D1389: bundle2: only seek to beginning of part in bundlerepo

[indygreg (Gregory Szorc)]

indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  For reasons still not yet fully understood by me, bundlerepo
  requires its changegroup bundle2 part to be seeked to beginning
  after part iteration. As far as I can tell, it is the only
  bundle2 part consumer that relies on this behavior.
  
  This seeking was performed in the generic iterparts() API. Again,
  I don't fully understand why it was here and not in bundlerepo.
  Probably historical reasons.
  
  What I do know is that all other bundle2 part consumers don't
  need this special behavior (assuming the tests are comprehensive).
  So, we move the code from bundle2's iterparts() to bundlerepo's
  consumption of iterparts().

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/bundle2.py
  mercurial/bundlerepo.py

CHANGE DETAILS

diff --git a/mercurial/bundlerepo.py b/mercurial/bundlerepo.py
--- a/mercurial/bundlerepo.py
+++ b/mercurial/bundlerepo.py
@@ -288,18 +288,26 @@
             self._bundlefile = bundle
             self._cgunpacker = None
 
-            hadchangegroup = False
+            cgpart = None
             for part in bundle.iterparts():
                 if part.type == 'changegroup':
-                    if hadchangegroup:
+                    if cgpart:
                         raise NotImplementedError("can't process "
                                                   "multiple changegroups")
-                    hadchangegroup = True
+                    cgpart = part
 
                 self._handlebundle2part(bundle, part)
 
-            if not hadchangegroup:
+            if not cgpart:
                 raise error.Abort(_("No changegroups found"))
+
+            # This is required to placate a later consumer, which expects
+            # the payload offset to be at the beginning of the changegroup.
+            # We need to do this after the iterparts() generator advances
+            # because iterparts() will seek to end of payload after the
+            # generator returns control to iterparts().
+            cgpart.seek(0, os.SEEK_SET)
+
         elif isinstance(bundle, changegroup.cg1unpacker):
             if bundle.compressed():
                 f = self._writetempbundle(bundle.read, '.hg10un',
diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -859,9 +859,7 @@
             # Ensure part is fully consumed so we can start reading the next
             # part.
             part.consume()
-            # But then seek back to the beginning so the code consuming this
-            # generator has a part that starts at 0.
-            part.seek(0, os.SEEK_SET)
+
             headerblock = self._readpartheader()
         indebug(self.ui, 'end of bundle2 stream')
 



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