Patchwork [4,of,5] bundle2: update callsites to explicitly specify mandatory parts

login
register
mail settings
Submitter Eric Sumner
Date Dec. 16, 2014, 9:57 p.m.
Message ID <9729f1cb29923ab0e4f2.1418767053@dev911.prn1.facebook.com>
Download mbox | patch
Permalink /patch/7127/
State Changes Requested
Delegated to: Pierre-Yves David
Headers show

Comments

Eric Sumner - Dec. 16, 2014, 9:57 p.m.
# HG changeset patch
# User Eric Sumner <ericsumner@fb.com>
# Date 1418424904 28800
#      Fri Dec 12 14:55:04 2014 -0800
# Node ID 9729f1cb29923ab0e4f2bad7a0b8193dbbd87a70
# Parent  cc5b8da4adb3c56a2485e177498e036badfb21c7
bundle2: update callsites to explicitly specify mandatory parts

Encoding whether or not a part is mandatory in the capitalization of the
parttype is unintuitive and error-prone.  This sequence of patches separates
these concerns in the API to reduce programmer error and pave the way for
a potential change in how this information is transmitted over the wire.

This updates all the callsites in core mercurial to use the new, more explicit
mechanism for declaring a part to be mandatory.  A followup patch will remove
the autodetection

Patch

diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -49,7 +49,7 @@ 
         if version is None:
             raise ValueError('bundler do not support common obsmarker format')
         stream = obsolete.encodemarkers(markers, True, version=version)
-        return bundler.newpart('B2X:OBSMARKERS', data=stream)
+        return bundler.newpart('b2x:obsmarkers', data=stream, mandatory=True)
     return None
 
 class pushoperation(object):
@@ -454,7 +454,9 @@ 
                                      pushop.remote,
                                      pushop.outgoing)
     if not pushop.force:
-        bundler.newpart('B2X:CHECK:HEADS', data=iter(pushop.remoteheads))
+        bundler.newpart('b2x:check:heads',
+                        data=iter(pushop.remoteheads),
+                        mandatory=True)
     b2caps = bundle2.bundle2caps(pushop.remote)
     version = None
     cgversions = b2caps.get('b2x:changegroup')
@@ -469,7 +471,7 @@ 
         cg = changegroup.getlocalchangegroupraw(pushop.repo, 'push',
                                                 pushop.outgoing,
                                                 version=version)
-    cgpart = bundler.newpart('B2X:CHANGEGROUP', data=cg)
+    cgpart = bundler.newpart('b2x:changegroup', data=cg, mandatory=True)
     if version is not None:
         cgpart.addparam('version', version)
     def handlereply(op):
diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
--- a/mercurial/wireproto.py
+++ b/mercurial/wireproto.py
@@ -837,7 +837,8 @@ 
             os.unlink(tempname)
     except error.BundleValueError, exc:
             bundler = bundle2.bundle20(repo.ui)
-            errpart = bundler.newpart('B2X:ERROR:UNSUPPORTEDCONTENT')
+            errpart = bundler.newpart('b2x:error:unsupportedcontent',
+                                      mandatory = True)
             if exc.parttype is not None:
                 errpart.addparam('parttype', exc.parttype)
             if exc.params:
@@ -854,8 +855,9 @@ 
             advargs = []
             if inst.hint is not None:
                 advargs.append(('hint', inst.hint))
-            bundler.addpart(bundle2.bundlepart('B2X:ERROR:ABORT',
-                                               manargs, advargs))
+            bundler.addpart(bundle2.bundlepart('b2x:error:abort',
+                                               manargs, advargs,
+                                               mandatory = True))
             return streamres(bundler.getchunks())
         else:
             sys.stderr.write("abort: %s\n" % inst)
@@ -863,7 +865,9 @@ 
     except error.PushRaced, exc:
         if getattr(exc, 'duringunbundle2', False):
             bundler = bundle2.bundle20(repo.ui)
-            bundler.newpart('B2X:ERROR:PUSHRACED', [('message', str(exc))])
+            bundler.newpart('b2x:error:pushraced',
+                            [('message', str(exc))],
+                            mandatory = True)
             return streamres(bundler.getchunks())
         else:
             return pusherr(str(exc))
diff --git a/tests/test-bundle2-exchange.t b/tests/test-bundle2-exchange.t
--- a/tests/test-bundle2-exchange.t
+++ b/tests/test-bundle2-exchange.t
@@ -364,7 +364,7 @@ 
   >     if reason == 'abort':
   >         bundler.newpart('test:abort')
   >     if reason == 'unknown':
