Merge pull request #1749 from GitoxideLabs/status

Byron · web-flow · commit 8d84818240d4 · 2025-01-08T19:54:57.000+01:00
various improvements
diff --git a/gix-status/tests/Cargo.toml b/gix-status/tests/Cargo.toml
@@ -30,7 +30,7 @@ gix-dir = { path = "../../gix-dir" }
 gix-odb = { path = "../../gix-odb" }
 gix-hash = { path = "../../gix-hash" }
 gix-object = { path = "../../gix-object" }
-gix-features = { path = "../../gix-features" }
+gix-features = { path = "../../gix-features", features = ["parallel"] }
 gix-pathspec = { path = "../../gix-pathspec" }
 gix-worktree = { path = "../../gix-worktree" }
 filetime = "0.2.15"
diff --git a/gix-status/tests/fixtures/generated-archives/.gitignore b/gix-status/tests/fixtures/generated-archives/.gitignore
@@ -2,3 +2,4 @@ status_unchanged.tar
 status_changed.tar
 symlink_stack.tar
 status_nonfile.tar
+status_unchanged_filter.tar
diff --git a/gix-status/tests/fixtures/status_unchanged_filter.sh b/gix-status/tests/fixtures/status_unchanged_filter.sh
@@ -0,0 +1,23 @@
+#!/usr/bin/env bash
+set -eu -o pipefail
+
+git init -q
+
+touch empty
+echo -n "content" >executable
+chmod +x executable
+
+mkdir dir
+echo "other content" >dir/content
+seq 5 >dir/content2
+mkdir dir/sub-dir
+(cd dir/sub-dir && ln -sf ../content symlink)
+
+git add -A
+git update-index --chmod=+x executable  # For Windows.
+git commit -m "Commit"
+
+git ls-files | xargs rm
+
+git config core.autocrlf true
+git checkout -f HEAD
diff --git a/gix-status/tests/status/index_as_worktree.rs b/gix-status/tests/status/index_as_worktree.rs
@@ -5,6 +5,7 @@ use std::sync::{
 
 use bstr::BStr;
 use filetime::{set_file_mtime, FileTime};
+use gix_filter::eol::AutoCrlf;
 use gix_index as index;
 use gix_index::Entry;
 use gix_status::index_as_worktree::Context;
@@ -38,40 +39,97 @@ fn fixture(name: &str, expected_status: &[Expectation<'_>]) -> Outcome {
 }
 
 fn nonfile_fixture(name: &str, expected_status: &[Expectation<'_>]) -> Outcome {
-    fixture_filtered_detailed("status_nonfile", name, &[], expected_status, |_| {}, false)
+    fixture_filtered_detailed(
+        "status_nonfile",
+        name,
+        &[],
+        expected_status,
+        |_| {},
+        false,
+        Default::default(),
+        false,
+    )
 }
 
 fn fixture_with_index(
     name: &str,
     prepare_index: impl FnMut(&mut gix_index::State),
     expected_status: &[Expectation<'_>],
 ) -> Outcome {
-    fixture_filtered_detailed(name, "", &[], expected_status, prepare_index, false)
+    fixture_filtered_detailed(
+        name,
+        "",
+        &[],
+        expected_status,
+        prepare_index,
+        false,
+        Default::default(),
+        false,
+    )
 }
 
 fn submodule_fixture(name: &str, expected_status: &[Expectation<'_>]) -> Outcome {
-    fixture_filtered_detailed("status_submodule", name, &[], expected_status, |_| {}, false)
+    fixture_filtered_detailed(
+        "status_submodule",
+        name,
+        &[],
+        expected_status,
+        |_| {},
+        false,
+        Default::default(),
+        false,
+    )
 }
 
 fn conflict_fixture(name: &str, expected_status: &[Expectation<'_>]) -> Outcome {
-    fixture_filtered_detailed("conflicts", name, &[], expected_status, |_| {}, false)
+    fixture_filtered_detailed(
+        "conflicts",
+        name,
+        &[],
+        expected_status,
+        |_| {},
+        false,
+        Default::default(),
+        false,
+    )
 }
 
 fn submodule_fixture_status(name: &str, expected_status: &[Expectation<'_>], submodule_dirty: bool) -> Outcome {
-    fixture_filtered_detailed("status_submodule", name, &[], expected_status, |_| {}, submodule_dirty)
+    fixture_filtered_detailed(
+        "status_submodule",
+        name,
+        &[],
+        expected_status,
+        |_| {},
+        submodule_dirty,
+        Default::default(),
+        false,
+    )
 }
 
 fn fixture_filtered(name: &str, pathspecs: &[&str], expected_status: &[Expectation<'_>]) -> Outcome {
-    fixture_filtered_detailed(name, "", pathspecs, expected_status, |_| {}, false)
+    fixture_filtered_detailed(
+        name,
+        "",
+        pathspecs,
+        expected_status,
+        |_| {},
+        false,
+        Default::default(),
+        false,
+    )
 }
 
+#[allow(clippy::too_many_arguments)]
 fn fixture_filtered_detailed(
     name: &str,
     subdir: &str,
     pathspecs: &[&str],
     expected_status: &[Expectation<'_>],
     mut prepare_index: impl FnMut(&mut gix_index::State),
     submodule_dirty: bool,
+    auto_crlf: gix_filter::eol::AutoCrlf,
+    use_odb: bool,
 ) -> Outcome {
     // This can easily happen in some fixtures, which can cause flakiness. It's time-dependent after all.
     fn ignore_racyclean(mut out: Outcome) -> Outcome {
@@ -105,26 +163,53 @@ fn fixture_filtered_detailed(
         &index,
         index.path_backing(),
     );
-    let outcome = index_as_worktree(
-        &index,
-        &worktree,
-        &mut recorder,
-        FastEq,
-        SubmoduleStatusMock { dirty: submodule_dirty },
-        gix_object::find::Never,
-        &mut gix_features::progress::Discard,
-        Context {
-            pathspec: search,
-            stack,
-            filter: Default::default(),
-            should_interrupt: &AtomicBool::default(),
-        },
-        Options {
-            fs: gix_fs::Capabilities::probe(&git_dir),
-            stat: TEST_OPTIONS,
-            ..Options::default()
-        },
-    )
+    let ctx = Context {
+        pathspec: search,
+        stack,
+        filter: gix_filter::Pipeline::new(
+            Default::default(),
+            gix_filter::pipeline::Options {
+                eol_config: gix_filter::eol::Configuration {
+                    auto_crlf,
+                    ..Default::default()
+                },
+                ..Default::default()
+            },
+        ),
+        should_interrupt: &AtomicBool::default(),
+    };
+    let options = Options {
+        fs: gix_fs::Capabilities::probe(&git_dir),
+        stat: TEST_OPTIONS,
+        ..Options::default()
+    };
+    let outcome = if use_odb {
+        let odb = gix_odb::at(git_dir.join("objects")).unwrap().into_arc().unwrap();
+        index_as_worktree(
+            &index,
+            &worktree,
+            &mut recorder,
+            FastEq,
+            SubmoduleStatusMock { dirty: submodule_dirty },
+            odb,
+            &mut gix_features::progress::Discard,
+            ctx,
+            options,
+        )
+    } else {
+        let odb = gix_object::find::Never;
+        index_as_worktree(
+            &index,
+            &worktree,
+            &mut recorder,
+            FastEq,
+            SubmoduleStatusMock { dirty: submodule_dirty },
+            &odb,
+            &mut gix_features::progress::Discard,
+            ctx,
+            options,
+        )
+    }
     .unwrap();
     recorder.records.sort_unstable_by_key(|r| r.relative_path);
     assert_eq!(records_to_tuple(recorder.records), expected_status);
@@ -256,6 +341,8 @@ fn replace_dir_with_file() {
         ],
         |_| {},
         false,
+        Default::default(),
+        false,
     );
     assert_eq!(
         out,
@@ -453,6 +540,28 @@ fn unchanged() {
     fixture("status_unchanged", &[]);
 }
 
+#[test]
+fn unchanged_despite_filter() {
+    let actual_outcome = fixture_filtered_detailed(
+        "status_unchanged_filter",
+        "",
+        &[],
+        &[],
+        |_| {},
+        false,
+        AutoCrlf::Enabled,
+        true, /* make ODB available */
+    );
+
+    let expected_outcome = Outcome {
+        entries_to_process: 5,
+        entries_processed: 5,
+        symlink_metadata_calls: 5,
+        ..Default::default()
+    };
+    assert_eq!(actual_outcome, expected_outcome,);
+}
+
 #[test]
 fn refresh() {
     let expected_outcome = Outcome {
diff --git a/gix/src/status/iter/mod.rs b/gix/src/status/iter/mod.rs
@@ -238,14 +238,22 @@ impl Iterator for Iter {
         #[cfg(feature = "parallel")]
         loop {
             let (rx, _join_worktree, _join_tree) = self.rx_and_join.as_ref()?;
-            match rx.recv().ok() {
-                Some(item) => {
+            match rx.recv_timeout(std::time::Duration::from_millis(25)) {
+                Ok(item) => {
                     if let Some(item) = self.maybe_keep_index_change(item) {
                         break Some(Ok(item));
                     }
                     continue;
                 }
-                None => {
+                // NOTE: this isn't necessary when index::from-tree also supports interrupts. As it stands,
+                //       on big repositories it can go up to 500ms which aren't interruptible, so this is another
+                //       way to not wait for this. Once it can be interrupted, this won't be needed anymore.
+                Err(std::sync::mpsc::RecvTimeoutError::Timeout) => {
+                    if self.should_interrupt.load(Ordering::SeqCst) {
+                        return None;
+                    }
+                }
+                Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => {
                     let (_rx, worktree_handle, tree_handle) = self.rx_and_join.take()?;
                     let tree_index = if let Some(handle) = tree_handle {
                         match handle.join().expect("no panic") {
diff --git a/gix/src/util.rs b/gix/src/util.rs
@@ -75,11 +75,10 @@ pub fn parallel_iter_drop<T, U, V>(
             return;
         }
     };
-    // Wait until there is time to respond before we undo the change.
-    if let Some(handle) = maybe_handle {
-        handle.join().ok();
-    }
-    handle.join().ok();
+    // Do not for the remaining threads. Everything but index-from-tree is interruptible, and that wouldn't
+    // take very long even with huge trees.
+    // If this every becomes a problem, just make `index::from-tree` interruptible, and keep waiting for handles here.
+    drop((maybe_handle, handle));
     undo.fetch_update(
         std::sync::atomic::Ordering::SeqCst,
         std::sync::atomic::Ordering::SeqCst,
