Page MenuHomePhabricator

rust-dirstate: rust implementation of dirstatemap
ClosedPublic

Authored by Alphare on Jul 10 2019, 10:34 AM.

Details

Summary

The dirstatemap is one of the last building blocks needed to get to a
dirstate.walk Rust implementation.

Disclaimer: This change is part of a big (10) series of patches, all of which
started as one big changeset that took a long time to write.
This dirstatemap implementation is a compromise in terms of complexity both
for me and for the reviewers. I chose to submit this patch right now because
while it is not perfect, it works and is simple enough (IMHO) to be reviewed.

The Python implementation uses a lot of lazy propertycaches, breaks
encapsulation and is used as an iterator in a lot of places, all of which
dictated the somewhat unidiomatic patterns in this change.
Like written in the comments, rewriting this struct to use the typestate
pattern might be a good idea, but this is a good first step.

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

Alphare created this revision.Jul 10 2019, 10:34 AM
kevincox accepted this revision.Jul 22 2019, 11:10 AM
kevincox added inline comments.
rust/hg-core/src/dirstate/dirstate_map.rs
192

Can you just do entry.mtime = MTIME_UNSET?

289

This .clone() looks unnecessary. If it is necessary you should probably accept a &DirstateParents.

307

It isn't clear to me why we don't set_parents if dirty_parents. This would make me think that parents is lazily calculated however set_parents sets parents and dirty_parents which would imply that this is not the case.

Alphare updated this revision to Diff 16051.Jul 24 2019, 11:23 AM
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.
yuja added a subscriber: yuja.Aug 17 2019, 3:50 AM

+ if let Some(ref mut file_fold_map) = self.file_fold_map {
+ file_fold_map
+ .remove::<Vec<u8>>(filename.to_ascii_uppercase().as_ref());

Looks incompatible with the dirstate class, which uses util.normcase().