Patchwork [3,of,6] lfs: add the 'lfs' requirement in the changegroup transaction introducing lfs

login
register
mail settings
Submitter Matt Harbison
Date Dec. 27, 2017, 8:27 a.m.
Message ID <a05cb56d887b0d94f050.1514363275@Envy>
Download mbox | patch
Permalink /patch/26468/
State Accepted
Headers show

Comments

Matt Harbison - Dec. 27, 2017, 8:27 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1514091100 18000
#      Sat Dec 23 23:51:40 2017 -0500
# Node ID a05cb56d887b0d94f0506fb6ad86a96dc0358c7d
# Parent  ee4253e1dcd9caddfb548a9a7650a3949c24ace2
lfs: add the 'lfs' requirement in the changegroup transaction introducing lfs

A hook like this is how largefiles manages to do the same.  Largefiles uses a
changegroup hook, but this uses pretxnchangegroup because that actually causes
the transaction to rollback in the unlikely event that writing the requirements
out fails.  Sadly, the requires file itself isn't rolled back if a subsequent
hook fails, but that seems trivial.

Now that commit, changegroup and convert are covered, I don't think there's any
way to get an lfs repo without the requirement.

The grep exit code is blotted out of some test-lfs-serve.t tests now showing the
requirement, because run-tests.py doesn't support conditionalizing the exit
code.

Patch

diff --git a/hgext/lfs/__init__.py b/hgext/lfs/__init__.py
--- a/hgext/lfs/__init__.py
+++ b/hgext/lfs/__init__.py
@@ -43,6 +43,7 @@ 
     filelog,
     hg,
     localrepo,
+    node,
     registrar,
     revlog,
     scmutil,
@@ -122,13 +123,21 @@ 
     if 'lfs' not in repo.requirements:
         def checkrequireslfs(ui, repo, **kwargs):
             if 'lfs' not in repo.requirements:
-                ctx = repo[kwargs['node']]
+                last = kwargs.get('node_last')
+                _bin = node.bin
+                if last:
+                    s = repo.set('%n:%n', _bin(kwargs['node']), _bin(last))
+                else:
+                    s = repo.set('%n', _bin(kwargs['node']))
+            for ctx in s:
                 # TODO: is there a way to just walk the files in the commit?
                 if any(ctx[f].islfs() for f in ctx.files() if f in ctx):
                     repo.requirements.add('lfs')
                     repo._writerequirements()
+                    break
 
         ui.setconfig('hooks', 'commit.lfs', checkrequireslfs, 'lfs')
+        ui.setconfig('hooks', 'pretxnchangegroup.lfs', checkrequireslfs, 'lfs')
 
 def wrapfilelog(filelog):
     wrapfunction = extensions.wrapfunction
diff --git a/tests/test-lfs-serve.t b/tests/test-lfs-serve.t
--- a/tests/test-lfs-serve.t
+++ b/tests/test-lfs-serve.t
@@ -130,9 +130,9 @@ 
 
 --------------------------------------------------------------------------------
 Case #3: client with lfs content and the extension enabled; server with
-non-lfs content, and the extension state controlled by #testcases.
+non-lfs content, and the extension state controlled by #testcases.  The server
+should have an 'lfs' requirement after it picks up its first commit with a blob.
 
-TODO: add the 'lfs' requirement on the server for each test in lfsremote-on
   $ echo 'this is a big lfs file' > lfs.bin
   $ hg ci -Aqm 'lfs'
   $ grep 'lfs' .hg/requires $SERVER_REQUIRES
@@ -144,15 +144,18 @@ 
   ValueError: no common changegroup version (lfsremote-off !)
   $ grep 'lfs' .hg/requires $SERVER_REQUIRES
   .hg/requires:lfs
+  $TESTTMP/server/.hg/requires:lfs (lfsremote-on !)
 
   $ hg clone -q http://localhost:$HGPORT $TESTTMP/client3_clone
