[PATCH 1 of 2 v2] bundle2.bundlepart: make mandatory part flag explicit in API

[Pierre-Yves David]

On 12/17/2014 03:42 PM, Eric Sumner wrote:
> # HG changeset patch
> # User Eric Sumner <ericsumner at fb.com>
> # Date 1418856629 28800
> #      Wed Dec 17 14:50:29 2014 -0800
> # Node ID 72d491a98c7a589f905b76328f584728c12f1d79
> # Parent  a4e1268f4ce180063d0ead294041d07dc816cb32
> bundle2.bundlepart: make mandatory part flag explicit in API

Pushed to the clowncopter after a couple of fixes.

>
> This makes all bundle2 parts mandatory unless they are expressly made advisory
> via the keyword parameter or the bundlepart.mandatory property.
>
> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
> --- a/mercurial/bundle2.py
> +++ b/mercurial/bundle2.py
> @@ -588,7 +588,7 @@
>       """
>
>       def __init__(self, parttype, mandatoryparams=(), advisoryparams=(),
> -                 data=''):
> +                 data='', mandatory = True):

for function argument, we are not using space around the "=" (from 
pep8). I've fixed a bunch of them.

>           self.id = None
>           self.type = parttype
>           self._data = data
> @@ -605,6 +605,7 @@
>           # - False: currently generated,
>           # - True: generation done.
>           self._generated = None
> +        self.mandatory = mandatory
>
>       # methods used to defines the part content
>       def __setdata(self, data):
> @@ -642,9 +643,13 @@
>               raise RuntimeError('part can only be consumed once')
>           self._generated = False
>           #### header
> +        if self.mandatory:
> +            parttype = self.type.upper()
> +        else:
> +            parttype = self.type.lower()
>           ## parttype
> -        header = [_pack(_fparttypesize, len(self.type)),
> -                  self.type, _pack(_fpartid, self.id),
> +        header = [_pack(_fparttypesize, len(parttype)),
> +                  parttype, _pack(_fpartid, self.id),
>                    ]
>           ## parameters
>           # count
> 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
> @@ -55,7 +55,8 @@
>     >     op.ui.write('received ping request (id %i)\n' % part.id)
>     >     if op.reply is not None and 'ping-pong' in op.reply.capabilities:
>     >         op.ui.write_err('replying to ping request (id %i)\n' % part.id)
> -  >         op.reply.newpart('test:pong', [('in-reply-to', str(part.id))])
> +  >         op.reply.newpart('test:pong', [('in-reply-to', str(part.id))],
> +  >                          mandatory = False)
>     >
>     > @bundle2.parthandler('test:debugreply')
>     > def debugreply(op, part):
> @@ -111,29 +112,30 @@
>     >             bundler.newpart('b2x:changegroup', data=cg.getchunks())
>     >
>     >     if opts['parts']:
> -  >        bundler.newpart('test:empty')
> +  >        bundler.newpart('test:empty', mandatory = False)
>     >        # add a second one to make sure we handle multiple parts
> -  >        bundler.newpart('test:empty')
> -  >        bundler.newpart('test:song', data=ELEPHANTSSONG)
> -  >        bundler.newpart('test:debugreply')
> +  >        bundler.newpart('test:empty', mandatory = False)
> +  >        bundler.newpart('test:song', data=ELEPHANTSSONG, mandatory = False)
> +  >        bundler.newpart('test:debugreply', mandatory = False)
>     >        mathpart = bundler.newpart('test:math')
>     >        mathpart.addparam('pi', '3.14')
>     >        mathpart.addparam('e', '2.72')
>     >        mathpart.addparam('cooking', 'raw', mandatory=False)
>     >        mathpart.data = '42'
> +  >        mathpart.mandatory = False
>     >        # advisory known part with unknown mandatory param
> -  >        bundler.newpart('test:song', [('randomparam','')])
> +  >        bundler.newpart('test:song', [('randomparam','')], mandatory = False)
>     >     if opts['unknown']:
>     >        bundler.newpart('test:UNKNOWN', data='some random content')
>     >     if opts['unknownparams']:
>     >        bundler.newpart('test:SONG', [('randomparams', '')])
>     >     if opts['parts']:
> -  >        bundler.newpart('test:ping')
> +  >        bundler.newpart('test:ping', mandatory = False)
>     >     if opts['genraise']:
>     >        def genraise():
>     >            yield 'first line\n'
>     >            raise RuntimeError('Someone set up us the bomb!')
> -  >        bundler.newpart('b2x:output', data=genraise())
> +  >        bundler.newpart('b2x:output', data=genraise(), mandatory = False)
>     >
>     >     if path is None:
>     >        file = sys.stdout
> @@ -608,12 +610,12 @@
>
>     $ cat ../reply.hg2
>     HG2Y\x00\x00\x00\x00\x00\x00\x00\x1f (esc)
> -  b2x:output\x00\x00\x00\x00\x00\x01\x0b\x01in-reply-to3\x00\x00\x00\xd9The choir starts singing: (esc)
> +  B2X:OUTPUT\x00\x00\x00\x00\x00\x01\x0b\x01in-reply-to3\x00\x00\x00\xd9The choir starts singing: (esc)
>         Patali Dirapata, Cromda Cromda Ripalo, Pata Pata, Ko Ko Ko
>         Bokoro Dipoulito, Rondi Rondi Pepino, Pata Pata, Ko Ko Ko
>         Emana Karassoli, Loucra Loucra Ponponto, Pata Pata, Ko Ko Ko.
>     \x00\x00\x00\x00\x00\x00\x00\x1f (esc)
> -  b2x:output\x00\x00\x00\x01\x00\x01\x0b\x01in-reply-to4\x00\x00\x00\xc9debugreply: capabilities: (esc)
> +  B2X:OUTPUT\x00\x00\x00\x01\x00\x01\x0b\x01in-reply-to4\x00\x00\x00\xc9debugreply: capabilities: (esc)
>     debugreply:     'city=!'
>     debugreply:         'celeste,ville'
>     debugreply:     'elephants'
> @@ -621,7 +623,7 @@
>     debugreply:         'celeste'
>     debugreply:     'ping-pong'
>     \x00\x00\x00\x00\x00\x00\x00\x1e	test:pong\x00\x00\x00\x02\x01\x00\x0b\x01in-reply-to7\x00\x00\x00\x00\x00\x00\x00\x1f (esc)
> -  b2x:output\x00\x00\x00\x03\x00\x01\x0b\x01in-reply-to7\x00\x00\x00=received ping request (id 7) (esc)
> +  B2X:OUTPUT\x00\x00\x00\x03\x00\x01\x0b\x01in-reply-to7\x00\x00\x00=received ping request (id 7) (esc)

Having the test outptu changing is an hint that you did not dropped 
mandatory-ness from multiple part creation. I've fixed them too.


-- 
Pierre-Yves David