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

[Yuya Nishihara]

On Sat, 14 Nov 2015 03:46:25 +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> # Date 1447437921 -32400
> #      Sat Nov 14 03:05:21 2015 +0900
> # Branch stable
> # Node ID f90cd44b9860347b58e04e2bf4228bb9566de3f5
> # Parent  24e29f6c10ed52935786e394954e01af8b0e258b
> cmdutil: delay examination of dirstate parents until editor invocation
> 
> Before this patch, 'mergeeditform()' immediately examines dirstate
> parents, even though it may be invoked outside wlock scope (normal "hg
> commit" is typical case).
> 
> This may show "ignoring unknown working parent" message occasionally,
> because current dirstate parents may not be contained in changelog,
> which is cached outside wlock scope (see issue4378 for detail).
> 
> In such case, "editform" may have ".normal" suffix, even if the
> working directory has actually two parents.
> 
> This patch delays examination of dirstate parents until editor
> invocation by returning callable object if examination of dirstate
> parents is needed.
> 
> This callable object is evaluated at external editor invocation, and
> wlock should be already acquired at that point.
> 
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -333,19 +333,26 @@ def logmessage(ui, opts):
>      return message
>  
>  def mergeeditform(ctxorbool, baseformname):
> -    """return appropriate editform name (referencing a committemplate)
> +    """return appropriate editform name or the callable object to get it
>  
>      'ctxorbool' is either a ctx to be committed, or a bool indicating whether
>      merging is committed.
>  
> -    This returns baseformname with '.merge' appended if it is a merge,
> -    otherwise '.normal' is appended.
> +    This (or the returned callable object) returns baseformname with
> +    '.merge' appended if it is a merge, otherwise '.normal' is appended.
>      """
>      if isinstance(ctxorbool, bool):
>          if ctxorbool:
>              return baseformname + ".merge"
> -    elif 1 < len(ctxorbool.parents()):
> -        return baseformname + ".merge"
> +    else:
> +        # delay accessing to changelog via localrepository.dirstate
> +        # (see issue4378 for detail)
> +        def geteditform():
> +            if 1 < len(ctxorbool.parents()):
> +                return baseformname + ".merge"
> +            else:
> +                return baseformname + ".normal"
> +        return geteditform

These patches make me think that the problem is the locking scope is too
narrow to do commit consistently. Is it wrong to take both lock and wlock
earlier, at cmdutil.commit() for example?