This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1957

streamclonebundle: make sure we accept new stream clone bundle spec
ClosedPublic

Authored by lothiraldan on Jan 31 2018, 11:25 AM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHGb837655c1509: streamclonebundle: make sure we accept new stream clone bundle spec
Summary

When asked specifically, the code do a sanity check on the clone bundle to
ensure it's a stream bundle. As we introduced a new stream bundle spec, update
the logic to support it.

With this final changeset, we can now announce safely a stream v2 clone bundle
and old clients would not crash trying to process it.

This changeset address the last comment from Gregory Szorc on the stream v2
bundle series.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

lothiraldan created this revision.Jan 31 2018, 11:25 AM
lothiraldan added a subscriber: indygreg.Feb 7 2018, 11:20 AM

@indygreg Could you take a look at the series before it gets too distant to merge cleanly?

lothiraldan added a comment.Feb 20 2018, 5:52 AM

Ping on this series

indygreg added a comment.Feb 20 2018, 11:53 PM

In general, I'm pretty happy with this series. Although I held off accepting changesets because I think we should have a mini discussion about content options versus capabilities before things get out of control and we duplicate too much logic.

I'd also like to note that bundle specs are intended to be user facing and their behavior should be as simple as possible for people. e.g. consider adding keywords to parameters instead of requiring parameters have values.

Also, while you are here, we should consider defining a v3 bundlespec that allows embedding of bookmarks, phases, obsolescence markers, etc. Basically all the modern features of bundle2 that modern peers and servers will negotiate with each other.

lothiraldan added a comment.Mar 15 2018, 10:06 AM

@indygreg I'm 👍 on having this discussion. When do you want to have it? What would be the best medium for it, a video-call?

lothiraldan updated this revision to Diff 7378.Mar 30 2018, 8:55 AM
indygreg accepted this revision.Mar 30 2018, 1:13 PM
This revision is now accepted and ready to land.Mar 30 2018, 1:13 PM
Closed by commit rHGb837655c1509: streamclonebundle: make sure we accept new stream clone bundle spec (authored by lothiraldan). · Explain WhyMar 30 2018, 1:14 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/exchange.py (16 lines)
Mtests/test-stream-bundle-v2.t (52 lines)

Diff 7388

mercurial/exchange.py

pass pass
except error.UnsupportedBundleSpecification: except error.UnsupportedBundleSpecification:
pass pass
m.append(attrs) m.append(attrs)
return m return m
def isstreamclonespec(bundlespec):
# Stream clone v1
if (bundlespec.compression == 'UN' and bundlespec.version == 's1'):
return True
# Stream clone v2
if (bundlespec.compression == 'UN' and bundlespec.version == '02' and \
bundlespec.contentopts.get('streamv2')):
return True
return False
def filterclonebundleentries(repo, entries, streamclonerequested=False): def filterclonebundleentries(repo, entries, streamclonerequested=False):
"""Remove incompatible clone bundle manifest entries. """Remove incompatible clone bundle manifest entries.
Accepts a list of entries parsed with ``parseclonebundlesmanifest`` Accepts a list of entries parsed with ``parseclonebundlesmanifest``
and returns a new list consisting of only the entries that this client and returns a new list consisting of only the entries that this client
should be able to apply. should be able to apply.
There is no guarantee we'll be able to apply all returned entries because There is no guarantee we'll be able to apply all returned entries because
the metadata we use to filter on may be missing or wrong. the metadata we use to filter on may be missing or wrong.
""" """
newentries = [] newentries = []
for entry in entries: for entry in entries:
spec = entry.get('BUNDLESPEC') spec = entry.get('BUNDLESPEC')
if spec: if spec:
try: try:
bundlespec = parsebundlespec(repo, spec, strict=True) bundlespec = parsebundlespec(repo, spec, strict=True)
# If a stream clone was requested, filter out non-streamclone # If a stream clone was requested, filter out non-streamclone
# entries. # entries.
comp = bundlespec.compression if streamclonerequested and not isstreamclonespec(bundlespec):
version = bundlespec.version
if streamclonerequested and (comp != 'UN' or version != 's1'):
repo.ui.debug('filtering %s because not a stream clone\n' % repo.ui.debug('filtering %s because not a stream clone\n' %
entry['URL']) entry['URL'])
continue continue
except error.InvalidBundleSpecification as e: except error.InvalidBundleSpecification as e:
repo.ui.debug(str(e) + '\n') repo.ui.debug(str(e) + '\n')
continue continue
except error.UnsupportedBundleSpecification as e: except error.UnsupportedBundleSpecification as e:

tests/test-stream-bundle-v2.t

getting B getting B
C: remote created -> g C: remote created -> g
getting C getting C
D: remote created -> g D: remote created -> g
getting D getting D
E: remote created -> g E: remote created -> g
getting E getting E
5 files updated, 0 files merged, 0 files removed, 0 files unresolved 5 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg clone --stream http://localhost:$HGPORT streamv2-clone-explicit --debug
using http://localhost:$HGPORT/
sending capabilities command
sending clonebundles command
applying clone bundle from http://localhost:$HGPORT1/bundle.hg
bundle2-input-bundle: with-transaction
bundle2-input-part: "stream2" (params: 3 mandatory) supported
applying stream bundle
11 files to transfer, 1.65 KB of data
adding [s] data/A.i (66 bytes)
adding [s] data/B.i (66 bytes)
adding [s] data/C.i (66 bytes)
adding [s] data/D.i (66 bytes)
adding [s] data/E.i (66 bytes)
adding [s] 00manifest.i (584 bytes)
adding [s] 00changelog.i (595 bytes)
adding [s] phaseroots (43 bytes)
adding [c] branch2-served (94 bytes)
adding [c] rbc-names-v1 (7 bytes)
adding [c] rbc-revs-v1 (40 bytes)
transferred 1.65 KB in *.* seconds (*/sec) (glob)
bundle2-input-part: total payload size 1840
bundle2-input-bundle: 0 parts total
finished applying clone bundle
query 1; heads
sending batch command
searching for changes
all remote heads known locally
no changes found
sending getbundle command
bundle2-input-bundle: with-transaction
bundle2-input-part: "listkeys" (params: 1 mandatory) supported
bundle2-input-part: "phase-heads" supported
bundle2-input-part: total payload size 24
bundle2-input-bundle: 1 parts total
checking for updated bookmarks
updating to branch default
resolving manifests
branchmerge: False, force: False, partial: False
ancestor: 000000000000, local: 000000000000+, remote: 9bc730a19041
A: remote created -> g
getting A
B: remote created -> g
getting B
C: remote created -> g
getting C
D: remote created -> g
getting D
E: remote created -> g
getting E
5 files updated, 0 files merged, 0 files removed, 0 files unresolved