This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10995

corruption: backout changeset 49fd21f32695 (issue6528)
ClosedPublic

Authored by marmoute on Jul 6 2021, 10:40 AM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG411dc27fd9fd: corruption: backout changeset 49fd21f32695 (issue6528)
rHG0e4c5030d4d2: corruption: backout changeset 49fd21f32695 (issue6528)
Summary

inverting the parent is masking copy information leading to bad content being
fetched and bad status result.

Since 49fd21f32695, exchange can actively swap these parent corrupting existing
changesets and triggering the corruption.

Data corruption are considered critical so backing this out and doing and
unscheduled release seems in order.

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

marmoute created this revision.Jul 6 2021, 10:40 AM
baymax updated this revision to Diff 28886.Jul 6 2021, 2:49 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)
⚠ This patch is intended for stable ⚠

joerg.sonnenberger added a comment.Jul 6 2021, 3:09 PM

Just the revert as is introduces other regressions. At the very least, we should preserve the sorting for changelog and keep the original order for filelog. The other part is that the filelog logic here is actively broken even before, so this doesn't fix the logic, just makes it less visibly broken.

marmoute added a comment.Jul 6 2021, 6:38 PM
In D10995#168148, @joerg.sonnenberger wrote:

Just the revert as is introduces other regressions.

Can you clarify what these regressions are ? The backout does not breaks any tests, do you have tests in your sleeve for these regression ?

At the very least, we should preserve the sorting for changelog and keep the original order for filelog.

Is there another changeset to backout to get this ?

What does this order preserving achieve and why is this necessary ?

The other part is that the filelog logic here is actively broken even before, so this doesn't fix the logic, just makes it less visibly broken.

What kind of brokenness are we talking about here ? What broken behavior does this trigger and what are the scenario for triggering them ?

marmoute added a commit: rHG0e4c5030d4d2: corruption: backout changeset 49fd21f32695 (issue6528).Jul 8 2021, 10:36 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG0e4c5030d4d2: corruption: backout changeset 49fd21f32695 (issue6528) (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 29007

mercurial/revlog.py

def parentrevs(self, rev): def parentrevs(self, rev):
try: try:
entry = self.index[rev] entry = self.index[rev]
except IndexError: except IndexError:
if rev == wdirrev: if rev == wdirrev:
raise error.WdirUnsupported raise error.WdirUnsupported
raise raise
if entry[5] == nullrev:
return entry[6], entry[5]
else:
return entry[5], entry[6] return entry[5], entry[6]
# fast parentrevs(rev) where rev isn't filtered # fast parentrevs(rev) where rev isn't filtered
_uncheckedparentrevs = parentrevs _uncheckedparentrevs = parentrevs
def node(self, rev): def node(self, rev):
try: try:
return self.index[rev][7] return self.index[rev][7]
except IndexError: except IndexError:
if rev == wdirrev: if rev == wdirrev:
raise error.WdirUnsupported raise error.WdirUnsupported
raise raise
# Derived from index values. # Derived from index values.
def end(self, rev): def end(self, rev):
return self.start(rev) + self.length(rev) return self.start(rev) + self.length(rev)
def parents(self, node): def parents(self, node):
i = self.index i = self.index
d = i[self.rev(node)] d = i[self.rev(node)]
# inline node() to avoid function call overhead return i[d[5]][7], i[d[6]][7] # map revisions to nodes inline
if d[5] == nullid:
return i[d[6]][7], i[d[5]][7]
else:
return i[d[5]][7], i[d[6]][7]
def chainlen(self, rev): def chainlen(self, rev):
return self._chaininfo(rev)[0] return self._chaininfo(rev)[0]
def _chaininfo(self, rev): def _chaininfo(self, rev):
chaininfocache = self._chaininfocache chaininfocache = self._chaininfocache
if rev in chaininfocache: if rev in chaininfocache:
return chaininfocache[rev] return chaininfocache[rev]

tests/test-issue6528.t

$ cat b.txt $ cat b.txt
foo foo
2 2
3 3
4 4
5 5
bar bar
$ hg status $ hg status
M b.txt (known-bad-output !)
$ hg debugrebuilddirstate $ hg debugrebuilddirstate
$ hg status $ hg status
M b.txt (known-bad-output !)
the history was altered the history was altered
in theory p1/p2 order does not matter but in practice p1 == nullid is used as a in theory p1/p2 order does not matter but in practice p1 == nullid is used as a
marker that some metadata are present and should be fetched. marker that some metadata are present and should be fetched.
$ hg debugrevlogindex b.txt $ hg debugrevlogindex b.txt
rev linkrev nodeid p1 p2 rev linkrev nodeid p1 p2
0 2 05b806ebe5ea 000000000000 000000000000 0 2 05b806ebe5ea 000000000000 000000000000
1 3 a58b36ad6b65 05b806ebe5ea 000000000000 (known-bad-output !) 1 3 a58b36ad6b65 000000000000 05b806ebe5ea
1 3 a58b36ad6b65 000000000000 05b806ebe5ea (missing-correct-output !)
Check commit Graph Check commit Graph
$ hg log -G $ hg log -G
@ changeset: 3:a1cc2bdca0aa @ changeset: 3:a1cc2bdca0aa
|\ tag: tip |\ tag: tip
| | parent: 2:615c6ccefd15 | | parent: 2:615c6ccefd15
| | parent: 1:373d507f4667 | | parent: 1:373d507f4667

tests/test-narrow-shallow-merges.t

| o 2 d1c61993ec83... outside 2d | o 2 d1c61993ec83... outside 2d
| |
o 1 bb96a08b062a inside 3 o 1 bb96a08b062a inside 3
| |
o 0 b844052e7b3b... outside 2c o 0 b844052e7b3b... outside 2c
$ hg log -T '{if(ellipsis,"...")}{node|short} {p1node|short} {p2node|short} {desc}\n' | sort $ hg log -T '{if(ellipsis,"...")}{node|short} {p1node|short} {p2node|short} {desc}\n' | sort
...2a20009de83e 3ac1f5779de3 000000000000 outside 10 ...2a20009de83e 000000000000 3ac1f5779de3 outside 10
...3ac1f5779de3 bb96a08b062a 465567bdfb2d merge a/b/c/d 9 ...3ac1f5779de3 bb96a08b062a 465567bdfb2d merge a/b/c/d 9
...8d874d57adea 7ef88b4dd4fa 000000000000 outside 12 ...8d874d57adea 7ef88b4dd4fa 000000000000 outside 12
...b844052e7b3b 000000000000 000000000000 outside 2c ...b844052e7b3b 000000000000 000000000000 outside 2c
...d1c61993ec83 000000000000 000000000000 outside 2d ...d1c61993ec83 000000000000 000000000000 outside 2d
465567bdfb2d d1c61993ec83 000000000000 inside 3 465567bdfb2d d1c61993ec83 000000000000 inside 3
7ef88b4dd4fa 2a20009de83e 000000000000 inside 11 7ef88b4dd4fa 2a20009de83e 000000000000 inside 11
bb96a08b062a b844052e7b3b 000000000000 inside 3 bb96a08b062a b844052e7b3b 000000000000 inside 3