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

login
register
mail settings
Submitter Boris Feld
Date Aug. 21, 2017, 8:44 a.m.
Message ID <7cb7e67addd11cdbecea.1503305040@FB>
Download mbox | patch
Permalink /patch/23175/
State Superseded
Headers show

Comments

Boris Feld - Aug. 21, 2017, 8:44 a.m.
# 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.
Denis Laxalde - Aug. 21, 2017, 11:06 a.m.
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?
Boris Feld - Aug. 21, 2017, 1:50 p.m.
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.

Patch

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
+    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
diff -r faad6d683f7a -r 7cb7e67addd1 tests/test-obsmarker-template.t
--- a/tests/test-obsmarker-template.t	Mon Jul 03 15:34:10 2017 +0200
+++ b/tests/test-obsmarker-template.t	Mon Jul 03 17:38:56 2017 +0200
@@ -12,7 +12,7 @@ 
   > [experimental]
   > stabilization=all
   > [templates]
-  > obsfatesuccessors = " as {join(successors, ", ")}"
+  > obsfatesuccessors = "{if(successors, " as ")}{join(successors, ", ")}"
   > obsfateverb = "{obsfateverb(successors, markers)}"
   > obsfateuserstmpl = "{if(users, " by {join(users, ", ")}")}"
   > obsfateusers = "{obsfateusers(successors, markers) % "{obsfateuserstmpl}"}"
@@ -167,7 +167,7 @@ 
   | @  a468dc9b3633
   |/     Obsfate: rewritten as 4:d004c8f274b9 by test2 (at 2001-04-19 04:25 +0000);
   | x  f137d23bb3e1
-  | |
+  | |    Obsfate: pruned by test1 (at 2009-02-13 23:31 +0000);
   | x  471f378eab4c
   |/     Obsfate: rewritten as 3:a468dc9b3633 by test1 (at 2009-02-13 23:31 +0000);
   o  ea207398892e
@@ -216,7 +216,7 @@ 
   | x  a468dc9b3633
   |/     Obsfate: rewritten as 4:d004c8f274b9 by test2 (at 2001-04-19 04:25 +0000);
   | x  f137d23bb3e1
-  | |
+  | |    Obsfate: pruned by test1 (at 2009-02-13 23:31 +0000);
   | x  471f378eab4c
   |/     Obsfate: rewritten as 3:a468dc9b3633 by test1 (at 2009-02-13 23:31 +0000);
   o  ea207398892e
@@ -227,7 +227,7 @@ 
   | x  a468dc9b3633
   |/     Obsfate: [{"markers": [["a468dc9b36338b14fdb7825f55ce3df4e71517ad", ["d004c8f274b9ec480a47a93c10dac5eee63adb78"], 0, [["user", "test2"]], [987654321.0, 0], null]], "successors": ["d004c8f274b9ec480a47a93c10dac5eee63adb78"]}]
   | x  f137d23bb3e1
-  | |
+  | |    Obsfate: [{"markers": [["f137d23bb3e11dc1daeb6264fac9cb2433782e15", [], 0, [["user", "test1"]], [1234567890.0, 0], ["471f378eab4c5e25f6c77f785b27c936efb22874"]]], "successors": []}]
   | x  471f378eab4c
   |/     Obsfate: [{"markers": [["471f378eab4c5e25f6c77f785b27c936efb22874", ["a468dc9b36338b14fdb7825f55ce3df4e71517ad"], 0, [["user", "test1"]], [1234567890.0, 0], null]], "successors": ["a468dc9b36338b14fdb7825f55ce3df4e71517ad"]}]
   o  ea207398892e
@@ -1220,7 +1220,7 @@ 
   o  f897c6137566
   |
   | @  471f378eab4c
-  |/
+  |/     Obsfate: pruned;
   o  ea207398892e
   
 
@@ -1637,11 +1637,11 @@ 
   
   $ hg fatelog
   @  471f378eab4c
-  |
+  |    Obsfate: pruned by test (at 1970-01-01 00:00 +0000);
   o  ea207398892e
   
   $ hg fatelog -v
   @  471f378eab4c
-  |
+  |    Obsfate: pruned by test (at 1970-01-01 00:00 +0000);
   o  ea207398892e