Patchwork D10829: dirstate-v2: Drop parent directory cache when removing a dirstate node

login
register
mail settings
Submitter phabricator
Date June 1, 2021, 4:52 p.m.
Message ID <differential-rev-PHID-DREV-7foh7uruxibzh43bb2gh-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/49147/
State Superseded
Headers show

Comments

phabricator - June 1, 2021, 4:52 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  The premise of the directory cache is that the dirstate contains child nodes
  for every entry that `read_dir` would return. When removing nodes, that may
  not be the case anymore so the cache should be invalidated.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

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
@@ -957,4 +957,18 @@ 
   $ hg status
   ? subdir/unknown
 
+  $ rm subdir/unknown
+  $ hg status
+
+Removing a node from the dirstate resets the cache for its parent directory
+
+  $ hg forget subdir/a
+  $ hg debugdirstate --dirs --no-dates | grep '^d'
+  d   0          0 set                 subdir
+  $ hg ci -qm '#1'
+  $ hg debugdirstate --dirs --no-dates | grep '^d'
+  d   0          0 unset               subdir
+  $ hg status
+  ? subdir/a
+
 #endif
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
@@ -712,11 +712,17 @@ 
             had_entry: bool,
             had_copy_source: bool,
         }
+
+        /// If this returns `Ok(Some((dropped, removed)))`, then
+        ///
+        /// * `dropped` is about the leaf node that was at `filename`
+        /// * `removed` is whether this particular level of recursion just
+        ///   removed a node in `nodes`.
         fn recur<'on_disk>(
             on_disk: &'on_disk [u8],
             nodes: &mut ChildNodes<'on_disk>,
             path: &HgPath,
-        ) -> Result<Option<Dropped>, DirstateV2ParseError> {
+        ) -> Result<Option<(Dropped, bool)>, DirstateV2ParseError> {
             let (first_path_component, rest_of_path) =
                 path.split_first_component();
             let node = if let Some(node) =
@@ -728,11 +734,21 @@ 
             };
             let dropped;
             if let Some(rest) = rest_of_path {
-                if let Some(d) = recur(on_disk, &mut node.children, rest)? {
+                if let Some((d, removed)) =
+                    recur(on_disk, &mut node.children, rest)?
+                {
                     dropped = d;
                     if dropped.was_tracked {
                         node.tracked_descendants_count -= 1;
                     }
+
+                    // Directory caches must be invalidated when removing a
+                    // child node
+                    if removed {
+                        if let NodeData::CachedDirectory { .. } = &node.data {
+                            node.data = NodeData::None
+                        }
+                    }
                 } else {
                     return Ok(None);
                 }
@@ -752,16 +768,18 @@ 
             }
             // After recursion, for both leaf (rest_of_path is None) nodes and
             // parent nodes, remove a node if it just became empty.
-            if !node.data.has_entry()
+            let remove = !node.data.has_entry()
                 && node.copy_source.is_none()
-                && node.children.is_empty()
-            {
+                && node.children.is_empty();
+            if remove {
                 nodes.make_mut(on_disk)?.remove(first_path_component);
             }
-            Ok(Some(dropped))
+            Ok(Some((dropped, remove)))
         }
 
-        if let Some(dropped) = recur(self.on_disk, &mut self.root, filename)? {
+        if let Some((dropped, _removed)) =
+            recur(self.on_disk, &mut self.root, filename)?
+        {
             if dropped.had_entry {
                 self.nodes_with_entry_count -= 1
             }