Saving info about each transaction (was Re: [PATCH 2 of 2] rollback: tell the user their previous commit message was saved)

Matt Mackall mpm at selenic.com
Sun Nov 22 11:05:20 CST 2009


On Sun, Nov 22, 2009 at 09:54:45AM -0500, Greg Ward wrote:
> [original patch description]
> > rollback: tell the user their previous commit message was saved
> > (issue1635)
> 
> [Matt's response]
> > I don't think this is the right place for this message as it'll be shown
> > for unrelated rollbacks.
> 
> I've been thinking about this.  The problem is that we don't know,
> after the fact, what the last transaction committed.  Was it a pull?
> push? unbundle? commit?  No way to know.
> 
> But that is a solvable problem.  And it opens the door to solving
> another problem that has bugged me for a while, namely "what
> changesets are being destroyed by this rollback?".  Here's the idea:

This is more or less what I proposed last time we talked about this.

>   * modify the transaction class to track two things: the nature of
> the transaction
>      (pull, push, unbundle, commit) and its contents (list of changeset IDs)

It's sufficient to simply list the first revision number, or
alternately, the tip revision before the transaction. So for now we
need two lines:

rev
action

>   * when committing the transaction, write that info to .hg/txninfo
>   * modify every bit of code that opens and commits a transaction to
> supply this
>      information (only 5-6 places in current hg-crew)
>   * make that information available at rollback time
> 
> Obviously, there's a lot of code out there in the world that needs to
> continue to work, so changes to the transaction class have to be done
> in a backwards-compatible way: add new methods, or optional args to
> __init__() and/or close().  And we have to be able to handle
> transactions that don't have .hg/txninfo at all.

This shouldn't touch transaction.py at all, as it has no idea what a
revision is. In other words, it's a layering violation. But
localrepo.transaction is a perfectly good place to put this.

> But this feature would enable several nifty things:
> 
>   * at rollback time, we would know if we were rolling back a single commit,
>     so could intelligently say "last commit message saved in
>   .hg/message"

It's easy enough to do this by catching exceptions in commit(). The
rollback code should not need to know about commit messages.

>   * in localrepository.destroyed(), we could know which nodes are being
>     destroyed, which should make it possible to invalidate less of the branch
>     head cache (and maybe less of the tag cache too, although it is less
>     dramatically affected by destroying nodes)

Doesn't seem like a good complexity/performance trade-off to me, given
that rollback is not supposed to be a common operation.

Looking closer at destroyed(), I think it's a bad idea. Cache-building
should be lazy. When we invalidate, we should just nuke the caches and
wait until someone actually wants them again to rebuild them.

>   * likewise, it would make it possible to add a meaningful "destroy" hook that
>     gets called after nodes have been destroyed (and possibly "predestroy" too,
>     although "pretxndestroy" is obviously impossible)
> 
> What do you think?  If people like this idea, I will keep the "save
> .hg/message" patch and drop the "tell user about .hg/message" patch
> for the time being, and do it right later.
> 
> Greg

-- 
Mathematics is the supreme nostalgia of our time.


More information about the Mercurial-devel mailing list