This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5113

revisions: when using prefixhexnode, ensure we prefix "0"
ClosedPublic

Authored by spectral on Oct 16 2018, 10:21 AM.

Details

Reviewers
martinvonz
Group Reviewers
hg-reviewers
Commits
rHGd916ed3ca951: revisions: when using prefixhexnode, ensure we prefix "0"
Summary

Previously, if using experimental.revisions.disambiguatewithin (and it didn't
include rev0), and '0' was the shortest identifier in that disambiguation set,
we printed it as the shortest *without* a prefix. This was because we had logic
to determine "if the prefix is a pure integer, but starts with 0, we don't need
to prefix with 'x': 01 is not a synonym for revision #1", but didn't handle the
case where prefix == 0 (which is a pure integer, and starts with 0... but it
*is* "rev0").

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

spectral created this revision.Oct 16 2018, 10:21 AM
martinvonz accepted this revision.Oct 17 2018, 7:43 PM
martinvonz added a subscriber: martinvonz.

I'll also note that this bug was there all along, but it got way more visible with with the disambiguatewithin stuff. It used to fail only when calculating the shortest prefix for nullid in a repo without any other revisions starting with hex digit 0. The following patch shows that (it failed back on 4.6 too). I'm folding it in in flight. I'm also fixing test-revisions.t (which already showed the bug, but I had not realized it was a bug :P).

diff --git a/tests/test-template-functions.t b/tests/test-template-functions.t

  • a/tests/test-template-functions.t

+++ b/tests/test-template-functions.t
@@ -804,6 +804,8 @@ Test shortest(node) function:

e777603221
bcc7ff960b
f7769ec2ab

+ $ hg log --template '{shortest(node, 1)}\n' -r null
+ 00

$ hg log --template '{node|shortest}\n' -l1
e777
This revision is now accepted and ready to land.Oct 17 2018, 7:43 PM
martinvonz added inline comments.Oct 17 2018, 7:49 PM
tests/test-template-functions.t
936–937

"We *do* identify the collision if..." is a little misleading because it's a different collision that's identified, so I've changed this to "We identify the collision with nullid if..." in flight.

Closed by commit rHGd916ed3ca951: revisions: when using prefixhexnode, ensure we prefix "0" (authored by spectral). · Explain WhyOct 17 2018, 7:50 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 12230

mercurial/scmutil.py

return node return node
def mayberevnum(repo, prefix): def mayberevnum(repo, prefix):
"""Checks if the given prefix may be mistaken for a revision number""" """Checks if the given prefix may be mistaken for a revision number"""
try: try:
i = int(prefix) i = int(prefix)
# if we are a pure int, then starting with zero will not be # if we are a pure int, then starting with zero will not be
# confused as a rev; or, obviously, if the int is larger # confused as a rev; or, obviously, if the int is larger
# than the value of the tip rev # than the value of the tip rev. We still need to disambiguate if
if prefix[0:1] == b'0' or i >= len(repo): # prefix == '0', since that *is* a valid revnum.
if (prefix != b'0' and prefix[0:1] == b'0') or i >= len(repo):
return False return False
return True return True
except ValueError: except ValueError:
return False return False
def shortesthexnodeidprefix(repo, node, minlength=1, cache=None): def shortesthexnodeidprefix(repo, node, minlength=1, cache=None):
"""Find the shortest unambiguous prefix that matches hexnode. """Find the shortest unambiguous prefix that matches hexnode.

tests/test-revisions.t

2:72 2:72
1:9 1:9
0:b 0:b
$ cat <<EOF >> .hg/hgrc $ cat <<EOF >> .hg/hgrc
> [experimental] > [experimental]
> revisions.disambiguatewithin=not 4 > revisions.disambiguatewithin=not 4
> EOF > EOF
$ hg l $ hg l
5:0 5:00
4:7ba5d 4:7ba5d
3:7b 3:7b
2:72 2:72
1:9 1:9
0:b 0:b
9 was unambiguous and still is 9 was unambiguous and still is
$ hg l -r 9 $ hg l -r 9
1:9 1:9

