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

[Pierre-Yves David]

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  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 
# Date 1479916365 0
#  Wed Nov 23 15:52:45 2016 +
# 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

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

[Kyle Lippincott via Mercurial-devel]

On Tue, Nov 29, 2016 at 9:37 AM, Augie Fackler  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 
> > ># Date 1479916365 0
> > >#  Wed Nov 23 15:52:45 2016 +
> > ># 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.


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

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

[Augie Fackler]

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 
> ># Date 1479916365 0
> >#  Wed Nov 23 15:52:45 2016 +
> ># 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.

>
>
> > 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
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Rémi Chaintron]

On Tue, 29 Nov 2016 at 06:59 Pierre-Yves David <
pierre-yves.da...@ens-lyon.org> wrote:

> It seems like the commit message needs to be updated too.


(Y)


>

> 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?
>

The original patch included an example composition of the `lfs` current
implementation with a `dummy` extension that also provide a non-commutative
(but reversible) operation on the filelog.

censor is actually a really awkward extension for me to prototype the
current implementation:
- The operation it provides is non-reversible and do not allow to
demonstrate composing non-commutative operations.
- I've been hacking on moving censor to the new design for modifying
filelog contents with mixed results: while this allowed me to completely
clean filelog of the censor related code, I've had trouble completely
getting revlog rid of it. Some changes and improvements in our design might
actually allow me to move forward on this now, I'll take a look at it
tomorrow.

I'll make sure to re-send the RFC patches for lfs + composition with the
'dummy' extension that provide a good example of how easy it is to write
these extensions and how they rely on the flag transforms.


>
> >  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
>
-- 
Rémi
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Pierre-Yves David]

[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 
# Date 1479916365 0
#  Wed Nov 23 15:52:45 2016 +
# 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?




 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

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

[Remi Chaintron]

# HG changeset patch
# User Remi Chaintron 
# Date 1479916365 0
#  Wed Nov 23 15:52:45 2016 +
# 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.

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