D3818: scmutil: make cleanupnodes optionally also fix the phase

[Yuya Nishihara]

> +def cleanupnodes(repo, replacements, operation, moves=None, metadata=None,
> +                 fixphase=False):
>      """do common cleanups when old nodes are replaced by new nodes
>  
>      That includes writing obsmarkers or stripping nodes, and moving bookmarks.
> @@ -825,11 +826,34 @@
>              newnode = newnodes[0]
>          moves[oldnode] = newnode
>  
> +    allnewnodes = [n for ns in replacements.values() for n in ns]
> +    toretract = {}
> +    toadvance = {}
> +    if fixphase:
> +        precursors = {}
> +        for oldnode, newnodes in replacements.items():
> +            for newnode in newnodes:
> +                precursors.setdefault(newnode, []).append(oldnode)
> +
> +        allnewnodes.sort(key=lambda n: repo[n].rev())
> +        newphase = {}
> +        def phase(ctx):
> +            return newphase.get(ctx.node(), ctx.phase())
> +        for newnode in allnewnodes:
> +            ctx = repo[newnode]
> +            oldphase = max(repo[oldnode].phase()
> +                           for oldnode in precursors[newnode])
> +            parentphase = max(phase(p) for p in ctx.parents())
> +            newphase[newnode] = max(oldphase, parentphase)
> +            if newphase[newnode] > ctx.phase():
> +                toretract.setdefault(newphase[newnode], []).append(newnode)
> +            elif newphase[newnode] < ctx.phase():
> +                toadvance.setdefault(newphase[newnode], []).append(newnode)
> +
>      with repo.transaction('cleanup') as tr:
>          # Move bookmarks
>          bmarks = repo._bookmarks
>          bmarkchanges = []
> -        allnewnodes = [n for ns in replacements.values() for n in ns]
>          for oldnode, newnode in moves.items():
>              oldbmarks = repo.nodebookmarks(oldnode)
>              if not oldbmarks:
> @@ -850,6 +874,11 @@
>          if bmarkchanges:
>              bmarks.applychanges(repo, tr, bmarkchanges)
>  
> +        for phase, nodes in toretract.items():
> +            phases.retractboundary(repo, tr, phase, nodes)
> +        for phase, nodes in toadvance.items():
> +            phases.advanceboundary(repo, tr, phase, nodes)

The logic looks good to me, but I'm not sure if recomputing a proper phase
move here is better than the original config override.

> --- a/hgext/fix.py
> +++ b/hgext/fix.py
> @@ -158,7 +158,7 @@
>                  del filedata[rev]
>  
>          replacements = {prec: [succ] for prec, succ in replacements.iteritems()}
> -        scmutil.cleanupnodes(repo, replacements, 'fix')
> +        scmutil.cleanupnodes(repo, replacements, 'fix', fixphase=True)

Perhaps this has to be wrapped by a transaction. Otherwise, I think a secret
changeset could be temporarily exposed until cleanupnodes().