Patchwork [3,of,3,gca-revset] revset: add a new commonancestorheads method

login
register
mail settings
Submitter Sean Farley
Date June 15, 2018, 7:10 a.m.
Message ID <d44266127f1a86af521d.1529046625@1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa>
Download mbox | patch
Permalink /patch/32161/
State New
Headers show

Comments

Sean Farley - June 15, 2018, 7:10 a.m.
# HG changeset patch
# User Sean Farley <sean@farley.io>
# Date 1527357855 -7200
#      Sat May 26 20:04:15 2018 +0200
# Branch gca-revset
# Node ID d44266127f1a86af521df2b2c088528c3f3b803a
# Parent  ab29cfd39f48432d8e7c38cdceb62980d5c22f09
revset: add a new commonancestorheads method

This is a new revset that provides a public interface to the
ancestor.commonancestorsheads method. Currently, we have no fast revset
way to get the ancestors of a list of changesets as returned by the
consensus bid merge algorithm (the one needed by criss-cross merges).

Previously, the only way to get these commits were (tested on
mozilla-central):

hg perfrevset 'heads(::a7cf55 and ::d8b15)'
! wall 4.988366 comb 4.960000 user 4.780000 sys 0.180000 (best of 3)

After this patch:

hg perfrevset 'commonancestorheads(a7cf55, d8b15)'
! wall 0.001790 comb 0.000000 user 0.000000 sys 0.000000 (best of 1340)

I'm not a fan of added more revset methods for performance but, yet,
here we are.
Yuya Nishihara - June 15, 2018, 11:37 a.m.
On Fri, 15 Jun 2018 00:10:25 -0700, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean@farley.io>
> # Date 1527357855 -7200
> #      Sat May 26 20:04:15 2018 +0200
> # Branch gca-revset
> # Node ID d44266127f1a86af521df2b2c088528c3f3b803a
> # Parent  ab29cfd39f48432d8e7c38cdceb62980d5c22f09
> revset: add a new commonancestorheads method
> 
> This is a new revset that provides a public interface to the
> ancestor.commonancestorsheads method. Currently, we have no fast revset
> way to get the ancestors of a list of changesets as returned by the
> consensus bid merge algorithm (the one needed by criss-cross merges).
> 
> Previously, the only way to get these commits were (tested on
> mozilla-central):
> 
> hg perfrevset 'heads(::a7cf55 and ::d8b15)'
> ! wall 4.988366 comb 4.960000 user 4.780000 sys 0.180000 (best of 3)
> 
> After this patch:
> 
> hg perfrevset 'commonancestorheads(a7cf55, d8b15)'
> ! wall 0.001790 comb 0.000000 user 0.000000 sys 0.000000 (best of 1340)
> 
> I'm not a fan of added more revset methods for performance but, yet,
> here we are.
> 
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> index de543df..4004f4b 100644
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -299,10 +299,34 @@ def adds(repo, subset, x):
>      """
>      # i18n: "adds" is a keyword
>      pat = getstring(x, _("adds requires a pattern"))
>      return checkstatus(repo, subset, pat, 1)
>  
> +@predicate('commonancestorheads(*changeset)', safe=True, weight=0.5)
> +def commonancestorheads(repo, subset, x):
> +    """Returns all greatest common ancestors 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.
> +
> +    These greatest common ancestors the same ones that the consesus bid merge
> +    will find.
> +
> +    """
> +    # i18n: "ancestor" is a keyword

Nit: garbage comment.

> +    from .ancestor import commonancestorsheads as cah

Please move the import line to top. I don't think there's import cycle.

> +    l = getlist(x)
> +    rl = fullreposet(repo)
> +    input_revs = []
> +
> +    # (getset(repo, rl, i) for i in l) generates a list of lists
> +    for revs in (getset(repo, rl, i) for i in l):
> +        for r in revs:
> +            input_revs.append(r)
> +
> +    return subset & cah(repo.changelog.parentrevs, *input_revs)

So, we no longer need the first two patches?

> +  $ hg log -r 'commonancestorheads(head())'
> +  changeset:   1:0f6b37dbe527
> +  user:        test
> +  date:        Thu Jan 01 00:00:00 1970 +0000
> +  summary:     1 first change f1
> +  
> +  changeset:   2:d1d156401c1b
> +  parent:      0:40494bf2444c
> +  user:        test
> +  date:        Thu Jan 01 00:00:00 1970 +0000
> +  summary:     2 first change f2

Can you add a test for variable number (0, 1, 2...) of arguments? I think
that's likely to regress.
Sean Farley - June 15, 2018, 6:19 p.m.
Yuya Nishihara <yuya@tcha.org> writes:

> On Fri, 15 Jun 2018 00:10:25 -0700, Sean Farley wrote:
>> # HG changeset patch
>> # User Sean Farley <sean@farley.io>
>> # Date 1527357855 -7200
>> #      Sat May 26 20:04:15 2018 +0200
>> # Branch gca-revset
>> # Node ID d44266127f1a86af521df2b2c088528c3f3b803a
>> # Parent  ab29cfd39f48432d8e7c38cdceb62980d5c22f09
>> revset: add a new commonancestorheads method
>> 
>> This is a new revset that provides a public interface to the
>> ancestor.commonancestorsheads method. Currently, we have no fast revset
>> way to get the ancestors of a list of changesets as returned by the
>> consensus bid merge algorithm (the one needed by criss-cross merges).
>> 
>> Previously, the only way to get these commits were (tested on
>> mozilla-central):
>> 
>> hg perfrevset 'heads(::a7cf55 and ::d8b15)'
>> ! wall 4.988366 comb 4.960000 user 4.780000 sys 0.180000 (best of 3)
>> 
>> After this patch:
>> 
>> hg perfrevset 'commonancestorheads(a7cf55, d8b15)'
>> ! wall 0.001790 comb 0.000000 user 0.000000 sys 0.000000 (best of 1340)
>> 
>> I'm not a fan of added more revset methods for performance but, yet,
>> here we are.
>> 
>> diff --git a/mercurial/revset.py b/mercurial/revset.py
>> index de543df..4004f4b 100644
>> --- a/mercurial/revset.py
>> +++ b/mercurial/revset.py
>> @@ -299,10 +299,34 @@ def adds(repo, subset, x):
>>      """
>>      # i18n: "adds" is a keyword
>>      pat = getstring(x, _("adds requires a pattern"))
>>      return checkstatus(repo, subset, pat, 1)
>>  
>> +@predicate('commonancestorheads(*changeset)', safe=True, weight=0.5)
>> +def commonancestorheads(repo, subset, x):
>> +    """Returns all greatest common ancestors 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.
>> +
>> +    These greatest common ancestors the same ones that the consesus bid merge
>> +    will find.
>> +
>> +    """
>> +    # i18n: "ancestor" is a keyword
>
> Nit: garbage comment.

