This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10491

dirstate-tree: Add add_file, remove_file, and drop_file
ClosedPublic

Authored by SimonSapin on Apr 20 2021, 10:17 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG7dfc598ddcfe: dirstate-tree: Add add_file, remove_file, and drop_file
rHGdd5376e8c2f4: dirstate-tree: Add add_file, remove_file, and drop_file
Summary

Again, various counters need to be kept up to date.

Diff Detail

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

Event Timeline

SimonSapin created this revision.Apr 20 2021, 10:17 AM
baymax updated this revision to Diff 27141.Apr 20 2021, 1:03 PM

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

Alphare added a subscriber: Alphare.Apr 22 2021, 5:18 AM
Alphare added inline comments.
rust/hg-core/src/dirstate_tree/dirstate_map.rs
338–339

Maybe leave a comment saying why old_state isn't useful in this implementation.

368

I think we should remove it from the tree.

SimonSapin added inline comments.May 3 2021, 6:07 AM
rust/hg-core/src/dirstate_tree/dirstate_map.rs
338–339

A later commit “dirstate-tree: Fold "tracked descendants" counter update in main walk” ends up using it. (The same info is already in the tree, but having it available before tree lookup enables some optimization.)

368

I agree. I’ll do that in a future separate patch.

SimonSapin updated this revision to Diff 27317.May 3 2021, 6:24 AM
Alphare accepted this revision.May 3 2021, 9:01 AM
This revision is now accepted and ready to land.May 3 2021, 9:01 AM

Revision Contents
Changeset List

Diff 27111

rust/hg-core/src/dirstate_tree/dirstate_map.rs

}); });
self.root.clear(); self.root.clear();
self.nodes_with_entry_count = 0; self.nodes_with_entry_count = 0;
self.nodes_with_copy_source_count = 0; self.nodes_with_copy_source_count = 0;
} }
fn add_file( fn add_file(
&mut self, &mut self,
_filename: &HgPath, filename: &HgPath,
_old_state: EntryState, _old_state: EntryState,
_entry: DirstateEntry, entry: DirstateEntry,
) -> Result<(), DirstateMapError> { ) -> Result<(), DirstateMapError> {
todo!() self.add_file_node(filename, entry, None);
Ok(())
AlphareUnsubmitted
Not Done

Maybe leave a comment saying why old_state isn't useful in this implementation.

Alphare: Maybe leave a comment saying why `old_state` isn't useful in this implementation.
SimonSapinAuthorUnsubmitted
Done

A later commit “dirstate-tree: Fold "tracked descendants" counter update in main walk” ends up using it. (The same info is already in the tree, but having it available before tree lookup enables some optimization.)

SimonSapin: A later commit “//dirstate-tree: Fold "tracked descendants" counter update in main walk//” ends…
} }
fn remove_file( fn remove_file(
&mut self, &mut self,
_filename: &HgPath, filename: &HgPath,
_old_state: EntryState, _old_state: EntryState,
_size: i32, size: i32,
) -> Result<(), DirstateMapError> { ) -> Result<(), DirstateMapError> {
todo!() let entry = DirstateEntry {
state: EntryState::Removed,
mode: 0,
size,
mtime: 0,
};
self.add_file_node(filename, entry, None);
Ok(())
} }
fn drop_file( fn drop_file(
&mut self, &mut self,
_filename: &HgPath, filename: &HgPath,
_old_state: EntryState, _old_state: EntryState,
) -> Result<bool, DirstateMapError> { ) -> Result<bool, DirstateMapError> {
todo!() if let Some(node) = Self::get_node_mut(&mut self.root, filename) {
let was_tracked = node.is_tracked_file();
let had_entry = node.entry.is_some();
let had_copy_source = node.copy_source.is_some();
// TODO: this leaves in the tree a "non-file" node. Should we
AlphareUnsubmitted
Not Done

I think we should remove it from the tree.

Alphare: I think we should remove it from the tree.
SimonSapinAuthorUnsubmitted
Done

I agree. I’ll do that in a future separate patch.

SimonSapin: I agree. I’ll do that in a future separate patch.
// remove the node instead, together with ancestor nodes for
// directories that become empty?
node.entry = None;
node.copy_source = None;
if had_entry {
self.nodes_with_entry_count -= 1
}
if had_copy_source {
self.nodes_with_copy_source_count -= 1
}
if was_tracked {
self.for_each_ancestor_node(filename, |node| {
node.tracked_descendants_count -= 1
})
}
Ok(had_entry)
} else {
Ok(false)
}
} }
fn clear_ambiguous_times(&mut self, filenames: Vec<HgPathBuf>, now: i32) { fn clear_ambiguous_times(&mut self, filenames: Vec<HgPathBuf>, now: i32) {
for filename in filenames { for filename in filenames {
if let Some(node) = Self::get_node_mut(&mut self.root, &filename) { if let Some(node) = Self::get_node_mut(&mut self.root, &filename) {
if let Some(entry) = node.entry.as_mut() { if let Some(entry) = node.entry.as_mut() {
clear_ambiguous_mtime(entry, now); clear_ambiguous_mtime(entry, now);
} }