changeset 41218:24a1f67bb75a

revset: enforce "%d" to be interpreted as literal revision number (API) (BC)

Before this change, `formatspec("%d", x)` results in `"%d" % int(x)`. This
seems simple and correct until you consider `nullrev`. In revset, a direct
"-1" symbol is equivalent to `tip` not `nullrev`. This is a subtle error that
went undetected for a while. Wrapping the revision number inside 'rev()'
remove the ambiguity, preserving nullrev value passed to formatspec.

It got caught by the rebase code, were the following wrongly returned `[1]`:

    repo.revs("children(%d) and ancestors(%ld)", 0, [nullrev])

This is flagged as API, because `%d` can be used for non-revision integer
argument of revset function. We probably need to introduce a new '%…'
substitution to allow literal integer (maybe `%i`). However, the `%d` usage is
currently widespread for revision number so it is important to fix this issue
for `%d`.  This choice is reinforced by the fact _intlist is implemented as
revisions only. Restricting `%d` to revision only makes things more
consistent.

This bug can become especially tricky since `_intlist` recognize `nullrev`
right. So `revs('%ld', [-1, 0])` → select `[nullrev, 0]` but `revs('%ld',
[-1])` is simplified and treated as `%d` selecting `[tip]`.

Another side effect is that "%d" of an unknown revision simply match nothing. It
was previously raising and error. This is consistent with what "%ld" (and
`_intlist`) is doing, so it seems like a good move.

--- a/mercurial/revsetlang.py	Thu Jan 10 16:03:07 2019 +0100
+++ b/mercurial/revsetlang.py	Thu Jan 10 15:23:58 2019 +0100
@@ -583,7 +583,7 @@
 
 def _formatargtype(c, arg):
     if c == 'd':
-        return '%d' % int(arg)
+        return 'rev(%d)' % int(arg)
     elif c == 's':
         return _quote(arg)
     elif c == 'r':
@@ -638,7 +638,7 @@
     Supported arguments:
 
     %r = revset expression, parenthesized
-    %d = int(arg), no quoting
+    %d = rev(int(arg)), no quoting
     %s = string(arg), escaped and single-quoted
     %b = arg.branch(), escaped and single-quoted
     %n = hex(arg), single-quoted
@@ -650,9 +650,9 @@
     >>> formatspec(b'%r:: and %lr', b'10 or 11', (b"this()", b"that()"))
     '(10 or 11):: and ((this()) or (that()))'
     >>> formatspec(b'%d:: and not %d::', 10, 20)
-    '10:: and not 20::'
+    'rev(10):: and not rev(20)::'
     >>> formatspec(b'%ld or %ld', [], [1])
-    "_list('') or 1"
+    "_list('') or rev(1)"
     >>> formatspec(b'keyword(%s)', b'foo\\xe9')
     "keyword('foo\\\\xe9')"
     >>> b = lambda: b'default'

--- a/mercurial/scmutil.py	Thu Jan 10 16:03:07 2019 +0100
+++ b/mercurial/scmutil.py	Thu Jan 10 15:23:58 2019 +0100
@@ -723,7 +723,7 @@
     allspecs = []
     for spec in specs:
         if isinstance(spec, int):
-            spec = revsetlang.formatspec('rev(%d)', spec)
+            spec = revsetlang.formatspec('%d', spec)
         allspecs.append(spec)
     return repo.anyrevs(allspecs, user=True, localalias=localalias)