( )⚙ D10017 tags: return set of invalid nodes from _tagsfromfnodes()

This is an archive of the discontinued Mercurial Phabricator instance.

tags: return set of invalid nodes from _tagsfromfnodes()
AbandonedPublic

Authored by pulkit on Feb 17 2021, 2:44 PM.

Details

Reviewers
Alphare
marmoute
Group Reviewers
hg-reviewers
Summary

We pass a list of file nodes to that function but some can be invalid because
they were read from a cache which was corrupted.

This patches catches LookupError which is being raised because of invalid
fnodes, builds a set of such nodes and return it to the callers.
In next patch, we will add logic to update cache in such cases.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

pulkit created this revision.Feb 17 2021, 2:44 PM
Alphare accepted this revision.Feb 23 2021, 4:42 AM
marmoute accepted this revision.Mar 1 2021, 4:00 AM
marmoute requested changes to this revision.Mar 1 2021, 4:04 AM
marmoute added inline comments.
tests/test-tags.t
454–456

The commit message in D10018 seems to imply that this output is incorrect. Can you flag the bad output with known-bad-output and missing-correct-output ?

This revision now requires changes to proceed.Mar 1 2021, 4:04 AM

I am looking at this more, and I don't think we are addressing the issue at the right location.

Here you deal with error while trying to process the data pointed by the tags fnodes and you don't have the necessary information to react well when that fnode is bad.

We should treat this error earlier (maybe in _filterfnodes, or right before it?). At that point I think we could validate that all the fnode we have are correct / or corrected before starting to process them. In such context maybe we could even write down the fnode cache before proceeding further (if it make sense transactionwise).

mercurial/tags.py
120–121

I don't think should not ignore error here.

pulkit planned changes to this revision.Mar 1 2021, 1:54 PM

Superseeded by D10083.

pulkit added a comment.Mar 1 2021, 1:56 PM

I am looking at this more, and I don't think we are addressing the issue at the right location.
Here you deal with error while trying to process the data pointed by the tags fnodes and you don't have the necessary information to react well when that fnode is bad.
We should treat this error earlier (maybe in _filterfnodes, or right before it?). At that point I think we could validate that all the fnode we have are correct / or corrected before starting to process them. In such context maybe we could even write down the fnode cache before proceeding further (if it make sense transactionwise).

Sent D10083 which handles this at much earlier stage.

pulkit abandoned this revision.Mar 9 2021, 2:15 AM