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@mercurial-scm.org> wrote:



On Tue, Nov 29, 2016 at 9:37 AM, Augie Fackler <r...@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 <r...@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
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to