( )⚙ D10706 dirstate-tree: Remove newly-empty nodes after removing a `DirstateEntry`

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10706

dirstate-tree: Remove newly-empty nodes after removing a `DirstateEntry`
ClosedPublic

Authored by SimonSapin on May 11 2021, 12:34 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG47ccab19bf9f: dirstate-tree: Remove newly-empty nodes after removing a `DirstateEntry`
Summary

This is actually necessary to make DirstateMap::has_dir correct, since it
assumes that a node without a DirstateEntry has at least one descedant node
with a DirstateEntry.

This bug would become apparent when a later changeset persists tree nodes
on disk in the "dirstate-v2" format.

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

SimonSapin created this revision.May 11 2021, 12:34 PM
baymax updated this revision to Diff 27913.May 14 2021, 10:25 AM

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

pulkit accepted this revision.May 15 2021, 6:36 PM
This revision is now accepted and ready to land.May 15 2021, 6:36 PM

Revision Contents
Changeset List

Diff 27930

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

dropped = Dropped { dropped = Dropped {
was_tracked: node was_tracked: node
.entry .entry
.as_ref() .as_ref()
.map_or(false, |entry| entry.state.is_tracked()), .map_or(false, |entry| entry.state.is_tracked()),
had_entry: node.entry.take().is_some(), had_entry: node.entry.take().is_some(),
had_copy_source: node.copy_source.take().is_some(), had_copy_source: node.copy_source.take().is_some(),
}; };
// TODO: this leaves in the tree a "non-file" node. Should we }
// remove the node instead, together with ancestor nodes for // After recursion, for both leaf (rest_of_path is None) nodes and
// directories that become empty? // parent nodes, remove a node if it just became empty.
if node.entry.is_none()
&& node.copy_source.is_none()
&& node.children.is_empty()
{
nodes.remove(first_path_component);
} }
Some(dropped) Some(dropped)
} }
if let Some(dropped) = recur(&mut self.root, filename) { if let Some(dropped) = recur(&mut self.root, filename) {
if dropped.had_entry { if dropped.had_entry {
self.nodes_with_entry_count -= 1 self.nodes_with_entry_count -= 1
} }

tests/test-status.t

$ rm a $ rm a
$ rm b $ rm b
$ hg status a $ hg status a
! a ! a
$ hg rm a $ hg rm a
$ hg rm b $ hg rm b
$ hg status a $ hg status a
R a R a
$ hg commit -qm '#1'
$ hg status a
a: $ENOENT$
Check using include flag with pattern when status does not need to traverse Check using include flag with pattern when status does not need to traverse
the working directory (issue6483) the working directory (issue6483)
$ cd .. $ cd ..
$ hg init issue6483 $ hg init issue6483
$ cd issue6483 $ cd issue6483
$ touch a.py b.rs $ touch a.py b.rs