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

Durham Goode durham at fb.com
Tue May 9 15:51:39 EDT 2017



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.

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).


More information about the Mercurial-devel mailing list