This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4088

changegroup: move revision maps to cgpacker
ClosedPublic

Authored by indygreg on Aug 3 2018, 5:04 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG0548f696795b: changegroup: move revision maps to cgpacker
Summary

And remove the underscores so the variables conform to our
naming convention.

The logic in _close() should be the only thing warranting scrutiny
during review.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Aug 3 2018, 5:04 PM
Closed by commit rHG0548f696795b: changegroup: move revision maps to cgpacker (authored by indygreg). · Explain WhyAug 6 2018, 2:57 AM
This revision was automatically updated to reflect the committed changes.
indygreg added subscribers: spectral, martinvonz, durin42.Aug 6 2018, 9:04 PM
indygreg added inline comments.
mercurial/changegroup.py
602–603

There are 2 bugs here.

One is dropping the _ prefix from clrevtolocalrev.

The other is aliasing the variables then calling .clear(), which has the effect of undoing both and effectively nerfing self._nextclrevtolocalrev.

Surprisingly no tests failed as a result of these bugs. So I'm not sure what's going on.

Maybe @spectral, @martinvonz, or @durin42 know what's up.

Revision Contents
Changeset List

PathPackages
Mmercurial/changegroup.py (50 lines)

Diff 9850

mercurial/changegroup.py

self._verbosenote = self._repo.ui.note self._verbosenote = self._repo.ui.note
else: else:
self._verbosenote = lambda s: None self._verbosenote = lambda s: None
# TODO the functionality keyed off of this should probably be # TODO the functionality keyed off of this should probably be
# controlled via arguments to group() that influence behavior. # controlled via arguments to group() that influence behavior.
self._changelogdone = False self._changelogdone = False
# Maps CL revs to per-revlog revisions. Cleared in close() at
# the end of each group.
self._clrevtolocalrev = {}
self._nextclrevtolocalrev = {}
# Maps changelog nodes to changelog revs. Filled in once
# during changelog stage and then left unmodified.
self._clnodetorev = {}
def _close(self): def _close(self):
# Ellipses serving mode. # Ellipses serving mode.
getattr(self, '_clrev_to_localrev', {}).clear() self._clrevtolocalrev.clear()
if getattr(self, '_next_clrev_to_localrev', {}): if self._nextclrevtolocalrev:
self._clrev_to_localrev = self._next_clrev_to_localrev self.clrevtolocalrev = self._nextclrevtolocalrev
del self._next_clrev_to_localrev self._nextclrevtolocalrev.clear()
indygregAuthorUnsubmitted
Not Done

There are 2 bugs here.

One is dropping the _ prefix from clrevtolocalrev.

The other is aliasing the variables then calling .clear(), which has the effect of undoing both and effectively nerfing self._nextclrevtolocalrev.

Surprisingly no tests failed as a result of these bugs. So I'm not sure what's going on.

Maybe @spectral, @martinvonz, or @durin42 know what's up.

