This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1329

bundle: allow bundlerepo to support alternative manifest implementations
ClosedPublic

Authored by durham on Nov 7 2017, 1:19 PM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHGa2dfc723b6b5: bundle: allow bundlerepo to support alternative manifest implementations
Summary

With our treemanifest logic, the manifests are no longer transported as part of
the changegroup and are no longer stored in a revlog. This means the
self.manifestlog line in bundlerepo.filestart no longer calls
_constructmanifest, and therefore does not consume the manifest portion of the
changegroup, which means filestart is not populated and we result in an infinite
loop.

The fix is to make filestart aware that self.manifestlog might not consume the
changegroup part, and consume it manually if necessary.

There's currently no way to test this in core, but our treemanifest extension
has tests to cover this.

Diff Detail

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

Event Timeline

durham created this revision.Nov 7 2017, 1:19 PM
indygreg accepted this revision.Nov 10 2017, 12:35 AM
indygreg added a subscriber: indygreg.

bundlerepo is a pile of hacks and this seems par for the course.

FWIW, I've been looking at rewriting bundle2's I/O layer in order to address issue 5691. If you anticipate making significant changes to bundlerepo or bundle2 in the near future, please let me know so we don't step on each other's toes.

This revision is now accepted and ready to land.Nov 10 2017, 12:35 AM
Closed by commit rHGa2dfc723b6b5: bundle: allow bundlerepo to support alternative manifest implementations (authored by durham). · Explain WhyNov 10 2017, 12:41 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/bundlerepo.py (17 lines)

Diff 3385

mercurial/bundlerepo.py

self.bundle.seek(self.manstart) self.bundle.seek(self.manstart)
# consume the header if it exists # consume the header if it exists
self.bundle.manifestheader() self.bundle.manifestheader()
linkmapper = self.unfiltered().changelog.rev linkmapper = self.unfiltered().changelog.rev
m = bundlemanifest(self.svfs, self.bundle, linkmapper) m = bundlemanifest(self.svfs, self.bundle, linkmapper)
self.filestart = self.bundle.tell() self.filestart = self.bundle.tell()
return m return m
def _consumemanifest(self):
"""Consumes the manifest portion of the bundle, setting filestart so the
file portion can be read."""
self.bundle.seek(self.manstart)
self.bundle.manifestheader()
for delta in self.bundle.deltaiter():
pass
self.filestart = self.bundle.tell()
@localrepo.unfilteredpropertycache @localrepo.unfilteredpropertycache
def manstart(self): def manstart(self):
self.changelog self.changelog
return self.manstart return self.manstart
@localrepo.unfilteredpropertycache @localrepo.unfilteredpropertycache
def filestart(self): def filestart(self):
self.manifestlog self.manifestlog
# If filestart was not set by self.manifestlog, that means the
# manifestlog implementation did not consume the manifests from the
# changegroup (ex: it might be consuming trees from a separate bundle2
# part instead). So we need to manually consume it.
if 'filestart' not in self.__dict__:
self._consumemanifest()
return self.filestart return self.filestart
def url(self): def url(self):
return self._url return self._url
def file(self, f): def file(self, f):
if not self.bundlefilespos: if not self.bundlefilespos:
self.bundle.seek(self.filestart) self.bundle.seek(self.filestart)