Use gix_fs::stack::ToNormalPathComponents everywhere.

Author: Byron

Cargo.lock

@@ -8,10 +8,10 @@ pub struct Options {
 }
 
 pub(crate) mod function {
-    use std::{borrow::Cow, io, path::Path};
-
     use anyhow::bail;
     use gix::bstr::BStr;
+    use std::borrow::Cow;
+    use std::{io, path::Path};
 
     use crate::{
         is_dir_to_mode,
@@ -44,7 +44,7 @@ pub(crate) mod function {
                         .ok()
                         .map(|m| is_dir_to_mode(m.is_dir()));
 
-                    let entry = cache.at_entry(path.as_slice(), mode)?;
+                    let entry = cache.at_entry(&path, mode)?;
                     if !entry.matching_attributes(&mut matches) {
                         continue;
                     }

gitoxide-core/src/repository/attributes/query.rs

@@ -48,7 +48,7 @@ pub fn query(
                     .metadata()
                     .ok()
                     .map(|m| is_dir_to_mode(m.is_dir()));
-                let entry = cache.at_entry(path.as_slice(), mode)?;
+                let entry = cache.at_entry(&path, mode)?;
                 let match_ = entry
                     .matching_exclude_pattern()
                     .and_then(|m| (show_ignore_patterns || !m.pattern.is_negative()).then_some(m));

gitoxide-core/src/repository/exclude.rs

@@ -19,6 +19,8 @@ doctest = false
 serde = ["dep:serde"]
 
 [dependencies]
+bstr = "1.5.0"
+gix-path = { version = "^0.10.14", path = "../gix-path" }
 gix-features = { version = "^0.40.0", path = "../gix-features", features = ["fs-read-dir"] }
 gix-utils = { version = "^0.1.14", path = "../gix-utils" }
 thiserror = "2.0.0"

gix-fs/Cargo.toml

@@ -1,8 +1,8 @@
+use crate::Stack;
+use bstr::{BStr, BString, ByteSlice};
 use std::ffi::OsStr;
 use std::path::{Component, Path, PathBuf};
 
-use crate::Stack;
-
 ///
 pub mod to_normal_path_components {
     use std::ffi::OsString;
@@ -13,6 +13,8 @@ pub mod to_normal_path_components {
     pub enum Error {
         #[error("Input path \"{path}\" contains relative or absolute components", path = std::path::Path::new(.0.as_os_str()).display())]
         NotANormalComponent(OsString),
+        #[error("Could not convert to UTF8 or from UTF8 due to ill-formed input")]
+        IllegalUtf8,
     }
 }
 
@@ -33,6 +35,47 @@ impl ToNormalPathComponents for &Path {
     }
 }
 
+impl ToNormalPathComponents for PathBuf {
+    fn to_normal_path_components(&self) -> impl Iterator<Item = Result<&OsStr, to_normal_path_components::Error>> {
+        self.components().map(|component| match component {
+            Component::Normal(os_str) => Ok(os_str),
+            _ => Err(to_normal_path_components::Error::NotANormalComponent(
+                self.as_os_str().to_owned(),
+            )),
+        })
+    }
+}
+
+impl ToNormalPathComponents for &BStr {
+    fn to_normal_path_components(&self) -> impl Iterator<Item = Result<&OsStr, to_normal_path_components::Error>> {
+        self.split(|b| *b == b'/').filter(|c| !c.is_empty()).map(|component| {
+            gix_path::try_from_byte_slice(component.as_bstr())
+                .map_err(|_| to_normal_path_components::Error::IllegalUtf8)
+                .map(Path::as_os_str)
+        })
+    }
+}
+
+impl ToNormalPathComponents for &str {
+    fn to_normal_path_components(&self) -> impl Iterator<Item = Result<&OsStr, to_normal_path_components::Error>> {
+        self.split('/').filter(|c| !c.is_empty()).map(|component| {
+            gix_path::try_from_byte_slice(component.as_bytes())
+                .map_err(|_| to_normal_path_components::Error::IllegalUtf8)
+                .map(Path::as_os_str)
+        })
+    }
+}
+
+impl ToNormalPathComponents for &BString {
+    fn to_normal_path_components(&self) -> impl Iterator<Item = Result<&OsStr, to_normal_path_components::Error>> {
+        self.split(|b| *b == b'/').filter(|c| !c.is_empty()).map(|component| {
+            gix_path::try_from_byte_slice(component.as_bstr())
+                .map_err(|_| to_normal_path_components::Error::IllegalUtf8)
+                .map(Path::as_os_str)
+        })
+    }
+}
+
 /// Access
 impl Stack {
     /// Returns the top-level path of the stack.

gix-fs/src/stack.rs

@@ -182,7 +182,7 @@ fn empty_paths_are_noop_if_no_path_was_pushed_before() {
     let mut s = Stack::new(root.clone());
 
     let mut r = Record::default();
-    s.make_relative_path_current("".as_ref(), &mut r).unwrap();
+    s.make_relative_path_current("", &mut r).unwrap();
     assert_eq!(
         s.current_relative().to_string_lossy(),
         "",
@@ -196,7 +196,7 @@ fn relative_components_are_invalid() {
     let mut s = Stack::new(root.clone());
 
     let mut r = Record::default();
-    let err = s.make_relative_path_current("a/..".as_ref(), &mut r).unwrap_err();
+    let err = s.make_relative_path_current(p("a/.."), &mut r).unwrap_err();
     assert_eq!(
         err.to_string(),
         format!(
@@ -205,7 +205,7 @@ fn relative_components_are_invalid() {
         )
     );
 
-    s.make_relative_path_current("a/./b".as_ref(), &mut r)
+    s.make_relative_path_current(p("a/./b"), &mut r)
         .expect("dot is ignored");
     assert_eq!(
         r,
@@ -221,7 +221,7 @@ fn relative_components_are_invalid() {
         if cfg!(windows) { r".\a\b" } else { "./a/b" },
         "dot is silently ignored"
     );
-    s.make_relative_path_current("a//b/".as_ref(), &mut r)
+    s.make_relative_path_current(p("a//b/"), &mut r)
         .expect("multiple-slashes are ignored");
     assert_eq!(
         r,
@@ -240,19 +240,19 @@ fn absolute_paths_are_invalid() -> crate::Result {
     let mut s = Stack::new(root.clone());
 
     let mut r = Record::default();
-    let err = s.make_relative_path_current("/".as_ref(), &mut r).unwrap_err();
+    let err = s.make_relative_path_current(p("/"), &mut r).unwrap_err();
     assert_eq!(
         err.to_string(),
         r#"Input path "/" contains relative or absolute components"#,
         "a leading slash is always considered absolute"
     );
-    s.make_relative_path_current("a/".as_ref(), &mut r)?;
+    s.make_relative_path_current(p("a/"), &mut r)?;
     assert_eq!(
         s.current(),
         p("./a/"),
         "trailing slashes aren't a problem at this stage, as they cannot cause a 'breakout'"
     );
-    s.make_relative_path_current(r"b\".as_ref(), &mut r)?;
+    s.make_relative_path_current(p(r"b\"), &mut r)?;
     assert_eq!(
         s.current(),
         p(r"./b\"),
@@ -314,10 +314,10 @@ fn delegate_calls_are_consistent() -> crate::Result {
     let mut s = Stack::new(root.clone());
 
     assert_eq!(s.current(), root);
-    assert_eq!(s.current_relative(), Path::new(""));
+    assert_eq!(s.current_relative(), p(""));
 
     let mut r = Record::default();
-    s.make_relative_path_current("a/b".as_ref(), &mut r)?;
+    s.make_relative_path_current("a/b", &mut r)?;
     let mut dirs = vec![root.clone(), root.join("a")];
     assert_eq!(
         r,
@@ -329,7 +329,7 @@ fn delegate_calls_are_consistent() -> crate::Result {
         "it pushes the root-directory first, then the intermediate one"
     );
 
-    s.make_relative_path_current("a/b2".as_ref(), &mut r)?;
+    s.make_relative_path_current("a/b2", &mut r)?;
     assert_eq!(
         r,
         Record {
@@ -340,7 +340,7 @@ fn delegate_calls_are_consistent() -> crate::Result {
         "dirs remain the same as b2 is a leaf/file, hence the new `push`"
     );
 
-    s.make_relative_path_current("c/d/e".as_ref(), &mut r)?;
+    s.make_relative_path_current("c/d/e", &mut r)?;
     dirs.pop();
     dirs.extend([root.join("c"), root.join("c").join("d")]);
     assert_eq!(
@@ -354,7 +354,7 @@ fn delegate_calls_are_consistent() -> crate::Result {
     );
 
     dirs.push(root.join("c").join("d").join("x"));
-    s.make_relative_path_current("c/d/x/z".as_ref(), &mut r)?;
+    s.make_relative_path_current("c/d/x/z", &mut r)?;
     assert_eq!(
         r,
         Record {
@@ -366,8 +366,8 @@ fn delegate_calls_are_consistent() -> crate::Result {
     );
 
     dirs.drain(1..).count();
-    s.make_relative_path_current("f".as_ref(), &mut r)?;
-    assert_eq!(s.current_relative(), Path::new("f"));
+    s.make_relative_path_current("f", &mut r)?;
+    assert_eq!(s.current_relative(), p("f"));
     assert_eq!(
         r,
         Record {
@@ -379,7 +379,7 @@ fn delegate_calls_are_consistent() -> crate::Result {
     );
 
     dirs.push(root.join("x"));
-    s.make_relative_path_current("x/z".as_ref(), &mut r)?;
+    s.make_relative_path_current("x/z", &mut r)?;
     assert_eq!(
         r,
         Record {
@@ -391,7 +391,7 @@ fn delegate_calls_are_consistent() -> crate::Result {
     );
 
     dirs.push(root.join("x").join("z"));
-    s.make_relative_path_current("x/z/a".as_ref(), &mut r)?;
+    s.make_relative_path_current("x/z/a", &mut r)?;
     assert_eq!(
         r,
         Record {
@@ -404,7 +404,7 @@ fn delegate_calls_are_consistent() -> crate::Result {
 
     dirs.push(root.join("x").join("z").join("a"));
     dirs.push(root.join("x").join("z").join("a").join("b"));
-    s.make_relative_path_current("x/z/a/b/c".as_ref(), &mut r)?;
+    s.make_relative_path_current("x/z/a/b/c", &mut r)?;
     assert_eq!(
         r,
         Record {
@@ -416,7 +416,7 @@ fn delegate_calls_are_consistent() -> crate::Result {
     );
 
     dirs.drain(1 /*root*/ + 1 /*x*/ + 1 /*x/z*/ ..).count();
-    s.make_relative_path_current("x/z".as_ref(), &mut r)?;
+    s.make_relative_path_current("x/z", &mut r)?;
     assert_eq!(
         r,
         Record {
@@ -432,15 +432,15 @@ fn delegate_calls_are_consistent() -> crate::Result {
         "the stack is state so keeps thinking it's a directory which is consistent. Git does it differently though."
     );
 
-    let err = s.make_relative_path_current("".as_ref(), &mut r).unwrap_err();
+    let err = s.make_relative_path_current(p(""), &mut r).unwrap_err();
     assert_eq!(
         err.to_string(),
         "empty inputs are not allowed",
         "this is to protect us from double-counting the root path next time a component is pushed, \
         and besides that really shouldn't happen"
     );
 
-    s.make_relative_path_current("leaf".as_ref(), &mut r)?;
+    s.make_relative_path_current("leaf", &mut r)?;
     dirs.drain(1..).count();
     assert_eq!(
         r,
@@ -452,7 +452,7 @@ fn delegate_calls_are_consistent() -> crate::Result {
         "reset as much as possible, with just a leaf-component and the root directory"
     );
 
-    s.make_relative_path_current("a//b".as_ref(), &mut r)?;
+    s.make_relative_path_current(p("a//b"), &mut r)?;
     dirs.push(root.join("a"));
     assert_eq!(
         r,
@@ -466,7 +466,7 @@ fn delegate_calls_are_consistent() -> crate::Result {
 
     #[cfg(not(windows))]
     {
-        s.make_relative_path_current(r"\/b".as_ref(), &mut r)?;
+        s.make_relative_path_current(r"\/b", &mut r)?;
         dirs.pop();
         dirs.push(root.join(r"\"));
         assert_eq!(
@@ -479,7 +479,7 @@ fn delegate_calls_are_consistent() -> crate::Result {
             "a backslash is a normal character outside of Windows, so it's fine to have it as component"
         );
 
-        s.make_relative_path_current(r"\".as_ref(), &mut r)?;
+        s.make_relative_path_current(r"\", &mut r)?;
         assert_eq!(
             r,
             Record {
@@ -494,7 +494,7 @@ fn delegate_calls_are_consistent() -> crate::Result {
             r"a backslash can also be a valid leaf component - here we only popped the 'b', leaving the \ 'directory'"
         );
 
-        s.make_relative_path_current(r"\\".as_ref(), &mut r)?;
+        s.make_relative_path_current(r"\\", &mut r)?;
         dirs.pop();
         assert_eq!(
             r,

gix-fs/tests/fs/stack.rs

@@ -1,10 +1,10 @@
+use crate::SymlinkCheck;
 use bstr::BStr;
+use gix_fs::stack::ToNormalPathComponents;
 use gix_fs::Stack;
 use std::borrow::Cow;
 use std::path::{Path, PathBuf};
 
-use crate::SymlinkCheck;
-
 impl SymlinkCheck {
     /// Create a new stack that starts operating at `root`.
     pub fn new(root: PathBuf) -> Self {
@@ -24,7 +24,7 @@ impl SymlinkCheck {
     /// ### Note
     ///
     /// On windows, no verification is performed, instead only the combined path is provided as usual.
-    pub fn verified_path(&mut self, relative_path: &Path) -> std::io::Result<&Path> {
+    pub fn verified_path(&mut self, relative_path: impl ToNormalPathComponents) -> std::io::Result<&Path> {
         self.inner.make_relative_path_current(relative_path, &mut Delegate)?;
         Ok(self.inner.current())
     }
@@ -34,7 +34,7 @@ impl SymlinkCheck {
     /// For convenience, this incarnation is tuned to be easy to use with Git paths, i.e. slash-separated `BString` path.
     pub fn verified_path_allow_nonexisting(&mut self, relative_path: &BStr) -> std::io::Result<Cow<'_, Path>> {
         let rela_path = gix_path::try_from_bstr(relative_path).map_err(std::io::Error::other)?;
-        if let Err(err) = self.verified_path(&rela_path) {
+        if let Err(err) = self.verified_path(rela_path.as_ref()) {
             if err.kind() == std::io::ErrorKind::NotFound {
                 Ok(Cow::Owned(self.inner.root().join(rela_path)))
             } else {