feat!: add support for submodule status

Previously, submodules where ignored. Now they are treated correctly
as 'directory' which is compared to what's in the worktree.

We also simplify blob handling.

Author: Byron

gitoxide-core/src/repository/status.rs

@@ -4,7 +4,7 @@ use gix::bstr::{BStr, BString};
 use gix::index::Entry;
 use gix::prelude::FindExt;
 use gix::Progress;
-use gix_status::index_as_worktree::content::FastEq;
+use gix_status::index_as_worktree::traits::FastEq;
 use gix_status::index_as_worktree::Change;
 
 pub enum Submodules {

gix-status/src/index_as_worktree/function.rs

@@ -7,8 +7,8 @@ use gix_features::parallel::{in_parallel_if, Reduce};
 
 use crate::{
     index_as_worktree::{
-        content,
-        content::CompareBlobs,
+        traits,
+        traits::{CompareBlobs, SubmoduleStatus},
         types::{Error, Options},
         Change, VisitEntry,
     },
@@ -26,20 +26,23 @@ use crate::{
 /// like whether a file has conflicts, and files that were added with `git add` are shown as a special
 /// changes despite not technically requiring a change to the index since `git add` already added the file to the index.
 #[allow(clippy::too_many_arguments)]
-pub fn index_as_worktree<'index, T, Find, E>(
+pub fn index_as_worktree<'index, T, U, Find, E1, E2>(
     index: &'index mut gix_index::State,
     worktree: &Path,
-    collector: &mut impl VisitEntry<'index, ContentChange = T>,
+    collector: &mut impl VisitEntry<'index, ContentChange = T, SubmoduleStatus = U>,
     compare: impl CompareBlobs<Output = T> + Send + Clone,
+    submodule: impl SubmoduleStatus<Output = U, Error = E2> + Send + Clone,
     find: Find,
     progress: &mut dyn gix_features::progress::Progress,
     pathspec: impl Pathspec + Send + Clone,
     options: Options,
 ) -> Result<(), Error>
 where
     T: Send,
-    E: std::error::Error + Send + Sync + 'static,
-    Find: for<'a> FnMut(&gix_hash::oid, &'a mut Vec<u8>) -> Result<gix_object::BlobRef<'a>, E> + Send + Clone,
+    U: Send,
+    E1: std::error::Error + Send + Sync + 'static,
+    E2: std::error::Error + Send + Sync + 'static,
+    Find: for<'a> FnMut(&gix_hash::oid, &'a mut Vec<u8>) -> Result<gix_object::BlobRef<'a>, E1> + Send + Clone,
 {
     // the order is absolutely critical here we use the old timestamp to detect racy index entries
     // (modified at or after the last index update) during the index update we then set those
@@ -78,16 +81,17 @@ where
                         options,
                     },
                     compare,
+                    submodule,
                     find,
                     pathspec,
                 )
             }
         },
