[PATCH 3 of 3 STABLE?] cmdutil: delay examination of dirstate parents until editor invocation

FUJIWARA Katsunori foozy at lares.dti.ne.jp
Wed Nov 18 11:45:28 CST 2015


At Wed, 18 Nov 2015 23:51:40 +0900,
Yuya Nishihara wrote:
> 
> On Wed, 18 Nov 2015 00:26:03 +0900, FUJIWARA Katsunori wrote:
> > At Tue, 17 Nov 2015 22:29:32 +0900,
> > Yuya Nishihara wrote:
> > > On Mon, 16 Nov 2015 18:54:00 -0800, Pierre-Yves David wrote:
> > > > Patch 1 seems correct in itself, Foozy, can you confirm we should still 
> > > > take it ?
> > > 
> > > I consider the patch 1 is also a locking scope issue. If repo.lock is taken,
> > > dirstate.parents() can see the fresh changelog after repo.invalidate().
> > 
> > After surrounding whole 'commands.commit()' with wlock and (s)lock
> > scope, invalidating changelog and so on at 'repo.wlock()' (like patch
> > #1 of this series) seems redundant, from point of view of Mercurial
> > source tree.
> > 
> > AFAIK, 'repo.branchheads()', 'cmdutil.mergeeditform()' and so on are
> > invoked inside wlock/(s)lock scope, except for current "hg commit"
> > without --amend code path.
> > 
> > On the other hand, patch #1 is still useful for 3rd party extensions,
> > which invoke 'repo.commit()' directly without acquisition of
> > wlock/(s)lock and expect it to refer recent 'dirstate.parents()'
> > correctly.
> 
> Perhaps repo.commit() can take "lock" again to make sure both dirstate
> and changelog are consistent?

From point of view of "dirstate parents is sensitive to changelog
changes", it seems reasonable to acquire "lock" explicitly at the
beginning of 'repo.commit()', as you mentioned.

I'll post #1 patch revised so (maybe after discussion about exact
change detection RFC).

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp


More information about the Mercurial-devel mailing list