Patchwork [8,of,9,V3] template: better prune support in obsfate

# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1499096336 -7200
#      Mon Jul 03 17:38:56 2017 +0200
# Node ID 7cb7e67addd11cdbecea49ec707e3e9b30bb8677
# Parent  faad6d683f7a30996007116d296df9ebf853c44d
# EXP-Topic obsfatetemplate
template: better prune support in obsfate

successorssets don't returns good results for pruned commit, add a workaround
for simple cases.

A proper fix would require a large rework of successorssets algorithm, I will
send a separate series for this refactoring.

Boris Feld a écrit :
> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1499096336 -7200
> #      Mon Jul 03 17:38:56 2017 +0200
> # Node ID 7cb7e67addd11cdbecea49ec707e3e9b30bb8677
> # Parent  faad6d683f7a30996007116d296df9ebf853c44d
> # EXP-Topic obsfatetemplate
> template: better prune support in obsfate
> 
> successorssets don't returns good results for pruned commit, add a workaround
> for simple cases.
> 
> A proper fix would require a large rework of successorssets algorithm, I will
> send a separate series for this refactoring.
> 
> diff -r faad6d683f7a -r 7cb7e67addd1 mercurial/obsutil.py
> --- a/mercurial/obsutil.py	Mon Jul 03 15:34:10 2017 +0200
> +++ b/mercurial/obsutil.py	Mon Jul 03 17:38:56 2017 +0200
> @@ -614,8 +614,32 @@
>   
>       ssets = successorssets(repo, ctx.node(), closest=True)
>   
> +    # closestsuccessors returns an empty list for pruned revisions, remap it
> +    # into a list containing en empty list for future processing

typo: "an" empty list

> +    if ssets == []:
> +        ssets = [[]]
> +
> +    # Try to recover pruned markers
> +    succsmap = repo.obsstore.successors
> +    fullsuccessorsets = [] # successor set + markers
> +    for sset in ssets:
> +        if sset:
> +            fullsuccessorsets.append(sset)
> +        else:
> +            # XXX we do not catch all prune markers (eg rewritten then pruned)
> +            # (fix me later)
> +            foundany = False
> +            for mark in succsmap.get(ctx.node(), ()):
> +                if not mark[1]:
> +                    foundany = True
> +                    sset = _succs()
> +                    sset.markers.add(mark)
> +                    fullsuccessorsets.append(sset)
> +            if not foundany:
> +                fullsuccessorsets.append(_succs())
> +
>       values = []
> -    for sset in ssets:
> +    for sset in fullsuccessorsets:
>           values.append({'successors': sset, 'markers': sset.markers})
>   
>       return values

Maybe I'm not familiar enough with these things, but I can't figure out
what the algorithm is doing. Would you mind explaining it in a comment?

On Mon, 2017-08-21 at 13:06 +0200, Denis Laxalde wrote:
> Boris Feld a écrit :
> > # HG changeset patch
> > # User Boris Feld <boris.feld@octobus.net>
> > # Date 1499096336 -7200
> > #      Mon Jul 03 17:38:56 2017 +0200
> > # Node ID 7cb7e67addd11cdbecea49ec707e3e9b30bb8677
> > # Parent  faad6d683f7a30996007116d296df9ebf853c44d
> > # EXP-Topic obsfatetemplate
> > template: better prune support in obsfate
> > 
> > successorssets don't returns good results for pruned commit, add a
> > workaround
> > for simple cases.
> > 
> > A proper fix would require a large rework of successorssets
> > algorithm, I will
> > send a separate series for this refactoring.
> > 
> > diff -r faad6d683f7a -r 7cb7e67addd1 mercurial/obsutil.py
> > --- a/mercurial/obsutil.py	Mon Jul 03 15:34:10 2017 +0200
> > +++ b/mercurial/obsutil.py	Mon Jul 03 17:38:56 2017 +0200
> > @@ -614,8 +614,32 @@
> >   
> >       ssets = successorssets(repo, ctx.node(), closest=True)
> >   
> > +    # closestsuccessors returns an empty list for pruned
> > revisions, remap it
> > +    # into a list containing en empty list for future processing
> 
> typo: "an" empty list

Fixed thanks

> 
> > +    if ssets == []:
> > +        ssets = [[]]
> > +
> > +    # Try to recover pruned markers
> > +    succsmap = repo.obsstore.successors
> > +    fullsuccessorsets = [] # successor set + markers
> > +    for sset in ssets:
> > +        if sset:
> > +            fullsuccessorsets.append(sset)
> > +        else:
> > +            # XXX we do not catch all prune markers (eg rewritten
> > then pruned)
> > +            # (fix me later)
> > +            foundany = False
> > +            for mark in succsmap.get(ctx.node(), ()):
> > +                if not mark[1]:
> > +                    foundany = True
> > +                    sset = _succs()
> > +                    sset.markers.add(mark)
> > +                    fullsuccessorsets.append(sset)
> > +            if not foundany:
> > +                fullsuccessorsets.append(_succs())
> > +
> >       values = []
> > -    for sset in ssets:
> > +    for sset in fullsuccessorsets:
> >           values.append({'successors': sset, 'markers':
> > sset.markers})
> >   
> >       return values
> 
> Maybe I'm not familiar enough with these things, but I can't figure
> out
> what the algorithm is doing. Would you mind explaining it in a
> comment?

I've added a comment on the next version of this series, here it is:

# successorsset return an empty set() when ctx or one of its successors
is
# pruned.
# In this case, walk the obs-markers tree again starting with ctx and
find the
# relevant pruning obs-makers, the ones without successors.
# Returning these markers allow us to compute some information about
its fate,
# like who pruned this changeset and when.

# XXX we do not catch all prune markers (eg rewritten then pruned) (fix
me
# later)

What do you think? Is the comment clear?


I will wait for yuja comment on the series before sending a V4.