-        |entries, (state, diff, find, pathspec)| {
+        |entries, (state, blobdiff, submdule, find, pathspec)| {
             entries
                 .iter_mut()
                 .filter_map(|entry| {
-                    let res = state.process(entry, diff, find, pathspec);
+                    let res = state.process(entry, blobdiff, submdule, find, pathspec);
                     count.fetch_add(1, Ordering::Relaxed);
                     res
                 })
@@ -109,19 +113,21 @@ struct State<'a, 'b> {
     options: &'a Options,
 }
 
-type StatusResult<'index, T> = Result<(&'index gix_index::Entry, &'index BStr, Option<Change<T>>, bool), Error>;
+type StatusResult<'index, T, U> = Result<(&'index gix_index::Entry, &'index BStr, Option<Change<T, U>>, bool), Error>;
 
 impl<'index> State<'_, 'index> {
-    fn process<T, Find, E>(
+    fn process<T, U, Find, E1, E2>(
         &mut self,
         entry: &'index mut gix_index::Entry,
         diff: &mut impl CompareBlobs<Output = T>,
+        submodule: &mut impl SubmoduleStatus<Output = U, Error = E2>,
         find: &mut Find,
         pathspec: &mut impl Pathspec,
-    ) -> Option<StatusResult<'index, T>>
+    ) -> Option<StatusResult<'index, T, U>>
     where
-        E: std::error::Error + Send + Sync + 'static,
-        Find: for<'a> FnMut(&gix_hash::oid, &'a mut Vec<u8>) -> Result<gix_object::BlobRef<'a>, E>,
+        E1: std::error::Error + Send + Sync + 'static,
+        E2: std::error::Error + Send + Sync + 'static,
+        Find: for<'a> FnMut(&gix_hash::oid, &'a mut Vec<u8>) -> Result<gix_object::BlobRef<'a>, E1>,
     {
         let conflict = match entry.stage() {
             0 => false,
@@ -140,7 +146,7 @@ impl<'index> State<'_, 'index> {
         if !pathspec.is_included(path, Some(false)) {
             return None;
         }
-        let status = self.compute_status(&mut *entry, path, diff, find);
+        let status = self.compute_status(&mut *entry, path, diff, submodule, find);
         Some(status.map(move |status| (&*entry, path, status, conflict)))
     }
 
@@ -183,18 +189,20 @@ impl<'index> State<'_, 'index> {
     /// which is a constant.
     ///
     /// Adapted from [here](https://github.com/Byron/gitoxide/pull/805#discussion_r1164676777).
-    fn compute_status<T, Find, E>(
+    fn compute_status<T, U, Find, E1, E2>(
         &mut self,
         entry: &mut gix_index::Entry,
-        git_path: &BStr,
+        rela_path: &BStr,
         diff: &mut impl CompareBlobs<Output = T>,
+        submodule: &mut impl SubmoduleStatus<Output = U, Error = E2>,
         find: &mut Find,
-    ) -> Result<Option<Change<T>>, Error>
+    ) -> Result<Option<Change<T, U>>, Error>
     where
-        E: std::error::Error + Send + Sync + 'static,
-        Find: for<'a> FnMut(&gix_hash::oid, &'a mut Vec<u8>) -> Result<gix_object::BlobRef<'a>, E>,
+        E1: std::error::Error + Send + Sync + 'static,
+        E2: std::error::Error + Send + Sync + 'static,
+        Find: for<'a> FnMut(&gix_hash::oid, &'a mut Vec<u8>) -> Result<gix_object::BlobRef<'a>, E1>,
     {
-        let worktree_path = gix_path::try_from_bstr(git_path).map_err(|_| Error::IllformedUtf8)?;
+        let worktree_path = gix_path::try_from_bstr(rela_path).map_err(|_| Error::IllformedUtf8)?;
         let worktree_path = match self.path_stack.verified_path(worktree_path.as_ref()) {
             Ok(path) => path,
             Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(Some(Change::Removed)),
@@ -206,11 +214,17 @@ impl<'index> State<'_, 'index> {
                 // if a file turned into a directory it was removed
                 // the only exception here are submodules which are
                 // part of the index despite being directories
-                //
-                // TODO: submodules:
-                //   if entry.mode.contains(Mode::COMMIT) &&
-                //     resolve_gitlink_ref(ce->name, "HEAD", &sub))
-                return Ok(Some(Change::Removed));
+                if entry.mode.is_submodule() {
+                    let status = submodule
+                        .status(entry, rela_path)
+                        .map_err(|err| Error::SubmoduleStatus {
+                            rela_path: rela_path.into(),
+                            source: Box::new(err),
+                        })?;
+                    return Ok(status.map(|status| Change::SubmoduleModification(status)));
+                } else {
+                    return Ok(Some(Change::Removed));
+                }
             }
             Ok(metadata) => metadata,
             Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(Some(Change::Removed)),
@@ -265,7 +279,7 @@ impl<'index> State<'_, 'index> {
             id: &entry.id,
             find,
         };
