[PATCH v2] histedit: make histedit aware of obsoletions not stored in state (issue4800)

[Pierre-Yves David]

(urg previous email sent too fast)

On 02/22/2016 03:50 PM, Kostia Balytskyi wrote:
> # HG changeset patch
> # User Kostia Balytskyi <ikostia at fb.com>
> # Date 1456152481 0
> #      Mon Feb 22 14:48:01 2016 +0000
> # Node ID 426bfeb7144bb8ad34aefa003f39be15217ccc54
> # Parent  28e1694ca60056d609ae2c8e0ad5cb2891416ea3
> histedit: make histedit aware of obsoletions not stored in state (issue4800)
>
> Before this change, when histedit exited to interactive session (during edit
> command for example), user could introduce obsoletion changes that would not
> be known to histedit. For example, user could've amended one of the commits
> (introducing an obsoletion change). The fact of this amendment would not be
> stored in histedit's state file and later, when histedit would try to process
> all the replacements that happened, one of the final successors (in its opinion)
> would turn out to be hidden. This behavior is described in issue4800. This
> commit fixes it.
>
> diff --git a/hg b/hg
> --- a/hg
> +++ b/hg
> @@ -39,5 +39,4 @@
>
>   for fp in (sys.stdin, sys.stdout, sys.stderr):
>       mercurial.util.setbinary(fp)
> -
>   mercurial.dispatch.run()
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -1386,6 +1386,34 @@
>                   hint=_('use "drop %s" to discard, see also: '
>                          '"hg help -e histedit.config"') % missing[0][:12])
>
> +def adjustreplacementsfrommarkers(repo, allsuccs, replaced, fullmapping):
> +    """Adjusts replacement structures with data from obsoletion markers
> +
> +    These structures are originally generated only based on
> +    histedit state and do not account for changes that are
> +    not recorded there. This function fixes that"""
> +    if not obsolete.isenabled(repo, obsolete.createmarkersopt):
> +        return
> +
> +    succstocheck = list(allsuccs)
> +    while succstocheck:
> +        n = succstocheck.pop()
> +        ctx = repo.unfiltered()[n]
> +        firstiteration = True
> +        for marker in obsolete.successormarkers(ctx):
> +            for nsucc in marker.succnodes():
> +                # nsucc is a successor of n
> +                if firstiteration:
> +                    # this is the first time we see that node n
> +                    # was replaced by something
> +                    firstiteration = False
> +                    replaced.add(n)

This update to replacement is actually getting me a bit surprised. cf 
below ?

> +                if nsucc not in allsuccs:
> +                    # save nsucc for further investigation
> +                    allsuccs.add(nsucc)
> +                    succstocheck.append(nsucc)
> +                fullmapping.setdefault(n, set()).add(nsucc)
> +
>   def processreplacement(state):
>       """process the list of replacements to return
>
> @@ -1402,6 +1430,7 @@
>           allsuccs.update(rep[1])
>           replaced.add(rep[0])
>           fullmapping.setdefault(rep[0], set()).update(rep[1])
> +    adjustreplacementsfrommarkers(state.repo, allsuccs, replaced, fullmapping)

We shouln't we read data related to marker before we start any 
processing? so that processreplacement just read and process full 
mapping in "on block" ?

-- 
Pierre-Yves David