tests/test-template-functions.t

$ hg log --template '{shortest(node)}\n' $ hg log --template '{shortest(node)}\n'
e777 e777
bcc7 bcc7
f776 f776
$ hg log --template '{shortest(node, 10)}\n' $ hg log --template '{shortest(node, 10)}\n'
e777603221 e777603221
bcc7ff960b bcc7ff960b
f7769ec2ab f7769ec2ab
$ hg log --template '{shortest(node, 1)}\n' -r null
00
$ hg log --template '{node|shortest}\n' -l1 $ hg log --template '{node|shortest}\n' -l1
e777 e777
$ hg log -r 0 -T '{shortest(node, "1{"0"}")}\n' $ hg log -r 0 -T '{shortest(node, "1{"0"}")}\n'
f7769ec2ab f7769ec2ab
$ hg log -r 0 -T '{shortest(node, "not an int")}\n' $ hg log -r 0 -T '{shortest(node, "not an int")}\n'
hg: parse error: shortest() expects an integer minlength hg: parse error: shortest() expects an integer minlength
[255] [255]
8:c5625 8:c5625
$ hg log -r 8:10 -T '{rev}:{node|shortest}\n' --hidden $ hg log -r 8:10 -T '{rev}:{node|shortest}\n' --hidden
8:c5625 8:c5625
9:c5623 9:c5623
10:c562d 10:c562d
$ cd .. $ cd ..
Test prefixhexnode when the first character of the hash is 0.
$ hg init hashcollision2
$ cd hashcollision2
$ cat <<EOF >> .hg/hgrc
> [experimental]
> evolution.createmarkers=True
> EOF
$ echo 0 > a
$ hg ci -qAm 0
$ echo 21 > a
$ hg ci -qm 21
$ hg up -q null
$ hg log -r0: -T '{rev}:{node}\n'
0:b4e73ffab476aa0ee32ed81ca51e07169844bc6a
1:0cf177ba2b1dc3862a00fb81715fec90950201be
we need the 'x' prefix to ensure we aren't colliding with rev0. We identify
the collision with nullid if we aren't using disambiguatewithin, so we need to set
martinvonzUnsubmitted
Not Done

"We *do* identify the collision if..." is a little misleading because it's a different collision that's identified, so I've changed this to "We identify the collision with nullid if..." in flight.

martinvonz: "We *do* identify the collision if..." is a little misleading because it's a different…
that as well.
$ hg --config experimental.revisions.disambiguatewithin='descendants(0)' \
> --config experimental.revisions.prefixhexnode=yes \
> log -r 1 -T '{rev}:{shortest(node, 0)}\n'
1:x0
$ hg debugobsolete 0cf177ba2b1dc3862a00fb81715fec90950201be
obsoleted 1 changesets
$ hg up -q 0
$ echo 61 > a
$ hg ci -m 61
$ hg log -r0: -T '{rev}:{node}\n'
0:b4e73ffab476aa0ee32ed81ca51e07169844bc6a
2:01384dde84b3a511ae0835f35ac40bd806c99bb8
we still have the 'x' prefix because '0' is still the shortest prefix, since
rev1's '0c' is hidden.
$ hg --config experimental.revisions.disambiguatewithin=0:-1-0 \
> --config experimental.revisions.prefixhexnode=yes \
> log -r 0:-1-0 -T '{rev}:{shortest(node, 0)}\n'
2:x0
we don't have the 'x' prefix on 2 because '01' is not a synonym for rev1.
$ hg --config experimental.revisions.disambiguatewithin=0:-1-0 \
> --config experimental.revisions.prefixhexnode=yes \
> log -r 0:-1-0 -T '{rev}:{shortest(node, 0)}\n' --hidden
1:0c
2:01
$ cd ..
Test pad function Test pad function
$ cd r $ cd r
$ hg log --template '{pad(rev, 20)} {author|user}\n' $ hg log --template '{pad(rev, 20)} {author|user}\n'
2 test 2 test
1 {node|short} 1 {node|short}
0 test 0 test