DigestSubset should not short-circuit when there are ignores involved. (

#12648)

Fixes #12661.

Signed-off-by: Andreas Stenius <andreas.stenius@svenskaspel.se>

[ci skip-build-wheels]

src/rust/engine/fs/src/lib.rs

@@ -310,6 +310,29 @@ impl GitignoreStyleExcludes {
       ::ignore::Match::Ignore(_) => true,
     }
   }
+
+  ///
+  /// Find out if a path has any ignore patterns for files/paths in its tree.
+  ///
+  /// Used by the IntermediateGlobbedFilesAndDirectories in snapshot_ops.rs,
+  /// to check if it may optimize the snapshot subset operation on this tree,
+  /// or need to check for excluded files/directories.
+  ///
+  pub fn maybe_is_parent_of_ignored_path(&self, path: &Path) -> bool {
+    match path.to_str() {
+      None => true,
+      Some(s) => {
+        for pattern in self.exclude_patterns().iter() {
+          if pattern.starts_with(s) || s.starts_with(pattern) {
+            // In case the pattern is shorter than path, we are inside a ignored tree, so both
+            // parent and child of ignored paths.
+            return true;
+          }
+        }
+        false
+      }
+    }
+  }
 }
 
 #[derive(Debug, Clone, Eq, Hash, PartialEq)]

src/rust/engine/fs/store/src/snapshot_ops.rs

@@ -406,6 +406,12 @@ impl IntermediateGlobbedFilesAndDirectories {
           RestrictedPathGlob::Wildcard { wildcard } => {
             // NB: We interpret globs such that the *only* way to have a glob match the contents of
             // a whole directory is to end in '/**' or '/**/*'.
+
+            if exclude.maybe_is_parent_of_ignored_path(&directory_path) {
+              // Leave this directory in todo_directories, so we process ignore patterns correctly.
+              continue;
+            }
+
             if *wildcard == *DOUBLE_STAR_GLOB {
               globbed_directories.insert(directory_path, directory_node.clone());
             } else if !globbed_directories.contains_key(&directory_path) {
@@ -425,7 +431,7 @@ impl IntermediateGlobbedFilesAndDirectories {
             if (*wildcard == *DOUBLE_STAR_GLOB) || (*wildcard == *SINGLE_STAR_GLOB) {
               // Here we short-circuit all cases which would swallow up a directory without
               // subsetting it or needing to perform any further recursive work.
-              let short_circuit: bool = match &remainder[..] {
+              let is_short_circuit_pattern: bool = match &remainder[..] {
                 [] => true,
                 // NB: Very often, /**/* is seen ending zsh-style globs, which means the same as
                 // ending in /**. Because we want to *avoid* recursing and just use the subdirectory
@@ -439,7 +445,9 @@ impl IntermediateGlobbedFilesAndDirectories {
                 }
                 _ => false,
               };
-              if short_circuit {
+              if is_short_circuit_pattern
+                && !exclude.maybe_is_parent_of_ignored_path(&directory_path)
+              {
                 globbed_directories.insert(directory_path, directory_node.clone());
                 continue;
               }

src/rust/engine/fs/store/src/snapshot_ops_tests.rs

@@ -95,7 +95,7 @@ async fn subset_single_files() {
 async fn subset_recursive_wildcard() {
   let (store, tempdir, posix_fs, digester) = setup();
 
-  let (merged_digest, _, _) = get_duplicate_rolands(
+  let (merged_digest, snapshot1, _) = get_duplicate_rolands(
     store.clone(),
     store.clone(),
     tempdir.path(),
@@ -120,6 +120,24 @@ async fn subset_recursive_wildcard() {
     .await
     .unwrap();
   assert_eq!(merged_digest, subset_roland2);
+
+  // ** should not include explicitly excluded files
+  let subset_params3 = make_subset_params(&["!subdir/roland2", "subdir/**"]);
+  let subset_roland3 = store
+    .clone()
+    .subset(merged_digest, subset_params3)
+    .await
+    .unwrap();
+  assert_eq!(subset_roland3, snapshot1.digest);
+
+  // ** should not include explicitly excluded files
+  let subset_params4 = make_subset_params(&["!subdir/roland2", "**"]);
+  let subset_roland4 = store
+    .clone()
+    .subset(merged_digest, subset_params4)
+    .await
+    .unwrap();
+  assert_eq!(subset_roland4, snapshot1.digest);
 }
 
 #[derive(Clone)]