This is an archive of the discontinued Mercurial Phabricator instance.

Differential D128

treemanifest: fix historypack corruption bug
ClosedPublic

Authored by durham on Jul 18 2017, 9:35 AM.
Tags
None
Subscribers
akushner

Details

Reviewers
mitrandir
Group Reviewers
Restricted Project
Commits
rFBHGX787cd2a2b239: treemanifest: fix historypack corruption bug
Summary

The history pack writer had a bug where if the same node was added to the
mutablehistorypack N times, it would write out that it had N entries, but then
it would only write a single entry. This caused corruption (the length value
didn't match the actual number of entries) that broke repack.

This primarily affected users who used the old version of treemanifest (where
trees were converted on the client side). The new version of treemanifest only
seems to repro this in rare cases, like when rebasing multiple commits that
create the same trees.

Test Plan

Added a test. It failed before and passes after.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
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.Jul 18 2017, 9:35 AM
mitrandir accepted this revision.Jul 18 2017, 10:34 AM
This revision is now accepted and ready to land.Jul 18 2017, 10:34 AM
mitrandir added a comment.Jul 18 2017, 10:37 AM

Wow, we already have 128 revs in this phabricator, time to party!

Closed by commit rFBHGX787cd2a2b239: treemanifest: fix historypack corruption bug (authored by durham). · Explain WhyJul 18 2017, 7:10 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mremotefilelog/historypack.py (21 lines)
Mtests/test-treemanifest.t (30 lines)

Diff 292

remotefilelog/historypack.py

copyfromlen, copyfromlen,
copyfrom)) copyfrom))
def _write(self): def _write(self):
for filename in sorted(self.fileentries): for filename in sorted(self.fileentries):
entries = self.fileentries[filename] entries = self.fileentries[filename]
sectionstart = self.packfp.tell() sectionstart = self.packfp.tell()
# Write the file section header
self.writeraw("%s%s%s" % (
struct.pack('!H', len(filename)),
filename,
struct.pack('!I', len(entries)),
))
sectionlen = constants.FILENAMESIZE + len(filename) + 4
# Write the file section content # Write the file section content
entrymap = dict((e[0], e) for e in entries) entrymap = dict((e[0], e) for e in entries)
def parentfunc(node): def parentfunc(node):
x, p1, p2, x, x, x = entrymap[node] x, p1, p2, x, x, x = entrymap[node]
parents = [] parents = []
if p1 != nullid: if p1 != nullid:
parents.append(p1) parents.append(p1)
if p2 != nullid: if p2 != nullid:
parents.append(p2) parents.append(p2)
return parents return parents
sortednodes = reversed(shallowutil.sortnodes( sortednodes = list(reversed(shallowutil.sortnodes(
(e[0] for e in entries), (e[0] for e in entries),
parentfunc)) parentfunc)))
# Write the file section header
self.writeraw("%s%s%s" % (
struct.pack('!H', len(filename)),
filename,
struct.pack('!I', len(sortednodes)),
))
sectionlen = constants.FILENAMESIZE + len(filename) + 4
rawstrings = [] rawstrings = []
# Record the node locations for the index # Record the node locations for the index
locations = self.entrylocations.setdefault(filename, {}) locations = self.entrylocations.setdefault(filename, {})
offset = sectionstart + sectionlen offset = sectionstart + sectionlen
for node in sortednodes: for node in sortednodes:
locations[node] = offset locations[node] = offset

tests/test-treemanifest.t

> reset = $TESTDIR/../hgext3rd/reset.py > reset = $TESTDIR/../hgext3rd/reset.py
> EOF > EOF
$ hg sparse -I subdir $ hg sparse -I subdir
$ hg reset '.^' $ hg reset '.^'
resetting without an active bookmark resetting without an active bookmark
saved backup bundle to $TESTTMP/client/.hg/strip-backup/27a577922312-3ad85b1a-backup.hg (glob) saved backup bundle to $TESTTMP/client/.hg/strip-backup/27a577922312-3ad85b1a-backup.hg (glob)
$ hg status $ hg status
M subdir/y M subdir/y
$ hg up -C .
1 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg sparse --reset
Test rebase two commits with same changes
$ echo >> subdir/y
$ hg commit -qm 'modify subdir/y #1'
$ hg up -q '.^'
$ echo >> x
$ echo >> subdir/y
$ hg commit -qm 'modify subdir/y #2'
$ hg up -q '.^'
$ echo >> noop
$ hg add noop
$ hg commit -Am 'rebase destination'
created new head
$ hg rebase -d 6 -s '4 + 5' --config rebase.singletransaction=True
rebasing 4:6052526a0d67 "modify subdir/y #1"
rebasing 5:79a69a1547d7 "modify subdir/y #2"
saved backup bundle to $TESTTMP/client/.hg/strip-backup/79a69a1547d7-fc6bc129-rebase.hg (glob)
$ hg debughistorypack .hg/store/packs/manifests/668716b2b93e2fa8151bca1a327a391746f0dcc0.histpack
Node P1 Node P2 Node Link Node Copy From
8026e03c5a35 8011431de863 000000000000 000000000000
5ca06dca517c 8011431de863 000000000000 000000000000
subdir
Node P1 Node P2 Node Link Node Copy From
ad0a48a2ec1e a4e2f032ee0f 000000000000 000000000000