This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3371

scmutil: make shortesthexnodeidprefix() take a full binary nodeid
ClosedPublic

Authored by martinvonz on Apr 14 2018, 3:20 AM.

Download Raw Diff

Details

Reviewers

None

Group Reviewers

hg-reviewers

Commits

rHG7b2955624777: scmutil: make shortesthexnodeidprefix() take a full binary nodeid
rHG1f473df0dffe: scmutil: make shortesthexnodeidprefix() take a full binary nodeid

Summary

The shortest() template function depended on the behavior of
revlog._partialmatch() for these types of inputs:

non-hex strings
ambiguous strings
too long strings

revlog._partialmatch() seems to return the input unchanged in these
cases, but we shouldn't depend on such a low-level function to match
the behavior we want in the user-facing template function. Instead,
let's handle these cases in the template function and always pass a
binary nodeid to _partialmatch().

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

martinvonz created this revision.Apr 14 2018, 3:20 AM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptApr 14 2018, 3:20 AM

Herald added a subscriber: mercurial-devel. · View Herald Transcript

martinvonz updated this revision to Diff 8286.Apr 14 2018, 12:21 PM

Queued thanks.

The shortest() template function depended on the behavior of
revlog._partialmatch() for these types of inputs:

I guess some of them would be bug.

mercurial/templatefuncs.py
605	One minor bug in this patch. `hg log -T '{shortest(node\|short)}\n' -r 'wdir()'` no longer works.

