( )⚙ D10016 tags: return tag cache source from _readtagcache()

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10016

tags: return tag cache source from _readtagcache()
AbandonedPublic

Authored by pulkit on Feb 17 2021, 2:44 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Summary

If the cache is invalid, _readtagcache() can return data which has a wrong
filenode. We want to fix the cache in such cases by replacing the wrong filenode
with a correct one. To do so, first the callers need to be aware from
which cache values returned by this function are read.

This patch adds a new return value which depicts the source of data return from
_readtagcache().

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

pulkit created this revision.Feb 17 2021, 2:44 PM
Alphare accepted this revision.Feb 23 2021, 4:41 AM
marmoute added a subscriber: marmoute.Mar 1 2021, 3:59 AM
marmoute added inline comments.
mercurial/tags.py
186–188

small nits: I would probably use 2 explicit line for this instead of a "one-liner" spawning 3 line:

cache_values = _readtagcache(ui, repo)
(heads, tagfnode, valid, cachetags, shouldwrite, source) = cache_values
409–413

what does this mean if "the cache data is read from hgtagsfnodes" ? Does this always result in cachetags being None ?

pulkit planned changes to this revision.Mar 1 2021, 1:53 PM

Superseeded by D10083.

pulkit abandoned this revision.Mar 9 2021, 2:14 AM

Revision Contents
Changeset List

PathPackages
Mmercurial/tags.py (20 lines)

Diff 25654

mercurial/tags.py

def findglobaltags(ui, repo): def findglobaltags(ui, repo):
"""Find global tags in a repo: return a tagsmap """Find global tags in a repo: return a tagsmap
tagsmap: tag name to (node, hist) 2-tuples. tagsmap: tag name to (node, hist) 2-tuples.
The tags cache is read and updated as a side-effect of calling. The tags cache is read and updated as a side-effect of calling.
""" """
(heads, tagfnode, valid, cachetags, shouldwrite) = _readtagcache(ui, repo) (heads, tagfnode, valid, cachetags, shouldwrite, source) = _readtagcache(
ui, repo
)
marmouteUnsubmitted
Not Done

small nits: I would probably use 2 explicit line for this instead of a "one-liner" spawning 3 line:

cache_values = _readtagcache(ui, repo)
(heads, tagfnode, valid, cachetags, shouldwrite, source) = cache_values
marmoute: small nits: I would probably use 2 explicit line for this instead of a "one-liner" spawning 3…
if cachetags is not None: if cachetags is not None:
assert not shouldwrite assert not shouldwrite
# XXX is this really 100% correct? are there oddball special # XXX is this really 100% correct? are there oddball special
# cases where a global tag should outrank a local tag but won't, # cases where a global tag should outrank a local tag but won't,
# because cachetags does not contain rank info? # because cachetags does not contain rank info?
alltags = {} alltags = {}
_updatetags(cachetags, alltags) _updatetags(cachetags, alltags)
return alltags return alltags
if repo.filtername: if repo.filtername:
filename = b'%s-%s' % (filename, repo.filtername) filename = b'%s-%s' % (filename, repo.filtername)
return filename return filename
def _readtagcache(ui, repo): def _readtagcache(ui, repo):
"""Read the tag cache. """Read the tag cache.
Returns a tuple (heads, fnodes, validinfo, cachetags, shouldwrite). Returns a tuple (heads, fnodes, validinfo, cachetags, shouldwrite, source).
If the cache is completely up-to-date, "cachetags" is a dict of the If the cache is completely up-to-date, "cachetags" is a dict of the
form returned by _readtags() and "heads", "fnodes", and "validinfo" are form returned by _readtags() and "heads", "fnodes", and "validinfo" are
None and "shouldwrite" is False. None and "shouldwrite" is False.
If the cache is not up to date, "cachetags" is None. "heads" is a list If the cache is not up to date, "cachetags" is None. "heads" is a list
of all heads currently in the repository, ordered from tip to oldest. of all heads currently in the repository, ordered from tip to oldest.
"validinfo" is a tuple describing cache validation info. This is used "validinfo" is a tuple describing cache validation info. This is used
when writing the tags cache. "fnodes" is a mapping from head to .hgtags when writing the tags cache. "fnodes" is a mapping from head to .hgtags
filenode. "shouldwrite" is True. filenode. "shouldwrite" is True.
"source" is from which cache the data is read. Possible values are:
tags2: when data is read from `tags2-<filtername>` cache
hgtagsfnodes: data is read from hgtagsfnodes cache
other: when data is read from source other than tags2 and
hgtagsfnodes cache
marmouteUnsubmitted
Not Done

