Patchwork [3,of,3,V4] revset: skip legacy lookup for revspec wrapped in 'revset(…)'

login
register
mail settings
Submitter Boris Feld
Date April 14, 2018, 1:07 p.m.
Message ID <e345b7103fef40be16f7.1523711226@FB>
Download mbox | patch
Permalink /patch/31029/
State New
Headers show

Comments

Boris Feld - April 14, 2018, 1:07 p.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1523369212 -7200
#      Tue Apr 10 16:06:52 2018 +0200
# Node ID e345b7103fef40be16f792cda46aa899d0ef8dd1
# Parent  304b6671aa1be37fc410edb38f2f2f3040ce6708
# EXP-Topic noname
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r e345b7103fef
revset: skip legacy lookup for revspec wrapped in 'revset(…)'

Currently, multiple labels can take forms that can be confused with revset
(eg: "rev(0)" is a valid tag). Since we look up for tags before evaluating
revset, this means a tag can shadow a valid revset at any time.

We now enforce the strict revset parsing when wrapped with 'revset(…)'. For now,
This only work on a whole revspec. This might change in the future if we improve
the implementation.

The feature is undocumented for now, keeping it in the experimental namespace.
In case a better approach to achieve the same goal is found.

The syntax looks like a revset but is not implemented as such for now. Since the
goal is to avoid some preprocessing that happens before revset parsing, we
cannot simply implement it as a revset predicate.

There was other approaches discussed over the mailing-list but they were less
convincing.

Having a configuration flag to disable legacy lookup have been considered but
discarded. There are too many common uses of ambiguous identifier (eg: '+',
'-' or '..') to have the legacy lookup mechanism turned off.

In addition, the  approach can control the parsing of each revset, making
it more flexible. For example, a revset used as the value of an existing
configuration option (eg: pushrev) could enforce its resolution as a revset (by
using the prefix) while user inputs would still use the legacy lookup.

In addition of offering a way to unambiguously input a revset, this prefix
allow skipping the name lookup providing a significant speedup in some case.
Yuya Nishihara - April 14, 2018, 1:30 p.m.
On Sat, 14 Apr 2018 15:07:06 +0200, Boris Feld wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1523369212 -7200
> #      Tue Apr 10 16:06:52 2018 +0200
> # Node ID e345b7103fef40be16f792cda46aa899d0ef8dd1
> # Parent  304b6671aa1be37fc410edb38f2f2f3040ce6708
> # EXP-Topic noname
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r e345b7103fef
> revset: skip legacy lookup for revspec wrapped in 'revset(…)'

>  def matchany(ui, specs, repo=None, localalias=None):
>      """Create a matcher that will include any revisions matching one of the
>      given specs
> @@ -2196,7 +2199,13 @@ def matchany(ui, specs, repo=None, local
>          lookup = lookupfn(repo)
>      parsedspecs = []
>      for s in specs:
> -        parsedspecs.append(revsetlang.parse(s, lookup))
> +        lookupthis = lookup
> +        stripped = s.strip()
> +        if (stripped.startswith(prefixrevset)
> +                and stripped.endswith(postfixrevset)):
> +            s = s[len(prefixrevset):-len(postfixrevset)]
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

This would confuse the error reporting. Instead maybe we can handle this
hack in revsetlang.py.

 1) nullify lookup function at revsetlang.parse(), _parsewith(), or
    tokenize()
 2) and, rewrite (func 'revset' x[2]) to x[2] by _analyze()

(2) would be something like

    elif op == 'func':
        f = getsymbol(x[1])
        if f == 'revset':
            return _analyze(x[2])

"hg debugrevspec -v" will show how the parsed tree will be rewritten.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -2177,6 +2177,9 @@  def match(ui, spec, repo=None):
     """Create a matcher for a single revision spec"""
     return matchany(ui, [spec], repo=repo)
 
+prefixrevset = 'revset('
+postfixrevset = ')'
+
 def matchany(ui, specs, repo=None, localalias=None):
     """Create a matcher that will include any revisions matching one of the
     given specs
