[PATCH 5 of 5 v4] changegroup3: enable on 'lfs' repo requirements

Pierre-Yves David pierre-yves.david at ens-lyon.org
Fri Dec 2 22:37:01 EST 2016



On 11/30/2016 06:35 AM, Martin von Zweigbergk wrote:
> +cc:remi
>
> On Nov 29, 2016 13:51, "Kyle Lippincott via Mercurial-devel" <
> mercurial-devel at mercurial-scm.org> wrote:
>
>
>
> On Tue, Nov 29, 2016 at 9:37 AM, Augie Fackler <raf at durin42.com> wrote:
>
>> On Tue, Nov 29, 2016 at 07:59:11AM +0100, Pierre-Yves David wrote:
>>> [cc martin because I've a question about some code mentioning
>> 'treemanifest'
>>>
>>> On 11/23/2016 06:39 PM, Remi Chaintron wrote:
>>>> # HG changeset patch
>>>> # User Remi Chaintron <remi at fb.com>
>>>> # Date 1479916365 0
>>>> #      Wed Nov 23 15:52:45 2016 +0000
>>>> # Branch stable
>>>> # Node ID b421c16161aed491fec20b600df5f1278b07bc1a
>>>> # Parent  75ee4746c198f039a39400e855e9335afc34f1dd
>>>> changegroup3: enable on 'lfs' repo requirements
>>>>
>>>> `changegroup3` is required by the `lfs` extension in order to send
>> flags for
>>>> revlog objects over the wire.
>>>
>>> It seems like the commit message needs to be updated too.
>>>
>>>> diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
>>>> --- a/mercurial/changegroup.py
>>>> +++ b/mercurial/changegroup.py
>>>> @@ -879,14 +879,16 @@
>>>> # Changegroup versions that can be applied to the repo
>>>> def supportedincomingversions(repo):
>>>>     versions = allsupportedversions(repo.ui)
>>>> -    if 'treemanifest' in repo.requirements:
>>>> +    if ('treemanifest' in repo.requirements or
>>>> +        'lfs' in repo.requirements):
>>>>         versions.add('03')
>>>
>>> I've not seen 'lfs' used anywhere yet so this changeset seems a bit
>>> premature. Given the code in this series I would expect the next step to
>> be
>>> a minimal usage of the new code (either by porting some of censor or by
>>> using a test extension). Then we can start working on exchanging these
>>> flagged changesets.
>>>
>>> In addition, this piece of code is suspicious. If we have a changelog
>> '03'
>>> available, we should be using it. I'm not sure why we only consider it
>> when
>>> treemanifest is used. Martin: is this the remain of a period where the
>> '03'
>>> format was experimental? Did we actually tested the new format now? Can
>> we
>>> drop this special case?
>>
>> Martin is on vacation (and you didn't add him to CC?), but we've been
>> using changegroup3 extensively, and I think we're happy with it. Added
>> spectral to confirm.
>>
>
> Yes, we've been using changegroup3 internally without issues for a while
> now. It's required for treemanifests and I'm pretty sure is required by
> recent versions of narrowhg.  I'm fine with freezing it and making it
> non-experimental.
>
>
> I agree, let's make it non-experimental. I assume that also means it's on
> by default and will be used be getbundle.

Yes please, we should do this ☺

Make sure we have a way to control the changegroup used though the 
'devel.legacy' class of option and that it is still tested.

> IIRC, "hg bundle" will still
> produce a cg2 bundle, though, unless cg3 is forced by the presence of the
> treemanifest requirement. So, in the same way, you'll still need to force
> it to use cg3 when the lfs requirement is there.
> Looking at the patch, that
> seems to mean removing the first hunk (after the patch the makes cg3 on by
> default), but keeping the second hunk (and third). Or maybe I'm just
> confusing it with how there is no way to specify cg3 to "hg bundle", but
> once it's on by default, it will get picked up. On that case, there will
> instead be some BC concern in that people with a new hg can not share a
> bundle with people with an old (current) hg. As I said, I'm not sure which,
> so please check for yourself (I guess I'm talking to Remi here).

bundle have various options to control what it is generating, it is 
likely that they is already a way to select the changegroup version to 
'03' from the command line (or we should had it).

At some point we can spin a new 'bundle' version that will use cg3 (and 
improvement like bookmark as port etc). We must make sure version that 
exist now stay consistent with their definition at the time they were 
introduced.

Cheers,

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list