mercurial/tags.py
497–498	lets drop a `_` between words here, if you don't mind. This is easier to read.
843–852	We do more that recomputing the nodes, we also update the cache right ? We should maybe update the function name (maybe refresh_invalide_node ?) and update the doc to point this out.

mercurial/tags.py
505–506	look slike my main an long comment about this got lost… I don't understand why we do a full filectx initialisation and data fetch here. What we want to do is only check if the node exist within the filelog. right? Why won't we explicitly grab de filelog and check that?

mercurial/tags.py
501	The official way to get this is `repo.file(b'.hgtags')` if I remember correctly. The current code is a bit abstraction-breaking.

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	26006		Mar 1 2021, 1:47 PM	★	★
Diff 2	26013		Mar 2 2021, 3:42 AM	★	★
Diff 3	26015		Mar 2 2021, 6:44 AM	★	★
Diff 4	26021		Mar 2 2021, 9:24 AM	★	★
Diff 5	26106		Mar 5 2021, 1:16 PM	★	★
Diff 6	26145	rHG9a31f65381ae1404db8c908197eda610dbecc080	Mar 1 2021, 1:32 PM	★	★

Status	Author	Revision
Closed	pulkit	D10083 tags: validate nodes in _getfnodes() and update cache in case of unknown nodes
Closed	pulkit	D10015 debugtagscache: verify that filenode is correct
Closed	mharbison72	D9812 tests: demonstrate a case where a corrupt tag cache causes an abort
Closed	pulkit	D10014 debugcommands: prevent using `is False`
Closed	pulkit	D10013 hgtagsfnodes: refactor code to compute fnode into separate fn
Closed	pulkit	D9994 error: remove shortening of node in error message

Diff 26145