Oops; yes, bad copy paste.

>> +    from .ancestor import commonancestorsheads as cah
>
> Please move the import line to top. I don't think there's import cycle.

D'oh.

>> +    l = getlist(x)
>> +    rl = fullreposet(repo)
>> +    input_revs = []
>> +
>> +    # (getset(repo, rl, i) for i in l) generates a list of lists
>> +    for revs in (getset(repo, rl, i) for i in l):
>> +        for r in revs:
>> +            input_revs.append(r)
>> +
>> +    return subset & cah(repo.changelog.parentrevs, *input_revs)
>
> So, we no longer need the first two patches?

... I didn't realize that until now. Yes, you're totally correct.

>> +  $ hg log -r 'commonancestorheads(head())'
>> +  changeset:   1:0f6b37dbe527
>> +  user:        test
>> +  date:        Thu Jan 01 00:00:00 1970 +0000
>> +  summary:     1 first change f1
>> +  
>> +  changeset:   2:d1d156401c1b
>> +  parent:      0:40494bf2444c
>> +  user:        test
>> +  date:        Thu Jan 01 00:00:00 1970 +0000
>> +  summary:     2 first change f2
>
> Can you add a test for variable number (0, 1, 2...) of arguments? I think
> that's likely to regress.

Ah, yeah, sure.

Further discussion: should introduce this new revset and deprecate the
older one? Or would it be possible to optimize the heads(::x and ::y)
syntax? Thoughts?
Yuya Nishihara - June 16, 2018, 1:57 a.m.
On Fri, 15 Jun 2018 11:19:47 -0700, Sean Farley wrote:
> >> # HG changeset patch
> >> # User Sean Farley <sean@farley.io>
> >> # Date 1527357855 -7200
> >> #      Sat May 26 20:04:15 2018 +0200
> >> # Branch gca-revset
> >> # Node ID d44266127f1a86af521df2b2c088528c3f3b803a
> >> # Parent  ab29cfd39f48432d8e7c38cdceb62980d5c22f09
> >> revset: add a new commonancestorheads method

> >> +  $ hg log -r 'commonancestorheads(head())'
> >> +  changeset:   1:0f6b37dbe527
> >> +  user:        test
> >> +  date:        Thu Jan 01 00:00:00 1970 +0000
> >> +  summary:     1 first change f1
> >> +  
> >> +  changeset:   2:d1d156401c1b
> >> +  parent:      0:40494bf2444c
> >> +  user:        test
> >> +  date:        Thu Jan 01 00:00:00 1970 +0000
> >> +  summary:     2 first change f2
> >
> > Can you add a test for variable number (0, 1, 2...) of arguments? I think
> > that's likely to regress.
> 
> Ah, yeah, sure.
> 
> Further discussion: should introduce this new revset and deprecate the
> older one?

