[PATCH 2 of 4 V2] bundle2: support unbundling empty part

[Pierre-Yves David]

On 03/28/2014 11:10 AM, David Soria Parra wrote:
> pierre-yves.david at ens-lyon.org writes:
>
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david at fb.com>
>> # Date 1395295443 25200
>> #      Wed Mar 19 23:04:03 2014 -0700
>> # Node ID e7c977d822a8c7921510c034e250ed52233fff41
>> # Parent  f5be6f95d76a9369b441da680cf2a3dd959da1f7
>> bundle2: support unbundling empty part
>>
>> We augment the unbundler to make it able to unbundle the empty part we are now
>> able to bundle.
>>
>> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
>> --- a/mercurial/bundle2.py
>> +++ b/mercurial/bundle2.py
>> @@ -238,23 +238,44 @@ class unbundle20(object):
>>               part = self._readpart()
>>           self.ui.debug('end of bundle2 stream\n')
>>
>>       def _readpart(self):
>>           """return None when an end of stream markers is reach"""
>> -        headersize = self._readexact(2)
>> -        assert headersize == '\0\0'
>> -        return None
>> +
>> +        headersize = self._unpack(_fpartheadersize)[0]
>> +        self.ui.debug('part header size: %i\n' % headersize)
>> +        if not headersize:
>> +            return None
>> +        headerblock = self._readexact(headersize)
>> +        # some utility to help reading from the header bloc
>
> bloc -> block
>
>> +        def fromheader(size, _offset=[0]):
>> +            """return the next <size> byte from the header"""
>> +            offset = _offset[0] #small hack to make this mutable
>> +            data = headerblock[offset:(offset + size)]
>> +            _offset[0] = offset + size
>> +            return data
>
> It might be just me, but I think this is a bit hacky and hard to follow.
> Can we either return the offset with data and just pass it around like
>
>    return data, offset+size
>
> and
>
>    typesize, offset = fromheader(1)
>    next, offset = fromheader(1, offset)
>
> or use a variable defined just before fromheader? Maybe yoru approach is
> a python standard and we should keep it, but i had to read twice to
> follow what it's actually doing.


In a prior version, _offset was defined right before fromheader. I 
decided to move it in the function parameter to make it clear is was 
used only here. (basically a static variable with for this unbundling.)

The result of from header is other directly fed to unpack so I'm not 
keen to use the first proposal (return data, newoffset).

-- 
Pierre-Yves