-  $ grep 'lfs' $TESTTMP/client3_clone/.hg/requires $SERVER_REQUIRES
-  [1]
+  $ grep 'lfs' $TESTTMP/client3_clone/.hg/requires $SERVER_REQUIRES || true
+  $TESTTMP/client3_clone/.hg/requires:lfs (lfsremote-on !)
+  $TESTTMP/server/.hg/requires:lfs (lfsremote-on !)
 
   $ hg init $TESTTMP/client3_pull
   $ hg -R $TESTTMP/client3_pull pull -q http://localhost:$HGPORT
-  $ grep 'lfs' $TESTTMP/client3_pull/.hg/requires $SERVER_REQUIRES
-  [1]
+  $ grep 'lfs' $TESTTMP/client3_pull/.hg/requires $SERVER_REQUIRES || true
+  $TESTTMP/client3_pull/.hg/requires:lfs (lfsremote-on !)
+  $TESTTMP/server/.hg/requires:lfs (lfsremote-on !)
 
 XXX: The difference here is the push failed above when the extension isn't
 enabled on the server.  The extension shouldn't need to mess with changegroup
@@ -185,12 +188,12 @@ 
   $ echo 'non-lfs' > nonlfs2.txt
   $ hg ci -Aqm 'non-lfs'
   $ grep 'lfs' .hg/requires $SERVER_REQUIRES
-  [1]
+  $TESTTMP/server/.hg/requires:lfs
 
   $ hg push -q --force
   warning: repository is unrelated
   $ grep 'lfs' .hg/requires $SERVER_REQUIRES
-  [1]
+  $TESTTMP/server/.hg/requires:lfs
 
 TODO: fail more gracefully.
 
@@ -199,6 +202,7 @@ 
   [255]
   $ grep 'lfs' $TESTTMP/client4_clone/.hg/requires $SERVER_REQUIRES
   grep: $TESTTMP/client4_clone/.hg/requires: $ENOENT$
+  $TESTTMP/server/.hg/requires:lfs
   [2]
 
 TODO: fail more gracefully.
@@ -208,7 +212,7 @@ 
   abort: HTTP Error 500: Internal Server Error
   [255]
   $ grep 'lfs' $TESTTMP/client4_pull/.hg/requires $SERVER_REQUIRES
-  [1]
+  $TESTTMP/server/.hg/requires:lfs
 
   $ hg identify http://localhost:$HGPORT
   03b080fa9d93
@@ -226,16 +230,18 @@ 
 
   $ hg push -q
   $ grep 'lfs' .hg/requires $SERVER_REQUIRES
-  [1]
+  $TESTTMP/server/.hg/requires:lfs
 
   $ hg clone -q http://localhost:$HGPORT $TESTTMP/client5_clone
   $ grep 'lfs' $TESTTMP/client5_clone/.hg/requires $SERVER_REQUIRES
-  [1]
+  $TESTTMP/client5_clone/.hg/requires:lfs
+  $TESTTMP/server/.hg/requires:lfs
 
   $ hg init $TESTTMP/client5_pull
   $ hg -R $TESTTMP/client5_pull pull -q http://localhost:$HGPORT
   $ grep 'lfs' $TESTTMP/client5_pull/.hg/requires $SERVER_REQUIRES
-  [1]
+  $TESTTMP/client5_pull/.hg/requires:lfs
+  $TESTTMP/server/.hg/requires:lfs
 
   $ hg identify http://localhost:$HGPORT
   c729025cc5e3
@@ -244,23 +250,24 @@ 
 Case #6: client with lfs content and the extension enabled; server with
 lfs content, and the extension enabled.
 
-TODO: add the 'lfs' requirement on the server for each test
-
   $ echo 'this is another lfs file' > lfs2.txt
   $ hg ci -Aqm 'lfs file with lfs client'
 
   $ hg push -q
   $ grep 'lfs' .hg/requires $SERVER_REQUIRES
   .hg/requires:lfs
