[PATCH 3 of 4 V2] bundle2: support for bundling and unbundling payload

David Soria Parra davidsp at fb.com
Fri Mar 28 13:25:22 CDT 2014


pierre-yves.david at ens-lyon.org writes:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at fb.com>
> # Date 1395297375 25200
> #      Wed Mar 19 23:36:15 2014 -0700
> # Node ID 1fbb708cc3821bbb247f409f26ff2a940ca297c5
> # Parent  e7c977d822a8c7921510c034e250ed52233fff41
> bundle2: support for bundling and unbundling payload
>
> We add the ability to bundle and unbundle a payload in parts. The payload is the
> actual binary data of the part. It is used to convey all the applicative data.
> For now we stick to very simple implementation with all the data fit in single
> chunk. This open the door to some bundle2 testing usage. This will be improved before
> bundle2 get used for real. We need to be able to stream the payload in multiple
> part to exchange any changegroup efficiently. This simple version will do for
> now.
>
> Bundling and unbundling are done in the same changeset because the test for
> parts is less modular. However, the result is not too complex.
>
> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
> --- a/mercurial/bundle2.py
> +++ b/mercurial/bundle2.py
> @@ -87,12 +87,17 @@ Binary format is as follow
>      :typename: alphanumerical part name
>      :option: we do not support option yet this denoted by two 16 bites zero.
>  
>  :payload:
>  
> -    The current payload is a 32bit integer with a value of 0. This is
> -    considered an "empty" payload.
> +    payload is a series of `<chunksize><chunkdata>`.
> +
> +    `chunksize` is a 32 bits integer, `chunkdata` are plain bytes (as much as
> +    `chunksize` says)` The payload part is concluded by a zero size chunk.
> +
> +    The current implementation always produces either zero or one chunk.
> +    This is an implementation limitation that will ultimatly be lifted.
>  """
>  
>  import util
>  import struct
>  import urllib
> @@ -107,10 +112,11 @@ from i18n import _
>  _magicstring = 'HG20'
>  
>  _fstreamparamsize = '>H'
>  _fpartheadersize = '>H'
>  _fparttypesize = '>B'
> +_fpayloadsize = '>I'
>  
>  class bundle20(object):
>      """represent an outgoing bundle2 container
>  
>      Use the `addparam` method to add stream level parameter. and `addpart` to
> @@ -254,15 +260,21 @@ class unbundle20(object):
>              _offset[0] = offset + size
>              return data
>          typesize = _unpack(_fparttypesize, fromheader(1))[0]
>          parttype = fromheader(typesize)
>          self.ui.debug('part type: "%s"\n' % parttype)
> -        current = part(parttype)
>          assert fromheader(2) == '\0\0' # no option for now
>          self.ui.debug('part parameters: 0\n')
> -        assert self._readexact(4) == '\0\0\0\0' #empty payload
> -        self.ui.debug('payload chunk size: 0\n')
> +        payload = []
> +        payloadsize = self._unpack(_fpayloadsize)[0]
> +        self.ui.debug('payload chunk size: %i\n' % payloadsize)
> +        while payloadsize:
> +            payload.append(self._readexact(payloadsize))
> +            payloadsize = self._unpack(_fpayloadsize)[0]
> +            self.ui.debug('payload chunk size: %i\n' % payloadsize)
> +        payload = ''.join(payload)
> +        current = part(parttype, data=payload)
>          return current
>  
>  
>  class part(object):
>      """A bundle2 part contains application level payload
> @@ -282,9 +294,14 @@ class part(object):
>                    '\0\0', # No option support for now.
>                   ]
>          headerchunk = ''.join(header)
>          yield _pack(_fpartheadersize, len(headerchunk))
>          yield headerchunk
> -        # force empty part for now
> -        yield '\0\0\0\0'
> +        # we only support fixed size data now.
> +        # This will be improved in the future.
> +        yield _pack(_fpayloadsize, len(self.data))
> +        yield self.data
> +        if len(self.data):
> +            # end of payload
> +            yield _pack(_fpayloadsize, 0)

so we only mark the ned of the part when we have a len(self.data) > 0
because otherwise the self.data itself will terminate the part.
shouldn't we rather go by

  if len(self.data):
    yield _pack(_fpayloadsize, len(self.data)
    yield self.data
  yield _pack(_fpayloadsize, 0)

and make it explizit to always return 0 in the end.


More information about the Mercurial-devel mailing list