Patchwork [3,of,3,V2] revset: cleanup of algorithm for `destination()` predicate

login
register
mail settings
Submitter Hannes Oldenburg
Date Sept. 11, 2016, 3:06 p.m.
Message ID <7eb48191ac8b5c679b8c.1473606360@localhost.localdomain>
Download mbox | patch
Permalink /patch/16584/
State Accepted
Headers show

Comments

Hannes Oldenburg - Sept. 11, 2016, 3:06 p.m.
# HG changeset patch
# User Hannes Oldenburg <hannes.christian.oldenburg@gmail.com>
# Date 1473552945 0
#      Sun Sep 11 00:15:45 2016 +0000
# Node ID 7eb48191ac8b5c679b8c606b2b83c0306180b97c
# Parent  05b4f301b5ade476f7f2236f7bf699a4f248297d
revset: cleanup of algorithm for `destination()` predicate

Previously the first `_getrevsource()` was treated as a special case, before
entering the loop and we kept track of two revs `r` and `src`, a target and
it's source respectively.

The argument for the correctness of the new approach goes like this:

We immediately add `r` to the lineage.
We update `r` to it's source, if it has no source `r` becomes None.
None is neither in `dests` nor in `sources`, so the lineage is thrown away
and the contents do not matter.
If however at some point, while following the sources, `r` is in `sources` or
`dests`, the lineage contains every preceding target and is added to `dests`.
timeless - Sept. 12, 2016, 6:47 a.m.
> We update `r` to it's source, if it has no source `r` becomes None.

This should say "its"
Augie Fackler - Sept. 12, 2016, 1:25 p.m.
On Sun, Sep 11, 2016 at 03:06:00PM +0000, Hannes Oldenburg wrote:
> # HG changeset patch
> # User Hannes Oldenburg <hannes.christian.oldenburg@gmail.com>
> # Date 1473552945 0
> #      Sun Sep 11 00:15:45 2016 +0000
> # Node ID 7eb48191ac8b5c679b8c606b2b83c0306180b97c
> # Parent  05b4f301b5ade476f7f2236f7bf699a4f248297d
> revset: cleanup of algorithm for `destination()` predicate

Nice cleanup. Queued, thanks

>
> Previously the first `_getrevsource()` was treated as a special case, before
> entering the loop and we kept track of two revs `r` and `src`, a target and
> it's source respectively.
>
> The argument for the correctness of the new approach goes like this:
>
> We immediately add `r` to the lineage.
> We update `r` to it's source, if it has no source `r` becomes None.
> None is neither in `dests` nor in `sources`, so the lineage is thrown away
> and the contents do not matter.
> If however at some point, while following the sources, `r` is in `sources` or
> `dests`, the lineage contains every preceding target and is added to `dests`.
>
> diff -r 05b4f301b5ad -r 7eb48191ac8b mercurial/revset.py
> --- a/mercurial/revset.py	Fri Sep 09 20:57:37 2016 +0000
> +++ b/mercurial/revset.py	Sun Sep 11 00:15:45 2016 +0000
> @@ -851,23 +851,20 @@
>      # transitive transplants and rebases to yield the same results as transitive
>      # grafts.
>      for r in subset:
> -        src = _getrevsource(repo, r)
>          lineage = list()
>          found = False
> -        while src is not None and not found:
> +        while r is not None and not found:
>              lineage.append(r)
> -
> +            r = _getrevsource(repo, r)
>              # The visited lineage is a match if the current source is in the arg
>              # set.  Since every candidate dest is visited by way of iterating
>              # subset, any dests further back in the lineage will be tested by a
>              # different iteration over subset.  Likewise, if the src was already
>              # selected, the current lineage can be selected without going back
>              # further.
> -            found = src in sources or src in dests
> -            r = src
> -            src = _getrevsource(repo, r)
> +            found = r in sources or r in dests
>          if found:
> -            dests.update(linage)
> +            dests.update(lineage)
>      return subset.filter(dests.__contains__,
>                           condrepr=lambda: '<destination %r>' % sorted(dests))
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff -r 05b4f301b5ad -r 7eb48191ac8b mercurial/revset.py
--- a/mercurial/revset.py	Fri Sep 09 20:57:37 2016 +0000
+++ b/mercurial/revset.py	Sun Sep 11 00:15:45 2016 +0000
@@ -851,23 +851,20 @@ 
     # transitive transplants and rebases to yield the same results as transitive
     # grafts.
     for r in subset:
-        src = _getrevsource(repo, r)
         lineage = list()
         found = False
-        while src is not None and not found:
+        while r is not None and not found:
             lineage.append(r)
-
+            r = _getrevsource(repo, r)
             # The visited lineage is a match if the current source is in the arg
             # set.  Since every candidate dest is visited by way of iterating
             # subset, any dests further back in the lineage will be tested by a
             # different iteration over subset.  Likewise, if the src was already
             # selected, the current lineage can be selected without going back
             # further.
-            found = src in sources or src in dests
-            r = src
-            src = _getrevsource(repo, r)
+            found = r in sources or r in dests
         if found:
-            dests.update(linage)
+            dests.update(lineage)
     return subset.filter(dests.__contains__,
                          condrepr=lambda: '<destination %r>' % sorted(dests))