Patchwork [4,of,4] transaction: include backup file in the "undo" transaction

login
register
mail settings
Submitter Pierre-Yves David
Date Jan. 17, 2015, 3:51 a.m.
Message ID <e97485e98326cc480fac.1421466676@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/7513/
State Accepted
Commit d251da5e0e841971cc54f2fc1fc4d70743056b5c
Headers show

Comments

Pierre-Yves David - Jan. 17, 2015, 3:51 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1421462054 28800
#      Fri Jan 16 18:34:14 2015 -0800
# Node ID e97485e98326cc480fac12b31b95bf26a63f63d0
# Parent  cb7106ff9861b85f1e2cfbf55f7694c66a596941
transaction: include backup file in the "undo" transaction

Once the transaction is closed, we now write transaction related data for
possible future undo. For now, we only do it for full file "backup" because
their were not handle at all in that case. In the future, we could move all the
current logic to set undo up (that currently exists in localrepository) inside
transaction itself, but it is not strictly requires to solve the current
situation.
Matt Mackall - Jan. 17, 2015, 8:50 p.m.
On Fri, 2015-01-16 at 19:51 -0800, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1421462054 28800
> #      Fri Jan 16 18:34:14 2015 -0800
> # Node ID e97485e98326cc480fac12b31b95bf26a63f63d0
> # Parent  cb7106ff9861b85f1e2cfbf55f7694c66a596941
> transaction: include backup file in the "undo" transaction