-        let content_change = diff.compare_blobs::<Error>(entry, metadata.len() as usize, read_file, read_blob)?;
+        let content_change = diff.compare_blobs(entry, metadata.len() as usize, read_file, read_blob)?;
         // This file is racy clean! Set the size to 0 so we keep detecting this as the file is updated.
         if content_change.is_some() && racy_clean {
             entry.stat.size = 0;
@@ -288,8 +302,10 @@ struct ReduceChange<'a, 'index, T: VisitEntry<'index>> {
     phantom: PhantomData<fn(&'index ())>,
 }
 
-impl<'index, T, C: VisitEntry<'index, ContentChange = T>> Reduce for ReduceChange<'_, 'index, C> {
-    type Input = Vec<StatusResult<'index, T>>;
+impl<'index, T, U, C: VisitEntry<'index, ContentChange = T, SubmoduleStatus = U>> Reduce
+    for ReduceChange<'_, 'index, C>
+{
+    type Input = Vec<StatusResult<'index, T, U>>;
 
     type FeedProduce = ();
 
@@ -327,7 +343,7 @@ where
     find: Find,
 }
 
-impl<'a> content::ReadDataOnce<'a, Error> for WorktreeBlob<'a> {
+impl<'a> traits::ReadDataOnce<'a> for WorktreeBlob<'a> {
     fn read_data(self) -> Result<&'a [u8], Error> {
         let res = read::data_to_buf_with_meta(
             self.path,
@@ -339,7 +355,7 @@ impl<'a> content::ReadDataOnce<'a, Error> for WorktreeBlob<'a> {
     }
 }
 
-impl<'a, Find, E> content::ReadDataOnce<'a, Error> for OdbBlob<'a, Find, E>
+impl<'a, Find, E> traits::ReadDataOnce<'a> for OdbBlob<'a, Find, E>
 where
     E: std::error::Error + Send + Sync + 'static,
     Find: FnMut(&gix_hash::oid, &'a mut Vec<u8>) -> Result<gix_object::BlobRef<'a>, E>,

gix-status/src/index_as_worktree/mod.rs

@@ -4,8 +4,8 @@ mod types;
 pub use types::{Change, Error, Options, VisitEntry};
 
 mod recorder;
-pub use recorder::Recorder;
+pub use recorder::{Record, Recorder};
 
-///
-pub mod content;
 pub(crate) mod function;
+///
+pub mod traits;

gix-status/src/index_as_worktree/recorder.rs

@@ -3,25 +3,46 @@ use gix_index as index;
 
 use crate::index_as_worktree::{Change, VisitEntry};
 
+/// A record of a change.
+///
+/// It's created either if there is a conflict or a change, or both.
+#[derive(Debug)]
+pub struct Record<'index, T, U> {
+    /// The index entry that is changed.
+    pub entry: &'index index::Entry,
+    /// The path to the entry.
+    pub relative_path: &'index BStr,
+    /// The change itself, or `None` if there is only a conflict.
+    pub change: Option<Change<T, U>>,
+    /// information about the conflict itself
+    pub conflict: bool,
+}
+
 /// Convenience implementation of [`VisitEntry`] that collects all non-trivial changes into a `Vec`.
 #[derive(Debug, Default)]
-pub struct Recorder<'index, T = ()> {
+pub struct Recorder<'index, T = (), U = ()> {
     /// collected changes, index entries without conflicts or changes are excluded.
-    pub records: Vec<(&'index BStr, Option<Change<T>>, bool)>,
+    pub records: Vec<Record<'index, T, U>>,
 }
 
-impl<'index, T: Send> VisitEntry<'index> for Recorder<'index, T> {
+impl<'index, T: Send, U: Send> VisitEntry<'index> for Recorder<'index, T, U> {
     type ContentChange = T;
+    type SubmoduleStatus = U;
 
     fn visit_entry(
         &mut self,
-        _entry: &'index index::Entry,
+        entry: &'index index::Entry,
         rela_path: &'index BStr,
-        status: Option<Change<Self::ContentChange>>,
+        change: Option<Change<Self::ContentChange, Self::SubmoduleStatus>>,
         conflict: bool,
     ) {
-        if conflict || status.is_some() {
-            self.records.push((rela_path, status, conflict))
+        if conflict || change.is_some() {
+            self.records.push(Record {
+                entry,
+                relative_path: rela_path,
+                change,
+                conflict,
+            })
         }
     }
 }

gix-status/src/index_as_worktree/content.rs renamed to gix-status/src/index_as_worktree/traits.rs

@@ -1,3 +1,5 @@
+use crate::index_as_worktree::Error;
+use bstr::BStr;
 use gix_hash::ObjectId;
 use gix_index as index;
 use index::Entry;
@@ -11,22 +13,33 @@ pub trait CompareBlobs {
     /// and allow reading its bytes using `worktree_blob`.
     /// If this function returns `None` the `entry` and the `worktree_blob` are assumed to be identical.
     /// Use `entry_blob` to obtain the data for the blob referred to by `entry`, allowing comparisons of the data itself.
-    fn compare_blobs<'a, E>(
+    fn compare_blobs<'a, 'b>(
         &mut self,
-        entry: &'a gix_index::Entry,
+        entry: &gix_index::Entry,
         worktree_blob_size: usize,
-        worktree_blob: impl ReadDataOnce<'a, E>,
-        entry_blob: impl ReadDataOnce<'a, E>,
-    ) -> Result<Option<Self::Output>, E>;
+        worktree_blob: impl ReadDataOnce<'a>,
+        entry_blob: impl ReadDataOnce<'b>,
+    ) -> Result<Option<Self::Output>, Error>;
+}
+
+/// Determine the status of a submodule, which always indicates that it changed if present.
+pub trait SubmoduleStatus {
+    /// The status result, describing in which way the submodule changed.
+    type Output;
+    /// A custom error that may occur while computing the submodule status.
+    type Error: std::error::Error + Send + Sync + 'static;
+
+    /// Compute the status of the submodule at `entry` and `rela_path`, or return `None` if no change was detected.
+    fn status(&mut self, entry: &gix_index::Entry, rela_path: &BStr) -> Result<Option<Self::Output>, Self::Error>;
 }
 
 /// Lazy borrowed access to blob data.
