( )⚙ D10083 tags: validate nodes in _getfnodes() and update cache in case of unknown nodes

This is an archive of the discontinued Mercurial Phabricator instance.

tags: validate nodes in _getfnodes() and update cache in case of unknown nodes
ClosedPublic

Authored by pulkit on Mar 1 2021, 1:47 PM.

Details

Summary

hgtagsfnodescache can contain unknown nodes due to cache corruption and this
lead to a traceback on operations like hg tags as we don't validate nodes.

This patch validates that all filenodes returned after hgtagsfnodescache are
known to the repository. If there exists any unknown filenode, we force
recompute it and update the cache.

The test change demonstrates the fix.

Diff Detail

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

Event Timeline

pulkit created this revision.Mar 1 2021, 1:47 PM
marmoute requested changes to this revision.Mar 1 2021, 2:05 PM
marmoute added a subscriber: marmoute.
marmoute added inline comments.
mercurial/tags.py
497–498

lets drop a _ between words here, if you don't mind. This is easier to read.

843–852

We do more that recomputing the nodes, we also update the cache right ? We should maybe update the function name (maybe refresh_invalide_node ?) and update the doc to point this out.

This revision now requires changes to proceed.Mar 1 2021, 2:05 PM
baymax updated this revision to Diff 26013.Mar 2 2021, 3:42 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

pulkit marked 2 inline comments as done.Mar 2 2021, 3:47 AM
marmoute added inline comments.Mar 2 2021, 3:56 AM
mercurial/tags.py
505–506

look slike my main an long comment about this got lost…

I don't understand why we do a full filectx initialisation and data fetch here. What we want to do is only check if the node exist within the filelog. right? Why won't we explicitly grab de filelog and check that?

baymax updated this revision to Diff 26015.Mar 2 2021, 6:44 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

pulkit marked an inline comment as done.Mar 2 2021, 7:34 AM
marmoute requested changes to this revision.Mar 2 2021, 8:28 AM
marmoute added inline comments.
mercurial/tags.py
501

The official way to get this is repo.file(b'.hgtags') if I remember correctly. The current code is a bit abstraction-breaking.

This revision now requires changes to proceed.Mar 2 2021, 8:28 AM
pulkit marked an inline comment as done.Mar 2 2021, 9:24 AM
pulkit updated this revision to Diff 26021.Mar 2 2021, 9:24 AM
marmoute accepted this revision.Mar 5 2021, 12:42 PM
baymax updated this revision to Diff 26106.Mar 5 2021, 1:16 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.