@@ -2196,7 +2199,13 @@  def matchany(ui, specs, repo=None, local
         lookup = lookupfn(repo)
     parsedspecs = []
     for s in specs:
-        parsedspecs.append(revsetlang.parse(s, lookup))
+        lookupthis = lookup
+        stripped = s.strip()
+        if (stripped.startswith(prefixrevset)
+                and stripped.endswith(postfixrevset)):
+            s = s[len(prefixrevset):-len(postfixrevset)]
+            lookupthis = None
+        parsedspecs.append(revsetlang.parse(s, lookupthis))
     if len(parsedspecs) == 1:
         tree = parsedspecs[0]
     else:
diff --git a/tests/test-legacy-lookup.t b/tests/test-legacy-lookup.t
--- a/tests/test-legacy-lookup.t
+++ b/tests/test-legacy-lookup.t
@@ -62,6 +62,12 @@  within a more advances revset
   $ hg log -r 'rev(0) and branch(default)'
   0:a87874c6ec31 first []
 
+with explicit revset resolution
+(still resolved as the label)
+
+  $ hg log -r 'revset(rev(0))'
+  0:a87874c6ec31 first []
+
 some of the above with quote to force its resolution as a label
 
   $ hg log -r ':"rev(0)"'
@@ -91,8 +97,13 @@  Test label with quote in them.
   $ hg log -r '("foo")'
   abort: unknown revision 'foo'!
   [255]
+  $ hg log -r 'revset("foo")'
+  abort: unknown revision 'foo'!
+  [255]
   $ hg log -r '("\"foo\"")'
   2:fb616635b18f Added tag rev(0) for changeset 43114e71eddd ["foo"]
+  $ hg log -r 'revset("\"foo\"")'
+  2:fb616635b18f Added tag rev(0) for changeset 43114e71eddd ["foo"]
 
 Test label with dash in them.
 
@@ -116,6 +127,9 @@  Test label with + in them.
   $ hg log -r '(foo+bar)'
   abort: unknown revision 'foo'!
   [255]
+  $ hg log -r 'revset(foo+bar)'
+  abort: unknown revision 'foo'!
+  [255]
   $ hg log -r '"foo+bar"'
   4:bbf52b87b370 Added tag foo-bar for changeset a50aae922707 [foo+bar]
   $ hg log -r '("foo+bar")'
@@ -129,6 +143,8 @@  Test tag with numeric version number.
   5:ff42fde8edbb Added tag foo+bar for changeset bbf52b87b370 [1.2]
   $ hg log -r '(1.2)'
   5:ff42fde8edbb Added tag foo+bar for changeset bbf52b87b370 [1.2]
+  $ hg log -r 'revset(1.2)'
+  5:ff42fde8edbb Added tag foo+bar for changeset bbf52b87b370 [1.2]
   $ hg log -r '"1.2"'
   5:ff42fde8edbb Added tag foo+bar for changeset bbf52b87b370 [1.2]
   $ hg log -r '("1.2")'
@@ -157,6 +173,9 @@  Test tag with parenthesis (but not a val
   $ hg log -r '(release_4.1(candidate1))'
   hg: parse error: unknown identifier: release_4.1
   [255]
+  $ hg log -r 'revset(release_4.1(candidate1))'
+  hg: parse error: unknown identifier: release_4.1
+  [255]
   $ hg log -r '"release_4.1(candidate1)"'
   6:db72e24fe069 Added tag 1.2 for changeset ff42fde8edbb [release_4.1(candidate1)]
   $ hg log -r '("release_4.1(candidate1)")'
@@ -182,6 +201,9 @@  Test tag with parenthesis and other func
   $ hg log -r '(release_4.1(arch=x86,arm))'
   hg: parse error: unknown identifier: release_4.1
   [255]
+  $ hg log -r 'revset(release_4.1(arch=x86,arm))'
+  hg: parse error: unknown identifier: release_4.1
+  [255]
   $ hg log -r '"release_4.1(arch=x86,arm)"'
   7:b29b25d7d687 Added tag release_4.1(candidate1) for changeset db72e24fe069 [release_4.1(arch=x86,arm)]
   $ hg log -r '("release_4.1(arch=x86,arm)")'
@@ -208,6 +230,9 @@  Test tag conflicting with revset functio
   $ hg log -r '(secret(team=foo,project=bar))'
   hg: parse error: secret takes no arguments
   [255]
+  $ hg log -r 'revset(secret(team=foo,project=bar))'
+  hg: parse error: secret takes no arguments
+  [255]
   $ hg log -r '"secret(team=foo,project=bar)"'
   8:6b2e2d4ea455 Added tag release_4.1(arch=x86,arm) for changeset b29b25d7d687 [secret(team=foo,project=bar)]
   $ hg log -r '("secret(team=foo,project=bar)")'
@@ -237,6 +262,11 @@  Test tag with space
   ((my little version)
        ^ here)
   [255]
+  $ hg log -r 'revset(my little version)'
+  hg: parse error at 3: invalid token
+  (my little version
+      ^ here)
+  [255]
   $ hg log -r '"my little version"'
   9:269192bf8fc3 Added tag secret(team=foo,project=bar) for changeset 6b2e2d4ea455 [my little version]
   $ hg log -r '("my little version")'