Patchwork revset: change ancestor to accept 0 or more arguments (issue3750)

login
register
mail settings
Submitter Paul Cavallaro
Date Feb. 1, 2013, 4:07 p.m.
Message ID <014dd5ef89d8504ee662.1359734831@devrs153.prn1.facebook.com>
Download mbox | patch
Permalink /patch/785/
State Accepted
Commit ae645d4f084c912018efdd89f85b438d751564b5
Headers show

Comments

Paul Cavallaro - Feb. 1, 2013, 4:07 p.m.
# HG changeset patch
# User Paul Cavallaro <ptc@fb.com>
# Date 1359404361 28800
# Branch stable
# Node ID 014dd5ef89d8504ee662ea508f32afaeba3f2f21
# Parent  692cbda1eb50fe30c70792cb1e9380b28769467c
revset: change ancestor to accept 0 or more arguments (issue3750)

Change ancestor to accept 0 or more arguments. The greatest common ancestor of a
single changeset is that changeset. If passed no arguments, the empty list is
returned.
Bryan O'Sullivan - Feb. 1, 2013, 5:26 p.m.
On Fri, Feb 1, 2013 at 8:07 AM, Paul Cavallaro <ptc@fb.com> wrote:

> revset: change ancestor to accept 0 or more arguments (issue3750)
>

Thanks. I'll apply this after the 2.5 freeze.
Bryan O'Sullivan - Feb. 1, 2013, 10:21 p.m.
On Fri, Feb 1, 2013 at 9:26 AM, Bryan O'Sullivan <bos@serpentine.com> wrote:

> Thanks. I'll apply this after the 2.5 freeze.


And both patches are now in. Thanks again!
Siddharth Agarwal - Feb. 7, 2013, 2:51 a.m.
On 02/01/2013 04:07 PM, Paul Cavallaro wrote:
> # HG changeset patch
> # User Paul Cavallaro <ptc@fb.com>
> # Date 1359404361 28800
> # Branch stable
> # Node ID 014dd5ef89d8504ee662ea508f32afaeba3f2f21
> # Parent  692cbda1eb50fe30c70792cb1e9380b28769467c
> revset: change ancestor to accept 0 or more arguments (issue3750)

Is the GCA operation commutative and associative, particularly when 
multiple revs are possible GCA candidates? If not, a left fold (which is 
what was picked here) and right fold could mean different things.
Bryan O'Sullivan - Feb. 7, 2013, 4:29 p.m.
On Wed, Feb 6, 2013 at 6:51 PM, Siddharth Agarwal <sid0@fb.com> wrote:

> Is the GCA operation commutative and associative, particularly when
> multiple revs are possible GCA candidates?
>

In principle yes, in practice we get that wrong today (and this is no worse
than the existing code).

When we get around to replacing the ancestor code with the more general GCA
code, I'll fix this.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -278,20 +278,32 @@ 
     return checkstatus(repo, subset, pat, 1)
 
 def ancestor(repo, subset, x):
-    """``ancestor(single, single)``
-    Greatest common ancestor of the two changesets.
+    """``ancestor(*changeset)``
+    Greatest common ancestor of the changesets.
+
+    Accepts 0 or more changesets.
+    Will return empty list when passed no args.
+    Greatest common ancestor of a single changeset is that changeset.
     """
     # i18n: "ancestor" is a keyword
-    l = getargs(x, 2, 2, _("ancestor requires two arguments"))
-    r = list(repo)
-    a = getset(repo, r, l[0])
-    b = getset(repo, r, l[1])
-    if len(a) != 1 or len(b) != 1:
-        # i18n: "ancestor" is a keyword
-        raise error.ParseError(_("ancestor arguments must be single revisions"))
-    an = [repo[a[0]].ancestor(repo[b[0]]).rev()]
+    l = getlist(x)
+    rl = list(repo)
+    anc = None
 
-    return [r for r in an if r in subset]
+    # (getset(repo, rl, i) for i in l) generates a list of lists
+    rev = repo.changelog.rev
+    ancestor = repo.changelog.ancestor
+    node = repo.changelog.node
+    for revs in (getset(repo, rl, i) for i in l):
+        for r in revs:
+            if anc is None:
+                anc = r
+            else:
+                anc = rev(ancestor(node(anc), node(r)))
+
+    if anc is not None and anc in subset:
+        return [anc]
+    return []
 
 def _ancestors(repo, subset, x, followfirst=False):
     args = getset(repo, list(repo), x)
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -218,17 +218,29 @@ 
   $ log 'date(2005) and 1::'
   4
 
+ancestor can accept 0 or more arguments
+
+  $ log 'ancestor()'
   $ log 'ancestor(1)'
-  hg: parse error: ancestor requires two arguments
-  [255]
+  1
   $ log 'ancestor(4,5)'
   1
   $ log 'ancestor(4,5) and 4'
+  $ log 'ancestor(0,0,1,3)'
+  0
+  $ log 'ancestor(3,1,5,3,5,1)'
+  1
+  $ log 'ancestor(0,1,3,5)'
+  0
+  $ log 'ancestor(1,2,3,4,5)'
+  1
   $ log 'ancestors(5)'
   0
   1
   3
   5
+  $ log 'ancestor(ancestors(5))'
+  0
   $ log 'author(bob)'
   2
   $ log 'author("re:bob|test")'