[PATCH 2 of 2] changegroup: delete unused 'bundlecaps' argument

[Martin von Zweigbergk]

On Tue, May 9, 2017 at 12:51 PM, Durham Goode <durham at fb.com> wrote:
>
>
> On 5/9/17 12:39 PM, Martin von Zweigbergk wrote:
>>
>>
>>
>> On Tue, May 9, 2017, 12:30 Durham Goode <durham at fb.com
>> <mailto:durham at fb.com>> wrote:
>>
>>     On 5/3/17 10:44 AM, Martin von Zweigbergk via Mercurial-devel wrote:
>>     > # HG changeset patch
>>     > # User Martin von Zweigbergk <martinvonz at google.com
>>     <mailto:martinvonz at google.com>>
>>     > # Date 1493794030 25200
>>     > #      Tue May 02 23:47:10 2017 -0700
>>     > # Node ID 06058cbb9209df54bfa0aaf9f7d10b836a1e3ee9
>>     > # Parent  bf2a70b5c015ff0720571c4512d67eb53f52b85e
>>     > changegroup: delete unused 'bundlecaps' argument
>>
>>     This was pretty heavily used by remotefilelog to tell the bundler to
>> not
>>     include filelogs in the changegroup. Unfortunately there aren't
>> really
>>     any other ways of communicating from the exchange layer to the
>> cgpacker
>>     layer.
>>
>>     If this was just a clean-up commit, could we roll it back? Otherwise I
>>     have to jump through a lot of hoops to make remotefilelog work again
>>     (potentially involving adding some sort of options that get passed
>> from
>>     the exchange layer down through to changegroup creation, just like
>>     bundlecaps were before).
>>
>>
>> Fine with me. It was just a cleanup. Do you think the bundlecaps
>> parameter is the right solution for your need long-term?
>
>
> Nah, it's not a good long-term solution. The bundlecaps parameter was a
> hacky solution from the beginning.
>
> Ideally we would have changegroup creation happen in it's own function at
> the exchange layer. That way an extension could hook in at the exchange
> layer (where they have access to bundle2 arguments, etc) and customize what
> cgpacker is used. The problem right now is that exchange layer never sees
> the cgpacker.  It calls changegroup.getlocalchangegroupraw(...) which
> returns a generator, at which point it's too late to modify the cgpacker.
>
> So if exchange code could be written like:
>
>   cg = createchangegroup(repo,..., bundle2args)
>   part = bundler.newpart('changegroup', data=cg.generate()
>
> then I as an extension could wrap the cg appropriately based on the
> bundle2args we see.
>
> But I'd rather not solve that right now while I'm working on treemanifest
> and hidden commit stuff.

Sure, that's fine with me. I was just trying to see if we wanted the
bundlespecs to stay "forever" or if there was a better long-term
solution.

>
> Note: the proposal above only works for bundle2. There is no alternative
> solution for bundle1, so if we drop bundlecaps entirely, remotefilelog has
> to drop support for bundle1. We don't use bundle1, but I don't know if other
> consumers might.
>
>
> I'll send a backout commit for now (it requires some merging with recent
> changes, so I'm happy to do the work there).

Thanks. Just FYI, I'm in meetings much of the day.