+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. 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).
>
> >
> >
> > > return versions
> > >
> > > # Changegroup versions that can be created from the repo
> > > def supportedoutgoingversions(repo):
> > > versions = allsupportedversions(repo.ui)
> > >- if 'treemanifest' in repo.requirements:
> > >+ if ('treemanifest' in repo.requirements or
> > >+ 'lfs' in repo.requirements):
> > > # Versions 01 and 02 support only flat manifests and it's just
> too
> > > # expensive to convert between the flat manifest and tree
> manifest on
> > > # the fly. Since tree manifests are hashed differently, all of
> history
> > >diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> > >--- a/mercurial/localrepo.py
> > >+++ b/mercurial/localrepo.py
> > >@@ -238,7 +238,7 @@
> > > class localrepository(object):
> > >
> > > supportedformats = set(('revlogv1', 'generaldelta', 'treemanifest',
> > >- 'manifestv2'))
> > >+ 'manifestv2', 'lfs'))
> > > _basesupported = supportedformats | set(('store', 'fncache',
> 'shared',
> > > 'dotencode'))
> > > openerreqs = set(('revlogv1', 'generaldelta', 'treemanifest',
> 'manifestv2'))
> > >_______________________________________________
> > >Mercurial-devel mailing list
> > >Mercurial-devel@mercurial-scm.org
> > >https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> > >
> >
> > --
> > Pierre-Yves David
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
--
--Kyle
Note:
If I've asked a question, and you're responding to me, please use *respond
all*, so that other people can read any solutions we come to!
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
