This is an archive of the discontinued Mercurial Phabricator instance.

Differential D522

revlog: abort on attempt to write null revision
ClosedPublic

Authored by martinvonz on Aug 25 2017, 6:59 PM.

Details

Reviewers
indygreg
quark
Group Reviewers
hg-reviewers
Commits
rHG9180f8f593f3: revlog: abort on attempt to write null revision
Summary

My repo got corrupted yesterday by something that ended up writing the
null revision to the revlog (nullid hash, not nullrev index, of
course). We use many extensions internally (narrowhg, remotefilelog,
evolve, internal extensions) and treemanifests are on. The null
revision was written to the changelog, the root manifest log, and one
subdirectory manifest log. I have no idea exactly why the null
revision was written, but it seems cheap enough to check that we
should fail instead of corrupting the repo.

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

martinvonz created this revision.Aug 25 2017, 6:59 PM
quark accepted this revision.Aug 25 2017, 7:27 PM
quark added a subscriber: quark.

Looks good to me. Maybe we also want to blacklist the working copy node.

quark resigned from this revision.Aug 25 2017, 7:28 PM

Actually, I think it's better to do this in _addrevision, which could cover more code paths in theory.

martinvonz added a comment.Aug 26 2017, 12:50 AM
In D522#8435, @quark wrote:

Looks good to me. Maybe we also want to blacklist the working copy node.

Makes sense. It's slightly different since it only applies to the changelog (but there are also several other magic hashes for manifests), so I'll do it a separate patch to avoid holding this patch hostage.

In D522#8437, @quark wrote:

Actually, I think it's better to do this in _addrevision, which could cover more code paths in theory.

Good point. I'll move it there.

martinvonz updated this revision to Diff 1319.Aug 26 2017, 12:59 AM
quark accepted this revision.Aug 26 2017, 11:58 AM
Closed by commit rHG9180f8f593f3: revlog: abort on attempt to write null revision (authored by martinvonz). · Explain WhyAug 28 2017, 10:01 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/revlog.py (3 lines)

Diff 1348

mercurial/revlog.py

see addrevision for argument descriptions. see addrevision for argument descriptions.
note: "addrevision" takes non-raw text, "_addrevision" takes raw text. note: "addrevision" takes non-raw text, "_addrevision" takes raw text.
invariants: invariants:
- rawtext is optional (can be None); if not set, cachedelta must be set. - rawtext is optional (can be None); if not set, cachedelta must be set.
if both are set, they must correspond to each other. if both are set, they must correspond to each other.
""" """
if node == nullid:
raise RevlogError(_("%s: attempt to add null revision") %
(self.indexfile))
btext = [rawtext] btext = [rawtext]
def buildtext(): def buildtext():
if btext[0] is not None: if btext[0] is not None:
return btext[0] return btext[0]
baserev = cachedelta[0] baserev = cachedelta[0]
delta = cachedelta[1] delta = cachedelta[1]
# special case deltas which replace entire base; no need to decode # special case deltas which replace entire base; no need to decode
# base revision. this neatly avoids censored bases, which throw when # base revision. this neatly avoids censored bases, which throw when