fix: assure an ignored repository clone is for deletion is recognized as repository (#1464)

Author: Byron

gix-dir/src/walk/classify.rs

@@ -214,37 +214,14 @@ pub fn path(
     };
 
     let mut maybe_upgrade_to_repository = |current_kind, find_harder: bool| {
-        if recurse_repositories {
-            return current_kind;
-        }
-        if find_harder {
-            let mut is_nested_repo = gix_discover::is_git(path).is_ok();
-            if is_nested_repo {
-                let git_dir_is_our_own =
-                    gix_path::realpath_opts(path, ctx.current_dir, gix_path::realpath::MAX_SYMLINKS)
-                        .ok()
-                        .map_or(false, |realpath_candidate| realpath_candidate == ctx.git_dir_realpath);
-                is_nested_repo = !git_dir_is_our_own;
-            }
-            if is_nested_repo {
-                return Some(entry::Kind::Repository);
-            }
-        }
-        path.push(gix_discover::DOT_GIT_DIR);
-        let mut is_nested_nonbare_repo = gix_discover::is_git(path).is_ok();
-        if is_nested_nonbare_repo {
-            let git_dir_is_our_own = gix_path::realpath_opts(path, ctx.current_dir, gix_path::realpath::MAX_SYMLINKS)
-                .ok()
-                .map_or(false, |realpath_candidate| realpath_candidate == ctx.git_dir_realpath);
-            is_nested_nonbare_repo = !git_dir_is_our_own;
-        }
-        path.pop();
-
-        if is_nested_nonbare_repo {
-            Some(entry::Kind::Repository)
-        } else {
-            current_kind
-        }
+        maybe_upgrade_to_repository(
+            current_kind,
+            find_harder,
+            recurse_repositories,
+            path,
+            ctx.current_dir,
+            ctx.git_dir_realpath,
+        )
     };
     if let Some(status) = maybe_status {
         if kind == Some(entry::Kind::Directory) && index_kind == Some(entry::Kind::Repository) {
@@ -302,6 +279,46 @@ pub fn path(
     Ok(out.with_status(status).with_kind(kind, index_kind))
 }
 
+pub fn maybe_upgrade_to_repository(
+    current_kind: Option<entry::Kind>,
+    find_harder: bool,
+    recurse_repositories: bool,
+    path: &mut PathBuf,
+    current_dir: &Path,
+    git_dir_realpath: &Path,
+) -> Option<entry::Kind> {
+    if recurse_repositories {
+        return current_kind;
+    }
+    if find_harder {
+        let mut is_nested_repo = gix_discover::is_git(path).is_ok();
+        if is_nested_repo {
+            let git_dir_is_our_own = gix_path::realpath_opts(path, current_dir, gix_path::realpath::MAX_SYMLINKS)
+                .ok()
+                .map_or(false, |realpath_candidate| realpath_candidate == git_dir_realpath);
+            is_nested_repo = !git_dir_is_our_own;
+        }
+        if is_nested_repo {
+            return Some(entry::Kind::Repository);
+        }
+    }
+    path.push(gix_discover::DOT_GIT_DIR);
+    let mut is_nested_nonbare_repo = gix_discover::is_git(path).is_ok();
+    if is_nested_nonbare_repo {
+        let git_dir_is_our_own = gix_path::realpath_opts(path, current_dir, gix_path::realpath::MAX_SYMLINKS)
+            .ok()
+            .map_or(false, |realpath_candidate| realpath_candidate == git_dir_realpath);
+        is_nested_nonbare_repo = !git_dir_is_our_own;
+    }
+    path.pop();
+
+    if is_nested_nonbare_repo {
+        Some(entry::Kind::Repository)
+    } else {
+        current_kind
+    }
+}
+
 /// Note that `rela_path` is used as buffer for convenience, but will be left as is when this function returns.
 /// Also note `maybe_file_type` will be `None` for entries that aren't up-to-date and files, for directories at least one entry must be uptodate.
 /// Returns `(maybe_file_type, Option<index_file_type>, flags)`, with the last option being a flag set only for sparse directories in the index.

gix-dir/src/walk/readdir.rs

@@ -6,7 +6,9 @@ use std::sync::atomic::Ordering;
 use crate::entry::{PathspecMatch, Status};
 use crate::walk::function::{can_recurse, emit_entry};
 use crate::walk::EmissionMode::CollapseDirectory;
-use crate::walk::{classify, Action, CollapsedEntriesEmissionMode, Context, Delegate, Error, Options, Outcome};
+use crate::walk::{
+    classify, Action, CollapsedEntriesEmissionMode, Context, Delegate, Error, ForDeletionMode, Options, Outcome,
+};
 use crate::{entry, walk, Entry, EntryRef};
 
 /// ### Deviation
@@ -57,7 +59,7 @@ pub(super) fn recursive(
         );
         current.push(file_name);
 
-        let info = classify::path(
+        let mut info = classify::path(
             current,
             current_bstr,
             if prev_len == 0 { 0 } else { prev_len + 1 },
@@ -90,10 +92,25 @@ pub(super) fn recursive(
             if action != Action::Continue {
                 return Ok((action, prevent_collapse));
             }
-        } else if !state.held_for_directory_collapse(current_bstr.as_bstr(), info, &opts) {
-            let action = emit_entry(Cow::Borrowed(current_bstr.as_bstr()), info, None, opts, out, delegate);
-            if action != Action::Continue {
-                return Ok((action, prevent_collapse));
+        } else {
+            if opts.for_deletion == Some(ForDeletionMode::IgnoredDirectoriesCanHideNestedRepositories)
+                && info.disk_kind == Some(entry::Kind::Directory)
+                && matches!(info.status, Status::Ignored(_))
+            {
+                info.disk_kind = classify::maybe_upgrade_to_repository(
+                    info.disk_kind,
+                    true,
+                    false,
+                    current,
+                    ctx.current_dir,
+                    ctx.git_dir_realpath,
+                );
+            }
+            if !state.held_for_directory_collapse(current_bstr.as_bstr(), info, &opts) {
+                let action = emit_entry(Cow::Borrowed(current_bstr.as_bstr()), info, None, opts, out, delegate);
+                if action != Action::Continue {
+                    return Ok((action, prevent_collapse));
+                }
             }
         }
         current_bstr.truncate(prev_len);

gix-dir/tests/fixtures/many.sh

@@ -431,3 +431,10 @@ EOF
     git commit -m "init"
   )
 )
+
+git init with-sub-repo
+(cd with-sub-repo
+  echo '*' > .gitignore
+  git add -f .gitignore
+  git clone ../dir-with-file sub-repo
+)

gix-dir/tests/walk/mod.rs

@@ -4434,3 +4434,62 @@ fn one_ignored_submodule() -> crate::Result {
     );
     Ok(())
 }
+
+#[test]
+fn ignored_sub_repo() -> crate::Result {
+    let root = fixture("with-sub-repo");
+    let ((out, _root), entries) = collect(&root, None, |keep, ctx| walk(&root, ctx, options_emit_all(), keep));
+    assert_eq!(
+        out,
+        walk::Outcome {
+            read_dir_calls: 1,
+            returned_entries: entries.len(),
+            seen_entries: 3,
+        }
+    );
+    assert_eq!(
+        entries,
+        &[
+            entry_nokind(".git", Pruned).with_property(DotGit).with_match(Always),
+            entry(".gitignore", Tracked, File),
+            entry("sub-repo", Ignored(Expendable), Directory),
+        ],
+        "without intent to delete, this looks like just like an untracked directory"
+    );
+
+    for ignored_emission_mode in [Matching, CollapseDirectory] {
+        for untracked_emission_mode in [Matching, CollapseDirectory] {
+            let ((out, _root), entries) = collect(&root, None, |keep, ctx| {
+                walk(
+                    &root,
+                    ctx,
+                    walk::Options {
+                        for_deletion: Some(ForDeletionMode::IgnoredDirectoriesCanHideNestedRepositories),
+                        emit_tracked: false,
+                        emit_ignored: Some(ignored_emission_mode),
+                        emit_untracked: untracked_emission_mode,
+                        ..options_emit_all()
+                    },
+                    keep,
+                )
+            });
+            assert_eq!(
+                out,
+                walk::Outcome {
+                    read_dir_calls: 1,
+                    returned_entries: entries.len(),
+                    seen_entries: 3,
+                }
+            );
+            assert_eq!(
+                entries,
+                &[
+                    entry_nokind(".git", Pruned).with_property(DotGit).with_match(Always),
+                    entry("sub-repo", Ignored(Expendable), Repository),
+                ],
+                "Even when ignored directories can hide repositories, we are able to detect top-level repositories"
+            );
+        }
+    }
+    Ok(())
+}