[PATCH STABLE?] commit: avoid a dirstate race with multiple commits in the same process

[Matt Mackall]

On Mon, 2011-03-14 at 14:48 -0400, Greg Ward wrote:
> # HG changeset patch
> # User Greg Ward <greg-hg at gerg.ca>
> # Date 1300128285 14400
> # Branch stable
> # Node ID 396982a5883383fc043d3f5ad78a0f9f0366bd06
> # Parent  4bfff063aed6fb4b3c8abe8a9ec51fe1e55725bf
> commit: avoid a dirstate race with multiple commits in the same process
> (issue2264, issue2516)
> 
> The race happens when two commits in a row change the same file but do
> not change its size, *if* those two commits happen in the same second
> in the same process while holding the same repo lock.  For example:
> 
>   commit i:
>     M a
>     M b
>   commit i+1:         # same process, same second, same repo lock
>     M b               # modify b without changing its size
>     M c
> 
> This first manifested in transplant, which is the most common way to
> do multiple commits in the same process. But it can manifest in any
> script or extension that does multiple commits under the same repo
> lock.
> 
> The problem was that dirstate failed to notice the changes to b when
> localrepo is doing the second commit, meaning that change gets left in
> the working directory. In the context of transplant, that means either
> a crash ("RuntimeError: nothing committed after transplant") or a
> silently inaccurate transplant, depending on whether any other files
> were modified by the second transplanted changeset.
> 
> The fix is to work a little harder in the second (and subsequent)
> commits run in the same process:
> - dirstate: factor out maybelookup() from write()
> - localrepo: add _lastcommitfiles attribute, and use it with
>   maybelookup() to force repo.status() to look harder at files
>   added/modified by the *previous* commit
> 
> Incidentally, there is a simpler fix: call dirstate.normallookup() on
> every file modified by commit() at the end of the commit.  The trouble
> with that solution is that it imposes a performance penalty on the
> common case: it means the next status-dependent hg command after every
> "hg commit" will be a little bit slower.  The patch here is more
> complex, but it only affects performance for the uncommon case of
> multiple commits in the same process.

I think you're going to need to break this up. There's a lot going on
here and some of it you haven't told us about. You might start by
introducing getfstime and we can debate whether that's the right thing.

-- 
Mathematics is the supreme nostalgia of our time.