fix: make it possible to use a submodule root for a full walk.

Previously, it would not allow to enter the repository, making
a walk impossible.

Author: Byron

gix-dir/src/entry.rs

@@ -168,15 +168,25 @@ impl Status {
     /// This implements the default rules of `git status`, which is good for a minimal traversal through
     /// tracked and non-ignored portions of a worktree.
     /// `for_deletion` is used to determine if recursion into a directory is allowed even though it otherwise wouldn't be.
+    /// If `worktree_root_is_repository` is `true`, then this status is part of the root of an iteration, and the corresponding
+    /// worktree root is a repository itself. This typically happens for submodules. In this case, recursion rules are relaxed
+    /// to allow traversing submodule worktrees.
     ///
     /// Use `pathspec_match` to determine if a pathspec matches in any way, affecting the decision to recurse.
     pub fn can_recurse(
         &self,
         file_type: Option<Kind>,
         pathspec_match: Option<PathspecMatch>,
         for_deletion: Option<ForDeletionMode>,
+        worktree_root_is_repository: bool,
     ) -> bool {
-        let is_dir_on_disk = file_type.map_or(false, |ft| ft.is_recursable_dir());
+        let is_dir_on_disk = file_type.map_or(false, |ft| {
+            if worktree_root_is_repository {
+                ft.is_dir()
+            } else {
+                ft.is_recursable_dir()
+            }
+        });
         if !is_dir_on_disk {
             return false;
         }

gix-dir/src/walk/classify.rs

@@ -4,8 +4,7 @@ use std::borrow::Cow;
 use crate::entry::PathspecMatch;
 use crate::walk::{Context, Error, ForDeletionMode, Options};
 use bstr::{BStr, BString, ByteSlice};
-use std::ffi::OsStr;
-use std::path::{Component, Path, PathBuf};
+use std::path::{Path, PathBuf};
 
 /// Classify the `worktree_relative_root` path and return the first `PathKind` that indicates that
 /// it isn't a directory, leaving `buf` with the path matching the returned `PathKind`,
@@ -16,28 +15,26 @@ pub fn root(
     worktree_relative_root: &Path,
     options: Options,
     ctx: &mut Context<'_>,
-) -> Result<Outcome, Error> {
+) -> Result<(Outcome, bool), Error> {
     buf.clear();
     let mut last_length = None;
     let mut path_buf = worktree_root.to_owned();
     // These initial values kick in if worktree_relative_root.is_empty();
-    let mut out = None;
-    for component in worktree_relative_root
-        .components()
-        .chain(if worktree_relative_root.as_os_str().is_empty() {
-            Some(Component::Normal(OsStr::new("")))
-        } else {
-            None
-        })
-    {
+    let file_kind = path_buf.symlink_metadata().map(|m| m.file_type().into()).ok();
+    let mut out = path(&mut path_buf, buf, 0, file_kind, || None, options, ctx)?;
+    let worktree_root_is_repository = out
+        .disk_kind
+        .map_or(false, |kind| matches!(kind, entry::Kind::Repository));
+
+    for component in worktree_relative_root.components() {
         if last_length.is_some() {
             buf.push(b'/');
         }
         path_buf.push(component);
         buf.extend_from_slice(gix_path::os_str_into_bstr(component.as_os_str()).expect("no illformed UTF8"));
         let file_kind = path_buf.symlink_metadata().map(|m| m.file_type().into()).ok();
 
-        let res = path(
+        out = path(
             &mut path_buf,
             buf,
             last_length.map(|l| l + 1 /* slash */).unwrap_or_default(),
@@ -46,16 +43,17 @@ pub fn root(
             options,
             ctx,
         )?;
-        out = Some(res);
-        if !res
-            .status
-            .can_recurse(res.disk_kind, res.pathspec_match, options.for_deletion)
-        {
+        if !out.status.can_recurse(
+            out.disk_kind,
+            out.pathspec_match,
+            options.for_deletion,
+            worktree_root_is_repository,
+        ) {
             break;
         }
         last_length = Some(buf.len());
     }
-    Ok(out.expect("One iteration of the loop at least"))
+    Ok((out, worktree_root_is_repository))
 }
 /// The product of [`path()`] calls.
 #[derive(Debug, Copy, Clone)]

gix-dir/src/walk/function.rs

@@ -60,15 +60,21 @@ pub fn walk(
     let (mut current, worktree_root_relative) = assure_no_symlink_in_root(worktree_root, &root)?;
     let mut out = Outcome::default();
     let mut buf = BString::default();
-    let root_info = classify::root(
+    let (root_info, worktree_root_is_repository) = classify::root(
         worktree_root,
         &mut buf,
         worktree_root_relative.as_ref(),
         options,
         &mut ctx,
     )?;
-    if !can_recurse(buf.as_bstr(), root_info, options.for_deletion, delegate) {
-        if buf.is_empty() && !matches!(root_info.disk_kind, Some(entry::Kind::Directory { .. })) {
+    if !can_recurse(
+        buf.as_bstr(),
+        root_info,
+        options.for_deletion,
+        worktree_root_is_repository, /* is root */
+        delegate,
+    ) {
+        if buf.is_empty() && !root_info.disk_kind.map_or(false, |kind| kind.is_dir()) {
             return Err(Error::WorktreeRootIsFile { root: root.to_owned() });
         }
         if options.precompose_unicode {
@@ -141,12 +147,17 @@ pub(super) fn can_recurse(
     rela_path: &BStr,
     info: classify::Outcome,
     for_deletion: Option<ForDeletionMode>,
+    is_root: bool,
     delegate: &mut dyn Delegate,
 ) -> bool {
     if info.disk_kind.map_or(true, |k| !k.is_dir()) {
         return false;
     }
-    delegate.can_recurse(EntryRef::from_outcome(Cow::Borrowed(rela_path), info), for_deletion)
+    delegate.can_recurse(
+        EntryRef::from_outcome(Cow::Borrowed(rela_path), info),
+        for_deletion,
+        is_root,
+    )
 }
 
 /// Possibly emit an entry to `for_each` in case the provided information makes that possible.

gix-dir/src/walk/mod.rs

@@ -69,12 +69,24 @@ pub trait Delegate {
     /// Use `for_deletion` to specify if the seen entries should ultimately be deleted, which may affect the decision
     /// of whether to resource or not.
     ///
+    /// If `worktree_root_is_repository` is `true`, then this status is part of the root of an iteration, and the corresponding
+    /// worktree root is a repository itself. This typically happens for submodules. In this case, recursion rules are relaxed
+    /// to allow traversing submodule worktrees.
+    ///
     /// Note that this method will see all directories, even though not all of them may end up being [emitted](Self::emit()).
     /// If this method returns `false`, the `entry` will always be emitted.
-    fn can_recurse(&mut self, entry: EntryRef<'_>, for_deletion: Option<ForDeletionMode>) -> bool {
-        entry
-            .status
-            .can_recurse(entry.disk_kind, entry.pathspec_match, for_deletion)
+    fn can_recurse(
+        &mut self,
+        entry: EntryRef<'_>,
+        for_deletion: Option<ForDeletionMode>,
+        worktree_root_is_repository: bool,
+    ) -> bool {
+        entry.status.can_recurse(
+            entry.disk_kind,
+            entry.pathspec_match,
+            for_deletion,
+            worktree_root_is_repository,
+        )
     }
 }
 

gix-dir/src/walk/readdir.rs

@@ -63,7 +63,13 @@ pub(super) fn recursive(
             ctx,
         )?;
 
-        if can_recurse(current_bstr.as_bstr(), info, opts.for_deletion, delegate) {
+        if can_recurse(
+            current_bstr.as_bstr(),
+            info,
+            opts.for_deletion,
+            false, /* is root */
+            delegate,
+        ) {
             let subdir_may_collapse = state.may_collapse(current);
             let (action, subdir_prevent_collapse) = recursive(
                 subdir_may_collapse,

gix-dir/tests/fixtures/many.sh

@@ -29,6 +29,13 @@ git init dir-with-tracked-file
   git commit -m "init"
 )
 
+git init repo-with-submodule
+(cd repo-with-submodule
+  git submodule add ../dir-with-tracked-file submodule
+  git commit -m "add submodule"
+  touch submodule/untracked
+)
+
 git init ignored-dir
 (cd ignored-dir
   mkdir dir

gix-dir/tests/walk/mod.rs

@@ -2631,6 +2631,130 @@ fn root_may_not_go_through_dot_git() -> crate::Result {
     Ok(())
 }
 
+#[test]
+fn root_at_submodule_repository_allows_walk() -> crate::Result {
+    let root = fixture("repo-with-submodule");
+    let troot = root.join("submodule");
+    let ((out, _root), entries) = try_collect_filtered_opts_collect_with_root(
+        &troot,
+        None,
+        Some(&troot),
+        |keep, ctx| {
+            walk(
+                &troot,
+                ctx,
+                walk::Options {
+                    emit_tracked: true,
+                    emit_untracked: Matching,
+                    ..options()
+                },
+                keep,
+            )
+        },
+        None::<&str>,
+        Options::git_dir("../.git/modules/submodule"),
+    )?;
+
+    assert_eq!(
+        out,
+        walk::Outcome {
+            read_dir_calls: 2,
+            returned_entries: entries.len(),
+            seen_entries: 3,
+        }
+    );
+
+    assert_eq!(
+        entries,
+        [entry("dir/file", Tracked, File), entry("untracked", Untracked, File)],
+        "this is a special case to allow walking submodules specifically, like a normal repository"
+    );
+    Ok(())
+}
+
+#[test]
+fn root_in_submodule_repository_allows_walk() -> crate::Result {
+    let root = fixture("repo-with-submodule");
+    let troot = root.join("submodule");
+    let ((out, _root), entries) = try_collect_filtered_opts_collect_with_root(
+        &troot,
+        None,
+        Some(&troot.join("dir")),
+        |keep, ctx| {
+            walk(
+                &troot,
+                ctx,
+                walk::Options {
+                    emit_tracked: true,
+                    emit_untracked: Matching,
+                    ..options()
+                },
+                keep,
+            )
+        },
+        None::<&str>,
+        Options::git_dir("../.git/modules/submodule"),
+    )?;
+
+    assert_eq!(
+        out,
+        walk::Outcome {
+            read_dir_calls: 1,
+            returned_entries: entries.len(),
+            seen_entries: 1,
+        }
+    );
+
+    assert_eq!(
+        entries,
+        [entry("dir/file", Tracked, File)],
+        "it's also working if the traversal root is inside the subdmodule"
+    );
+    Ok(())
+}
+
+#[test]
+fn root_in_submodule_from_superproject_repository_allows_walk() -> crate::Result {
+    let root = fixture("repo-with-submodule");
+    let troot = root.join("submodule").join("dir");
+    let ((out, _root), entries) = try_collect_filtered_opts_collect_with_root(
+        &root,
+        None,
+        Some(&troot),
+        |keep, ctx| {
+            walk(
+                &troot,
+                ctx,
+                walk::Options {
+                    emit_tracked: true,
+                    emit_untracked: Matching,
+                    ..options()
+                },
+                keep,
+            )
+        },
+        None::<&str>,
+        Default::default(),
+    )?;
+
+    assert_eq!(
+        out,
+        walk::Outcome {
+            read_dir_calls: 1,
+            returned_entries: entries.len(),
+            seen_entries: 1,
+        }
+    );
+
+    assert_eq!(
+        entries,
+        [entry("file", Untracked, File)],
+        "there is no index that has 'file' in it (it's 'dir/file'), hence it's untracked.\
+        But the traversal is possible, even though it might not make the most sense."
+    );
+    Ok(())
+}
+
 #[test]
 fn root_enters_directory_with_dot_git_in_reconfigured_worktree_tracked() -> crate::Result {
     let root = fixture("nonstandard-worktree");
@@ -2797,7 +2921,8 @@ fn root_may_not_go_through_submodule() -> crate::Result {
     assert_eq!(
         entries,
         [entry("submodule", Tracked, Repository)],
-        "it refuses to start traversal in a submodule, thus it ends in the directory that is the submodule"
+        "it refuses to start traversal in a submodule, thus it ends in the directory that is the submodule, \
+        if the root is another repository"
     );
     Ok(())
 }