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

[Matt Mackall]

On Sat, 2011-09-17 at 11:38 -0400, Greg Ward wrote:
> 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.

I think we should skip it. I think the current behavior is pretty
obviously buggy and I'm a little amazed it's taken us this long to
notice it. But we don't want to be stuck with a pointless message going
forward.

-- 
Mathematics is the supreme nostalgia of our time.