Patchwork revset: make follow() reject more than one start revisions

login
register
mail settings
Submitter Yuya Nishihara
Date Oct. 12, 2016, 1:07 p.m.
Message ID <4960509763a61ae37103.1476277679@mimosa>
Download mbox | patch
Permalink /patch/17043/
State Accepted
Headers show

Comments

Yuya Nishihara - Oct. 12, 2016, 1:07 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1476131409 -7200
#      Mon Oct 10 22:30:09 2016 +0200
# Node ID 4960509763a61ae3710346131f218cf3bf3f3d32
# Parent  30b2ca4bcf3588de9d949b3eefb2669cb6f8a18a
revset: make follow() reject more than one start revisions

Taking only the last revision is inconsistent because ancestors(set) follows
all revisions given, and theoretically follow(startrev=set) == ancestors(set).
I'm planning to add a support for multiple start revisions, but that won't fit
to the 4.0 time frame. So reject multiple revisions now to avoid future BC.

len(revs) might be slow if revs were large, but we don't care since a valid
revs should have only one element.
Pierre-Yves David - Oct. 16, 2016, 12:20 p.m.
On 10/12/2016 03:07 PM, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1476131409 -7200
> #      Mon Oct 10 22:30:09 2016 +0200
> # Node ID 4960509763a61ae3710346131f218cf3bf3f3d32
> # Parent  30b2ca4bcf3588de9d949b3eefb2669cb6f8a18a
> revset: make follow() reject more than one start revisions

Pushed thanks

> Taking only the last revision is inconsistent because ancestors(set) follows
> all revisions given, and theoretically follow(startrev=set) == ancestors(set).
> I'm planning to add a support for multiple start revisions, but that won't fit
> to the 4.0 time frame. So reject multiple revisions now to avoid future BC.

Looking forward for that

> len(revs) might be slow if revs were large, but we don't care since a valid
> revs should have only one element.

Maybe we could use two iteration step to check that the second one is 
empty? That would imply less computation if the set is large.

If this end up being a common need we could extend the smartrevset class

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -1027,10 +1027,11 @@  def _follow(repo, subset, x, name, follo
         x = getstring(l[0], _("%s expected a pattern") % name)
         rev = None
         if len(l) >= 2:
-            rev = getset(repo, fullreposet(repo), l[1]).last()
-            if rev is None:
+            revs = getset(repo, fullreposet(repo), l[1])
+            if len(revs) != 1:
                 raise error.RepoLookupError(
-                        _("%s: starting revision set cannot be empty") % name)
+                        _("%s expected one starting revision") % name)
+            rev = revs.last()
             c = repo[rev]
         matcher = matchmod.match(repo.root, repo.getcwd(), [x],
                                  ctx=repo[rev], default='path')