[PATCH 2 of 2] rollback: only restore dirstate (et. al.) when appropriate

Greg Ward greg-hg at gerg.ca
Sat Sep 17 10:38:38 CDT 2011


On Fri, Sep 16, 2011 at 3:42 PM, Matt Mackall <mpm at selenic.com> wrote:
> On Thu, 2011-09-15 at 22:22 -0400, Greg Ward wrote:
>> # HG changeset patch
>> # User Greg Ward <greg at gerg.ca>
>> # Date 1316139696 14400
>> # Node ID b33ba776a9977a4b1460eebb5acd9dc954da03c5
>> # Parent  23fe96275fabc99c33f61da305f81948ea5c61df
>> rollback: only restore dirstate (et. al.) when appropriate.
[...]
> I think it would be better to simply truncate history and then see if
> the working directory parent has vanished rather than looking at the
> may-not-be-present-or-correct transaction description.

OK, I got this working. Here is the second half of
localrepository._rollback() in my latest patch:

        if dryrun:
            return 0

        parents = self.dirstate.parents()
        transaction.rollback(self.sopener, self.sjoin('undo'), ui.warn)
        self.invalidate()

        parentgone = (parents[0] not in self.changelog.nodemap or
                      parents[1] not in self.changelog.nodemap)
        if parentgone:
            util.rename(self.join('undo.dirstate'), self.join('dirstate'))
            if os.path.exists(self.join('undo.bookmarks')):
                util.rename(self.join('undo.bookmarks'),
                            self.join('bookmarks'))
            try:
                branch = self.opener.read('undo.branch')
                self.dirstate.setbranch(branch)
            except IOError:
                ui.warn(_('named branch could not be reset: '
                          'current branch is still \'%s\'\n')
                        % self.dirstate.branch())

            self.dirstate.invalidate()
            self.destroyed()
            parents = tuple([p.rev() for p in self.parents()])
            if len(parents) > 1:
                ui.status(_('working directory now based on '
                            'revisions %d and %d\n') % parents)
            else:
                ui.status(_('working directory now based on '
                            'revision %d\n') % parents)
        else:
            ui.status(_('working directory parent not changed\n'))
        return 0

This causes lots of test failures, e.g.

--- /home/greg/src/hg-crew/tests/test-acl.t
+++ /home/greg/src/hg-crew/tests/test-acl.t.err
@@ -121,7 +121,7 @@
   updating the branch cache
   checking for updated bookmarks
   repository tip rolled back to revision 0 (undo push)
-  working directory now based on revision 0
+  working directory parent not changed
   0:6675d58eff77


Before I update all those tests, I'd like your opinion on the "working
directory parent not changed" message. IMHO if rollback had *always*
worked this way, it would be unnecessary (silence is golden and all
that). But since we are deliberately changing behaviour, it seems like
a good idea to say so. Either way, I have to update a bunch of test
scripts, but I'd rather only do it once.

Luckily, there don't appear to be any tests that depend on the current
"restore dirstate et. al. unconditionally" behaviour, so I got off
easy there.

Greg


More information about the Mercurial-devel mailing list