feat!: improve index_as_worktree() function signature.

This is accomplished by providing a context which now contains only
statically known types, among them also a pathspec search. This allows
the attribute stack to be passed and can thus cheaply be cloned, instead
of being created internally.

Author: Byron

Cargo.lock

@@ -22,6 +22,7 @@ gix-path = { version = "^0.10.5", path = "../gix-path" }
 gix-features = { version = "^0.38.0", path = "../gix-features" }
 gix-filter = { version = "^0.9.0", path = "../gix-filter" }
 gix-worktree = { version = "^0.31.0", path = "../gix-worktree", default-features = false, features = ["attributes"] }
+gix-pathspec = { version = "^0.6.0", path = "../gix-pathspec" }
 
 thiserror = "1.0.26"
 filetime = "0.2.15"

gix-status/Cargo.toml

@@ -2,7 +2,7 @@ use std::{
     io,
     path::Path,
     slice::Chunks,
-    sync::atomic::{AtomicBool, AtomicU64, AtomicUsize, Ordering},
+    sync::atomic::{AtomicU64, AtomicUsize, Ordering},
 };
 
 use bstr::BStr;
@@ -11,30 +11,30 @@ use gix_features::parallel::{in_parallel_if, Reduce};
 use gix_filter::pipeline::convert::ToGitOutcome;
 use gix_object::FindExt;
 
+use crate::index_as_worktree::Context;
 use crate::{
     index_as_worktree::{
         traits,
         traits::{read_data::Stream, CompareBlobs, SubmoduleStatus},
         types::{Error, Options},
         Change, Conflict, EntryStatus, Outcome, VisitEntry,
     },
-    Pathspec, SymlinkCheck,
+    SymlinkCheck,
 };
 
 /// Calculates the changes that need to be applied to an `index` to match the state of the `worktree` and makes them
 /// observable in `collector`, along with information produced by `compare` which gets to see blobs that may have changes, and
 /// `submodule` which can take a look at submodules in detail to produce status information (BASE version if its conflicting).
 /// `options` are used to configure the operation.
 ///
+/// Note `worktree` must be the root path of the worktree, not a path inside of the worktree.
+///
 /// Note that `index` may require changes to be up-to-date with the working tree and avoid expensive computations by updating
 /// respective entries with stat information from the worktree, and its timestamp is adjusted to the current time for which it
 /// will be considered fresh. All changes that would be applied to the index are delegated to the caller, which receives these
 /// as [`EntryStatus`].
 /// The `pathspec` is used to determine which index entries to check for status in the first place.
 ///
-/// `should_interrupt` can be used to stop all processing.
-/// `filter` is used to convert worktree files back to their internal git representation. For this to be correct,
-/// [`Options::attributes`] must be configured as well.
 /// `objects` is used to access the version of an object in the object database for direct comparison.
 ///
 /// **It's important to note that the `index` should have its [timestamp updated](gix_index::State::set_timestamp()) with a timestamp
@@ -47,7 +47,7 @@ use crate::{
 /// stats like `git status` would if it had to determine the hash.
 /// If that happened, the index should be written back after updating the entries with these updated stats, see [Outcome::skipped].
 ///
-/// Thus some care has to be taken to do the right thing when letting the index match the worktree by evaluating the changes observed
+/// Thus, some care has to be taken to do the right thing when letting the index match the worktree by evaluating the changes observed
 /// by the `collector`.
 #[allow(clippy::too_many_arguments)]
 pub fn index_as_worktree<'index, T, U, Find, E>(
@@ -58,10 +58,13 @@ pub fn index_as_worktree<'index, T, U, Find, E>(
     submodule: impl SubmoduleStatus<Output = U, Error = E> + Send + Clone,
     objects: Find,
     progress: &mut dyn gix_features::progress::Progress,
-    pathspec: impl Pathspec + Send + Clone,
-    filter: gix_filter::Pipeline,
-    should_interrupt: &AtomicBool,
-    mut options: Options,
+    Context {
+        pathspec,
+        stack,
+        filter,
+        should_interrupt,
+    }: Context<'_>,
+    options: Options,
 ) -> Result<Outcome, Error>
 where
     T: Send,
@@ -84,20 +87,13 @@ where
         .prefixed_entries_range(pathspec.common_prefix())
         .unwrap_or(0..index.entries().len());
 
-    let stack = gix_worktree::Stack::from_state_and_ignore_case(
-        worktree,
-        options.fs.ignore_case,
-        gix_worktree::stack::State::AttributesStack(std::mem::take(&mut options.attributes)),
-        index,
-        index.path_backing(),
-    );
     let (entries, path_backing) = (index.entries(), index.path_backing());
     let mut num_entries = entries.len();
     let entry_index_offset = range.start;
     let entries = &entries[range];
 
     let _span = gix_features::trace::detail!("gix_status::index_as_worktree", 
-                                             num_entries = entries.len(), 
+                                             num_entries = entries.len(),
                                              chunk_size = chunk_size,
                                              thread_limit = ?thread_limit);
 