what does this mean if "the cache data is read from hgtagsfnodes" ? Does this always result in cachetags being None ?

marmoute: what does this mean if "the cache data is read from `hgtagsfnodes`" ? Does this always result…
If the cache is not up to date, the caller is responsible for reading tag If the cache is not up to date, the caller is responsible for reading tag
info from each returned head. (See findglobaltags().) info from each returned head. (See findglobaltags().)
""" """
try: try:
cachefile = repo.cachevfs(_filename(repo), b'r') cachefile = repo.cachevfs(_filename(repo), b'r')
# force reading the file for static-http # force reading the file for static-http
cachelines = iter(cachefile) cachelines = iter(cachefile)
except IOError: except IOError:
# have been destroyed by strip or rollback.) # have been destroyed by strip or rollback.)
if ( if (
cacherev == tiprev cacherev == tiprev
and cachenode == tipnode and cachenode == tipnode
and cachehash == scmutil.filteredhash(repo, tiprev) and cachehash == scmutil.filteredhash(repo, tiprev)
): ):
tags = _readtags(ui, repo, cachelines, cachefile.name) tags = _readtags(ui, repo, cachelines, cachefile.name)
cachefile.close() cachefile.close()
return (None, None, None, tags, False) return (None, None, None, tags, False, 'tags2')
if cachefile: if cachefile:
cachefile.close() # ignore rest of file cachefile.close() # ignore rest of file
valid = (tiprev, tipnode, scmutil.filteredhash(repo, tiprev)) valid = (tiprev, tipnode, scmutil.filteredhash(repo, tiprev))
repoheads = repo.heads() repoheads = repo.heads()
# Case 2 (uncommon): empty repo; get out quickly and don't bother # Case 2 (uncommon): empty repo; get out quickly and don't bother
# writing an empty cache. # writing an empty cache.
if repoheads == [nullid]: if repoheads == [nullid]:
return ([], {}, valid, {}, False) return ([], {}, valid, {}, False, 'other')
# Case 3 (uncommon): cache file missing or empty. # Case 3 (uncommon): cache file missing or empty.
# Case 4 (uncommon): tip rev decreased. This should only happen # Case 4 (uncommon): tip rev decreased. This should only happen
# when we're called from localrepository.destroyed(). Refresh the # when we're called from localrepository.destroyed(). Refresh the
# cache so future invocations will not see disappeared heads in the # cache so future invocations will not see disappeared heads in the
# cache. # cache.
# Case 5 (common): tip has changed, so we've added/replaced heads. # Case 5 (common): tip has changed, so we've added/replaced heads.
# As it happens, the code to handle cases 3, 4, 5 is the same. # As it happens, the code to handle cases 3, 4, 5 is the same.
# N.B. in case 4 (nodes destroyed), "new head" really means "newly # N.B. in case 4 (nodes destroyed), "new head" really means "newly
# exposed". # exposed".
if not len(repo.file(b'.hgtags')): if not len(repo.file(b'.hgtags')):
# No tags have ever been committed, so we can avoid a # No tags have ever been committed, so we can avoid a
# potentially expensive search. # potentially expensive search.
return ([], {}, valid, None, True) return ([], {}, valid, None, True, 'other')
# Now we have to lookup the .hgtags filenode for every new head. # Now we have to lookup the .hgtags filenode for every new head.
# This is the most expensive part of finding tags, so performance # This is the most expensive part of finding tags, so performance
# depends primarily on the size of newheads. Worst case: no cache # depends primarily on the size of newheads. Worst case: no cache
# file, so newheads == repoheads. # file, so newheads == repoheads.
# Reversed order helps the cache ('repoheads' is in descending order) # Reversed order helps the cache ('repoheads' is in descending order)
cachefnode = _getfnodes(ui, repo, reversed(repoheads)) cachefnode = _getfnodes(ui, repo, reversed(repoheads))
# Caller has to iterate over all heads, but can use the filenodes in # Caller has to iterate over all heads, but can use the filenodes in
# cachefnode to get to each .hgtags revision quickly. # cachefnode to get to each .hgtags revision quickly.
return (repoheads, cachefnode, valid, None, True) return (repoheads, cachefnode, valid, None, True, 'hgtagsfnodes')
def _getfnodes(ui, repo, nodes): def _getfnodes(ui, repo, nodes):
"""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.
""" """