This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3755

remotefilectx: when looking for linknodes, use unfiltered repo
ClosedPublic

Authored by spectral on Jun 15 2018, 5:17 PM.
Tags
None
Subscribers
quark

Details

Reviewers
quark
Group Reviewers
Restricted Project
Commits
rFBHGXf20ef49301c4: remotefilectx: when looking for linknodes, use unfiltered repo
Summary

Currently, remotefilelog updates the linknode whenever a commit containing the
file is created, including during things like amend - even if the file is not
touched. This causes some issues if that node is subsequently stripped for
whatever reason, either manually or as part of a --abort of the operation that
created it (for more information and a sample way of reproducing this, see
https://bitbucket.org/facebook/hg-experimental/issues/1/hg-revert-r-then-hg-shelve-can-cause-bad)

We are experimenting with *not* updating the linknode if the file revision
exists already and the existing one is valid. This is the behavior that stock
Mercurial seems to have, it will not update the linkrev unless it needs to.

Since the linknode is not updated, it is very likely for it to point to a
filtered node. When tracing file moves, we try to use the linknode and then
either fall back to a slow path or fail completely because it's filtered. This
patch resolves that issue: when looking for linknodes, use the unfiltered repo.
This also mimics what Mercurial does.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

spectral created this revision.Jun 15 2018, 5:17 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 15 2018, 5:17 PM
quark accepted this revision.Jun 15 2018, 10:42 PM
quark added a subscriber: quark.

Pushed. Thanks!

This revision is now accepted and ready to land.Jun 15 2018, 10:42 PM
Closed by commit rFBHGXf20ef49301c4: remotefilectx: when looking for linknodes, use unfiltered repo (authored by spectral). · Explain WhyJun 15 2018, 11:42 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mremotefilelog/remotefilectx.py (4 lines)

Diff 9106

remotefilelog/remotefilectx.py

return self._changeid return self._changeid
elif '_changectx' in self.__dict__: elif '_changectx' in self.__dict__:
return self._changectx.rev() return self._changectx.rev()
elif '_descendantrev' in self.__dict__: elif '_descendantrev' in self.__dict__:
# this file context was created from a revision with a known # this file context was created from a revision with a known
# descendant, we can (lazily) correct for linkrev aliases # descendant, we can (lazily) correct for linkrev aliases
linknode = self._adjustlinknode(self._path, self._filelog, linknode = self._adjustlinknode(self._path, self._filelog,
self._filenode, self._descendantrev) self._filenode, self._descendantrev)
return self._repo.changelog.rev(linknode) return self._repo.unfiltered().changelog.rev(linknode)
else: else:
return self.linkrev() return self.linkrev()
def filectx(self, fileid, changeid=None): def filectx(self, fileid, changeid=None):
'''opens an arbitrary revision of the file without '''opens an arbitrary revision of the file without
opening a new filelog''' opening a new filelog'''
return remotefilectx(self._repo, self._path, fileid=fileid, return remotefilectx(self._repo, self._path, fileid=fileid,
filelog=self._filelog, changeid=changeid) filelog=self._filelog, changeid=changeid)
or the user-passed rev we're looking back from. It may contain two revs or the user-passed rev we're looking back from. It may contain two revs
when there is a merge going on, or zero revs when a root node with no when there is a merge going on, or zero revs when a root node with no
parents is being created. parents is being created.
""" """
if not revs: if not revs:
return False return False
try: try:
# Use the C fastpath to check if the given linknode is correct. # Use the C fastpath to check if the given linknode is correct.
cl = self._repo.changelog cl = self._repo.unfiltered().changelog
return any(cl.isancestor(linknode, cl.node(r)) for r in revs) return any(cl.isancestor(linknode, cl.node(r)) for r in revs)
except error.LookupError: except error.LookupError:
# The linknode read from the blob may have been stripped or # The linknode read from the blob may have been stripped or
# otherwise not present in the repository anymore. Do not fail hard # otherwise not present in the repository anymore. Do not fail hard
# in this case. Instead, return false and continue the search for # in this case. Instead, return false and continue the search for
# the correct linknode. # the correct linknode.
return False return False