@@ -253,7 +249,7 @@ impl<'index> State<'_, 'index> {
         entries: &'index [gix_index::Entry],
         entry: &'index gix_index::Entry,
         entry_index: usize,
-        pathspec: &mut impl Pathspec,
+        pathspec: &mut gix_pathspec::Search,
         diff: &mut impl CompareBlobs<Output = T>,
         submodule: &mut impl SubmoduleStatus<Output = U, Error = E>,
         objects: &Find,
@@ -273,7 +269,20 @@ impl<'index> State<'_, 'index> {
             return None;
         }
         let path = entry.path_in(self.path_backing);
-        if !pathspec.is_included(path, Some(false)) {
+        let is_excluded = pathspec
+            .pattern_matching_relative_path(
+                path,
+                Some(entry.mode.is_submodule()),
+                &mut |relative_path, case, is_dir, out| {
+                    self.attr_stack
+                        .set_case(case)
+                        .at_entry(relative_path, Some(is_dir), objects)
+                        .map_or(false, |platform| platform.matching_attributes(out))
+                },
+            )
+            .map_or(true, |m| m.is_excluded());
+
+        if is_excluded {
             self.skipped_by_pathspec.fetch_add(1, Ordering::Relaxed);
             return None;
         }

gix-status/src/index_as_worktree/function.rs

@@ -1,7 +1,7 @@
 //! Changes between an index and a worktree.
 ///
 mod types;
-pub use types::{Change, Conflict, EntryStatus, Error, Options, Outcome, VisitEntry};
+pub use types::{Change, Conflict, Context, EntryStatus, Error, Options, Outcome, VisitEntry};
 
 mod recorder;
 pub use recorder::{Record, Recorder};

gix-status/src/index_as_worktree/mod.rs

@@ -1,4 +1,5 @@
 use bstr::{BStr, BString};
+use std::sync::atomic::AtomicBool;
 
 /// The error returned by [index_as_worktree()`](crate::index_as_worktree()).
 #[derive(Debug, thiserror::Error)]
@@ -30,11 +31,25 @@ pub struct Options {
     pub thread_limit: Option<usize>,
     /// Options that control how stat comparisons are made when checking if a file is fresh.
     pub stat: gix_index::entry::stat::Options,
-    /// Pre-configured state to allow processing attributes.
+}
+
+/// The context for [index_as_worktree()`](crate::index_as_worktree()).
+#[derive(Clone)]
+pub struct Context<'a> {
+    /// The pathspec to limit the amount of paths that are checked. Can be empty to allow all paths.
+    pub pathspec: gix_pathspec::Search,
+    /// A stack pre-configured to allow accessing attributes for each entry, as required for `filter`
+    /// and possibly pathspecs.
+    pub stack: gix_worktree::Stack,
+    /// A filter to be able to perform conversions from and to the worktree format.
+    ///
+    /// It is needed to potentially refresh the index with data read from the worktree, which needs to be converted back
+    /// to the form stored in Git.
     ///
-    /// These are needed to potentially refresh the index with data read from the worktree, which needs to be converted back
-    /// to the form stored in git.
-    pub attributes: gix_worktree::stack::state::Attributes,
+    /// Note that for this to be correct, the attribute `stack` must be configured correctly as well.
+    pub filter: gix_filter::Pipeline,
+    /// A flag to query to learn if cancellation is requested.
+    pub should_interrupt: &'a AtomicBool,
 }
 
 /// Provide additional information collected during the runtime of [`index_as_worktree()`](crate::index_as_worktree()).

gix-status/src/index_as_worktree/types.rs

@@ -8,27 +8,9 @@
 //! While also being able to check check if the working tree is dirty, quickly.
 #![deny(missing_docs, rust_2018_idioms, unsafe_code)]
 
-use bstr::BStr;
-
 pub mod index_as_worktree;
 pub use index_as_worktree::function::index_as_worktree;
 
-/// A trait to facilitate working working with pathspecs.
-pub trait Pathspec {
-    /// Return the portion of the prefix among all of the pathspecs involved in this search, or an empty string if
-    /// there is none. It doesn't have to end at a directory boundary though, nor does it denote a directory.
-    ///
-    /// Note that the common_prefix is always matched case-sensitively, and it is useful to skip large portions of input.
-    /// Further, excluded pathspecs don't participate which makes this common prefix inclusive. To work correctly though,
-    /// one will have to additionally match paths that have the common prefix with that pathspec itself to assure it is
-    /// not excluded.
-    fn common_prefix(&self) -> &BStr;
-
-    /// Return `true` if `relative_path` is included in this pathspec.
-    /// `is_dir` is `true` if `relative_path` is a directory.
-    fn is_included(&mut self, relative_path: &BStr, is_dir: Option<bool>) -> bool;
-}
-
 /// A stack that validates we are not going through a symlink in a way that is read-only.
 ///
 /// It can efficiently validate paths when these are queried in sort-order, which leads to each component

