On 12/16/2016 09:46 PM, Augie Fackler wrote:
On Wed, Dec 14, 2016 at 11:58:10AM +0000, Remi Chaintron wrote:
# HG changeset patch
# User Remi Chaintron <r...@fb.com>
# Date 1481639041 0
# Tue Dec 13 14:24:01 2016 +0000
# Branch stable
# Node ID 8120778471f01b41d01f8d13b4176b32d2b8482c
# Parent 217f7db4dae3309bec78e27c85cc90e924b109be
changegroup: enable version 03 on 'lfs' requirement
This also needs to disable 01 and 02 if it spots lfs (or treemanifests
for that matter).
TLDR; there is issue, but not were we though they were. However, we can
just drop that patch and have 'lfs' set 'experimental.changegroup3' in
'reposetup'
Looks at bit deeper, the change regarding changegroup version is
actually fine¹. The part that only adds '03' is
'supportedincomingversions'. receiving '01' and '02' version is fine as
long as their don't contains 'lfs' entries. But the peer generating the
bundle is responsible to ensure it emit correct ones.
The second side of the coin 'supportedoutgoingversions' is already
discarding '01' and '02' and is re-used for on disk bundle.
(to be honest, there is some confusing bit in the core implementation
here, I've sent a small series to clarify that)
However, I've found another problematic bug in last part of the
changesets. adding 'lfs' to "supportedformats" will make core pretend it
can handle 'lfs' respository while it can't. Only the 'lfs' extension can.
Overall having to mention 'lfs' anywhere in core look suspicious and I
don't think it is necessary. We already have "experimental.changegroup3"
config. The 'lfs' extensions just need to set it to True at reposetup time.
(also, it would be nice if someone (ideally from Google) could look into
making cg3 enable by default).
(And ideally, we would have a low level change in changegroup building
that make sure we are not bundling anything with a flag in a changegroup
version that don't support it. )
Version 03 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
@@ -239,7 +239,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'))
This is the bit of code I was talking about earlier. If I understand
this correctly, this new code seems to indicate core support 'lfs'
natively, which is not the case.
So this code will let a vanillia mercurial open 'lfs' repository without
actual 'lfs' support.
Cheers,
--
Pierre-Yves David
[1] mea culpa for making the initial flagging as that "error" in an IRC
conversation with Augie. Sorry about that false positive.
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