indygreg: There are 2 bugs here. One is dropping the `_` prefix from `clrevtolocalrev`. The other is…
self._changelogdone = True self._changelogdone = True
return closechunk() return closechunk()
def _fileheader(self, fname): def _fileheader(self, fname):
return chunkheader(len(fname)) + fname return chunkheader(len(fname)) + fname
# Extracted both for clarity and for overriding in extensions. # Extracted both for clarity and for overriding in extensions.
# a case like 882681bc3166 and its ancestors (back about 10 # a case like 882681bc3166 and its ancestors (back about 10
# revisions or so) in the main hg repo. # revisions or so) in the main hg repo.
# #
# The one invariant we *know* holds is that the new (potentially # The one invariant we *know* holds is that the new (potentially
# bogus) DAG shape will be valid if we order the nodes in the # bogus) DAG shape will be valid if we order the nodes in the
# order that they're introduced in dramatis personae by the # order that they're introduced in dramatis personae by the
# changelog, so what we do is we sort the non-changelog histories # changelog, so what we do is we sort the non-changelog histories
# by the order in which they are used by the changelog. # by the order in which they are used by the changelog.
if util.safehasattr(self, '_full_nodes') and self._clnode_to_rev: if util.safehasattr(self, '_full_nodes') and self._clnodetorev:
key = lambda n: self._clnode_to_rev[lookup(n)] key = lambda n: self._clnodetorev[lookup(n)]
return [store.rev(n) for n in sorted(nodelist, key=key)] return [store.rev(n) for n in sorted(nodelist, key=key)]
# for generaldelta revlogs, we linearize the revs; this will both be # for generaldelta revlogs, we linearize the revs; this will both be
# much quicker and generate a much smaller bundle # much quicker and generate a much smaller bundle
if (store._generaldelta and self._reorder is None) or self._reorder: if (store._generaldelta and self._reorder is None) or self._reorder:
dag = dagutil.revlogdag(store) dag = dagutil.revlogdag(store)
return dag.linearize(set(store.rev(n) for n in nodelist)) return dag.linearize(set(store.rev(n) for n in nodelist))
else: else:
n = c[0] n = c[0]
# Record the first changeset introducing this manifest # Record the first changeset introducing this manifest
# version. # version.
mfs.setdefault(n, x) mfs.setdefault(n, x)
# Set this narrow-specific dict so we have the lowest # Set this narrow-specific dict so we have the lowest
# manifest revnum to look up for this cl revnum. (Part of # manifest revnum to look up for this cl revnum. (Part of
# mapping changelog ellipsis parents to manifest ellipsis # mapping changelog ellipsis parents to manifest ellipsis
# parents) # parents)
self._next_clrev_to_localrev.setdefault(cl.rev(x), self._nextclrevtolocalrev.setdefault(cl.rev(x),
mfrevlog.rev(n)) mfrevlog.rev(n))
# We can't trust the changed files list in the changeset if the # We can't trust the changed files list in the changeset if the
# client requested a shallow clone. # client requested a shallow clone.
if self._isshallow: if self._isshallow:
changedfiles.update(mfl[c[0]].read().keys()) changedfiles.update(mfl[c[0]].read().keys())
else: else:
changedfiles.update(c[3]) changedfiles.update(c[3])
else: else:
# Defining this function has a side-effect of overriding the # Defining this function has a side-effect of overriding the
# function of the same name that was passed in as an argument. # function of the same name that was passed in as an argument.
# TODO have caller pass in appropriate function. # TODO have caller pass in appropriate function.
def linknodes(flog, fname): def linknodes(flog, fname):
for c in commonctxs: for c in commonctxs:
try: try:
fnode = c.filenode(fname) fnode = c.filenode(fname)
self._clrev_to_localrev[c.rev()] = flog.rev(fnode) self._clrevtolocalrev[c.rev()] = flog.rev(fnode)
except error.ManifestLookupError: except error.ManifestLookupError:
pass pass
links = oldlinknodes(flog, fname) links = oldlinknodes(flog, fname)
if len(links) != len(mfdicts): if len(links) != len(mfdicts):
for mf, lr in mfdicts: for mf, lr in mfdicts:
fnode = mf.get(fname, None) fnode = mf.get(fname, None)
if fnode in links: if fnode in links:
links[fnode] = min(links[fnode], lr, key=clrev) links[fnode] = min(links[fnode], lr, key=clrev)
flags=store.flags(rev), flags=store.flags(rev),
deltachunks=(prefix, delta), deltachunks=(prefix, delta),
) )
def _revisiondeltanarrow(self, store, rev, prev, linknode): def _revisiondeltanarrow(self, store, rev, prev, linknode):
# build up some mapping information that's useful later. See # build up some mapping information that's useful later. See
# the local() nested function below. # the local() nested function below.
if not self._changelogdone: if not self._changelogdone:
self._clnode_to_rev[linknode] = rev self._clnodetorev[linknode] = rev
linkrev = rev linkrev = rev
self._clrev_to_localrev[linkrev] = rev self._clrevtolocalrev[linkrev] = rev
else: else:
linkrev = self._clnode_to_rev[linknode] linkrev = self._clnodetorev[linknode]
self._clrev_to_localrev[linkrev] = rev self._clrevtolocalrev[linkrev] = rev
# This is a node to send in full, because the changeset it # This is a node to send in full, because the changeset it
# corresponds to was a full changeset. # corresponds to was a full changeset.
if linknode in self._full_nodes: if linknode in self._full_nodes:
return self._revisiondeltanormal(store, rev, prev, linknode) return self._revisiondeltanormal(store, rev, prev, linknode)
# At this point, a node can either be one we should skip or an # At this point, a node can either be one we should skip or an
# ellipsis. If it's not an ellipsis, bail immediately. # ellipsis. If it's not an ellipsis, bail immediately.
return nullrev return nullrev
if not self._changelogdone: if not self._changelogdone:
# If we're doing the changelog, it's possible that we # If we're doing the changelog, it's possible that we
# have a parent that is already on the client, and we # have a parent that is already on the client, and we
# need to store some extra mapping information so that # need to store some extra mapping information so that
# our contained ellipsis nodes will be able to resolve # our contained ellipsis nodes will be able to resolve
# their parents. # their parents.
if clrev not in self._clrev_to_localrev: if clrev not in self._clrevtolocalrev:
clnode = store.node(clrev) clnode = store.node(clrev)
self._clnode_to_rev[clnode] = clrev self._clnodetorev[clnode] = clrev
return clrev return clrev
# Walk the ellipsis-ized changelog breadth-first looking for a # Walk the ellipsis-ized changelog breadth-first looking for a
# change that has been linked from the current revlog. # change that has been linked from the current revlog.
# #
# For a flat manifest revlog only a single step should be necessary # For a flat manifest revlog only a single step should be necessary
# as all relevant changelog entries are relevant to the flat # as all relevant changelog entries are relevant to the flat
# manifest. # manifest.
# #
# For a filelog or tree manifest dirlog however not every changelog # For a filelog or tree manifest dirlog however not every changelog
# entry will have been relevant, so we need to skip some changelog # entry will have been relevant, so we need to skip some changelog
# nodes even after ellipsis-izing. # nodes even after ellipsis-izing.
walk = [clrev] walk = [clrev]
while walk: while walk:
p = walk[0] p = walk[0]
walk = walk[1:] walk = walk[1:]
if p in self._clrev_to_localrev: if p in self._clrevtolocalrev:
return self._clrev_to_localrev[p] return self._clrevtolocalrev[p]
elif p in self._full_nodes: elif p in self._full_nodes:
walk.extend([pp for pp in self._repo.changelog.parentrevs(p) walk.extend([pp for pp in self._repo.changelog.parentrevs(p)
if pp != nullrev]) if pp != nullrev])
elif p in self._precomputed_ellipsis: elif p in self._precomputed_ellipsis:
walk.extend([pp for pp in self._precomputed_ellipsis[p] walk.extend([pp for pp in self._precomputed_ellipsis[p]
if pp != nullrev]) if pp != nullrev])
else: else:
# In this case, we've got an ellipsis with parents # In this case, we've got an ellipsis with parents
shallow=depth is not None) shallow=depth is not None)
# Give the packer the list of nodes which should not be # Give the packer the list of nodes which should not be
# ellipsis nodes. We store this rather than the set of nodes # ellipsis nodes. We store this rather than the set of nodes
# that should be an ellipsis because for very large histories # that should be an ellipsis because for very large histories
# we expect this to be significantly smaller. # we expect this to be significantly smaller.
packer._full_nodes = relevant_nodes packer._full_nodes = relevant_nodes
# Maps ellipsis revs to their roots at the changelog level. # Maps ellipsis revs to their roots at the changelog level.
packer._precomputed_ellipsis = ellipsisroots packer._precomputed_ellipsis = ellipsisroots
# Maps CL revs to per-revlog revisions. Cleared in close() at
# the end of each group.
packer._clrev_to_localrev = {}
packer._next_clrev_to_localrev = {}
# Maps changelog nodes to changelog revs. Filled in once
# during changelog stage and then left unmodified.
packer._clnode_to_rev = {}
return packer.generate(common, visitnodes, False, source) return packer.generate(common, visitnodes, False, source)