Making rollback safer

Greg Ward greg at gerg.ca
Sun Sep 11 20:45:40 CDT 2011


What a coincidence: I was all set to sit down tonight and work on
favourite pet peeve (rollback is particularly dangerous with shared
clones) when someone opened issue2998 (rollback is really dangerous if
your working dir is updated to the wrong changeset). I don't claim to
be able to kill both birds with one stone, but I think a similar
approach will work for both.

First, my pet peeve: rollback with shared clones. The problem is that
you have two working dirs sharing the same repo store. If you commit
and rollback in the same working dir, no problem. But if you commit,
change to a different working dir, and then rollback, Mercurial does
bad stuff because it only has access to .hg/store/undo, not
.hg/undo.*. The transaction in the store is correctly truncated, but
it leaves at least one working dir in a bad state: "warning: ignoring
unknown working parent d7cd497f77e1!".

Proposed fix: when writing a transaction
(localrepository._writejournal()), write .hg/store/undo.wdir
containing repo.root. Then in rollback, make sure that repo.root ==
contents of that file. If not, abort. Add a --force option in case
someone really wants to shoot themselves in the foot.

I have a patch and it seems to work. Please let me know if you think
this is a dumb approach -- otherwise I'll send it soon.

Now for issue2998: commit, update back to an old changeset, then
rollback. In that case, 'update' carefully and deliberately loses any
changes made in your recent commit. That's its *job*, after all. Then
'rollback' does nothing at all to the working dir (part of *its*
contract) and truncates the transaction. Result: the changes you just
committed are utterly totally completely irretrievably lost. Oops.
Yes, people should RTFM and not use rollback until they understand
e-x-a-c-t-l-y what it does, but this is the real world. People make
mistakes, misunderstand the docs, ignore the docs, assume Mercurial
has got their back, etc.

Proposed fix: at rollback time, we already know the revnum of the
latest changeset that will be destroyed by rollback, since it's in
undo.desc. Use that info: if working dir parent rev != rev in
undo.desc, abort the rollback. As above, allow the user to do stupid
things with --force. I don't have a patch for this yet, but it looks
straightforward.

Opinions?

Greg


More information about the Mercurial-devel mailing list