Patchwork [1,of,9,V3] obsolete: introduce a _succs class

login
register
mail settings
Submitter Boris Feld
Date Aug. 21, 2017, 8:43 a.m.
Message ID <328789121fa66b99c50f.1503305033@FB>
Download mbox | patch
Permalink /patch/23168/
State Superseded
Headers show

Comments

Boris Feld - Aug. 21, 2017, 8:43 a.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1499036035 -7200
#      Mon Jul 03 00:53:55 2017 +0200
# Node ID 328789121fa66b99c50ffa2fb139df700cf9fdc8
# Parent  3cfc9070245fbaa8c4c010327f24d579a416370f
# EXP-Topic obsfatetemplate
obsolete: introduce a _succs class

It will be useful later when we will be adding markers to _succs in order to
represent a successorset with the list of markers from the root to each
successors sets. This information will be needed for the obsfate template I will
introduce.

Makes it a subclass of list so all callers will continue to work.
Denis Laxalde - Aug. 21, 2017, 10:45 a.m.
Boris Feld a écrit :
> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1499036035 -7200
> #      Mon Jul 03 00:53:55 2017 +0200
> # Node ID 328789121fa66b99c50ffa2fb139df700cf9fdc8
> # Parent  3cfc9070245fbaa8c4c010327f24d579a416370f
> # EXP-Topic obsfatetemplate
> obsolete: introduce a _succs class
> 
> It will be useful later when we will be adding markers to _succs in order to
> represent a successorset with the list of markers from the root to each
> successors sets. This information will be needed for the obsfate template I will
> introduce.
> 
> Makes it a subclass of list so all callers will continue to work.
> 
> diff -r 3cfc9070245f -r 328789121fa6 mercurial/obsutil.py
> --- a/mercurial/obsutil.py	Wed Aug 16 10:44:06 2017 -0700
> +++ b/mercurial/obsutil.py	Mon Jul 03 00:53:55 2017 +0200
> @@ -327,6 +327,9 @@
>               obsoleted.add(rev)
>       return obsoleted
>   
> +class _succs(list):
> +    """small class to represent a successors with some metadata about it"""
> +
>   def successorssets(repo, initialnode, closest=False, cache=None):
>       """Return set of all latest successors of initial nodes
>   
> @@ -445,11 +448,11 @@
>               # case (2): end of walk.
>               if current in repo:
>                   # We have a valid successors.
> -                cache[current] = [(current,)]
> +                cache[current] = [_succs((current,))]
>               else:
>                   # Final obsolete version is unknown locally.
>                   # Do not count that as a valid successors
> -                cache[current] = []
> +                cache[current] = _succs()

While equivalent by value (at this point), it seems to me that this
should be kept as an empty list in "else" case (to keep
`type(cache[current]) == list`).

>           else:
>               # cases (3) and (4)
>               #
> @@ -487,7 +490,7 @@
>                       if suc not in cache:
>                           if suc in stackedset:
>                               # cycle breaking
> -                            cache[suc] = []
> +                            cache[suc] = _succs()
>                           else:
>                               # case (3) If we have not computed successors sets
>                               # of one of those successors we add it to the
> @@ -521,13 +524,13 @@
>                   succssets = []
>                   for mark in sorted(succmarkers[current]):
>                       # successors sets contributed by this marker
> -                    markss = [[]]
> +                    markss = [_succs()]
>                       for suc in mark[1]:
>                           # cardinal product with previous successors
>                           productresult = []
>                           for prefix in markss:
>                               for suffix in cache[suc]:
> -                                newss = list(prefix)
> +                                newss = _succs(prefix)
>                                   for part in suffix:
>                                       # do not duplicated entry in successors set
>                                       # first entry wins.
Boris Feld - Aug. 21, 2017, 1:01 p.m.
On Mon, 2017-08-21 at 12:45 +0200, Denis Laxalde wrote:
> Boris Feld a écrit :
> > # HG changeset patch
> > # User Boris Feld <boris.feld@octobus.net>
> > # Date 1499036035 -7200
> > #      Mon Jul 03 00:53:55 2017 +0200
> > # Node ID 328789121fa66b99c50ffa2fb139df700cf9fdc8
> > # Parent  3cfc9070245fbaa8c4c010327f24d579a416370f
> > # EXP-Topic obsfatetemplate
> > obsolete: introduce a _succs class
> > 
> > It will be useful later when we will be adding markers to _succs in
> > order to
> > represent a successorset with the list of markers from the root to
> > each
> > successors sets. This information will be needed for the obsfate
> > template I will
> > introduce.
> > 
> > Makes it a subclass of list so all callers will continue to work.
> > 
> > diff -r 3cfc9070245f -r 328789121fa6 mercurial/obsutil.py
> > --- a/mercurial/obsutil.py	Wed Aug 16 10:44:06 2017 -0700
> > +++ b/mercurial/obsutil.py	Mon Jul 03 00:53:55 2017 +0200
> > @@ -327,6 +327,9 @@
> >               obsoleted.add(rev)
> >       return obsoleted
> >   
> > +class _succs(list):
> > +    """small class to represent a successors with some metadata
> > about it"""
> > +
> >   def successorssets(repo, initialnode, closest=False, cache=None):
> >       """Return set of all latest successors of initial nodes
> >   
> > @@ -445,11 +448,11 @@
> >               # case (2): end of walk.
> >               if current in repo:
> >                   # We have a valid successors.
> > -                cache[current] = [(current,)]
> > +                cache[current] = [_succs((current,))]
> >               else:
> >                   # Final obsolete version is unknown locally.
> >                   # Do not count that as a valid successors
> > -                cache[current] = []
> > +                cache[current] = _succs()
> 
> While equivalent by value (at this point), it seems to me that this
> should be kept as an empty list in "else" case (to keep
> `type(cache[current]) == list`).

