Patchwork [4,of,4] revset: avoid returning duplicates when returning ancestors

login
register
mail settings
Submitter Pierre-Yves David
Date May 4, 2015, 10:25 p.m.
Message ID <81d963f466607d9425d7.1430778355@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/8882/
State Accepted
Headers show

Comments

Pierre-Yves David - May 4, 2015, 10:25 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1395874550 25200
#      Wed Mar 26 15:55:50 2014 -0700
# Node ID 81d963f466607d9425d79adfed374c360fbcb5f1
# Parent  dffaf3d15f9587d3f4e00a03cd288108c5cc78fe
revset: avoid returning duplicates when returning ancestors

Before this patch, _revancestors were giving false result when a revision was
duplicated in the input. Duplicated entry are rare but may happen when using the
`%lx` notation internally.

This series has no visible impact on the performance of the function according
to benchmark.
Martin von Zweigbergk - May 6, 2015, 9:05 p.m.
I have now pushed patches 1 and 2, followed by my own patch, followed by 3
and 4, to clowncopter. Thanks!

On Mon, May 4, 2015 at 3:29 PM Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1395874550 25200
> #      Wed Mar 26 15:55:50 2014 -0700
> # Node ID 81d963f466607d9425d79adfed374c360fbcb5f1
> # Parent  dffaf3d15f9587d3f4e00a03cd288108c5cc78fe
> revset: avoid returning duplicates when returning ancestors
>
> Before this patch, _revancestors were giving false result when a revision
> was
> duplicated in the input. Duplicated entry are rare but may happen when
> using the
> `%lx` notation internally.
>
> This series has no visible impact on the performance of the function
> according
> to benchmark.
>
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -36,17 +36,17 @@ def _revancestors(repo, revs, followfirs
>              irevs, inputrev = None, None
>
>          seen = set()
>          while h:
>              current = -heapq.heappop(h)
> +            if irevs is not None and current == inputrev:
> +                try:
> +                    inputrev = irevs.next()
> +                    heapq.heappush(h, -inputrev)
> +                except StopIteration:
> +                    irevs, inputrev = None, None
>              if current not in seen:
> -                if irevs is not None and current == inputrev:
> -                    try:
> -                        inputrev = irevs.next()
> -                        heapq.heappush(h, -inputrev)
> -                    except StopIteration:
> -                        irevs, inputrev = None, None
>                  seen.add(current)
>                  yield current
>                  for parent in cl.parentrevs(current)[:cut]:
>                      if parent != node.nullrev:
>                          heapq.heappush(h, -parent)
> diff --git a/tests/test-revset.t b/tests/test-revset.t
> --- a/tests/test-revset.t
> +++ b/tests/test-revset.t
> @@ -1,7 +1,29 @@
>    $ HGENCODING=utf-8
>    $ export HGENCODING
> +  $ cat > testrevset.py << EOF
> +  > import mercurial.revset
> +  >
> +  > baseset = mercurial.revset.baseset
> +  >
> +  > def r3232(repo, subset, x):
> +  >     """"simple revset that return [3,2,3,2]
> +  >
> +  >     revisions duplicated on purpose.
> +  >     """
> +  >     if 3 not in subset:
> +  >        if 2 in subset:
> +  >            return baseset([2,2])
> +  >        return baseset()
> +  >     return baseset([3,3,2,2])
> +  >
> +  > mercurial.revset.symbols['r3232'] = r3232
> +  > EOF
> +  $ cat >> $HGRCPATH << EOF
> +  > [extensions]
> +  > testrevset=$TESTTMP/testrevset.py
> +  > EOF
>
>    $ try() {
>    >   hg debugrevspec --debug "$@"
>    > }
>
> @@ -309,17 +331,26 @@ ancestor can accept 0 or more arguments
>    1
>    $ log 'ancestor(0,1,3,5)'
>    0
>    $ log 'ancestor(1,2,3,4,5)'
>    1
> +
> +test ancestors
> +
>    $ log 'ancestors(5)'
>    0
>    1
>    3
>    5
>    $ log 'ancestor(ancestors(5))'
>    0
> +  $ log '::r3232()'
> +  0
> +  1
> +  2
> +  3
> +
>    $ log 'author(bob)'
>    2
>    $ log 'author("re:bob|test")'
>    0
>    1
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -36,17 +36,17 @@  def _revancestors(repo, revs, followfirs
             irevs, inputrev = None, None
 
         seen = set()
         while h:
             current = -heapq.heappop(h)
+            if irevs is not None and current == inputrev:
+                try:
+                    inputrev = irevs.next()
+                    heapq.heappush(h, -inputrev)
+                except StopIteration:
+                    irevs, inputrev = None, None
             if current not in seen:
-                if irevs is not None and current == inputrev:
-                    try:
-                        inputrev = irevs.next()
-                        heapq.heappush(h, -inputrev)
-                    except StopIteration:
-                        irevs, inputrev = None, None
                 seen.add(current)
                 yield current
                 for parent in cl.parentrevs(current)[:cut]:
                     if parent != node.nullrev:
                         heapq.heappush(h, -parent)
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -1,7 +1,29 @@ 
   $ HGENCODING=utf-8
   $ export HGENCODING
+  $ cat > testrevset.py << EOF
+  > import mercurial.revset
+  > 
+  > baseset = mercurial.revset.baseset
+  > 
+  > def r3232(repo, subset, x):
+  >     """"simple revset that return [3,2,3,2]
+  > 
+  >     revisions duplicated on purpose.
+  >     """
+  >     if 3 not in subset:
+  >        if 2 in subset:
+  >            return baseset([2,2])
+  >        return baseset()
+  >     return baseset([3,3,2,2])
+  > 
+  > mercurial.revset.symbols['r3232'] = r3232
+  > EOF
+  $ cat >> $HGRCPATH << EOF
+  > [extensions]
+  > testrevset=$TESTTMP/testrevset.py
+  > EOF
 
   $ try() {
   >   hg debugrevspec --debug "$@"
   > }
 
@@ -309,17 +331,26 @@  ancestor can accept 0 or more arguments
   1
   $ log 'ancestor(0,1,3,5)'
   0
   $ log 'ancestor(1,2,3,4,5)'
   1
+
+test ancestors
+
   $ log 'ancestors(5)'
   0
   1
   3
   5
   $ log 'ancestor(ancestors(5))'
   0
+  $ log '::r3232()'
+  0
+  1
+  2
+  3
+
   $ log 'author(bob)'
   2
   $ log 'author("re:bob|test")'
   0
   1