mercurial/tags.py

	"""return .hgtags fnodes for a list of changeset nodes			"""return .hgtags fnodes for a list of changeset nodes

	Return value is a {node: fnode} mapping. There will be no entry for nodes			Return value is a {node: fnode} mapping. There will be no entry for nodes
	without a '.hgtags' file.			without a '.hgtags' file.
	"""			"""
	starttime = util.timer()			starttime = util.timer()
	fnodescache = hgtagsfnodescache(repo.unfiltered())			fnodescache = hgtagsfnodescache(repo.unfiltered())
	cachefnode = {}			cachefnode = {}
				validated_fnodes = set()
				unknown_entries = set()
				marmouteUnsubmitted Done lets drop a `_` between words here, if you don't mind. This is easier to read. marmoute: lets drop a `_` between words here, if you don't mind. This is easier to read.
	for node in nodes:			for node in nodes:
	fnode = fnodescache.getfnode(node)			fnode = fnodescache.getfnode(node)
				flog = repo.file(b'.hgtags')
				marmouteUnsubmitted Done The official way to get this is `repo.file(b'.hgtags')` if I remember correctly. The current code is a bit abstraction-breaking. marmoute: The official way to get this is `repo.file(b'.hgtags')` if I remember correctly. The current…
				if fnode != nullid:
				if fnode not in validated_fnodes:
				if flog.hasnode(fnode):
				validated_fnodes.add(fnode)
				else:
				marmouteUnsubmitted Done look slike my main an long comment about this got lost… I don't understand why we do a full filectx initialisation and data fetch here. What we want to do is only check if the node exist within the filelog. right? Why won't we explicitly grab de filelog and check that? marmoute: look slike my main an long comment about this got lost… I don't understand why we do a full…
				unknown_entries.add(node)
				cachefnode[node] = fnode

				if unknown_entries:
				fixed_nodemap = fnodescache.refresh_invalid_nodes(unknown_entries)
				for node, fnode in pycompat.iteritems(fixed_nodemap):
	if fnode != nullid:			if fnode != nullid:
	cachefnode[node] = fnode			cachefnode[node] = fnode

	fnodescache.write()			fnodescache.write()

	duration = util.timer() - starttime			duration = util.timer() - starttime
	ui.log(			ui.log(
	b'tagscache',			b'tagscache',
	b'%d/%d cache hits/lookups in %0.4f seconds\n',			b'%d/%d cache hits/lookups in %0.4f seconds\n',
	fnodescache.hitcount,			fnodescache.hitcount,
	ctx = self._repo[node]			ctx = self._repo[node]

	# Do a lookup first to avoid writing if nothing has changed.			# Do a lookup first to avoid writing if nothing has changed.
	if self.getfnode(ctx.node(), computemissing=False) == fnode:			if self.getfnode(ctx.node(), computemissing=False) == fnode:
	return			return

	self._writeentry(ctx.rev() * _fnodesrecsize, node[0:4], fnode)			self._writeentry(ctx.rev() * _fnodesrecsize, node[0:4], fnode)

				def refresh_invalid_nodes(self, nodes):
				"""recomputes file nodes for a given set of nodes which has unknown
				filenodes for them in the cache
				Also updates the in-memory cache with the correct filenode.
				Caller needs to take care about calling `.write()` so that updates are
				persisted.
				Returns a map {node: recomputed fnode}
				"""
				fixed_nodemap = {}
				for node in nodes:
				marmouteUnsubmitted Done We do more that recomputing the nodes, we also update the cache right ? We should maybe update the function name (maybe refresh_invalide_node ?) and update the doc to point this out. marmoute: We do more that recomputing the nodes, we also update the cache right ? We should maybe update…
				fnode = self._computefnode(node)
				fixed_nodemap[node] = fnode
				self.setfnode(node, fnode)
				return fixed_nodemap

	def _writeentry(self, offset, prefix, fnode):			def _writeentry(self, offset, prefix, fnode):
	# Slices on array instances only accept other array.			# Slices on array instances only accept other array.
	entry = bytearray(prefix + fnode)			entry = bytearray(prefix + fnode)
	self._raw[offset : offset + _fnodesrecsize] = entry			self._raw[offset : offset + _fnodesrecsize] = entry
	# self._dirtyoffset could be None.			# self._dirtyoffset could be None.
	self._dirtyoffset = min(self._dirtyoffset or 0, offset or 0)			self._dirtyoffset = min(self._dirtyoffset or 0, offset or 0)

	def write(self):			def write(self):

tests/test-tags.t

	5 8dbfe60eff306a54259cfe007db9e330e7ecf866 0c04f2a8deadde17fab7422878ee5a2dadbc943d (unknown node)			5 8dbfe60eff306a54259cfe007db9e330e7ecf866 0c04f2a8deadde17fab7422878ee5a2dadbc943d (unknown node)

	$ rm -f .hg/cache/tags2-visible			$ rm -f .hg/cache/tags2-visible
	$ hg debugtagscache \| tail -2			$ hg debugtagscache \| tail -2
	4 0c192d7d5e6b78a714de54a2e9627952a877e25a 0c04f2a8af31de17fab7422878ee5a2dadbc943d			4 0c192d7d5e6b78a714de54a2e9627952a877e25a 0c04f2a8af31de17fab7422878ee5a2dadbc943d
	5 8dbfe60eff306a54259cfe007db9e330e7ecf866 0c04f2a8deadde17fab7422878ee5a2dadbc943d (unknown node)			5 8dbfe60eff306a54259cfe007db9e330e7ecf866 0c04f2a8deadde17fab7422878ee5a2dadbc943d (unknown node)

	$ hg tags			$ hg tags
	abort: data/.hgtags.i@0c04f2a8deadde17fab7422878ee5a2dadbc943d: no match found			tip 5:8dbfe60eff30
	[50]			bar 1:78391a272241

	BUG: Unless this file is restored, the `hg tags` in the next unix-permissions			BUG: Unless this file is restored, the `hg tags` in the next unix-permissions
	conditional will fail: "abort: data/.hgtags.i@0c04f2a8dead: no match found"			conditional will fail: "abort: data/.hgtags.i@0c04f2a8dead: no match found"

	$ mv .hg/cache/hgtagsfnodes1.bak .hg/cache/hgtagsfnodes1			$ mv .hg/cache/hgtagsfnodes1.bak .hg/cache/hgtagsfnodes1

	#if unix-permissions no-root			#if unix-permissions no-root
	Errors writing to .hgtags fnodes cache are silently ignored			Errors writing to .hgtags fnodes cache are silently ignored

			Path	Packages
M			mercurial/tags.py (29 lines)
M			tests/test-tags.t (4 lines)

This is an archive of the discontinued Mercurial Phabricator instance.

tags: validate nodes in _getfnodes() and update cache in case of unknown nodes
ClosedPublic

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 26145

mercurial/tags.py

tests/test-tags.t

This is an archive of the discontinued Mercurial Phabricator instance.

tags: validate nodes in _getfnodes() and update cache in case of unknown nodesClosedPublic

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 26145

mercurial/tags.py

tests/test-tags.t

tags: validate nodes in _getfnodes() and update cache in case of unknown nodes
ClosedPublic

Revision Contents
Changeset List