gix-status/src/lib.rs

@@ -25,6 +25,7 @@ gix-hash = { path = "../../gix-hash" }
 gix-object = { path = "../../gix-object" }
 gix-features = { path = "../../gix-features" }
 gix-pathspec = { path = "../../gix-pathspec" }
+gix-worktree = { path = "../../gix-worktree" }
 filetime = "0.2.15"
 bstr = { version = "1.3.0", default-features = false }
 

gix-status/tests/Cargo.toml

@@ -7,6 +7,7 @@ use bstr::BStr;
 use filetime::{set_file_mtime, FileTime};
 use gix_index as index;
 use gix_index::Entry;
+use gix_status::index_as_worktree::Context;
 use gix_status::{
     index_as_worktree,
     index_as_worktree::{
@@ -93,6 +94,13 @@ fn fixture_filtered_detailed(
     let mut recorder = Recorder::default();
     let search = gix_pathspec::Search::from_specs(to_pathspecs(pathspecs), None, std::path::Path::new(""))
         .expect("valid specs can be normalized");
+    let stack = gix_worktree::Stack::from_state_and_ignore_case(
+        worktree.clone(),
+        false,
+        gix_worktree::stack::State::AttributesStack(Default::default()),
+        &index,
+        index.path_backing(),
+    );
     let outcome = index_as_worktree(
         &index,
         &worktree,
@@ -101,9 +109,12 @@ fn fixture_filtered_detailed(
         SubmoduleStatusMock { dirty: submodule_dirty },
         gix_object::find::Never,
         &mut gix_features::progress::Discard,
-        Pathspec(search),
-        Default::default(),
-        &AtomicBool::default(),
+        Context {
+            pathspec: search,
+            stack,
+            filter: Default::default(),
+            should_interrupt: &AtomicBool::default(),
+        },
         Options {
             fs: gix_fs::Capabilities::probe(&git_dir),
             stat: TEST_OPTIONS,
@@ -606,6 +617,19 @@ fn racy_git() {
 
     let count = Arc::new(AtomicUsize::new(0));
     let counter = CountCalls(count.clone(), FastEq);
+    let stack = gix_worktree::Stack::from_state_and_ignore_case(
+        worktree,
+        false,
+        gix_worktree::stack::State::AttributesStack(Default::default()),
+        &index,
+        index.path_backing(),
+    );
+    let ctx = Context {
+        pathspec: default_pathspec(),
+        stack,
+        filter: Default::default(),
+        should_interrupt: &AtomicBool::default(),
+    };
     let out = index_as_worktree(
         &index,
         worktree,
@@ -614,9 +638,7 @@ fn racy_git() {
         SubmoduleStatusMock { dirty: false },
         gix_object::find::Never,
         &mut gix_features::progress::Discard,
-        Pathspec::default(),
-        Default::default(),
-        &AtomicBool::default(),
+        ctx.clone(),
         Options {
             fs,
             stat: TEST_OPTIONS,
@@ -653,9 +675,7 @@ fn racy_git() {
         SubmoduleStatusMock { dirty: false },
         gix_object::find::Never,
         &mut gix_features::progress::Discard,
-        Pathspec::default(),
-        Default::default(),
-        &AtomicBool::default(),
+        ctx,
         Options {
             fs,
             stat: TEST_OPTIONS,
@@ -696,27 +716,6 @@ fn racy_git() {
     );
 }
 
-#[derive(Clone)]
-struct Pathspec(gix_pathspec::Search);
-
-impl Default for Pathspec {
-    fn default() -> Self {
-        let search = gix_pathspec::Search::from_specs(to_pathspecs(&[]), None, std::path::Path::new(""))
-            .expect("empty is always valid");
-        Self(search)
-    }
-}
-
-impl gix_status::Pathspec for Pathspec {
-    fn common_prefix(&self) -> &BStr {
-        self.0.common_prefix()
-    }
-
-    fn is_included(&mut self, relative_path: &BStr, is_dir: Option<bool>) -> bool {
-        self.0
-            .pattern_matching_relative_path(relative_path, is_dir, &mut |_, _, _, _| {
-                unreachable!("we don't use attributes in our pathspecs")
-            })
-            .map_or(false, |m| !m.is_excluded())
-    }
+fn default_pathspec() -> gix_pathspec::Search {
+    gix_pathspec::Search::from_specs(to_pathspecs(&[]), None, std::path::Path::new("")).expect("empty is always valid")
 }