Patchwork D1954: bundle: add the possibility to bundle a stream v2 part

login
register
mail settings
Submitter phabricator
Date Jan. 31, 2018, 4:25 p.m.
Message ID <differential-rev-PHID-DREV-hqbsdqqjwz5xnaxq4xch-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/27090/
State Superseded
Headers show

Comments

phabricator - Jan. 31, 2018, 4:25 p.m.
lothiraldan created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/bundle2.py

CHANGE DETAILS




To: lothiraldan, #hg-reviewers
Cc: mercurial-devel
phabricator - Feb. 2, 2018, 9:33 a.m.
lothiraldan added a comment.


  Now that we have more time to review this series, this is the part I am the least comfortable with. I think we could/should extract the stream part generator somewhere else than exchange.py. Maybe bundle2.py directly but I'm not sure. Any ideas?

REPOSITORY
  rHG Mercurial

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

To: lothiraldan, #hg-reviewers
Cc: mercurial-devel
phabricator - March 19, 2018, 8:39 p.m.
indygreg requested changes to this revision.
indygreg added a comment.
This revision now requires changes to proceed.


  In https://phab.mercurial-scm.org/D1954#33576, @lothiraldan wrote:
  
  > Now that we have more time to review this series, this is the part I am the least comfortable with. I think we could/should extract the stream part generator somewhere else than exchange.py. Maybe bundle2.py directly but I'm not sure. Any ideas?
  
  
  Yup. You've stumbled upon something: we have 2 variations of code that produce bundle2 data. We have `bundle2._addpartsfromopts()` and `exchange.getbundlechunks()`. There is redundancy between the two. For example, creation of the `changegroup` part.
  
  I would *very* much like to see this code unified. Probably in `bundle2.py`. What makes it difficult to unify is that each implementation is determining *what data should I generate* via different mechanisms. `bundle2.py` is using a dict of [content] options. `exchange.py` is using bundle2 capabilities, which come from the client.
  
  If you squint hard enough, these are both almost the same thing. I would like to think that we could normalize b2caps and content options down to a single data structure that described what to generate and we could pass that into a common *generate bundle2 content* function. I /think/ we want to unify on content options. But I'm not sure.
  
  Things are a bit more difficult than they should be because bundle2 is both used for data at rest and in flight. It represents both static data and the request/results of an RPC call over the wire protocol. e.g. the `check:*`, `error:*`, and `reply:*` parts don't make a lot of sense for local-only bundle2 bundles. Those are there mainly to facilitate wire protocol features. So any unified *create a bundle* or *apply a bundle* functionality need to account for the fact that there are special bundle2 parts and functionality that are unique to the wire protocol.
  
  Anyway, I'd fix the immediate concern of having to import `exchange` by moving the code for creating this bundle2 part into `bundle2.py` and having the part generator in `exchange` call into it.

REPOSITORY
  rHG Mercurial

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

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

Patch

diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -1601,6 +1601,16 @@ 
         phasedata = phases.binaryencode(headsbyphase)
         bundler.newpart('phase-heads', data=phasedata)
 
+    if opts.get('streamv2', False):
+        # The generation of the stream part is currently in exchange but
+        # importing exchange create an import cycle. We could extract the
+        # generation of the stream part in bundle2 or another file.
+        from . import exchange
+        # The streamv2 part doesn't uses the source argument, we can refactor
+        # this once the bundle part generator has been extracted
+        source = None
+        exchange._getbundlestream2(bundler, repo, source, stream=True)
+
 def addparttagsfnodescache(repo, bundler, outgoing):
     # we include the tags fnode cache for the bundle changeset
     # (as an optional parts)