These are queued for default, thanks.
Adrian Buehlmann - March 3, 2015, 2:47 p.m.
On 2015-01-17 04:51, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1421462054 28800
> #      Fri Jan 16 18:34:14 2015 -0800
> # Node ID e97485e98326cc480fac12b31b95bf26a63f63d0
> # Parent  cb7106ff9861b85f1e2cfbf55f7694c66a596941
> transaction: include backup file in the "undo" transaction
> 
> Once the transaction is closed, we now write transaction related data for
> possible future undo. For now, we only do it for full file "backup" because
> their were not handle at all in that case. In the future, we could move all the
> current logic to set undo up (that currently exists in localrepository) inside
> transaction itself, but it is not strictly requires to solve the current
> situation.
> 
> diff --git a/mercurial/transaction.py b/mercurial/transaction.py
> --- a/mercurial/transaction.py
> +++ b/mercurial/transaction.py
> @@ -403,10 +403,11 @@ class transaction(object):
>                          raise
>                      # Abort may be raise by read only opener
>                      self.report("couldn't remote %s: %s\n"
>                                  % (vfs.join(b), inst))
>          self.entries = []
> +        self._writeundo()
>          if self.after:
>              self.after()
>          if self.opener.isfile(self.journal):
>              self.opener.unlink(self.journal)
>          if self.opener.isfile(self._backupjournal):
> @@ -438,10 +439,36 @@ class transaction(object):
>          '''abort the transaction (generally called on error, or when the
>          transaction is not explicitly committed before going out of
>          scope)'''
>          self._abort()
>  
> +    def _writeundo(self):
> +        """write transaction data for possible future undo call"""
> +        if self.undoname is None:
> +            return
> +        undobackupfile = self.opener.open("%s.backupfiles" % self.undoname, 'w')
> +        undobackupfile.write('%d\n' % version)
> +        for l, f, b, c in self._backupentries:
> +            if not f:  # temporary file
> +                continue
> +            if not b:
> +                u = ''
> +            else:
> +                if l not in self._vfsmap and c:
> +                    self.report("couldn't remote %s: unknown cache location"
> +                                "%s\n" % (b, l))
> +                    continue
> +                vfs = self._vfsmap[l]
> +                base, name = vfs.split(b)
> +                assert name.startswith(self.journal), name
> +                uname = name.replace(self.journal, self.undoname, 1)
> +                u = vfs.reljoin(base, uname)
> +                util.copyfile(vfs.join(b), vfs.join(u), hardlink=True)
> +            undobackupfile.write("%s\0%s\0%s\0%d\n" % (l, f, u, c))
> +        undobackupfile.close()
> +
> +
>      def _abort(self):
>          self.count = 0
>          self.usages = 0
>          self.file.close()
>          self._backupsfile.close()
> diff --git a/tests/test-fncache.t b/tests/test-fncache.t
> --- a/tests/test-fncache.t
> +++ b/tests/test-fncache.t
> @@ -79,10 +79,11 @@ Non store repo:
>    .hg/dirstate
>    .hg/last-message.txt
>    .hg/phaseroots
>    .hg/requires
>    .hg/undo
> +  .hg/undo.backupfiles
>    .hg/undo.bookmarks
>    .hg/undo.branch
>    .hg/undo.desc
>    .hg/undo.dirstate
>    .hg/undo.phaseroots
> @@ -112,10 +113,11 @@ Non fncache repo:
>    .hg/store/data
>    .hg/store/data/tst.d.hg
>    .hg/store/data/tst.d.hg/_foo.i
>    .hg/store/phaseroots
>    .hg/store/undo
> +  .hg/store/undo.backupfiles
>    .hg/store/undo.phaseroots
>    .hg/undo.bookmarks
>    .hg/undo.branch
>    .hg/undo.desc
>    .hg/undo.dirstate
> diff --git a/tests/test-hardlinks.t b/tests/test-hardlinks.t
> --- a/tests/test-hardlinks.t
> +++ b/tests/test-hardlinks.t
> @@ -48,10 +48,12 @@ Prepare repo r1:
>    1 r1/.hg/store/data/d1/f2.i
>    1 r1/.hg/store/data/f1.i
>    1 r1/.hg/store/fncache
>    1 r1/.hg/store/phaseroots
>    1 r1/.hg/store/undo
> +  1 r1/.hg/store/undo.backup.fncache
> +  1 r1/.hg/store/undo.backupfiles
>    1 r1/.hg/store/undo.phaseroots
>  
>  
>  Create hardlinked clone r2:
>  
> @@ -78,10 +80,12 @@ Repos r1 and r2 should now contain hardl
>    2 r1/.hg/store/data/d1/f2.i
>    2 r1/.hg/store/data/f1.i
>    2 r1/.hg/store/fncache
>    1 r1/.hg/store/phaseroots
>    1 r1/.hg/store/undo
> +  1 r1/.hg/store/undo.backup.fncache
> +  1 r1/.hg/store/undo.backupfiles
>    1 r1/.hg/store/undo.phaseroots
>  
>    $ nlinksdir r2/.hg/store
>    2 r2/.hg/store/00changelog.i
>    2 r2/.hg/store/00manifest.i
> @@ -97,10 +101,11 @@ Repo r3 should not be hardlinked:
>    1 r3/.hg/store/data/d1/f2.i
>    1 r3/.hg/store/data/f1.i
>    1 r3/.hg/store/fncache
>    1 r3/.hg/store/phaseroots
>    1 r3/.hg/store/undo
> +  1 r3/.hg/store/undo.backupfiles
>    1 r3/.hg/store/undo.phaseroots
>  
>  
>  Create a non-inlined filelog in r3:
>  
> @@ -122,10 +127,13 @@ Create a non-inlined filelog in r3:
>    1 r3/.hg/store/data/d1/f2.i
>    1 r3/.hg/store/data/f1.i
>    1 r3/.hg/store/fncache
>    1 r3/.hg/store/phaseroots
>    1 r3/.hg/store/undo
> +  1 r3/.hg/store/undo.backup.fncache
> +  1 r3/.hg/store/undo.backup.phaseroots
> +  1 r3/.hg/store/undo.backupfiles
>    1 r3/.hg/store/undo.phaseroots
>  
>  Push to repo r1 should break up most hardlinks in r2:
>  
>    $ hg -R r2 verify
> @@ -149,11 +157,11 @@ Push to repo r1 should break up most har
>    $ nlinksdir r2/.hg/store
>    1 r2/.hg/store/00changelog.i
>    1 r2/.hg/store/00manifest.i
>    1 r2/.hg/store/data/d1/f2.i
>    2 r2/.hg/store/data/f1.i
> -  1 r2/.hg/store/fncache
> +  2 r2/.hg/store/fncache

Could you please carefully explain why the expected hardlinkcount of the
fncache file in repo r2 at this point of the test needs to be changed to
2 here?

(This is now released changeset d251da5e0e84 in Mercurial. See also bug
4546)