+  $TESTTMP/server/.hg/requires:lfs
 
   $ hg clone -q http://localhost:$HGPORT $TESTTMP/client6_clone
   $ grep 'lfs' $TESTTMP/client6_clone/.hg/requires $SERVER_REQUIRES
-  [1]
+  $TESTTMP/client6_clone/.hg/requires:lfs
+  $TESTTMP/server/.hg/requires:lfs
 
   $ hg init $TESTTMP/client6_pull
   $ hg -R $TESTTMP/client6_pull pull -q http://localhost:$HGPORT
   $ grep 'lfs' $TESTTMP/client6_pull/.hg/requires $SERVER_REQUIRES
-  [1]
+  $TESTTMP/client6_pull/.hg/requires:lfs
+  $TESTTMP/server/.hg/requires:lfs
 
   $ hg identify http://localhost:$HGPORT
   d3b84d50eacb
diff --git a/tests/test-lfs-test-server.t b/tests/test-lfs-test-server.t
--- a/tests/test-lfs-test-server.t
+++ b/tests/test-lfs-test-server.t
@@ -53,6 +53,7 @@ 
   adding manifests
   adding file changes
   added 1 changesets with 1 changes to 1 files
+  calling hook pretxnchangegroup.lfs: hgext.lfs.checkrequireslfs
 
 Clear the cache to force a download
   $ rm -rf `hg config lfs.usercache`
diff --git a/tests/test-lfs.t b/tests/test-lfs.t
--- a/tests/test-lfs.t
+++ b/tests/test-lfs.t
@@ -56,7 +56,7 @@ 
   > url=file:$TESTTMP/dummy-remote/
   > EOF
 
-TODO: Push to a local non-lfs repo with the extension enabled should add the
+Push to a local non-lfs repo with the extension enabled will add the
 lfs requirement
 
   $ grep lfs $TESTTMP/server/.hg/requires
@@ -70,8 +70,9 @@ 
   adding manifests
   adding file changes
   added 2 changesets with 2 changes to 2 files
+  calling hook pretxnchangegroup.lfs: hgext.lfs.checkrequireslfs
   $ grep lfs $TESTTMP/server/.hg/requires
-  [1]
+  lfs
 
 # Unknown URL scheme
 
@@ -91,11 +92,11 @@ 
 
 # Pull from server
 
-TODO: Pulling a local lfs repo into a local non-lfs repo with the extension
-enabled should add the lfs requirement
+Pulling a local lfs repo into a local non-lfs repo with the extension
+enabled adds the lfs requirement
 
   $ grep lfs .hg/requires $TESTTMP/server/.hg/requires
-  [1]
+  $TESTTMP/server/.hg/requires:lfs
   $ hg pull default
   pulling from $TESTTMP/server
   requesting all changes
@@ -106,7 +107,8 @@ 
   new changesets b29ba743f89d:00c137947d30
   (run 'hg update' to get a working copy)
   $ grep lfs .hg/requires $TESTTMP/server/.hg/requires
-  [1]
+  .hg/requires:lfs
+  $TESTTMP/server/.hg/requires:lfs
 
 # Check the blobstore is not yet populated
   $ [ -d .hg/store/lfs/objects ]
@@ -872,7 +874,7 @@ 
   $ hg commit -m 'rm A'
   $ cd ..
 
-TODO: Unbundling adds a requirement to a non-lfs repo, if necessary.
+Unbundling adds a requirement to a non-lfs repo, if necessary.
 
   $ hg bundle -R $TESTTMP/repo-del -qr 0 --base null nolfs.hg
   $ hg bundle -R convert_lfs2 -qr tip --base null lfs.hg
@@ -882,7 +884,7 @@ 
   [1]
   $ hg pull -R unbundle -q lfs.hg
   $ grep lfs unbundle/.hg/requires
-  [1]
+  lfs
 
   $ hg init no_lfs
   $ cat >> no_lfs/.hg/hgrc <<EOF