-pub trait ReadDataOnce<'a, E> {
+pub trait ReadDataOnce<'a> {
     /// Returns the contents of this blob.
     ///
     /// This potentially performs IO and other expensive operations
     /// and should only be called when necessary.
-    fn read_data(self) -> Result<&'a [u8], E>;
+    fn read_data(self) -> Result<&'a [u8], Error>;
 }
 
 /// Compares to blobs by comparing their size and oid, and only looks at the file if
@@ -37,13 +50,13 @@ pub struct FastEq;
 impl CompareBlobs for FastEq {
     type Output = ();
 
-    fn compare_blobs<'a, E>(
+    fn compare_blobs<'a, 'b>(
         &mut self,
-        entry: &'a Entry,
+        entry: &Entry,
         worktree_blob_size: usize,
-        worktree_blob: impl ReadDataOnce<'a, E>,
-        _entry_blob: impl ReadDataOnce<'a, E>,
-    ) -> Result<Option<Self::Output>, E> {
+        worktree_blob: impl ReadDataOnce<'a>,
+        _entry_blob: impl ReadDataOnce<'b>,
+    ) -> Result<Option<Self::Output>, Error> {
         // make sure to account for racily smudged entries here so that they don't always keep
         // showing up as modified even after their contents have changed again, to a potentially
         // unmodified state. That means that we want to ignore stat.size == 0 for non_empty_blobs.
@@ -66,13 +79,13 @@ pub struct HashEq;
 impl CompareBlobs for HashEq {
     type Output = ObjectId;
 
-    fn compare_blobs<'a, E>(
+    fn compare_blobs<'a, 'b>(
         &mut self,
-        entry: &'a Entry,
+        entry: &Entry,
         _worktree_blob_size: usize,
-        worktree_blob: impl ReadDataOnce<'a, E>,
-        _entry_blob: impl ReadDataOnce<'a, E>,
-    ) -> Result<Option<Self::Output>, E> {
+        worktree_blob: impl ReadDataOnce<'a>,
+        _entry_blob: impl ReadDataOnce<'b>,
+    ) -> Result<Option<Self::Output>, Error> {
         let blob = worktree_blob.read_data()?;
         let file_hash = gix_object::compute_hash(entry.id.kind(), gix_object::Kind::Blob, blob);
         Ok((entry.id != file_hash).then_some(file_hash))