This is an archive of the discontinued Mercurial Phabricator instance.

rewriteutil: fix crash when a rewritten message references f{6,64}
ClosedPublic

Authored by durin42 on Jul 29 2021, 4:39 PM.

Details

Summary

Without this, the rewriteutil logic thinks it's found a reference to the wdir
pseudo-revision, and then tries to look it up and rewrite it. Stop it from
doing that.

Amusingly, I had trouble working with this changeset when I didn't
describe the defect above using a regular expression, because it would
trigger the bug in my installed version of hg.

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

durin42 created this revision.Jul 29 2021, 4:39 PM
mharbison72 added inline comments.
mercurial/rewriteutil.py
210–216

I can't test because I lack easy access to a filesystem with execbit support, but will 0{6,64} also fail?

Also, I didn't think about this when I swiped this from evolve, but should we be locking the repo before doing anything? Otherwise, the lookup here could succeed, but the unfi[fullnode] below could fail.

durin42 marked an inline comment as done.Jul 30 2021, 11:02 AM
durin42 added inline comments.
mercurial/rewriteutil.py
210–216

I don't think so, because nullid will return a node, which won't have any successors.

I don't think we need to lock the repo here.

durin42 marked an inline comment as done.Jul 30 2021, 2:11 PM
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.
mharbison72 added inline comments.Jul 30 2021, 2:11 PM
mercurial/rewriteutil.py
210–216

I don't think so, because nullid will return a node, which won't have any successors.

OK, I knew repo[nullid] would return something valid, but I wasn't sure about the changelog stuff inside scmutil.resolvehexnodeidprefix. I tweaked the phab test locally, and it looks like it properly stays at 00..00 though, so it looks like it works.