# HG changeset patch # User Boris Feld <boris.f...@octobus.net> # Date 1547130238 -3600 # Thu Jan 10 15:23:58 2019 +0100 # Node ID 18309ce7974f4479b089f0dc6f849afd19f9e629 # Parent 4005f4a1c34ec9121983bb07b4a1895892645f30 # EXP-Topic revs-efficiency # Available At https://bitbucket.org/octobus/mercurial-devel/ # hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 18309ce7974f 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. diff --git a/mercurial/revsetlang.py b/mercurial/revsetlang.py --- a/mercurial/revsetlang.py +++ b/mercurial/revsetlang.py @@ -583,7 +583,7 @@ def _quote(s): 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 @@ def formatspec(expr, *args): 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 @@ def formatspec(expr, *args): >>> 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' diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py --- a/mercurial/scmutil.py +++ b/mercurial/scmutil.py @@ -723,7 +723,7 @@ def revrange(repo, specs, localalias=Non 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) _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel