largefiles: fix inappropriate locking (issue3182)

Don't lock/write on operations that should be readonly (status).
Always lock when writing the lfdirstate (rollback).
Don't write lfdirstate until after committing; state isn't actually changed
until the commit is complete.

hgext/largefiles/lfutil.py

@@ -154,11 +154,7 b' def openlfdirstate(ui, repo):'
 
     # If the largefiles dirstate does not exist, populate and create
     # it. This ensures that we create it on the first meaningful
-    # largefiles operation in a new clone. It also gives us an easy
-    # way to forcibly rebuild largefiles state:
-    #   rm .hg/largefiles/dirstate && hg status
-    # Or even, if things are really messed up:
-    #   rm -rf .hg/largefiles && hg status
+    # largefiles operation in a new clone.
     if not os.path.exists(os.path.join(admin, 'dirstate')):
         util.makedirs(admin)
         matcher = getstandinmatcher(repo)
@@ -172,27 +168,19 b' def openlfdirstate(ui, repo):'
             except OSError, err:
                 if err.errno != errno.ENOENT:
                     raise
-
-        lfdirstate.write()
-
     return lfdirstate
 
 def lfdirstate_status(lfdirstate, repo, rev):
-    wlock = repo.wlock()
-    try:
-        match = match_.always(repo.root, repo.getcwd())
-        s = lfdirstate.status(match, [], False, False, False)
-        unsure, modified, added, removed, missing, unknown, ignored, clean = s
-        for lfile in unsure:
-            if repo[rev][standin(lfile)].data().strip() != \
-                    hashfile(repo.wjoin(lfile)):
-                modified.append(lfile)
-            else:
-                clean.append(lfile)
-                lfdirstate.normal(lfile)
-        lfdirstate.write()
-    finally:
-        wlock.release()
+    match = match_.always(repo.root, repo.getcwd())
+    s = lfdirstate.status(match, [], False, False, False)
+    unsure, modified, added, removed, missing, unknown, ignored, clean = s
+    for lfile in unsure:
+        if repo[rev][standin(lfile)].data().strip() != \
+                hashfile(repo.wjoin(lfile)):
+            modified.append(lfile)
+        else:
+            clean.append(lfile)
+            lfdirstate.normal(lfile)
     return (modified, added, removed, missing, unknown, ignored, clean)
 
 def listlfiles(repo, rev=None, matcher=None):

hgext/largefiles/overrides.py

@@ -910,15 +910,19 b' def override_rollback(orig, ui, repo, **'
     result = orig(ui, repo, **opts)
     merge.update(repo, node=None, branchmerge=False, force=True,
         partial=lfutil.isstandin)
-    lfdirstate = lfutil.openlfdirstate(ui, repo)
-    lfiles = lfutil.listlfiles(repo)
-    oldlfiles = lfutil.listlfiles(repo, repo[None].parents()[0].rev())
-    for file in lfiles:
-        if file in oldlfiles:
-            lfdirstate.normallookup(file)
-        else:
-            lfdirstate.add(file)
-    lfdirstate.write()
+    wlock = repo.wlock()
+    try:
+        lfdirstate = lfutil.openlfdirstate(ui, repo)
+        lfiles = lfutil.listlfiles(repo)
+        oldlfiles = lfutil.listlfiles(repo, repo[None].parents()[0].rev())
+        for file in lfiles:
+            if file in oldlfiles:
+                lfdirstate.normallookup(file)
+            else:
+                lfdirstate.add(file)
+        lfdirstate.write()
+    finally:
+        wlock.release()
     return result
 
 def override_transplant(orig, ui, repo, *revs, **opts):

hgext/largefiles/reposetup.py

@@ -147,9 +147,6 b' def reposetup(ui, repo):'
                 result = super(lfiles_repo, self).status(node1, node2, m,
                     True, clean, unknown, listsubrepos)
                 if working:
-                    # hold the wlock while we read largefiles and
-                    # update the lfdirstate
-                    wlock = repo.wlock()
                     try:
                         # Any non-largefiles that were explicitly listed must be
                         # taken out or lfdirstate.status will report an error.
@@ -186,7 +183,6 b' def reposetup(ui, repo):'
                                 else:
                                     clean.append(lfile)
                                     lfdirstate.normal(lfile)
-                            lfdirstate.write()
                         else:
                             tocheck = unsure + modified + added + clean
                             modified, added, clean = [], [], []
@@ -201,10 +197,9 b' def reposetup(ui, repo):'
                                         clean.append(lfile)
                                 else:
                                     added.append(lfile)
+                    finally:
                         # Replace the original ignore function
                         lfdirstate._ignore = orig_ignore
-                    finally:
-                        wlock.release()
 
                     for standin in ctx1.manifest():
                         if not lfutil.isstandin(standin):
@@ -324,10 +319,13 b' def reposetup(ui, repo):'
                             if not os.path.exists(
                                     repo.wjoin(lfutil.standin(lfile))):
                                 lfdirstate.drop(lfile)
+
+                    result = orig(text=text, user=user, date=date, match=match,
+                                    force=force, editor=editor, extra=extra)
+                    # This needs to be after commit; otherwise precommit hooks
+                    # get the wrong status
                     lfdirstate.write()
-
-                    return orig(text=text, user=user, date=date, match=match,
-                                    force=force, editor=editor, extra=extra)
+                    return result
 
                 for f in match.files():
                     if lfutil.isstandin(f):
@@ -359,7 +357,6 b' def reposetup(ui, repo):'
                         lfdirstate.normal(lfile)
                     else:
                         lfdirstate.drop(lfile)
-                lfdirstate.write()
 
                 # Cook up a new matcher that only matches regular files or
                 # standins corresponding to the big files requested by the
@@ -400,8 +397,12 b' def reposetup(ui, repo):'
                         return f in standins
 
                 match.matchfn = matchfn
-                return orig(text=text, user=user, date=date, match=match,
+                result = orig(text=text, user=user, date=date, match=match,
                                 force=force, editor=editor, extra=extra)
+                # This needs to be after commit; otherwise precommit hooks
+                # get the wrong status
+                lfdirstate.write()
+                return result
             finally:
                 wlock.release()
 

tests/test-largefiles-small-disk.t

@@ -65,4 +65,3 b' makes copies instead of hardlinks:'
 The largefile is not created in .hg/largefiles:
 
   $ ls bob/.hg/largefiles
-  dirstate