Deprecate which?

> Or would it be possible to optimize the heads(::x and ::y)
> syntax? Thoughts?

For trivial case where x and y are symbols, that's possible. But we can't
rewrite heads(::head()) to commonancestorheads(head()) since ::(x + y) means
any ancestors of x or y. That's why I said we would need new function anyway.

If we don't like the commonancestorheads() function, maybe we can add
commonancestors(x) and rewrite heads(commonancestors(x)) to
_commonancestorsheads(x).
Sean Farley - June 26, 2018, 10:37 p.m.
Yuya Nishihara <yuya@tcha.org> writes:

> On Fri, 15 Jun 2018 11:19:47 -0700, Sean Farley wrote:
>> >> # HG changeset patch
>> >> # User Sean Farley <sean@farley.io>
>> >> # Date 1527357855 -7200
>> >> #      Sat May 26 20:04:15 2018 +0200
>> >> # Branch gca-revset
>> >> # Node ID d44266127f1a86af521df2b2c088528c3f3b803a
>> >> # Parent  ab29cfd39f48432d8e7c38cdceb62980d5c22f09
>> >> revset: add a new commonancestorheads method
>
>> >> +  $ hg log -r 'commonancestorheads(head())'
>> >> +  changeset:   1:0f6b37dbe527
>> >> +  user:        test
>> >> +  date:        Thu Jan 01 00:00:00 1970 +0000
>> >> +  summary:     1 first change f1
>> >> +  
>> >> +  changeset:   2:d1d156401c1b
>> >> +  parent:      0:40494bf2444c
>> >> +  user:        test
>> >> +  date:        Thu Jan 01 00:00:00 1970 +0000
>> >> +  summary:     2 first change f2
>> >
>> > Can you add a test for variable number (0, 1, 2...) of arguments? I think
>> > that's likely to regress.
>> 
>> Ah, yeah, sure.
>> 
>> Further discussion: should introduce this new revset and deprecate the
>> older one?
>
> Deprecate which?

Nevermind, I was confused.

>> Or would it be possible to optimize the heads(::x and ::y)
>> syntax? Thoughts?
>
> For trivial case where x and y are symbols, that's possible. But we can't
> rewrite heads(::head()) to commonancestorheads(head()) since ::(x + y) means
> any ancestors of x or y. That's why I said we would need new function anyway.
>
> If we don't like the commonancestorheads() function, maybe we can add
> commonancestors(x) and rewrite heads(commonancestors(x)) to
> _commonancestorsheads(x).

Dammit, Yuya, you are a genius. I wish I was half as smart. A new
serious is incoming.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
index de543df..4004f4b 100644
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -299,10 +299,34 @@  def adds(repo, subset, x):
     """
     # i18n: "adds" is a keyword
     pat = getstring(x, _("adds requires a pattern"))
     return checkstatus(repo, subset, pat, 1)
 
+@predicate('commonancestorheads(*changeset)', safe=True, weight=0.5)
+def commonancestorheads(repo, subset, x):
+    """Returns all greatest common ancestors 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.
+
+    These greatest common ancestors the same ones that the consesus bid merge
+    will find.
+
+    """
+    # i18n: "ancestor" is a keyword
+    from .ancestor import commonancestorsheads as cah
+    l = getlist(x)
+    rl = fullreposet(repo)
+    input_revs = []
+
+    # (getset(repo, rl, i) for i in l) generates a list of lists
+    for revs in (getset(repo, rl, i) for i in l):
+        for r in revs:
+            input_revs.append(r)
+
+    return subset & cah(repo.changelog.parentrevs, *input_revs)
+
 @predicate('ancestor(*changeset)', safe=True, weight=0.5)
 def ancestor(repo, subset, x):
     """A greatest common ancestor of the changesets.
 
     Accepts 0 or more changesets.
diff --git a/tests/test-merge-criss-cross.t b/tests/test-merge-criss-cross.t
index 4901da2..f2a96e7 100644
--- a/tests/test-merge-criss-cross.t
+++ b/tests/test-merge-criss-cross.t
@@ -159,10 +159,25 @@  Criss cross merging
   merging f2
   3 files updated, 0 files merged, 0 files removed, 1 files unresolved
   use 'hg resolve' to retry unresolved file merges or 'hg merge --abort' to abandon
   [1]
 
+Test the greatest common ancestor returning multiple changesets
+
+  $ hg log -r 'commonancestorheads(head())'
+  changeset:   1:0f6b37dbe527
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     1 first change f1
+  
+  changeset:   2:d1d156401c1b
+  parent:      0:40494bf2444c
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     2 first change f2
+  
+
 Redo merge with merge.preferancestor="*" to enable bid merge
 
   $ rm f*
   $ hg up -qC .
   $ hg merge -v --debug --tool internal:dump 5 --config merge.preferancestor="*"