Patchwork [2,of,2,stable] largefiles: more safe handling of interruptions while updating modifications

login
register
mail settings
Submitter Mads Kiilerich
Date Oct. 16, 2016, 12:30 a.m.
Message ID <dcd190cb73a0740be570.1476577801@madski>
Download mbox | patch
Permalink /patch/17093/
State Accepted
Headers show

Comments

Mads Kiilerich - Oct. 16, 2016, 12:30 a.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1476577785 -7200
#      Sun Oct 16 02:29:45 2016 +0200
# Node ID dcd190cb73a0740be570bdfbc1090066c38e1173
# Parent  2105975810c843d016ba930c44e63699af2703a6
largefiles: more safe handling of interruptions while updating modifications

Largefiles are fragile with the design where dirstate and lfdirstate must be
kept in sync.

To be less fragile, mark all clean largefiles as unsure ("normallookup") before
updating standins. After standins have been updated and we know exactly which
largefile standins actually was changed, mark the unchanged largefiles back to
clean ("normal").

This will make the failure mode more safe. If interrupted, the next command
will continue to perform extra hashing of all largefiles. That will do that all
largefiles that are out of sync with their standin will be marked dirty and
they will show up in status and can be cleaned with update --clean.
Yuya Nishihara - Oct. 17, 2016, 2:28 p.m.
On Sun, 16 Oct 2016 02:30:01 +0200, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1476577785 -7200
> #      Sun Oct 16 02:29:45 2016 +0200
> # Node ID dcd190cb73a0740be570bdfbc1090066c38e1173
> # Parent  2105975810c843d016ba930c44e63699af2703a6
> largefiles: more safe handling of interruptions while updating modifications
> 
> Largefiles are fragile with the design where dirstate and lfdirstate must be
> kept in sync.
> 
> To be less fragile, mark all clean largefiles as unsure ("normallookup") before
> updating standins. After standins have been updated and we know exactly which
> largefile standins actually was changed, mark the unchanged largefiles back to
> clean ("normal").

This sounds good to me and the change looks correct. Queued, thanks.

Patch

diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -1385,7 +1385,8 @@  def mergeupdate(orig, repo, node, branch
         lfdirstate = lfutil.openlfdirstate(repo.ui, repo)
         unsure, s = lfdirstate.status(matchmod.always(repo.root,
                                                     repo.getcwd()),
-                                      [], False, False, False)
+                                      [], False, True, False)
+        oldclean = set(s.clean)
         pctx = repo['.']
         for lfile in unsure + s.modified:
             lfileabs = repo.wvfs.join(lfile)
@@ -1397,9 +1398,13 @@  def mergeupdate(orig, repo, node, branch
                                 lfutil.getexecutable(lfileabs))
             if (standin in pctx and
                 lfhash == lfutil.readstandin(repo, lfile, '.')):
-                lfdirstate.normal(lfile)
+                oldclean.add(lfile)
         for lfile in s.added:
             lfutil.updatestandin(repo, lfutil.standin(lfile))
+        # mark all clean largefiles as dirty, just in case the update gets
+        # interrupted before largefiles and lfdirstate are synchronized
+        for lfile in oldclean:
+            lfdirstate.normallookup(lfile)
         lfdirstate.write()
 
         oldstandins = lfutil.getstandinsstate(repo)
@@ -1408,6 +1413,13 @@  def mergeupdate(orig, repo, node, branch
 
         newstandins = lfutil.getstandinsstate(repo)
         filelist = lfutil.getlfilestoupdate(oldstandins, newstandins)
+
+        # to avoid leaving all largefiles as dirty and thus rehash them, mark
+        # all the ones that didn't change as clean
+        for lfile in oldclean.difference(filelist):
+            lfdirstate.normal(lfile)
+        lfdirstate.write()
+
         if branchmerge or force or partial:
             filelist.extend(s.deleted + s.removed)
 
diff --git a/tests/test-largefiles-update.t b/tests/test-largefiles-update.t
--- a/tests/test-largefiles-update.t
+++ b/tests/test-largefiles-update.t
@@ -732,16 +732,16 @@  bit correctly on the platform being unaw
 
 #endif
 
-Test a fatal error interrupting an update. lfdirstate doesn't realize that
-.hglf has been updated while the largefile hasn't. Status thus shows a clean
-state ... but rebuilding lfdirstate and checking all hashes reveals it isn't
-clean.
+Test a fatal error interrupting an update. Verify that status report dirty
+files correctly after an interrupted update. Also verify that checking all
+hashes reveals it isn't clean.
 
 Start with clean dirstates:
   $ hg up -qcr "8^"
   $ sleep 1
   $ hg st
-Update standins without updating largefiles:
+Update standins without updating largefiles - large1 is modified and largeX is
+added:
   $ cat << EOF > ../crashupdatelfiles.py
   > import hgext.largefiles.lfutil
   > def getlfilestoupdate(oldstandins, newstandins):
@@ -750,29 +750,31 @@  Update standins without updating largefi
   > EOF
   $ hg up -Cr "8" --config extensions.crashupdatelfiles=../crashupdatelfiles.py
   [7]
-Check large1 content and status:
-BUG: largeX is R and large1 is not M and update does nothing
+Check large1 content and status ... and that update will undo modifications:
+BUG: large is R
   $ cat large1
   large1 in #3
   $ hg st
+  M large1
   R largeX
   $ hg up -Cr .
-  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  getting changed largefiles
+  1 largefiles updated, 0 removed
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ cat large1
+  manually modified before 'hg transplant --continue'
   $ hg st
   R largeX
-Force largefiles rehashing and check again - revealing modifications that
-update now can remove:
+Force largefiles rehashing and check again - which makes it realize that largeX
+not has been removed but just doesn't exist:
   $ rm .hg/largefiles/dirstate
   $ hg st
-  M large1
   ! largeX
   $ hg up -Cr .
   getting changed largefiles
-  2 largefiles updated, 0 removed
-  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  1 largefiles updated, 0 removed
+  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg st
-  $ cat large1
-  manually modified before 'hg transplant --continue'
 
   $ cd ..