This is an archive of the discontinued Mercurial Phabricator instance.

rust-dirstatemap: add `set_possibly_dirty` method
ClosedPublic

Authored by Alphare on Apr 6 2022, 10:11 AM.

Details

Summary

This is the new API that Python has already migrated to.

Diff Detail

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

Event Timeline

Alphare created this revision.Apr 6 2022, 10:11 AM
marmoute added inline comments.
rust/hg-core/src/dirstate_tree/dirstate_map.rs
811

(Same question as in D12456)

Alphare added inline comments.Apr 8 2022, 8:24 AM
rust/hg-core/src/dirstate_tree/dirstate_map.rs
811

I don't think this should affect anything. Setting an entry as  possibly_dirty does not have any effect on its being tracked or having a copy source, or an entry. The next call to dirstate.status will use some other method to reset its state. Or am I missing something?

marmoute added inline comments.Apr 8 2022, 8:47 AM
rust/hg-core/src/dirstate_tree/dirstate_map.rs
811

My question is more "it seems like this call could insert a node, yet nothing is done to adjust the counter associated to this insertion". Is this a legitimate concerns ?

Alphare added inline comments.Apr 8 2022, 8:50 AM
rust/hg-core/src/dirstate_tree/dirstate_map.rs
811

This can't insert a node because of the check in the public function. There is a strong case to use get_node_mut instead, but it would have to be adjusted to have an ancestors callback as well. I propose to fix D12456 in the state that it is, and then add that change on top of the stack.

marmoute added inline comments.Apr 8 2022, 9:00 AM
rust/hg-core/src/dirstate_tree/dirstate_map.rs
811

It seems like we are in a "This function is correct because *unrelated* code never call it with argument that makes it wrong" That seems dangerous, and either and assert or a narrower function use would be safer.

Doing that fix at the top of the stack up seems fines if that helps.

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.
baymax updated this revision to Diff 32930.Apr 8 2022, 12:29 PM

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

baymax updated this revision to Diff 33032.Apr 12 2022, 10:57 AM

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

baymax updated this revision to Diff 33114.Apr 12 2022, 12:06 PM

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