Closed by commit rHG1f473df0dffe: scmutil: make shortesthexnodeidprefix() take a full binary nodeid (authored by martinvonz). · Explain WhyApr 15 2018, 9:42 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			hgext/show.py (5 lines)
M			mercurial/scmutil.py (4 lines)
M			mercurial/templatefuncs.py (19 lines)
M			tests/test-command-template.t (15 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	8256		Apr 14 2018, 3:20 AM	★	★
Diff 2	8286		Apr 14 2018, 12:21 PM	★	★
Diff 3	8303	rHG1f473df0dffe697d9640018c2e73ced70ddbe9c1	Apr 14 2018, 3:13 AM	★	★

Status	Author	Revision
Closed	martinvonz	D3370 scmutil: introduce shortesthexnodeidprefix()
Closed	martinvonz	D3463 directaccess: use resolvehexnodeidprefix() instead of _partialmatch()
Closed	martinvonz	D3371 scmutil: make shortesthexnodeidprefix() take a full binary nodeid
Closed	martinvonz	D3313 scmutil: make shortesthexnodeidprefix() use unfiltered repo
Abandoned	martinvonz	D3312 revlog: move shortest() to scmutil.shortesthexnodeidprefix() (API)
Closed	martinvonz	D3311 revset: use resolvehexnodeidprefix() in id() predicate (BC)
Closed	martinvonz	D3310 scmutil: use resolvehexnodeidprefix() from revsymbol()
Closed	martinvonz	D3309 scmutil: rename resolvepartialhexnodeid() to resolvehexnodeidprefix()
Closed	martinvonz	D3308 scmutil: document that isrevsymbol() raises on ambiguous node prefix
Closed	martinvonz	D3307 context: set stack level for deprecation warning

Diff 8303

hgext/show.py

	Aliases that would conflict with existing registrations will not be			Aliases that would conflict with existing registrations will not be
	performed.			performed.
	"""			"""

	from __future__ import absolute_import			from __future__ import absolute_import

	from mercurial.i18n import _			from mercurial.i18n import _
	from mercurial.node import (			from mercurial.node import (
	hex,
	nullrev,			nullrev,
	)			)
	from mercurial import (			from mercurial import (
	cmdutil,			cmdutil,
	commands,			commands,
	destutil,			destutil,
	error,			error,
	formatter,			formatter,
	The number of revisions in the repo is taken into account to prevent			The number of revisions in the repo is taken into account to prevent
	a numeric node prefix from conflicting with an integer revision number.			a numeric node prefix from conflicting with an integer revision number.
	If we fail to do this, a value of e.g. ``10023`` could mean either			If we fail to do this, a value of e.g. ``10023`` could mean either
	revision 10023 or node ``10023abc...``.			revision 10023 or node ``10023abc...``.
	"""			"""
	if not revs:			if not revs:
	return minlen			return minlen
	cl = repo.changelog			cl = repo.changelog
	return max(len(scmutil.shortesthexnodeidprefix(repo, hex(cl.node(r)),			return max(len(scmutil.shortesthexnodeidprefix(repo, cl.node(r), minlen))
	minlen)) for r in revs)			for r in revs)

	# Adjust the docstring of the show command so it shows all registered views.			# Adjust the docstring of the show command so it shows all registered views.
	# This is a bit hacky because it runs at the end of module load. When moved			# This is a bit hacky because it runs at the end of module load. When moved
	# into core or when another extension wants to provide a view, we'll need			# into core or when another extension wants to provide a view, we'll need
	# to do this more robustly.			# to do this more robustly.
	# TODO make this more robust.			# TODO make this more robust.
	def _updatedocstring():			def _updatedocstring():
	longest = max(map(len, showview._table.keys()))			longest = max(map(len, showview._table.keys()))

mercurial/scmutil.py

	# Uses unfiltered repo because it's faster when prefix is ambiguous/			# Uses unfiltered repo because it's faster when prefix is ambiguous/
	# This matches the shortesthexnodeidprefix() function below.			# This matches the shortesthexnodeidprefix() function below.
	node = repo.unfiltered().changelog._partialmatch(prefix)			node = repo.unfiltered().changelog._partialmatch(prefix)
	if node is None:			if node is None:
	return			return
	repo.changelog.rev(node) # make sure node isn't filtered			repo.changelog.rev(node) # make sure node isn't filtered
	return node			return node

	def shortesthexnodeidprefix(repo, hexnode, minlength=1):			def shortesthexnodeidprefix(repo, node, minlength=1):
	"""Find the shortest unambiguous prefix that matches hexnode."""			"""Find the shortest unambiguous prefix that matches hexnode."""
	# _partialmatch() of filtered changelog could take O(len(repo)) time,			# _partialmatch() of filtered changelog could take O(len(repo)) time,
	# which would be unacceptably slow. so we look for hash collision in			# which would be unacceptably slow. so we look for hash collision in
	# unfiltered space, which means some hashes may be slightly longer.			# unfiltered space, which means some hashes may be slightly longer.
	return repo.unfiltered().changelog.shortest(hexnode, minlength)			return repo.unfiltered().changelog.shortest(hex(node), minlength)

	def isrevsymbol(repo, symbol):			def isrevsymbol(repo, symbol):
	"""Checks if a symbol exists in the repo.			"""Checks if a symbol exists in the repo.

	See revsymbol() for details. Raises error.LookupError if the symbol is an			See revsymbol() for details. Raises error.LookupError if the symbol is an
	ambiguous nodeid prefix.			ambiguous nodeid prefix.
	"""			"""
	try:			try:

mercurial/templatefuncs.py

	# templatefuncs.py - common template functions			# templatefuncs.py - common template functions
	#			#
	# Copyright 2005, 2006 Matt Mackall <mpm@selenic.com>			# Copyright 2005, 2006 Matt Mackall <mpm@selenic.com>
	#			#
	# This software may be used and distributed according to the terms of the			# This software may be used and distributed according to the terms of the
	# GNU General Public License version 2 or any later version.			# GNU General Public License version 2 or any later version.

	from __future__ import absolute_import			from __future__ import absolute_import

	import re			import re

	from .i18n import _			from .i18n import _
				from .node import (
				bin,
				)
	from . import (			from . import (
	color,			color,
	encoding,			encoding,
	error,			error,
	minirst,			minirst,
	obsutil,			obsutil,
	pycompat,			pycompat,
	registrar,			registrar,
	@templatefunc('shortest(node, minlength=4)')			@templatefunc('shortest(node, minlength=4)')
	def shortest(context, mapping, args):			def shortest(context, mapping, args):
	"""Obtain the shortest representation of			"""Obtain the shortest representation of
	a node."""			a node."""
	if not (1 <= len(args) <= 2):			if not (1 <= len(args) <= 2):
	# i18n: "shortest" is a keyword			# i18n: "shortest" is a keyword
	raise error.ParseError(_("shortest() expects one or two arguments"))			raise error.ParseError(_("shortest() expects one or two arguments"))

	node = evalstring(context, mapping, args[0])			hexnode = evalstring(context, mapping, args[0])

	minlength = 4			minlength = 4
	if len(args) > 1:			if len(args) > 1:
	minlength = evalinteger(context, mapping, args[1],			minlength = evalinteger(context, mapping, args[1],
	# i18n: "shortest" is a keyword			# i18n: "shortest" is a keyword
	_("shortest() expects an integer minlength"))			_("shortest() expects an integer minlength"))

	repo = context.resource(mapping, 'ctx')._repo			repo = context.resource(mapping, 'ctx')._repo
				if len(hexnode) > 40:
				return hexnode
				elif len(hexnode) == 40:
				try:
				node = bin(hexnode)
				except TypeError:
				return hexnode
				else:
				try:
				node = scmutil.resolvehexnodeidprefix(repo, hexnode)
				except (error.LookupError, error.WdirUnsupported):
				return hexnode
				yujaUnsubmitted Not Done One minor bug in this patch. `hg log -T '{shortest(node\|short)}\n' -r 'wdir()'` no longer works. yuja: One minor bug in this patch. `hg log -T '{shortest(node\|short)}\n' -r 'wdir()'` no longer works.
				if not node:
				return hexnode
	return scmutil.shortesthexnodeidprefix(repo, node, minlength)			return scmutil.shortesthexnodeidprefix(repo, node, minlength)

	@templatefunc('strip(text[, chars])')			@templatefunc('strip(text[, chars])')
	def strip(context, mapping, args):			def strip(context, mapping, args):
	"""Strip characters from a string. By default,			"""Strip characters from a string. By default,
	strips all leading and trailing whitespace."""			strips all leading and trailing whitespace."""
	if not (1 <= len(args) <= 2):			if not (1 <= len(args) <= 2):
	# i18n: "strip" is a keyword			# i18n: "strip" is a keyword

tests/test-command-template.t

	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]

	$ hg log -r 'wdir()' -T '{node\|shortest}\n'			$ hg log -r 'wdir()' -T '{node\|shortest}\n'
	ffff			ffff

				$ hg log --template '{shortest("f")}\n' -l1
				f

				$ hg log --template '{shortest("0123456789012345678901234567890123456789")}\n' -l1
				0123456789012345678901234567890123456789

				$ hg log --template '{shortest("01234567890123456789012345678901234567890123456789")}\n' -l1
				01234567890123456789012345678901234567890123456789

				$ hg log --template '{shortest("not a hex string")}\n' -l1
				not a hex string

				$ hg log --template '{shortest("not a hex string, but it'\''s 40 bytes long")}\n' -l1
				not a hex string, but it's 40 bytes long

	$ cd ..			$ cd ..

	Test shortest(node) with the repo having short hash collision:			Test shortest(node) with the repo having short hash collision:

	$ hg init hashcollision			$ hg init hashcollision
	$ cd hashcollision			$ cd hashcollision
	$ cat <<EOF >> .hg/hgrc			$ cat <<EOF >> .hg/hgrc
	> [experimental]			> [experimental]