Merge pull request #1730 from GitoxideLabs/fix-1729

Byron · web-flow · commit 6822689fca04 · 2024-12-20T13:19:49.000+01:00
fix!: consider non-files as pruned officially.
diff --git a/gitoxide-core/src/repository/clean.rs b/gitoxide-core/src/repository/clean.rs
@@ -161,11 +161,16 @@ pub(crate) mod function {
             if entry.disk_kind.is_none() {
                 entry.disk_kind = workdir
                     .join(gix::path::from_bstr(entry.rela_path.as_bstr()))
-                    .metadata()
+                    .symlink_metadata()
                     .ok()
-                    .and_then(|e| gix::dir::entry::Kind::try_from_file_type(e.file_type()));
+                    .map(|e| e.file_type().into());
             }
-            let mut disk_kind = entry.disk_kind.expect("present if not pruned");
+            let Some(mut disk_kind) = entry.disk_kind else {
+                if debug {
+                    writeln!(err, "DBG: ignoring unreadable entry at '{}' ", entry.rela_path).ok();
+                }
+                continue;
+            };
             if !keep {
                 if debug {
                     writeln!(err, "DBG: prune '{}' as -x or -p is missing", entry.rela_path).ok();
@@ -183,6 +188,12 @@ pub(crate) mod function {
             }
 
             match disk_kind {
+                Kind::NonFile => {
+                    if debug {
+                        writeln!(err, "DBG: skipped non-file at '{}'", entry.rela_path).ok();
+                    }
+                    continue;
+                }
                 Kind::File | Kind::Symlink => {}
                 Kind::Directory => {
                     if !directories {
@@ -254,6 +265,7 @@ pub(crate) mod function {
                     "WOULD remove"
                 },
                 suffix = match disk_kind {
+                    Kind::NonFile => unreachable!("always skipped earlier"),
                     Kind::Directory if entry.property == Some(gix::dir::entry::Property::EmptyDirectory) => {
                         " empty"
                     }
diff --git a/gix-dir/src/entry.rs b/gix-dir/src/entry.rs
@@ -1,6 +1,7 @@
 use crate::walk::ForDeletionMode;
 use crate::{Entry, EntryRef};
 use std::borrow::Cow;
+use std::fs::FileType;
 
 /// A way of attaching additional information to an [Entry] .
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Ord, PartialOrd)]
@@ -25,6 +26,10 @@ pub enum Property {
 /// The kind of the entry, seated in their kinds available on disk.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Ord, PartialOrd)]
 pub enum Kind {
+    /// Something that is not a file, like a named pipe or character device.
+    ///
+    /// These can only exist in the filesystem.
+    NonFile,
     /// The entry is a blob, executable or not.
     File,
     /// The entry is a symlink.
@@ -147,20 +152,17 @@ impl Entry {
     }
 }
 
-impl Kind {
-    /// Try to represent the file type `t` as `Entry`, or return `None` if it cannot be represented.
-    ///
-    /// The latter can happen if it's a `pipe` for instance.
-    pub fn try_from_file_type(t: std::fs::FileType) -> Option<Self> {
-        Some(if t.is_dir() {
+impl From<std::fs::FileType> for Kind {
+    fn from(value: FileType) -> Self {
+        if value.is_dir() {
             Kind::Directory
-        } else if t.is_symlink() {
+        } else if value.is_symlink() {
             Kind::Symlink
-        } else if t.is_file() {
+        } else if value.is_file() {
             Kind::File
         } else {
-            return None;
-        })
+            Kind::NonFile
+        }
     }
 }
 
diff --git a/gix-dir/src/walk/classify.rs b/gix-dir/src/walk/classify.rs
@@ -20,10 +20,7 @@ pub fn root(
     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 file_kind = path_buf
-        .symlink_metadata()
-        .ok()
-        .and_then(|m| entry::Kind::try_from_file_type(m.file_type()));
+    let file_kind = path_buf.symlink_metadata().ok().map(|m| m.file_type().into());
     let mut out = path(&mut path_buf, buf, 0, file_kind, || None, options, ctx)?;
     let worktree_root_is_repository = out
         .disk_kind
@@ -35,10 +32,7 @@ pub fn root(
         }
         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()
-            .ok()
-            .and_then(|m| entry::Kind::try_from_file_type(m.file_type()));
+        let file_kind = path_buf.symlink_metadata().ok().map(|m| m.file_type().into());
 
         out = path(
             &mut path_buf,
diff --git a/gix-dir/src/walk/readdir.rs b/gix-dir/src/walk/readdir.rs
@@ -64,7 +64,7 @@ pub(super) fn recursive(
             current_bstr,
             if prev_len == 0 { 0 } else { prev_len + 1 },
             None,
-            || entry.file_type().ok().and_then(entry::Kind::try_from_file_type),
+            || entry.file_type().ok().map(Into::into),
             opts,
             ctx,
         )?;
diff --git a/gix-dir/tests/dir/walk.rs b/gix-dir/tests/dir/walk.rs
@@ -66,7 +66,11 @@ fn one_top_level_fifo() {
         }
     );
 
-    assert_eq!(entries, &[], "Non-files are simply pruned by default");
+    assert_eq!(
+        entries,
+        &[entry("top", Untracked, NonFile),],
+        "Non-files are like normal files, but with a different state"
+    );
 }
 
 #[test]
@@ -99,10 +103,11 @@ fn fifo_in_traversal() {
         &[
             entry_nokind(".git", Pruned).with_property(DotGit).with_match(Always),
             entry("dir-with-file/nested-file", Untracked, File),
+            entry("dir/nested", Untracked, NonFile),
             entry("file", Untracked, File),
+            entry("top", Untracked, NonFile),
         ],
-        "Non-files are not even pruned, they are ignored entirely.\
-        If one day this isn't what we want, we can create an own filetype for them"
+        "Non-files only differ by their disk-kind"
     );
 }
 
diff --git a/gix-status/src/index_as_worktree/types.rs b/gix-status/src/index_as_worktree/types.rs
@@ -103,7 +103,12 @@ impl Outcome {
 pub enum Change<T = (), U = ()> {
     /// This corresponding file does not exist in the worktree anymore.
     Removed,
-    /// The type of file changed compared to the worktree, i.e. a symlink s now a file.
+    /// The type of file changed compared to the worktree, i.e. a symlink is now a file, or a file was replaced with a named pipe.
+    ///
+    /// ### Deviation
+    ///
+    /// A change to a non-file is marked as `modification` in Git, but that's related to the content which we can't evaluate.
+    /// Hence, a type-change is considered more appropriate.
     Type,
     /// This worktree file was modified in some form, like a permission change or content change or both,
     /// as compared to this entry.
diff --git a/gix-status/src/index_as_worktree_with_renames/mod.rs b/gix-status/src/index_as_worktree_with_renames/mod.rs
@@ -158,7 +158,7 @@ pub(super) mod function {
 
             let tracker = options
                 .rewrites
-                .map(gix_diff::rewrites::Tracker::<rewrite::ModificationOrDirwalkEntry<'index, T, U>>::new)
+                .map(gix_diff::rewrites::Tracker::<ModificationOrDirwalkEntry<'index, T, U>>::new)
                 .zip(filter);
             let rewrite_outcome = match tracker {
                 Some((mut tracker, (mut filter, mut attrs))) => {
@@ -168,12 +168,12 @@ pub(super) mod function {
                         let (change, location) = match event {
                             Event::IndexEntry(record) => {
                                 let location = Cow::Borrowed(record.relative_path);
-                                (rewrite::ModificationOrDirwalkEntry::Modification(record), location)
+                                (ModificationOrDirwalkEntry::Modification(record), location)
                             }
                             Event::DirEntry(entry, collapsed_directory_status) => {
                                 let location = Cow::Owned(entry.rela_path.clone());
                                 (
-                                    rewrite::ModificationOrDirwalkEntry::DirwalkEntry {
+                                    ModificationOrDirwalkEntry::DirwalkEntry {
                                         id: rewrite::calculate_worktree_id(
                                             options.object_hash,
                                             worktree,
@@ -222,7 +222,7 @@ pub(super) mod function {
                                     }
                                 }
                                 Some(src) => {
-                                    let rewrite::ModificationOrDirwalkEntry::DirwalkEntry {
+                                    let ModificationOrDirwalkEntry::DirwalkEntry {
                                         id,
                                         entry,
                                         collapsed_directory_status,
@@ -387,8 +387,11 @@ pub(super) mod function {
 
         impl<T, U> gix_dir::walk::Delegate for Delegate<'_, '_, T, U> {
             fn emit(&mut self, entry: EntryRef<'_>, collapsed_directory_status: Option<Status>) -> Action {
-                let entry = entry.to_owned();
-                self.tx.send(Event::DirEntry(entry, collapsed_directory_status)).ok();
+                // Status never shows untracked non-files
+                if entry.disk_kind != Some(gix_dir::entry::Kind::NonFile) {
+                    let entry = entry.to_owned();
+                    self.tx.send(Event::DirEntry(entry, collapsed_directory_status)).ok();
+                }
 
                 if self.should_interrupt.load(Ordering::Relaxed) {
                     Action::Cancel
@@ -466,6 +469,10 @@ pub(super) mod function {
                     ModificationOrDirwalkEntry::Modification(c) => c.entry.mode.to_tree_entry_mode(),
                     ModificationOrDirwalkEntry::DirwalkEntry { entry, .. } => entry.disk_kind.map(|kind| {
                         match kind {
+                            Kind::NonFile => {
+                                // Trees are never tracked for rewrites, so we 'pretend'.
+                                gix_object::tree::EntryKind::Tree
+                            }
                             Kind::File => gix_object::tree::EntryKind::Blob,
                             Kind::Symlink => gix_object::tree::EntryKind::Link,
                             Kind::Repository | Kind::Directory => gix_object::tree::EntryKind::Tree,
@@ -500,6 +507,10 @@ pub(super) mod function {
             };
 
             Ok(match kind {
+                Kind::NonFile => {
+                    // Go along with unreadable files, they are passed along without rename tracking.
+                    return Ok(object_hash.null());
+                }
                 Kind::File => {
                     let platform = attrs
                         .at_entry(rela_path, None, objects)
diff --git a/gix-status/tests/Cargo.toml b/gix-status/tests/Cargo.toml
@@ -12,8 +12,8 @@ publish = false
 rust-version = "1.65"
 
 [[test]]
-name = "worktree"
-path = "worktree.rs"
+name = "status"
+path = "status/mod.rs"
 
 [features]
 gix-features-parallel = ["gix-features/parallel"]
diff --git a/gix-status/tests/fixtures/generated-archives/.gitignore b/gix-status/tests/fixtures/generated-archives/.gitignore
@@ -1,3 +1,4 @@
 status_unchanged.tar
 status_changed.tar
 symlink_stack.tar
+status_nonfile.tar
diff --git a/gix-status/tests/fixtures/status_nonfile.sh b/gix-status/tests/fixtures/status_nonfile.sh
@@ -0,0 +1,19 @@
+#!/usr/bin/env bash
+set -eu -o pipefail
+
+git init -q untracked
+(cd untracked
+  touch file && git add file && git commit -m "just to get an index for the test-suite"
+
+  mkfifo pipe
+  git status
+)
+
+git init -q tracked-swapped
+(cd tracked-swapped
+  touch file && git add file && git commit -m "it starts out as trackable file"
+
+  rm file && mkfifo file
+  git status
+)
+
diff --git a/gix-status/tests/status/index_as_worktree.rs b/gix-status/tests/status/index_as_worktree.rs
@@ -37,6 +37,10 @@ fn fixture(name: &str, expected_status: &[Expectation<'_>]) -> Outcome {
     fixture_filtered(name, &[], expected_status)
 }
 
+fn nonfile_fixture(name: &str, expected_status: &[Expectation<'_>]) -> Outcome {
+    fixture_filtered_detailed("status_nonfile", name, &[], expected_status, |_| {}, false)
+}
+
 fn fixture_with_index(
     name: &str,
     prepare_index: impl FnMut(&mut gix_index::State),
@@ -185,6 +189,19 @@ fn status_removed() -> EntryStatus {
     Change::Removed.into()
 }
 
+#[test]
+#[cfg(unix)]
+fn nonfile_untracked_are_not_visible() {
+    // And generally, untracked aren't visible here.
+    nonfile_fixture("untracked", &[]);
+}
+
+#[test]
+#[cfg(unix)]
+fn tracked_changed_to_non_file() {
+    nonfile_fixture("tracked-swapped", &[(BStr::new(b"file"), 0, Change::Type.into())]);
+}
+
 #[test]
 fn removed() {
     let out = fixture(
diff --git a/gix-status/tests/status/index_as_worktree_with_renames.rs b/gix-status/tests/status/index_as_worktree_with_renames.rs
diff --git a/gix-status/tests/status/mod.rs b/gix-status/tests/status/mod.rs
diff --git a/gix-status/tests/status/stack.rs b/gix-status/tests/status/stack.rs
diff --git a/gix-status/tests/worktree.rs b/gix-status/tests/worktree.rs
