[PATCH 1 of 9 changegroup-apis] exchange: use early return

Gregory Szorc gregory.szorc at gmail.com
Thu Aug 4 10:44:10 EDT 2016



On Aug 4, 2016, at 06:58, Kevin Bullock <kbullock+mercurial at ringworld.org> wrote:

>> On Aug 3, 2016, at 22:53, Gregory Szorc <gregory.szorc at gmail.com> wrote:
>> 
>>>> On Wed, Aug 3, 2016 at 1:56 AM, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:
>>>> On 08/02/2016 04:50 AM, Gregory Szorc wrote:
>>>> 
>>>> I think the end state of this series involves a "changegroup emitter"
>>>> being fed into some kind of "bundler," preferably as a generator. Using
>>>> the cg1unpacker for this purpose is kind of silly, as it necessitates a
>>>> util.chunkbuffer. The good news is the "changegroupemitter" as it stands
>>>> can easily be represented as both a generator of chunks or a file
>>>> handle. So by introducing this object and requiring callers to use it,
>>>> we can start transitioning callers to not use util.chunkbuffer without
>>>> having to change APIs on changegroup.py. It's much more flexible.
>>> This paragraph highlight what seems to be the most important thing to
>>> address in this discussion. You seems to try to avoid touching the
>>> existing changegroup API as much as possible and I do not understand
>>> why. My general feeling is that it should be possible to upgrade the
>>> existing class to fit your needs, but your seems to think otherwise. I'm
>>> curious to find out why as you usually have pretty good reason for doing
>>> things?
>> I avoided changing the cg<N>packer and cg<N>unpacker classes because I didn't want to write a 40 part series :)
>> 
>> The cg<N>unpacker class in particular is a bit gnarly because it performs multiple tasks:
>> 
>> 1) Exposes a file API from a possibly compressed input file object (read, seek, tell, close)
>> 2) Defines the composition of the changegroup framing protocol
>> 3) Converts a file object to a generator of changegroup chunks
>> 4) Applies a changegroup to a repo
>> 
>> This class is doing too much and needs to be split. To be honest, I'm not sure how. I do know refactoring it will be a long series :)
> 
> Sounds like we should come up with a strategy for this at the sprint, if it's not resolved by then. I've added it to the wiki page.

I want to work this out prior, as it's related to some performance improvements I'd like to get into the next release. I think Pierre-Yves and I can find time to agree before October :)


More information about the Mercurial-devel mailing list