This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3312

revlog: move shortest() to scmutil.shortesthexnodeidprefix() (API)
AbandonedPublic

Authored by martinvonz on Apr 13 2018, 2:00 PM.

Download Raw Diff

Details

Reviewers

indygreg

Group Reviewers

hg-reviewers

Summary

I apparently moved this function from templater.py in 448725a2ef73
(templater: extract shortest() logic from template function,
2017-09-15). Now we have scmutil.resolvehexnodeidprefix(), so it makes
sense to have this method next to it.

Note that the change in show.py also makes it so the conversion from
revnum to nodeid is done on the filtered repo, but that should be
inconsequential since the revs are all from the filtered repo anyway.

Diff Detail

Repository

rHG Mercurial

Lint

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

martinvonz created this revision.Apr 13 2018, 2:00 PM

Herald added a reviewer: indygreg. · View Herald TranscriptApr 13 2018, 2:00 PM

Herald added a reviewer: hg-reviewers. · View Herald Transcript

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

martinvonz added a child revision: D3313: scmutil: make shortesthexnodeidprefix() use unfiltered repo.Apr 13 2018, 2:00 PM

I haven't looked at the patch yet, but I also wrote something like this as part of storage refactors. IMO the shortest node logic has no business being on the generic revlog implementation. I never submitted that patch since removing filelog's inheritance of revlog worked around the problem for me.

IIRC, @quark said it's in revlog because the implementation may vary
depending on storage, such as looking up in radix tree and rewind?

In D3312#53423, @yuja wrote:

IIRC, @quark said it's in revlog because the implementation may vary
depending on storage, such as looking up in radix tree and rewind?

That's reasonable. Perhaps I'll just make shortesthexnodeidprefix() a thin wrapper then, so all it does (at the end of this series) is to delegate to the unfiltered repo. Does that seem okay with you?

martinvonz abandoned this revision.Apr 14 2018, 12:14 PM

Revision Contents
Changeset List

			Path	Packages
M			hgext/show.py (7 lines)
M			mercurial/revlog.py (38 lines)
M			mercurial/scmutil.py (40 lines)
M			mercurial/templatefuncs.py (4 lines)

Status	Author	Revision
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	D3370 scmutil: introduce shortesthexnodeidprefix()
Closed	martinvonz	D3463 directaccess: use resolvehexnodeidprefix() instead of _partialmatch()
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 8118

hgext/show.py

	formatter,			formatter,
	graphmod,			graphmod,
	logcmdutil,			logcmdutil,
	phases,			phases,
	pycompat,			pycompat,
	registrar,			registrar,
	revset,			revset,
	revsetlang,			revsetlang,
				scmutil,
	)			)

	# Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core' for			# Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core' for
	# extensions which SHIP WITH MERCURIAL. Non-mainline extensions should			# extensions which SHIP WITH MERCURIAL. Non-mainline extensions should
	# be specifying the version(s) of Mercurial they are tested with, or			# be specifying the version(s) of Mercurial they are tested with, or
	# leave the attribute unspecified.			# leave the attribute unspecified.
	testedwith = 'ships-with-hg-core'			testedwith = 'ships-with-hg-core'

	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
	# don't use filtered repo because it's slow. see templater.shortest().			# don't use filtered repo because it's slow. see templater.shortest().
	cl = repo.unfiltered().changelog			cl = repo.changelog
	return max(len(cl.shortest(hex(cl.node(r)), minlen)) for r in revs)			return max(len(scmutil.shortesthexnodeidprefix(repo.unfiltered(),
				hex(cl.node(r)),
				minlen)) 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/revlog.py

	if n is not None:			if n is not None:
	return n			return n
	n = self._partialmatch(id)			n = self._partialmatch(id)
	if n:			if n:
	return n			return n

	raise LookupError(id, self.indexfile, _('no match found'))			raise LookupError(id, self.indexfile, _('no match found'))

	def shortest(self, hexnode, minlength=1):
	"""Find the shortest unambiguous prefix that matches hexnode."""
	def isvalid(test):
	try:
	if self._partialmatch(test) is None:
	return False

	try:
	i = int(test)
	# if we are a pure int, then starting with zero will not be
	# confused as a rev; or, obviously, if the int is larger
	# than the value of the tip rev
	if test[0] == '0' or i > len(self):
	return True
	return False
	except ValueError:
	return True
	except error.RevlogError:
	return False
	except error.WdirUnsupported:
	# single 'ff...' match
	return True

	shortest = hexnode
	startlength = max(6, minlength)
	length = startlength
	while True:
	test = hexnode[:length]
	if isvalid(test):
	shortest = test
	if length == minlength or length > startlength:
	return shortest
	length -= 1
	else:
	length += 1
	if len(shortest) <= length:
	return shortest

	def cmp(self, node, text):			def cmp(self, node, text):
	"""compare text with a given file revision			"""compare text with a given file revision

	returns True if text is different than what is stored.			returns True if text is different than what is stored.
	"""			"""
	p1, p2 = self.parents(node)			p1, p2 = self.parents(node)
	return hash(text, p1, p2) != node			return hash(text, p1, p2) != node

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 "shortest" template function.			# This matches the "shortest" template function.
	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):
				"""Find the shortest unambiguous prefix that matches hexnode."""
				cl = repo.changelog
				def isvalid(test):
				try:
				if cl._partialmatch(test) is None:
				return False

				try:
				i = int(test)
				# if we are a pure int, then starting with zero will not be
				# confused as a rev; or, obviously, if the int is larger
				# than the value of the tip rev
				if test[0] == '0' or i > len(cl):
				return True
				return False
				except ValueError:
				return True
				except error.RevlogError:
				return False
				except error.WdirUnsupported:
				# single 'ff...' match
				return True

				shortest = hexnode
				startlength = max(6, minlength)
				length = startlength
				while True:
				test = hexnode[:length]
				if isvalid(test):
				shortest = test
				if length == minlength or length > startlength:
				return shortest
				length -= 1
				else:
				length += 1
				if len(shortest) <= length:
				return shortest


	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
	an ambiguous nodeid prefix.			an ambiguous nodeid prefix.
	"""			"""
	try:			try:
	revsymbol(repo, symbol)			revsymbol(repo, symbol)

mercurial/templatefuncs.py

	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"))

	# _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.
	cl = context.resource(mapping, 'ctx')._repo.unfiltered().changelog			repo = context.resource(mapping, 'ctx')._repo
	return cl.shortest(node, minlength)			return scmutil.shortesthexnodeidprefix(repo.unfiltered(), 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
	raise error.ParseError(_("strip expects one or two arguments"))			raise error.ParseError(_("strip expects one or two arguments"))

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	8118			Apr 13 2018, 1:59 PM	★	★