You are right, cache values type should be list. I've fixed this
specific line and another for cycle-breaking (just below). It will be
part of a V4.

> 
> >           else:
> >               # cases (3) and (4)
> >               #
> > @@ -487,7 +490,7 @@
> >                       if suc not in cache:
> >                           if suc in stackedset:
> >                               # cycle breaking
> > -                            cache[suc] = []
> > +                            cache[suc] = _succs()
> >                           else:
> >                               # case (3) If we have not computed
> > successors sets
> >                               # of one of those successors we add
> > it to the
> > @@ -521,13 +524,13 @@
> >                   succssets = []
> >                   for mark in sorted(succmarkers[current]):
> >                       # successors sets contributed by this marker
> > -                    markss = [[]]
> > +                    markss = [_succs()]
> >                       for suc in mark[1]:
> >                           # cardinal product with previous
> > successors
> >                           productresult = []
> >                           for prefix in markss:
> >                               for suffix in cache[suc]:
> > -                                newss = list(prefix)
> > +                                newss = _succs(prefix)
> >                                   for part in suffix:
> >                                       # do not duplicated entry in
> > successors set
> >                                       # first entry wins.

Patch

diff -r 3cfc9070245f -r 328789121fa6 mercurial/obsutil.py
--- a/mercurial/obsutil.py	Wed Aug 16 10:44:06 2017 -0700
+++ b/mercurial/obsutil.py	Mon Jul 03 00:53:55 2017 +0200
@@ -327,6 +327,9 @@ 
             obsoleted.add(rev)
     return obsoleted
 
+class _succs(list):
+    """small class to represent a successors with some metadata about it"""
+
 def successorssets(repo, initialnode, closest=False, cache=None):
     """Return set of all latest successors of initial nodes
 
@@ -445,11 +448,11 @@ 
             # case (2): end of walk.
             if current in repo:
                 # We have a valid successors.
-                cache[current] = [(current,)]
+                cache[current] = [_succs((current,))]
             else:
                 # Final obsolete version is unknown locally.
                 # Do not count that as a valid successors
-                cache[current] = []
+                cache[current] = _succs()
         else:
             # cases (3) and (4)
             #
@@ -487,7 +490,7 @@ 
                     if suc not in cache:
                         if suc in stackedset:
                             # cycle breaking
-                            cache[suc] = []
+                            cache[suc] = _succs()
                         else:
                             # case (3) If we have not computed successors sets
                             # of one of those successors we add it to the
@@ -521,13 +524,13 @@ 
                 succssets = []
                 for mark in sorted(succmarkers[current]):
                     # successors sets contributed by this marker
-                    markss = [[]]
+                    markss = [_succs()]
                     for suc in mark[1]:
                         # cardinal product with previous successors
                         productresult = []
                         for prefix in markss:
                             for suffix in cache[suc]:
-                                newss = list(prefix)
+                                newss = _succs(prefix)
                                 for part in suffix:
                                     # do not duplicated entry in successors set
                                     # first entry wins.