fix: assure Action::Cancel doesn't run into unreachable code.

Author: Byron

gix-dir/src/walk/function.rs

@@ -83,7 +83,6 @@ pub fn walk(
         &mut out,
         &mut state,
     )?;
-    assert_eq!(state.on_hold.len(), 0, "BUG: must be fully consumed");
     gix_trace::debug!(statistics = ?out);
     Ok(out)
 }

gix-dir/src/walk/readdir.rs

@@ -68,7 +68,7 @@ pub(super) fn recursive(
                 recursive(false, current, current_bstr, info, ctx, opts, delegate, out, state)?;
             prevent_collapse |= subdir_prevent_collapse;
             if action != Action::Continue {
-                break;
+                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);

gix-dir/tests/walk/mod.rs

@@ -1,10 +1,12 @@
-use gix_dir::walk;
+use gix_dir::{walk, EntryRef};
 use pretty_assertions::assert_eq;
 
 use crate::walk_utils::{
     collect, collect_filtered, entry, entry_dirstat, entry_nokind, entry_nomatch, entryps, entryps_dirstat, fixture,
-    fixture_in, options, options_emit_all, try_collect, try_collect_filtered_opts, EntryExt, Options,
+    fixture_in, options, options_emit_all, try_collect, try_collect_filtered_opts, try_collect_filtered_opts_collect,
+    EntryExt, Options,
 };
+use gix_dir::entry;
 use gix_dir::entry::Kind::*;
 use gix_dir::entry::PathspecMatch::*;
 use gix_dir::entry::Status::*;
@@ -1823,7 +1825,7 @@ fn root_may_not_go_through_dot_git() -> crate::Result {
 #[test]
 fn root_enters_directory_with_dot_git_in_reconfigured_worktree_tracked() -> crate::Result {
     let root = fixture("nonstandard-worktree");
-    let (out, entries) = try_collect_filtered_opts(
+    let (out, entries) = try_collect_filtered_opts_collect(
         &root,
         |keep, ctx| {
             walk(
@@ -1857,7 +1859,7 @@ fn root_enters_directory_with_dot_git_in_reconfigured_worktree_tracked() -> crat
         "everything is tracked, so it won't try to detect git repositories anyway"
     );
 
-    let (out, entries) = try_collect_filtered_opts(
+    let (out, entries) = try_collect_filtered_opts_collect(
         &root,
         |keep, ctx| {
             walk(
@@ -1891,7 +1893,7 @@ fn root_enters_directory_with_dot_git_in_reconfigured_worktree_tracked() -> crat
 #[test]
 fn root_enters_directory_with_dot_git_in_reconfigured_worktree_untracked() -> crate::Result {
     let root = fixture("nonstandard-worktree-untracked");
-    let (_out, entries) = try_collect_filtered_opts(
+    let (_out, entries) = try_collect_filtered_opts_collect(
         &root,
         |keep, ctx| {
             walk(
@@ -2091,6 +2093,69 @@ fn root_can_be_pruned_early_with_pathspec() -> crate::Result {
     Ok(())
 }
 
+#[test]
+fn cancel_with_collection_does_not_fail() -> crate::Result {
+    struct CancelDelegate {
+        emits_left_until_cancel: usize,
+    }
+
+    impl gix_dir::walk::Delegate for CancelDelegate {
+        fn emit(&mut self, _entry: EntryRef<'_>, _collapsed_directory_status: Option<entry::Status>) -> walk::Action {
+            if self.emits_left_until_cancel == 0 {
+                walk::Action::Cancel
+            } else {
+                self.emits_left_until_cancel -= 1;
+                dbg!(self.emits_left_until_cancel);
+                walk::Action::Continue
+            }
+        }
+    }
+
+    for (idx, fixture_name) in [
+        "nonstandard-worktree",
+        "nonstandard-worktree-untracked",
+        "dir-with-file",
+        "expendable-and-precious",
+        "subdir-untracked-and-ignored",
+        "empty-and-untracked-dir",
+        "complex-empty",
+        "type-mismatch-icase-clash-file-is-dir",
+    ]
+    .into_iter()
+    .enumerate()
+    {
+        let root = fixture(fixture_name);
+        let mut dlg = CancelDelegate {
+            emits_left_until_cancel: idx,
+        };
+        let _out = try_collect_filtered_opts(
+            &root,
+            |keep, ctx| {
+                walk(
+                    &root,
+                    &root,
+                    ctx,
+                    walk::Options {
+                        emit_untracked: CollapseDirectory,
+                        emit_ignored: Some(CollapseDirectory),
+                        emit_empty_directories: true,
+                        emit_tracked: true,
+                        for_deletion: Some(Default::default()),
+                        emit_pruned: true,
+                        ..options()
+                    },
+                    keep,
+                )
+            },
+            None::<&str>,
+            &mut dlg,
+            Options::default(),
+        )?;
+        // Note that this also doesn't trigger an error - the caller has to deal with that.
+    }
+    Ok(())
+}
+
 #[test]
 fn file_root_is_shown_if_pathspec_matches_exactly() -> crate::Result {
     let root = fixture("dir-with-file");
@@ -2751,7 +2816,7 @@ fn partial_checkout_cone_and_non_one() -> crate::Result {
 #[test]
 fn type_mismatch() {
     let root = fixture("type-mismatch");
-    let (out, entries) = try_collect_filtered_opts(
+    let (out, entries) = try_collect_filtered_opts_collect(
         &root,
         |keep, ctx| {
             walk(
@@ -2794,7 +2859,7 @@ fn type_mismatch() {
          The typechange is visible only when there is an entry in the index, of course"
     );
 
-    let (out, entries) = try_collect_filtered_opts(
+    let (out, entries) = try_collect_filtered_opts_collect(
         &root,
         |keep, ctx| {
             walk(
@@ -2839,7 +2904,7 @@ fn type_mismatch() {
 #[test]
 fn type_mismatch_ignore_case() {
     let root = fixture("type-mismatch-icase");
-    let (out, entries) = try_collect_filtered_opts(
+    let (out, entries) = try_collect_filtered_opts_collect(
         &root,
         |keep, ctx| {
             walk(
@@ -2879,7 +2944,7 @@ fn type_mismatch_ignore_case() {
         "this is the same as in the non-icase version, which means that icase lookup works"
     );
 
-    let (out, entries) = try_collect_filtered_opts(
+    let (out, entries) = try_collect_filtered_opts_collect(
         &root,
         |keep, ctx| {
             walk(
@@ -2923,7 +2988,7 @@ fn type_mismatch_ignore_case() {
 #[test]
 fn type_mismatch_ignore_case_clash_dir_is_file() {
     let root = fixture("type-mismatch-icase-clash-dir-is-file");
-    let (out, entries) = try_collect_filtered_opts(
+    let (out, entries) = try_collect_filtered_opts_collect(
         &root,
         |keep, ctx| {
             walk(
@@ -2964,7 +3029,7 @@ fn type_mismatch_ignore_case_clash_dir_is_file() {
 #[test]
 fn type_mismatch_ignore_case_clash_file_is_dir() {
     let root = fixture("type-mismatch-icase-clash-file-is-dir");
-    let (out, entries) = try_collect_filtered_opts(
+    let (out, entries) = try_collect_filtered_opts_collect(
         &root,
         |keep, ctx| {
             walk(

gix-dir/tests/walk_utils/mod.rs

@@ -169,15 +169,27 @@ pub fn try_collect_filtered(
     cb: impl FnOnce(&mut dyn walk::Delegate, walk::Context) -> Result<walk::Outcome, walk::Error>,
     patterns: impl IntoIterator<Item = impl AsRef<BStr>>,
 ) -> Result<(walk::Outcome, Entries), walk::Error> {
-    try_collect_filtered_opts(worktree_root, cb, patterns, Default::default())
+    try_collect_filtered_opts_collect(worktree_root, cb, patterns, Default::default())
+}
+
+pub fn try_collect_filtered_opts_collect(
+    worktree_root: &Path,
+    cb: impl FnOnce(&mut dyn walk::Delegate, walk::Context) -> Result<walk::Outcome, walk::Error>,
+    patterns: impl IntoIterator<Item = impl AsRef<BStr>>,
+    options: Options<'_>,
+) -> Result<(walk::Outcome, Entries), walk::Error> {
+    let mut dlg = gix_dir::walk::delegate::Collect::default();
+    let outcome = try_collect_filtered_opts(worktree_root, cb, patterns, &mut dlg, options)?;
+    Ok((outcome, dlg.into_entries_by_path()))
 }
 
 pub fn try_collect_filtered_opts(
     worktree_root: &Path,
     cb: impl FnOnce(&mut dyn walk::Delegate, walk::Context) -> Result<walk::Outcome, walk::Error>,
     patterns: impl IntoIterator<Item = impl AsRef<BStr>>,
+    delegate: &mut dyn gix_dir::walk::Delegate,
     Options { fresh_index, git_dir }: Options<'_>,
-) -> Result<(walk::Outcome, Entries), walk::Error> {
+) -> Result<walk::Outcome, walk::Error> {
     let git_dir = worktree_root.join(git_dir.unwrap_or(".git"));
     let mut index = std::fs::read(git_dir.join("index")).ok().map_or_else(
         || gix_index::State::new(gix_index::hash::Kind::Sha1),
@@ -229,10 +241,9 @@ pub fn try_collect_filtered_opts(
 
     let cwd = gix_fs::current_dir(false).expect("valid cwd");
     let git_dir_realpath = gix_path::realpath_opts(&git_dir, &cwd, gix_path::realpath::MAX_SYMLINKS).unwrap();
-    let mut dlg = gix_dir::walk::delegate::Collect::default();
     let lookup = index.prepare_icase_backing();
-    let outcome = cb(
-        &mut dlg,
+    cb(
+        delegate,
         walk::Context {
             git_dir_realpath: &git_dir_realpath,
             current_dir: &cwd,
@@ -243,9 +254,7 @@ pub fn try_collect_filtered_opts(
             excludes: Some(&mut stack),
             objects: &gix_object::find::Never,
         },
-    )?;
-
-    Ok((outcome, dlg.into_entries_by_path()))
+    )
 }
 
 pub struct Options<'a> {