-  >         bundler.newpart('TEST:UNKNOWN')
+  >         bundler.newpart('test:unknown', mandatory=True)
   >     if reason == 'race':
   >         # 20 Bytes of crap
   >         bundler.newpart('b2x:check:heads', data='01234567890123456789')
diff --git a/tests/test-bundle2-format.t b/tests/test-bundle2-format.t
--- a/tests/test-bundle2-format.t
+++ b/tests/test-bundle2-format.t
@@ -124,9 +124,9 @@ 
   >        # advisory known part with unknown mandatory param
   >        bundler.newpart('test:song', [('randomparam','')])
   >     if opts['unknown']:
-  >        bundler.newpart('test:UNKNOWN', data='some random content')
+  >        bundler.newpart('test:unknown', data='some random content', mandatory=True)
   >     if opts['unknownparams']:
-  >        bundler.newpart('test:SONG', [('randomparams', '')])
+  >        bundler.newpart('test:song', [('randomparams', '')], mandatory=True)
   >     if opts['parts']:
   >        bundler.newpart('test:ping')
   >     if opts['genraise']:
diff --git a/tests/test-bundle2-remote-changegroup.t b/tests/test-bundle2-remote-changegroup.t
--- a/tests/test-bundle2-remote-changegroup.t
+++ b/tests/test-bundle2-remote-changegroup.t
@@ -32,11 +32,11 @@ 
   >           python expression as parameters. The python expression is
   >           evaluated with eval, and is expected to be a dict.
   >     """
-  >     def newpart(name, data=''):
+  >     def newpart(name, data='', **kwargs):
   >         """wrapper around bundler.newpart adding an extra part making the
   >         client output information about each processed part"""
   >         bundler.newpart('b2x:output', data=name)
-  >         part = bundler.newpart(name, data=data)
+  >         part = bundler.newpart(name, data=data, **kwargs)
   >         return part
   > 
   >     for line in open(repo.join('bundle2maker'), 'r'):
@@ -50,13 +50,13 @@ 
   >            bundledata = open(file, 'rb').read()
   >            digest = util.digester.preferred(b2caps['digests'])
   >            d = util.digester([digest], bundledata)
-  >            part = newpart('B2X:REMOTE-CHANGEGROUP')
+  >            part = newpart('b2x:remote-changegroup', mandatory = True)
   >            part.addparam('url', url)
   >            part.addparam('size', str(len(bundledata)))
   >            part.addparam('digests', digest)
   >            part.addparam('digest:%s' % digest, d[digest])
   >         elif verb == 'raw-remote-changegroup':
-  >            part = newpart('B2X:REMOTE-CHANGEGROUP')
+  >            part = newpart('b2x:remote-changegroup', mandatory = True)
   >            for k, v in eval(args).items():
   >                part.addparam(k, str(v))
   >         elif verb == 'changegroup':
@@ -65,7 +65,7 @@ 
   >             heads = [repo.lookup(r) for r in repo.revs(heads)]
   >             cg = changegroup.getchangegroup(repo, 'changegroup',
   >                 heads=heads, common=common)
-  >             newpart('B2X:CHANGEGROUP', cg.getchunks())
+  >             newpart('b2x:changegroup', cg.getchunks(), mandatory = True)
   >         else:
   >             raise Exception('unknown verb')
   > 
@@ -137,7 +137,7 @@ 
   $ hg pull -R clone ssh://user@dummy/repo
   pulling from ssh://user@dummy/repo
   searching for changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   adding changesets
   adding manifests
   adding file changes
@@ -180,12 +180,12 @@ 
   $ hg pull -R clone ssh://user@dummy/repo
   pulling from ssh://user@dummy/repo
   searching for changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   adding changesets
   adding manifests
   adding file changes
   added 2 changesets with 2 changes to 2 files (+1 heads)
-  remote: B2X:CHANGEGROUP
+  remote: b2x:changegroup
   adding changesets
   adding manifests
   adding file changes
@@ -228,12 +228,12 @@ 
   $ hg pull -R clone ssh://user@dummy/repo
   pulling from ssh://user@dummy/repo
   searching for changes
-  remote: B2X:CHANGEGROUP
+  remote: b2x:changegroup
   adding changesets
   adding manifests
   adding file changes
   added 2 changesets with 2 changes to 2 files (+1 heads)
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   adding changesets
   adding manifests
   adding file changes
@@ -279,17 +279,17 @@ 
   $ hg pull -R clone ssh://user@dummy/repo
   pulling from ssh://user@dummy/repo
   searching for changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   adding changesets
   adding manifests
   adding file changes
   added 2 changesets with 2 changes to 2 files (+1 heads)
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   adding changesets
   adding manifests
   adding file changes
   added 2 changesets with 1 changes to 1 files
-  remote: B2X:CHANGEGROUP
+  remote: b2x:changegroup
   adding changesets
   adding manifests
   adding file changes
@@ -324,7 +324,7 @@ 
   > EOF
   $ hg clone ssh://user@dummy/repo clone
   requesting all changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   adding changesets
   adding manifests
   adding file changes
@@ -338,7 +338,7 @@ 
   > EOF
   $ hg clone ssh://user@dummy/repo clone
   requesting all changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   adding changesets
   adding manifests
   adding file changes
@@ -354,7 +354,7 @@ 
   > EOF
   $ hg clone ssh://user@dummy/repo clone
   requesting all changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   adding changesets
   adding manifests
   adding file changes
@@ -372,7 +372,7 @@ 
   > EOF
   $ hg clone ssh://user@dummy/repo clone
   requesting all changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   adding changesets
   adding manifests
   adding file changes
@@ -388,7 +388,7 @@ 
   > EOF
   $ hg clone ssh://user@dummy/repo clone
   requesting all changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   adding changesets
   adding manifests
   adding file changes
@@ -404,7 +404,7 @@ 
   > EOF
   $ hg clone ssh://user@dummy/repo clone
   requesting all changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   adding changesets
   adding manifests
   adding file changes
@@ -433,12 +433,12 @@ 
   $ hg pull -R clone ssh://user@dummy/repo
   pulling from ssh://user@dummy/repo
   searching for changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   adding changesets
   adding manifests
   adding file changes
   added 2 changesets with 2 changes to 2 files (+1 heads)
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   adding changesets
   adding manifests
   adding file changes
@@ -467,7 +467,7 @@ 
   $ hg pull -R clone ssh://user@dummy/repo
   pulling from ssh://user@dummy/repo
   searching for changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   abort: remote-changegroup: missing "url" param
   [255]
 
@@ -479,7 +479,7 @@ 
   $ hg pull -R clone ssh://user@dummy/repo
   pulling from ssh://user@dummy/repo
   searching for changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   abort: remote-changegroup: missing "size" param
   [255]
 
@@ -491,7 +491,7 @@ 
   $ hg pull -R clone ssh://user@dummy/repo
   pulling from ssh://user@dummy/repo
   searching for changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   abort: remote-changegroup: invalid value for param "size"
   [255]
 
@@ -503,7 +503,7 @@ 
   $ hg pull -R clone ssh://user@dummy/repo
   pulling from ssh://user@dummy/repo
   searching for changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   adding changesets
   adding manifests
   adding file changes
@@ -522,7 +522,7 @@ 
   $ hg pull -R clone ssh://user@dummy/repo
   pulling from ssh://user@dummy/repo
   searching for changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   abort: missing support for b2x:remote-changegroup - digest:foo
   [255]
 
@@ -534,7 +534,7 @@ 
   $ hg pull -R clone ssh://user@dummy/repo
   pulling from ssh://user@dummy/repo
   searching for changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   abort: remote-changegroup: missing "digest:sha1" param
   [255]
 
@@ -546,7 +546,7 @@ 
   $ hg pull -R clone ssh://user@dummy/repo
   pulling from ssh://user@dummy/repo
   searching for changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   abort: remote-changegroup does not support ssh urls
   [255]
 
@@ -561,7 +561,7 @@ 
   $ hg pull -R clone ssh://user@dummy/repo
   pulling from ssh://user@dummy/repo
   searching for changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   abort: http://localhost:$HGPORT/notbundle.hg: not a Mercurial bundle
   [255]
 
@@ -576,7 +576,7 @@ 
   $ hg pull -R clone ssh://user@dummy/repo
   pulling from ssh://user@dummy/repo
   searching for changes
-  remote: B2X:REMOTE-CHANGEGROUP
+  remote: b2x:remote-changegroup
   abort: http://localhost:$HGPORT/notbundle10.hg: not a bundle version 1.0
   [255]