| Submitter | Remi Chaintron |
|---|---|
| Date | Dec. 14, 2016, 11:58 a.m. |
| Message ID | <8120778471f01b41d01f.1481716690@remi-mbp2> |
| Download | mbox | patch |
| Permalink | /patch/17904/ |
| State | Accepted |
| Headers | show |
Comments
On Wed, Dec 14, 2016 at 11:58:10AM +0000, Remi Chaintron wrote: > # HG changeset patch > # User Remi Chaintron <remi@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). > > 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')) > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
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 <remi@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,
Patch
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'))