Patchwork D1329: bundle: allow bundlerepo to support alternative manifest implementations

login
register
mail settings
Submitter phabricator
Date Nov. 7, 2017, 6:19 p.m.
Message ID <differential-rev-PHID-DREV-7mr3oulwdi6ooobpoygm-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/25410/
State Superseded
Headers show

Comments

phabricator - Nov. 7, 2017, 6:19 p.m.
durham created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION 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.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1329

AFFECTED FILES
  mercurial/bundlerepo.py

CHANGE DETAILS




To: durham, #hg-reviewers
Cc: mercurial-devel
phabricator - Nov. 10, 2017, 5:35 a.m.
indygreg accepted this revision.
indygreg added a comment.
This revision is now accepted and ready to land.


  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.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1329

To: durham, #hg-reviewers, indygreg
Cc: indygreg, mercurial-devel

Patch

diff --git a/mercurial/bundlerepo.py b/mercurial/bundlerepo.py
--- a/mercurial/bundlerepo.py
+++ b/mercurial/bundlerepo.py
@@ -352,14 +352,31 @@ 
         self.filestart = self.bundle.tell()
         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
     def manstart(self):
         self.changelog
         return self.manstart
 
     @localrepo.unfilteredpropertycache
     def filestart(self):
         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
 
     def url(self):