Patchwork [4,of,4,V3] revset: skip legacy lookup for revset starting with "set:" (BC)

login
register
mail settings
Submitter Boris Feld
Date April 11, 2018, 9:50 a.m.
Message ID <1fa181fa9b18860c1ec9.1523440217@FB>
Download mbox | patch
Permalink /patch/30678/
State Superseded
Headers show

Comments

Boris Feld - April 11, 2018, 9:50 a.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1523369212 -7200
#      Tue Apr 10 16:06:52 2018 +0200
# Node ID 1fa181fa9b18860c1ec97a31def9bcfcc6959057
# Parent  b52727111f7f17524fd8e8982af59b5f372cc7c3
# EXP-Topic noname
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 1fa181fa9b18
revset: skip legacy lookup for revset starting with "set:" (BC)

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 take a similar path as for fileset and enforce the strict revset parsing
when prefixed with 'set:'. Since 'set:' is a valid revset, this will break
user who uses "set" as a label and has existing legitimate revset starting
with "set:" (programming languages faces the same problem when introducing new
keywords, they have to drop compatibility for identifiers with the same name).

This is an unfortunate but seems acceptable. This is a one-time occurrence
that will impact a corner case. This 'set:' prefix is the "best" approach we
could think of: simple clear and consistent with what fileset use.

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

Having a config 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 prefix approach can control the parsing of each revset,
making it more flexible. For example, a revset used as the value of an
existing config 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.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -2177,6 +2177,8 @@  def match(ui, spec, repo=None, legacycom
     """Create a matcher for a single revision spec"""
     return matchany(ui, [spec], repo=repo, legacycompat=legacycompat)
 
+prefixrevset = 'set:'
+
 def matchany(ui, specs, repo=None, localalias=None, legacycompat=True):
     """Create a matcher that will include any revisions matching one of the
     given specs
@@ -2196,7 +2198,11 @@  def matchany(ui, specs, repo=None, local
         lookup = lookupfn(repo)
     parsedspecs = []
     for s in specs:
-        parsedspecs.append(revsetlang.parse(s, lookup))
+        lookupthis = lookup
+        if s.startswith(prefixrevset):
+            s = s[len(prefixrevset):]
+            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 'set: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 'set:"foo"'
+  abort: unknown revision 'foo'!
+  [255]
   $ hg log -r '("\"foo\"")'
   2:fb616635b18f Added tag rev(0) for changeset 43114e71eddd ["foo"]
+  $ hg log -r 'set:"\"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 'set: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 'set: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 'set: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 'set: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 'set: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 'set:(my little version)'
+  hg: parse error at 4: unexpected token: symbol
+  ((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")'