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

login
register
mail settings
Submitter phabricator
Date May 11, 2021, 4:35 p.m.
Message ID <differential-rev-PHID-DREV-qhcqkf23gpcolridzgm6-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/49025/
State Superseded
Headers show

Comments

phabricator - May 11, 2021, 4:35 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION 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.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D10706

AFFECTED FILES
  rust/hg-core/src/dirstate_tree/dirstate_map.rs
  tests/test-status.t

CHANGE DETAILS




To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/tests/test-status.t b/tests/test-status.t
--- a/tests/test-status.t
+++ b/tests/test-status.t
@@ -711,6 +711,9 @@ 
   $ hg rm b
   $ hg status 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
 the working directory (issue6483)
diff --git a/rust/hg-core/src/dirstate_tree/dirstate_map.rs b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
--- a/rust/hg-core/src/dirstate_tree/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
@@ -379,9 +379,14 @@ 
                     had_entry: node.entry.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
-                // directories that become empty?
+            }
+            // After recursion, for both leaf (rest_of_path is None) nodes and
+            // 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)
         }