comparison mercurial/scmutil.py @ 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.
author Boris Feld <boris.feld@octobus.net>
date Thu, 10 Jan 2019 15:23:58 +0100
parents 929999d963b8
children 17941fc53ae9
comparison
equal deleted inserted replaced
41217:afa884015e66 41218:24a1f67bb75a
721 integer revisions. 721 integer revisions.
722 """ 722 """
723 allspecs = [] 723 allspecs = []
724 for spec in specs: 724 for spec in specs:
725 if isinstance(spec, int): 725 if isinstance(spec, int):
726 spec = revsetlang.formatspec('rev(%d)', spec) 726 spec = revsetlang.formatspec('%d', spec)
727 allspecs.append(spec) 727 allspecs.append(spec)
728 return repo.anyrevs(allspecs, user=True, localalias=localalias) 728 return repo.anyrevs(allspecs, user=True, localalias=localalias)
729 729
730 def meaningfulparents(repo, ctx): 730 def meaningfulparents(repo, ctx):
731 """Return list of meaningful (or all if debug) parentrevs for rev. 731 """Return list of meaningful (or all if debug) parentrevs for rev.