[PATCH] rollback: try to prevent nasty messes with shared clones

Greg Ward greg-hg at gerg.ca
Tue Sep 13 20:26:16 CDT 2011


# HG changeset patch
# User Greg Ward <greg at gerg.ca>
# Date 1315963560 14400
# Node ID 20561153542c382a2ef33ba9e296c2f73d3606c3
# Parent  06c791dab7707265e830f0e4ba343aad0a4c8df5
rollback: try to prevent nasty messes with shared clones.

Trouble happens with shared clones (multiple working dirs all sharing
the same repo store) if you commit in one working dir and then
rollback that transaction in a different working dir: rollback uses
partial info (.hg/store/undo but no .hg/undo.*) and generally makes a
mess of things. Try to avoid that by refusing to rollback unless we're
in the same working dir where the last transaction was created from.

This is v2: now with passing tests! No change to actual implementation
relative to original post (2011/09/12).

Possible points of contention:

  * storing the working dir path in a file named journal.wdir (then
    undo.dir) is a bit weird, since that data isn't needed for the
    actual rollback: it's used needed to prevent nasty rollback messes.
    But it's so darn convenient! all the infrastructure I needed was
    right there.

  * making localrepository._writejournal() responsible for creating
    .hg/store/journal.wdir might be wrong: perhaps it should be created
    the same way as .hg/store/journal, with localrepository deciding the
    filename and transaction actually creating the file

  * the error message is a bit long

  * new test script: I should probably try to fold this into the
    existing test-rollback.t, but I wanted to get something working
    first

diff --git a/hgext/keyword.py b/hgext/keyword.py
--- a/hgext/keyword.py
+++ b/hgext/keyword.py
@@ -589,12 +589,12 @@
                 kwt.restrict = restrict
             return n
 
-        def rollback(self, dryrun=False):
+        def rollback(self, dryrun=False, force=False):
             wlock = self.wlock()
             try:
                 if not dryrun:
                     changed = self['.'].files()
-                ret = super(kwrepo, self).rollback(dryrun)
+                ret = super(kwrepo, self).rollback(dryrun, force)
                 if not dryrun:
                     ctx = self['.']
                     modified, added = _preselect(self[None].status(), changed)
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -4388,7 +4388,8 @@
     finally:
         wlock.release()
 
- at command('rollback', dryrunopts)
+ at command('rollback', dryrunopts +
+         [('f', 'force', False, _('ignore safety measures'))])
 def rollback(ui, repo, **opts):
     """roll back the last transaction (dangerous)
 
@@ -4418,7 +4419,8 @@
 
     Returns 0 on success, 1 if no rollback data is available.
     """
-    return repo.rollback(opts.get('dry_run'))
+    return repo.rollback(dryrun=opts.get('dry_run'),
+                         force=opts.get('force'))
 
 @command('root', [])
 def root(ui, repo):
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -718,7 +718,16 @@
         return tr
 
     def _writejournal(self, desc):
-        # save dirstate for rollback
+        # save pre-transaction state for rollback
+        # - path to working dir (in case of shared clones)
+        # - content of dirstate
+        # - current branch
+        # - current len(repo) (tip rev + 1), description of transaction
+        #   (N.B. if desc contains a newline, then undo.desc is effectively
+        #   a 3-tuple, not a 2-tuple!)
+        # - current bookmark
+
+        self.sopener.write('journal.wdir', '%s\n' % self.root)
         try:
             ds = self.opener.read("dirstate")
         except IOError:
@@ -735,8 +744,11 @@
         else:
             self.opener.write('journal.bookmarks', '')
 
-        return (self.sjoin('journal'), self.join('journal.dirstate'),
-                self.join('journal.branch'), self.join('journal.desc'),
+        return (self.sjoin('journal'),
+                self.sjoin('journal.wdir'),
+                self.join('journal.dirstate'),
+                self.join('journal.branch'),
+                self.join('journal.desc'),
                 self.join('journal.bookmarks'))
 
     def recover(self):
@@ -754,20 +766,33 @@
         finally:
             lock.release()
 
-    def rollback(self, dryrun=False):
+    def rollback(self, dryrun=False, force=False):
         wlock = lock = None
         try:
             wlock = self.wlock()
             lock = self.lock()
             if os.path.exists(self.sjoin("undo")):
-                return self._rollback(dryrun)
+                return self._rollback(dryrun, force)
             else:
                 self.ui.warn(_("no rollback information available\n"))
                 return 1
         finally:
             release(lock, wlock)
 
-    def _rollback(self, dryrun):
+    def _rollback(self, dryrun, force):
+        # check preconditions
+        try:
+            wdir = self.sopener.read('undo.wdir').strip()
+        except IOError:
+            pass
+        else:
+            if not force and wdir != self.root:
+                raise util.Abort(
+                    _('attempt to rollback from a different working dir '
+                      'than where this transaction was created (%s) '
+                      '(use -f to force: this will make a mess)')
+                    % wdir)
+
         try:
             args = self.opener.read("undo.desc").splitlines()
             if len(args) >= 3 and self.ui.verbose:
diff --git a/tests/test-debugcomplete.t b/tests/test-debugcomplete.t
--- a/tests/test-debugcomplete.t
+++ b/tests/test-debugcomplete.t
@@ -257,7 +257,7 @@
   rename: after, force, include, exclude, dry-run
   resolve: all, list, mark, unmark, no-status, tool, include, exclude
   revert: all, date, rev, no-backup, include, exclude, dry-run
-  rollback: dry-run
+  rollback: dry-run, force
   root: 
   showconfig: untrusted
   tag: force, local, rev, remove, edit, message, date, user
diff --git a/tests/test-fncache.t b/tests/test-fncache.t
--- a/tests/test-fncache.t
+++ b/tests/test-fncache.t
@@ -80,6 +80,7 @@
   .hg/undo.branch
   .hg/undo.desc
   .hg/undo.dirstate
+  .hg/undo.wdir
   $ cd ..
 
 Non fncache repo:
@@ -103,6 +104,7 @@
   .hg/store/data/tst.d.hg
   .hg/store/data/tst.d.hg/_foo.i
   .hg/store/undo
+  .hg/store/undo.wdir
   .hg/undo.bookmarks
   .hg/undo.branch
   .hg/undo.desc
diff --git a/tests/test-hardlinks.t b/tests/test-hardlinks.t
--- a/tests/test-hardlinks.t
+++ b/tests/test-hardlinks.t
@@ -46,6 +46,7 @@
   1 r1/.hg/store/data/f1.i
   1 r1/.hg/store/fncache
   1 r1/.hg/store/undo
+  1 r1/.hg/store/undo.wdir
 
 
 Create hardlinked clone r2:
@@ -74,6 +75,7 @@
   2 r1/.hg/store/data/f1.i
   2 r1/.hg/store/fncache
   1 r1/.hg/store/undo
+  1 r1/.hg/store/undo.wdir
 
   $ nlinksdir r2/.hg/store
   2 r2/.hg/store/00changelog.i
@@ -91,6 +93,7 @@
   1 r3/.hg/store/data/f1.i
   1 r3/.hg/store/fncache
   1 r3/.hg/store/undo
+  1 r3/.hg/store/undo.wdir
 
 
 Create a non-inlined filelog in r3:
@@ -111,6 +114,7 @@
   1 r3/.hg/store/data/f1.i
   1 r3/.hg/store/fncache
   1 r3/.hg/store/undo
+  1 r3/.hg/store/undo.wdir
 
 Push to repo r1 should break up most hardlinks in r2:
 
@@ -194,6 +198,7 @@
   2 r4/.hg/store/data/f1.i
   2 r4/.hg/store/fncache
   2 r4/.hg/store/undo
+  2 r4/.hg/store/undo.wdir
   2 r4/.hg/undo.bookmarks
   2 r4/.hg/undo.branch
   2 r4/.hg/undo.desc
@@ -223,6 +228,7 @@
   2 r4/.hg/store/data/f1.i
   2 r4/.hg/store/fncache
   2 r4/.hg/store/undo
+  2 r4/.hg/store/undo.wdir
   2 r4/.hg/undo.bookmarks
   2 r4/.hg/undo.branch
   2 r4/.hg/undo.desc
diff --git a/tests/test-hup.t b/tests/test-hup.t
--- a/tests/test-hup.t
+++ b/tests/test-hup.t
@@ -17,4 +17,4 @@
   rollback completed
   killed!
   $ echo .hg/* .hg/store/*
-  .hg/00changelog.i .hg/journal.bookmarks .hg/journal.branch .hg/journal.desc .hg/journal.dirstate .hg/requires .hg/store .hg/store/00changelog.i .hg/store/00changelog.i.a
+  .hg/00changelog.i .hg/journal.bookmarks .hg/journal.branch .hg/journal.desc .hg/journal.dirstate .hg/requires .hg/store .hg/store/00changelog.i .hg/store/00changelog.i.a .hg/store/journal.wdir
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
@@ -77,6 +77,7 @@
   00660 ./.hg/store/data/foo.i
   00660 ./.hg/store/fncache
   00660 ./.hg/store/undo
+  00660 ./.hg/store/undo.wdir
   00660 ./.hg/undo.bookmarks
   00660 ./.hg/undo.branch
   00660 ./.hg/undo.desc
@@ -118,6 +119,7 @@
   00660 ../push/.hg/store/data/foo.i
   00660 ../push/.hg/store/fncache
   00660 ../push/.hg/store/undo
+  00660 ../push/.hg/store/undo.wdir
   00660 ../push/.hg/undo.bookmarks
   00660 ../push/.hg/undo.branch
   00660 ../push/.hg/undo.desc
diff --git a/tests/test-rollback-safe.t b/tests/test-rollback-safe.t
new file mode 100644
--- /dev/null
+++ b/tests/test-rollback-safe.t
@@ -0,0 +1,36 @@
+setup: repo with two changesets
+  $ hg init repo1
+  $ cd repo1
+  $ echo a > a
+  $ hg -q commit -A -m'rev 0: add a'
+  $ echo b > b
+  $ hg -q commit -A -m'rev 1: add b'
+  $ cat >> $HGRCPATH <<EOF
+  > [extensions]
+  > share =
+  > EOF
+
+keep a backup copy for repeated rollback attempts
+  $ cd ..
+  $ tar -cf repo1.tar repo1
+
+attempt rollback in a shared clone
+  $ hg share repo1 share
+  updating working directory
+  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ cd share
+  $ hg rollback
+  abort: attempt to rollback from a different working dir than where this transaction was created ($TESTTMP/repo1) (use -f to force: this will make a mess)
+  [255]
+  $ hg tip -q
+  1:d7cd497f77e1
+  $ hg rollback -f
+  rolling back unknown transaction
+  abort: No such file or directory
+  [255]
+  $ hg tip -q
+  0:b9795f9c9ebd
+  $ hg status
+  warning: ignoring unknown working parent d7cd497f77e1!
+  M a
+  M b


More information about the Mercurial-devel mailing list