This is an archive of the discontinued Mercurial Phabricator instance.

Differential D646

changegroup: fix to allow empty manifest parts
ClosedPublic

Authored by durham on Sep 6 2017, 9:39 PM.

Details

Reviewers
dsp
Group Reviewers
hg-reviewers
Commits
rHGbbdca7e460c0: changegroup: fix to allow empty manifest parts
rHGe7660cef5510: changegroup: fix to allow empty manifest parts
Summary

The current chunk reading algorithm relied on counting the number of empty
chunks and comparing it to the number of chunk lists it expected (1 list of
files for cg1 and cg2, and 1 list of files + 1 list of trees for cg3). This
implicitly assumed that both the changelog part and the manifestlog part were
never empty (since them being empty would cause it to count it as one list being
done, and screw up the count). In our treemanifest code, the manifest section
could be empty, so we need to handle that case.

This patches refactors that code to be more explicit about how it counts the
expected parts.

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

durham created this revision.Sep 6 2017, 9:39 PM
dsp accepted this revision.Sep 8 2017, 2:54 PM
Closed by commit rHGe7660cef5510: changegroup: fix to allow empty manifest parts (authored by durham). · Explain WhySep 11 2017, 10:14 AM
This revision was automatically updated to reflect the committed changes.
martinvonz added a subscriber: martinvonz.Sep 11 2017, 1:27 PM
martinvonz added inline comments.
mercurial/changegroup.py
225

I'll s/empty/noentries/ in flight.

Revision Contents
Changeset List

PathPackages
Mmercurial/changegroup.py (39 lines)

Diff 1711

mercurial/changegroup.py

def getchunks(self): def getchunks(self):
"""returns all the chunks contains in the bundle """returns all the chunks contains in the bundle
Used when you need to forward the binary stream to a file or another Used when you need to forward the binary stream to a file or another
network API. To do so, it parse the changegroup data, otherwise it will network API. To do so, it parse the changegroup data, otherwise it will
block in case of sshrepo because it don't know the end of the stream. block in case of sshrepo because it don't know the end of the stream.
""" """
# an empty chunkgroup is the end of the changegroup # For changegroup 1 and 2, we expect 3 parts: changelog, manifestlog,
# a changegroup has at least 2 chunkgroups (changelog and manifest). # and a list of filelogs. For changegroup 3, we expect 4 parts:
# after that, changegroup versions 1 and 2 have a series of groups # changelog, manifestlog, a list of tree manifestlogs, and a list of
# with one group per file. changegroup 3 has a series of directory # filelogs.
# manifests before the files. #
count = 0 # Changelog and manifestlog parts are terminated with empty chunks. The
emptycount = 0 # tree and file parts are a list of entry sections. Each entry section
while emptycount < self._grouplistcount: # is a series of chunks terminating in an empty chunk. The list of these
empty = True # entry sections is terminated in yet another empty chunk, so we know
count += 1 # we've reached the end of the tree/file list when we reach an empty
# chunk that was proceeded by no non-empty chunks.
parts = 0
while parts < 2 + self._grouplistcount:
noentries = True
while True: while True:
chunk = getchunk(self) chunk = getchunk(self)
if not chunk: if not chunk:
if empty and count > 2: # The first two empty chunks represent the end of the
emptycount += 1 # changelog and the manifestlog portions. The remaining
# empty chunks represent either A) the end of individual
# tree or file entries in the file list, or B) the end of
# the entire list. It's the end of the entire list if there
# were no entries (i.e. empty is True).
martinvonzUnsubmitted
Not Done

I'll s/empty/noentries/ in flight.

martinvonz: I'll s/empty/noentries/ in flight.
if parts < 2:
parts += 1
elif noentries:
parts += 1
break break
empty = False noentries = False
yield chunkheader(len(chunk)) yield chunkheader(len(chunk))
pos = 0 pos = 0
while pos < len(chunk): while pos < len(chunk):
next = pos + 2**20 next = pos + 2**20
yield chunk[pos:next] yield chunk[pos:next]
pos = next pos = next
yield closechunk() yield closechunk()