Patchwork D6182: bundle2: handle compression in _forwardchunks

login
register
mail settings
Submitter phabricator
Date April 2, 2019, 5:49 p.m.
Message ID <differential-rev-PHID-DREV-ce4f52hsxh2kmvj4d3pq-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/39443/
State Superseded
Headers show

Comments

phabricator - April 2, 2019, 5:49 p.m.
joerg.sonnenberger created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  _forwardchunks is used to compensate for getbundle protocol deficits.
  Since it transparently decodes the payload, it also needs to remove the
  corresponding compression parameter in case the server decides to send
  one. This the wire protocol part of issue 5990.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/bundle2.py
  tests/test-pull-bundle.t

CHANGE DETAILS




To: joerg.sonnenberger, #hg-reviewers
Cc: mercurial-devel
phabricator - April 16, 2019, 5:18 p.m.
durin42 added a comment.


  Ugh I'm sorry - I evidently reviewed this and never pushed submit. :(

INLINE COMMENTS

> bundle2.py:846
> +                if k.lower() != 'compression':
> +                    oparams.append(p)
> +            outparams = ' '.join(outparams)

so...what's going on with oparams here? should it be outparams?

(is there more to this series that would get this code covered by tests?)

REPOSITORY
  rHG Mercurial

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

To: joerg.sonnenberger, #hg-reviewers
Cc: durin42, mercurial-devel
phabricator - April 16, 2019, 8:23 p.m.
joerg.sonnenberger added inline comments.

INLINE COMMENTS

> durin42 wrote in bundle2.py:846
> so...what's going on with oparams here? should it be outparams?
> 
> (is there more to this series that would get this code covered by tests?)

Yes, left-over from earlier rename round. I think the code should handle multiple parameters, since I have no idea when we might run into them. At the same time, I have no idea right now how to test that case.

REPOSITORY
  rHG Mercurial

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

To: joerg.sonnenberger, #hg-reviewers
Cc: durin42, mercurial-devel

Patch

diff --git a/tests/test-pull-bundle.t b/tests/test-pull-bundle.t
--- a/tests/test-pull-bundle.t
+++ b/tests/test-pull-bundle.t
@@ -120,6 +120,38 @@ 
   * sending pullbundle "1.hg" (glob)
   $ rm repo/.hg/blackbox.log
 
+Test pullbundle functionality for incoming
+
+  $ cd repo
+  $ hg --config blackbox.track=debug --debug serve -p $HGPORT2 -d --pid-file=../repo.pid
+  listening at http://*:$HGPORT2/ (bound to $LOCALIP:$HGPORT2) (glob) (?)
+  $ cat ../repo.pid >> $DAEMON_PIDS
+  $ cd ..
+  $ hg clone http://localhost:$HGPORT2/ repo.pullbundle2a -r 0
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  new changesets bbd179dfa0a7 (1 drafts)
+  updating to branch default
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ cd repo.pullbundle2a
+  $ hg incoming -r ed1b79f46b9a
+  comparing with http://localhost:$HGPORT2/
+  searching for changes
+  changeset:   1:ed1b79f46b9a
+  tag:         tip
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     change foo
+  
+  $ cd ..
+  $ killdaemons.py
+  $ grep 'sending pullbundle ' repo/.hg/blackbox.log
+  * sending pullbundle "0.hg" (glob)
+  * sending pullbundle "1.hg" (glob)
+  $ rm repo/.hg/blackbox.log
+
 Test recovery from misconfigured server sending no new data
 
   $ cd repo
diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -834,12 +834,21 @@ 
         if paramssize < 0:
             raise error.BundleValueError('negative bundle param size: %i'
                                          % paramssize)
-        yield _pack(_fstreamparamsize, paramssize)
         if paramssize:
             params = self._readexact(paramssize)
             self._processallparams(params)
-            yield params
-            assert self._compengine.bundletype()[1] == 'UN'
+            # The payload itself is decompressed below, so drop
+            # the compression parameter passed down to compensate.
+            outparams = []
+            for p in params.split(' '):
+                k, v = p.split('=', 1)
+                if k.lower() != 'compression':
+                    oparams.append(p)
+            outparams = ' '.join(outparams)
+            yield _pack(_fstreamparamsize, len(outparams))
+            yield outparams
+        else:
+            yield _pack(_fstreamparamsize, paramssize)
         # From there, payload might need to be decompressed
         self._fp = self._compengine.decompressorreader(self._fp)
         emptycount = 0