>  
>    $ hg -R r2 verify
>    checking changesets
>    checking manifests
>    crosschecking files in changesets and manifests
> @@ -174,11 +182,11 @@ Committing a change to f1 in r1 must bre
>    $ nlinksdir r2/.hg/store
>    1 r2/.hg/store/00changelog.i
>    1 r2/.hg/store/00manifest.i
>    1 r2/.hg/store/data/d1/f2.i
>    1 r2/.hg/store/data/f1.i
> -  1 r2/.hg/store/fncache
> +  2 r2/.hg/store/fncache
>  
>  
>    $ cd r3
>    $ hg tip --template '{rev}:{node|short}\n'
>    11:a6451b6bc41f
> @@ -208,10 +216,13 @@ r4 has hardlinks in the working dir (not
>    2 r4/.hg/store/data/d1/f2.i
>    2 r4/.hg/store/data/f1.i
>    2 r4/.hg/store/fncache
>    2 r4/.hg/store/phaseroots
>    2 r4/.hg/store/undo
> +  2 r4/.hg/store/undo.backup.fncache
> +  2 r4/.hg/store/undo.backup.phaseroots
> +  2 r4/.hg/store/undo.backupfiles
>    2 r4/.hg/store/undo.phaseroots
>    2 r4/.hg/undo.bookmarks
>    2 r4/.hg/undo.branch
>    2 r4/.hg/undo.desc
>    2 r4/.hg/undo.dirstate
> @@ -240,10 +251,13 @@ Update back to revision 11 in r4 should 
>    2 r4/.hg/store/data/d1/f2.i
>    2 r4/.hg/store/data/f1.i
>    2 r4/.hg/store/fncache
>    2 r4/.hg/store/phaseroots
>    2 r4/.hg/store/undo
> +  2 r4/.hg/store/undo.backup.fncache
> +  2 r4/.hg/store/undo.backup.phaseroots
> +  2 r4/.hg/store/undo.backupfiles
>    2 r4/.hg/store/undo.phaseroots
>    2 r4/.hg/undo.bookmarks
>    2 r4/.hg/undo.branch
>    2 r4/.hg/undo.desc
>    2 r4/.hg/undo.dirstate
> diff --git a/tests/test-hook.t b/tests/test-hook.t
> --- a/tests/test-hook.t
> +++ b/tests/test-hook.t
> @@ -156,10 +156,12 @@ more there after
>    data
>    fncache
>    journal.phaseroots
>    phaseroots
>    undo
> +  undo.backup.fncache
> +  undo.backupfiles
>    undo.phaseroots
>  
>  
>  precommit hook can prevent commit
>  
> diff --git a/tests/test-inherit-mode.t b/tests/test-inherit-mode.t
> --- a/tests/test-inherit-mode.t
> +++ b/tests/test-inherit-mode.t
> @@ -79,10 +79,11 @@ new directories are setgid
>    00660 ./.hg/store/data/dir/bar.i
>    00660 ./.hg/store/data/foo.i
>    00660 ./.hg/store/fncache
>    00660 ./.hg/store/phaseroots
>    00660 ./.hg/store/undo
> +  00660 ./.hg/store/undo.backupfiles
>    00660 ./.hg/store/undo.phaseroots
>    00660 ./.hg/undo.bookmarks
>    00660 ./.hg/undo.branch
>    00660 ./.hg/undo.desc
>    00660 ./.hg/undo.dirstate
> @@ -123,10 +124,11 @@ group can still write everything
>    00770 ../push/.hg/store/data/dir/
>    00660 ../push/.hg/store/data/dir/bar.i
>    00660 ../push/.hg/store/data/foo.i
>    00660 ../push/.hg/store/fncache
>    00660 ../push/.hg/store/undo
> +  00660 ../push/.hg/store/undo.backupfiles
>    00660 ../push/.hg/store/undo.phaseroots
>    00660 ../push/.hg/undo.bookmarks
>    00660 ../push/.hg/undo.branch
>    00660 ../push/.hg/undo.desc
>    00660 ../push/.hg/undo.dirstate

Patch

diff --git a/mercurial/transaction.py b/mercurial/transaction.py
--- a/mercurial/transaction.py
+++ b/mercurial/transaction.py
@@ -403,10 +403,11 @@  class transaction(object):
                         raise
                     # Abort may be raise by read only opener
                     self.report("couldn't remote %s: %s\n"
                                 % (vfs.join(b), inst))
         self.entries = []
+        self._writeundo()
         if self.after:
             self.after()
         if self.opener.isfile(self.journal):
             self.opener.unlink(self.journal)
         if self.opener.isfile(self._backupjournal):
@@ -438,10 +439,36 @@  class transaction(object):
         '''abort the transaction (generally called on error, or when the
         transaction is not explicitly committed before going out of
         scope)'''
         self._abort()
 
+    def _writeundo(self):
+        """write transaction data for possible future undo call"""
+        if self.undoname is None:
+            return
+        undobackupfile = self.opener.open("%s.backupfiles" % self.undoname, 'w')
+        undobackupfile.write('%d\n' % version)
+        for l, f, b, c in self._backupentries:
+            if not f:  # temporary file
+                continue
+            if not b:
+                u = ''
+            else:
+                if l not in self._vfsmap and c:
+                    self.report("couldn't remote %s: unknown cache location"
+                                "%s\n" % (b, l))
+                    continue
+                vfs = self._vfsmap[l]
+                base, name = vfs.split(b)
+                assert name.startswith(self.journal), name
+                uname = name.replace(self.journal, self.undoname, 1)
+                u = vfs.reljoin(base, uname)
+                util.copyfile(vfs.join(b), vfs.join(u), hardlink=True)
+            undobackupfile.write("%s\0%s\0%s\0%d\n" % (l, f, u, c))
+        undobackupfile.close()
+
+
     def _abort(self):
         self.count = 0
         self.usages = 0
         self.file.close()
         self._backupsfile.close()
diff --git a/tests/test-fncache.t b/tests/test-fncache.t
--- a/tests/test-fncache.t
+++ b/tests/test-fncache.t
@@ -79,10 +79,11 @@  Non store repo:
   .hg/dirstate
   .hg/last-message.txt
   .hg/phaseroots
   .hg/requires
   .hg/undo
+  .hg/undo.backupfiles
   .hg/undo.bookmarks
   .hg/undo.branch
   .hg/undo.desc
   .hg/undo.dirstate
   .hg/undo.phaseroots
@@ -112,10 +113,11 @@  Non fncache repo:
   .hg/store/data
   .hg/store/data/tst.d.hg
   .hg/store/data/tst.d.hg/_foo.i
   .hg/store/phaseroots
   .hg/store/undo
+  .hg/store/undo.backupfiles
   .hg/store/undo.phaseroots
   .hg/undo.bookmarks
   .hg/undo.branch
   .hg/undo.desc
   .hg/undo.dirstate
diff --git a/tests/test-hardlinks.t b/tests/test-hardlinks.t
--- a/tests/test-hardlinks.t
+++ b/tests/test-hardlinks.t
@@ -48,10 +48,12 @@  Prepare repo r1:
   1 r1/.hg/store/data/d1/f2.i
   1 r1/.hg/store/data/f1.i
   1 r1/.hg/store/fncache
   1 r1/.hg/store/phaseroots
   1 r1/.hg/store/undo
+  1 r1/.hg/store/undo.backup.fncache
+  1 r1/.hg/store/undo.backupfiles
   1 r1/.hg/store/undo.phaseroots
 
 
 Create hardlinked clone r2:
 
@@ -78,10 +80,12 @@  Repos r1 and r2 should now contain hardl
   2 r1/.hg/store/data/d1/f2.i
   2 r1/.hg/store/data/f1.i
   2 r1/.hg/store/fncache
   1 r1/.hg/store/phaseroots
   1 r1/.hg/store/undo
+  1 r1/.hg/store/undo.backup.fncache
+  1 r1/.hg/store/undo.backupfiles
   1 r1/.hg/store/undo.phaseroots
 
   $ nlinksdir r2/.hg/store
   2 r2/.hg/store/00changelog.i
   2 r2/.hg/store/00manifest.i
@@ -97,10 +101,11 @@  Repo r3 should not be hardlinked:
   1 r3/.hg/store/data/d1/f2.i
   1 r3/.hg/store/data/f1.i
   1 r3/.hg/store/fncache
   1 r3/.hg/store/phaseroots
   1 r3/.hg/store/undo
+  1 r3/.hg/store/undo.backupfiles
   1 r3/.hg/store/undo.phaseroots
 
 
 Create a non-inlined filelog in r3:
 
@@ -122,10 +127,13 @@  Create a non-inlined filelog in r3:
   1 r3/.hg/store/data/d1/f2.i
   1 r3/.hg/store/data/f1.i
   1 r3/.hg/store/fncache
   1 r3/.hg/store/phaseroots
   1 r3/.hg/store/undo
+  1 r3/.hg/store/undo.backup.fncache
+  1 r3/.hg/store/undo.backup.phaseroots
+  1 r3/.hg/store/undo.backupfiles
   1 r3/.hg/store/undo.phaseroots
 
 Push to repo r1 should break up most hardlinks in r2:
 
   $ hg -R r2 verify
@@ -149,11 +157,11 @@  Push to repo r1 should break up most har
   $ nlinksdir r2/.hg/store
   1 r2/.hg/store/00changelog.i
   1 r2/.hg/store/00manifest.i
   1 r2/.hg/store/data/d1/f2.i
   2 r2/.hg/store/data/f1.i
-  1 r2/.hg/store/fncache
+  2 r2/.hg/store/fncache
 
   $ hg -R r2 verify
   checking changesets
   checking manifests
   crosschecking files in changesets and manifests
@@ -174,11 +182,11 @@  Committing a change to f1 in r1 must bre
   $ nlinksdir r2/.hg/store
   1 r2/.hg/store/00changelog.i
   1 r2/.hg/store/00manifest.i
   1 r2/.hg/store/data/d1/f2.i
   1 r2/.hg/store/data/f1.i
-  1 r2/.hg/store/fncache
+  2 r2/.hg/store/fncache
 
 
   $ cd r3
   $ hg tip --template '{rev}:{node|short}\n'
   11:a6451b6bc41f
@@ -208,10 +216,13 @@  r4 has hardlinks in the working dir (not
   2 r4/.hg/store/data/d1/f2.i
   2 r4/.hg/store/data/f1.i
   2 r4/.hg/store/fncache
   2 r4/.hg/store/phaseroots
   2 r4/.hg/store/undo
+  2 r4/.hg/store/undo.backup.fncache
+  2 r4/.hg/store/undo.backup.phaseroots
+  2 r4/.hg/store/undo.backupfiles
   2 r4/.hg/store/undo.phaseroots
   2 r4/.hg/undo.bookmarks
   2 r4/.hg/undo.branch
   2 r4/.hg/undo.desc
   2 r4/.hg/undo.dirstate
@@ -240,10 +251,13 @@  Update back to revision 11 in r4 should 
   2 r4/.hg/store/data/d1/f2.i
   2 r4/.hg/store/data/f1.i
   2 r4/.hg/store/fncache
   2 r4/.hg/store/phaseroots
   2 r4/.hg/store/undo
+  2 r4/.hg/store/undo.backup.fncache
+  2 r4/.hg/store/undo.backup.phaseroots
+  2 r4/.hg/store/undo.backupfiles
   2 r4/.hg/store/undo.phaseroots
   2 r4/.hg/undo.bookmarks
   2 r4/.hg/undo.branch
   2 r4/.hg/undo.desc
   2 r4/.hg/undo.dirstate
diff --git a/tests/test-hook.t b/tests/test-hook.t
--- a/tests/test-hook.t
+++ b/tests/test-hook.t
@@ -156,10 +156,12 @@  more there after
   data
   fncache
   journal.phaseroots
   phaseroots
   undo
+  undo.backup.fncache
+  undo.backupfiles
   undo.phaseroots
 
 
 precommit hook can prevent commit
 
diff --git a/tests/test-inherit-mode.t b/tests/test-inherit-mode.t
--- a/tests/test-inherit-mode.t
+++ b/tests/test-inherit-mode.t
@@ -79,10 +79,11 @@  new directories are setgid
   00660 ./.hg/store/data/dir/bar.i
   00660 ./.hg/store/data/foo.i
   00660 ./.hg/store/fncache
   00660 ./.hg/store/phaseroots
   00660 ./.hg/store/undo
+  00660 ./.hg/store/undo.backupfiles
   00660 ./.hg/store/undo.phaseroots
   00660 ./.hg/undo.bookmarks
   00660 ./.hg/undo.branch
   00660 ./.hg/undo.desc
   00660 ./.hg/undo.dirstate
@@ -123,10 +124,11 @@  group can still write everything
   00770 ../push/.hg/store/data/dir/
   00660 ../push/.hg/store/data/dir/bar.i
   00660 ../push/.hg/store/data/foo.i
   00660 ../push/.hg/store/fncache
   00660 ../push/.hg/store/undo
+  00660 ../push/.hg/store/undo.backupfiles
   00660 ../push/.hg/store/undo.phaseroots
   00660 ../push/.hg/undo.bookmarks
   00660 ../push/.hg/undo.branch
   00660 ../push/.hg/undo.desc
   00660 ../push/.hg/undo.dirstate