diff --git a/gix-negotiate/tests/baseline/mod.rs b/gix-negotiate/tests/baseline/mod.rs index aefbd313f8b..84f71f04801 100644 --- a/gix-negotiate/tests/baseline/mod.rs +++ b/gix-negotiate/tests/baseline/mod.rs @@ -71,7 +71,7 @@ fn run() -> crate::Result { // } for tip in lookup_names(&["HEAD"]).into_iter().chain( refs.iter()? - .prefixed("refs/heads".as_ref())? + .prefixed(b"refs/heads".as_bstr())? .filter_map(Result::ok) .map(|r| r.target.into_id()), ) { diff --git a/gix-ref/src/namespace.rs b/gix-ref/src/namespace.rs index 1483b1c47e8..cba0b460c73 100644 --- a/gix-ref/src/namespace.rs +++ b/gix-ref/src/namespace.rs @@ -1,4 +1,4 @@ -use std::path::{Path, PathBuf}; +use std::{borrow::Cow, path::Path}; use gix_object::bstr::{BStr, BString, ByteSlice, ByteVec}; @@ -18,10 +18,9 @@ impl Namespace { gix_path::from_byte_slice(&self.0) } /// Append the given `prefix` to this namespace so it becomes usable for prefixed iteration. - pub fn into_namespaced_prefix(mut self, prefix: &Path) -> PathBuf { - let prefix = gix_path::into_bstr(prefix); - self.0.push_str(prefix.as_ref()); - gix_path::to_native_path_on_windows(self.0).into_owned() + pub fn into_namespaced_prefix(mut self, prefix: &BStr) -> Cow<'_, BStr> { + self.0.push_str(prefix); + gix_path::to_unix_separators_on_windows(self.0) } pub(crate) fn into_namespaced_name(mut self, name: &FullNameRef) -> FullName { self.0.push_str(name.as_bstr()); diff --git a/gix-ref/src/store/file/loose/iter.rs b/gix-ref/src/store/file/loose/iter.rs index 0bfbbd91587..99ebc2d7f4d 100644 --- a/gix-ref/src/store/file/loose/iter.rs +++ b/gix-ref/src/store/file/loose/iter.rs @@ -1,22 +1,26 @@ -use std::path::{Path, PathBuf}; +use std::{ + borrow::Cow, + path::{Path, PathBuf}, +}; use gix_features::fs::walkdir::DirEntryIter; use gix_object::bstr::ByteSlice; -use crate::{file::iter::LooseThenPacked, store_impl::file, BString, FullName}; +use crate::{file::iter::LooseThenPacked, store_impl::file, BStr, BString, FullName}; /// An iterator over all valid loose reference paths as seen from a particular base directory. pub(in crate::store_impl::file) struct SortedLoosePaths { pub(crate) base: PathBuf, - filename_prefix: Option, + /// A optional prefix to match against if the prefix is not the same as the `file_walk` path. + prefix: Option, file_walk: Option, } impl SortedLoosePaths { - pub fn at(path: &Path, base: PathBuf, filename_prefix: Option, precompose_unicode: bool) -> Self { + pub fn at(path: &Path, base: PathBuf, prefix: Option, precompose_unicode: bool) -> Self { SortedLoosePaths { base, - filename_prefix, + prefix, file_walk: path.is_dir().then(|| { // serial iteration as we expect most refs in packed-refs anyway. gix_features::fs::walkdir_sorted_new( @@ -41,23 +45,9 @@ impl Iterator for SortedLoosePaths { continue; } let full_path = entry.path().into_owned(); - if let Some((prefix, name)) = self - .filename_prefix - .as_deref() - .and_then(|prefix| full_path.file_name().map(|name| (prefix, name))) - { - match gix_path::os_str_into_bstr(name) { - Ok(name) => { - if !name.starts_with(prefix) { - continue; - } - } - Err(_) => continue, // TODO: silently skipping ill-formed UTF-8 on windows - maybe this can be better? - } - } let full_name = full_path .strip_prefix(&self.base) - .expect("prefix-stripping cannot fail as prefix is our root"); + .expect("prefix-stripping cannot fail as base is within our root"); let full_name = match gix_path::try_into_bstr(full_name) { Ok(name) => { let name = gix_path::to_unix_separators_on_windows(name); @@ -65,7 +55,11 @@ impl Iterator for SortedLoosePaths { } Err(_) => continue, // TODO: silently skipping ill-formed UTF-8 on windows here, maybe there are better ways? }; - + if let Some(prefix) = &self.prefix { + if !full_name.starts_with(prefix) { + continue; + } + } if gix_validate::reference::name_partial(full_name.as_bstr()).is_ok() { let name = FullName(full_name); return Some(Ok((full_path, name))); @@ -93,7 +87,10 @@ impl file::Store { /// Return an iterator over all loose references that start with the given `prefix`. /// /// Otherwise it's similar to [`loose_iter()`][file::Store::loose_iter()]. - pub fn loose_iter_prefixed(&self, prefix: &Path) -> std::io::Result> { - self.iter_prefixed_packed(prefix, None) + pub fn loose_iter_prefixed<'a>( + &self, + prefix: impl Into>, + ) -> std::io::Result> { + self.iter_prefixed_packed(prefix.into(), None) } } diff --git a/gix-ref/src/store/file/mod.rs b/gix-ref/src/store/file/mod.rs index 1f8321f4354..3c7d20e8c94 100644 --- a/gix-ref/src/store/file/mod.rs +++ b/gix-ref/src/store/file/mod.rs @@ -1,9 +1,6 @@ -use std::{ - borrow::Cow, - path::{Path, PathBuf}, -}; +use std::path::PathBuf; -use crate::{bstr::BStr, store::WriteReflog, Namespace}; +use crate::{store::WriteReflog, Namespace}; /// A store for reference which uses plain files. /// @@ -92,11 +89,6 @@ pub struct Transaction<'s, 'p> { packed_refs: transaction::PackedRefs<'p>, } -pub(in crate::store_impl::file) fn path_to_name<'a>(path: impl Into>) -> Cow<'a, BStr> { - let path = gix_path::into_bstr(path.into()); - gix_path::to_unix_separators_on_windows(path) -} - /// pub mod loose; mod overlay_iter; diff --git a/gix-ref/src/store/file/overlay_iter.rs b/gix-ref/src/store/file/overlay_iter.rs index 0e39e5af05f..0920b7bff95 100644 --- a/gix-ref/src/store/file/overlay_iter.rs +++ b/gix-ref/src/store/file/overlay_iter.rs @@ -7,9 +7,9 @@ use std::{ }; use crate::{ - file::{loose, loose::iter::SortedLoosePaths, path_to_name}, + file::{loose, loose::iter::SortedLoosePaths}, store_impl::{file, packed}, - BString, FullName, Namespace, Reference, + BStr, FullName, Namespace, Reference, }; /// An iterator stepping through sorted input of loose references and packed references, preferring loose refs over otherwise @@ -195,12 +195,11 @@ impl Platform<'_> { self.store.iter_packed(self.packed.as_ref().map(|b| &***b)) } - /// As [`iter(…)`][file::Store::iter()], but filters by `prefix`, i.e. "refs/heads". - /// - /// Please note that "refs/heads" or "refs\\heads" is equivalent to "refs/heads/" - pub fn prefixed(&self, prefix: &Path) -> std::io::Result> { + /// As [`iter(…)`][file::Store::iter()], but filters by `prefix`, i.e. "refs/heads/" or + /// "refs/heads/feature-". + pub fn prefixed<'a>(&self, prefix: impl Into>) -> std::io::Result> { self.store - .iter_prefixed_packed(prefix, self.packed.as_ref().map(|b| &***b)) + .iter_prefixed_packed(prefix.into(), self.packed.as_ref().map(|b| &***b)) } } @@ -242,22 +241,19 @@ pub(crate) enum IterInfo<'a> { /// The top-level directory as boundary of all references, used to create their short-names after iteration base: &'a Path, /// The original prefix - prefix: Cow<'a, Path>, - /// The remainder of the prefix that wasn't a valid path - remainder: Option, + prefix: Cow<'a, BStr>, /// If `true`, we will convert decomposed into precomposed unicode. precompose_unicode: bool, }, } impl<'a> IterInfo<'a> { - fn prefix(&self) -> Option<&Path> { + fn prefix(&self) -> Option> { match self { IterInfo::Base { .. } => None, - IterInfo::PrefixAndBase { prefix, .. } => Some(*prefix), - IterInfo::ComputedIterationRoot { prefix, .. } | IterInfo::BaseAndIterRoot { prefix, .. } => { - prefix.as_ref().into() - } + IterInfo::PrefixAndBase { prefix, .. } => Some(gix_path::into_bstr(*prefix)), + IterInfo::BaseAndIterRoot { prefix, .. } => Some(gix_path::into_bstr(prefix.clone())), + IterInfo::ComputedIterationRoot { prefix, .. } => Some(prefix.clone()), } } @@ -281,48 +277,42 @@ impl<'a> IterInfo<'a> { IterInfo::ComputedIterationRoot { iter_root, base, - prefix: _, - remainder, + prefix, precompose_unicode, - } => SortedLoosePaths::at(&iter_root, base.into(), remainder, precompose_unicode), + } => SortedLoosePaths::at(&iter_root, base.into(), Some(prefix.into_owned()), precompose_unicode), } .peekable() } - fn from_prefix(base: &'a Path, prefix: Cow<'a, Path>, precompose_unicode: bool) -> std::io::Result { - if prefix.is_absolute() { + fn from_prefix( + base: &'a Path, + prefix: impl Into>, + precompose_unicode: bool, + ) -> std::io::Result { + let prefix = prefix.into(); + let prefix_path = gix_path::from_bstring(prefix.clone().into_owned()); + if prefix_path.is_absolute() { return Err(std::io::Error::new( std::io::ErrorKind::InvalidInput, - "prefix must be a relative path, like 'refs/heads'", + "prefix must be a relative path, like 'refs/heads/'", )); } use std::path::Component::*; - if prefix.components().any(|c| matches!(c, CurDir | ParentDir)) { + if prefix_path.components().any(|c| matches!(c, CurDir | ParentDir)) { return Err(std::io::Error::new( std::io::ErrorKind::InvalidInput, "Refusing to handle prefixes with relative path components", )); } - let iter_root = base.join(prefix.as_ref()); - if iter_root.is_dir() { + let iter_root = base.join(&prefix_path); + if prefix.ends_with(b"/") { Ok(IterInfo::BaseAndIterRoot { base, iter_root, - prefix, + prefix: prefix_path.into(), precompose_unicode, }) } else { - let filename_prefix = iter_root - .file_name() - .map(ToOwned::to_owned) - .map(|p| { - gix_path::try_into_bstr(PathBuf::from(p)) - .map(std::borrow::Cow::into_owned) - .map_err(|_| { - std::io::Error::new(std::io::ErrorKind::InvalidInput, "prefix contains ill-formed UTF-8") - }) - }) - .transpose()?; let iter_root = iter_root .parent() .expect("a parent is always there unless empty") @@ -331,7 +321,6 @@ impl<'a> IterInfo<'a> { base, prefix, iter_root, - remainder: filename_prefix, precompose_unicode, }) } @@ -374,30 +363,28 @@ impl file::Store { } } - /// As [`iter(…)`][file::Store::iter()], but filters by `prefix`, i.e. "refs/heads". - /// - /// Please note that "refs/heads" or "refs\\heads" is equivalent to "refs/heads/" + /// As [`iter(…)`][file::Store::iter()], but filters by `prefix`, i.e. "refs/heads/" or + /// "refs/heads/feature-". pub fn iter_prefixed_packed<'s, 'p>( &'s self, - prefix: &Path, + prefix: Cow<'_, BStr>, packed: Option<&'p packed::Buffer>, ) -> std::io::Result> { match self.namespace.as_ref() { None => { - let git_dir_info = IterInfo::from_prefix(self.git_dir(), prefix.into(), self.precompose_unicode)?; + let git_dir_info = IterInfo::from_prefix(self.git_dir(), prefix.clone(), self.precompose_unicode)?; let common_dir_info = self .common_dir() - .map(|base| IterInfo::from_prefix(base, prefix.into(), self.precompose_unicode)) + .map(|base| IterInfo::from_prefix(base, prefix, self.precompose_unicode)) .transpose()?; self.iter_from_info(git_dir_info, common_dir_info, packed) } Some(namespace) => { - let prefix = namespace.to_owned().into_namespaced_prefix(prefix); - let git_dir_info = - IterInfo::from_prefix(self.git_dir(), prefix.clone().into(), self.precompose_unicode)?; + let prefix = namespace.to_owned().into_namespaced_prefix(&prefix); + let git_dir_info = IterInfo::from_prefix(self.git_dir(), prefix.clone(), self.precompose_unicode)?; let common_dir_info = self .common_dir() - .map(|base| IterInfo::from_prefix(base, prefix.into(), self.precompose_unicode)) + .map(|base| IterInfo::from_prefix(base, prefix, self.precompose_unicode)) .transpose()?; self.iter_from_info(git_dir_info, common_dir_info, packed) } @@ -416,7 +403,7 @@ impl file::Store { iter_packed: match packed { Some(packed) => Some( match git_dir_info.prefix() { - Some(prefix) => packed.iter_prefixed(path_to_name(prefix).into_owned()), + Some(prefix) => packed.iter_prefixed(prefix.into_owned()), None => packed.iter(), } .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))? diff --git a/gix-ref/tests/fixtures/generated-archives/make_packed_ref_repository.tar b/gix-ref/tests/fixtures/generated-archives/make_packed_ref_repository.tar index 87d999da6ba..4f956f6262c 100644 Binary files a/gix-ref/tests/fixtures/generated-archives/make_packed_ref_repository.tar and b/gix-ref/tests/fixtures/generated-archives/make_packed_ref_repository.tar differ diff --git a/gix-ref/tests/fixtures/generated-archives/make_packed_ref_repository_for_overlay.tar b/gix-ref/tests/fixtures/generated-archives/make_packed_ref_repository_for_overlay.tar index b6b126990a7..b588e583f6d 100644 Binary files a/gix-ref/tests/fixtures/generated-archives/make_packed_ref_repository_for_overlay.tar and b/gix-ref/tests/fixtures/generated-archives/make_packed_ref_repository_for_overlay.tar differ diff --git a/gix-ref/tests/fixtures/generated-archives/make_ref_repository.tar b/gix-ref/tests/fixtures/generated-archives/make_ref_repository.tar index d239944d93b..76d6c84863f 100644 Binary files a/gix-ref/tests/fixtures/generated-archives/make_ref_repository.tar and b/gix-ref/tests/fixtures/generated-archives/make_ref_repository.tar differ diff --git a/gix-ref/tests/fixtures/make_packed_ref_repository.sh b/gix-ref/tests/fixtures/make_packed_ref_repository.sh index 3a81139d5aa..5c2bf2b8258 100755 --- a/gix-ref/tests/fixtures/make_packed_ref_repository.sh +++ b/gix-ref/tests/fixtures/make_packed_ref_repository.sh @@ -10,9 +10,12 @@ git branch d1 git branch A mkdir -p .git/refs/remotes/origin +mkdir -p .git/refs/prefix/feature/sub/dir cp .git/refs/heads/main .git/refs/remotes/origin/ cp .git/refs/heads/main .git/refs/d1 +cp .git/refs/heads/main .git/refs/prefix/feature-suffix +cp .git/refs/heads/main .git/refs/prefix/feature/sub/dir/algo echo "ref: refs/remotes/origin/main" > .git/refs/remotes/origin/HEAD echo "notahexsha" > .git/refs/broken diff --git a/gix-ref/tests/fixtures/make_packed_ref_repository_for_overlay.sh b/gix-ref/tests/fixtures/make_packed_ref_repository_for_overlay.sh index 03574b865ae..5c231bae009 100755 --- a/gix-ref/tests/fixtures/make_packed_ref_repository_for_overlay.sh +++ b/gix-ref/tests/fixtures/make_packed_ref_repository_for_overlay.sh @@ -10,8 +10,11 @@ git branch A git tag -m "tag object" tag-object mkdir -p .git/refs/remotes/origin +mkdir -p .git/refs/prefix/feature/sub/dir cp .git/refs/heads/main .git/refs/remotes/origin/ +cp .git/refs/heads/main .git/refs/prefix/feature-suffix +cp .git/refs/heads/main .git/refs/prefix/feature/sub/dir/algo echo "ref: refs/remotes/origin/main" > .git/refs/remotes/origin/HEAD diff --git a/gix-ref/tests/fixtures/make_ref_repository.sh b/gix-ref/tests/fixtures/make_ref_repository.sh index 38080cddfca..d89948193fa 100755 --- a/gix-ref/tests/fixtures/make_ref_repository.sh +++ b/gix-ref/tests/fixtures/make_ref_repository.sh @@ -10,9 +10,12 @@ git branch d1 git branch A mkdir -p .git/refs/remotes/origin +mkdir -p .git/refs/prefix/feature/sub/dir cp .git/refs/heads/main .git/refs/remotes/origin/ cp .git/refs/heads/main .git/refs/d1 +cp .git/refs/heads/main .git/refs/prefix/feature-suffix +cp .git/refs/heads/main .git/refs/prefix/feature/sub/dir/algo echo "ref: refs/remotes/origin/main" > .git/refs/remotes/origin/HEAD echo "notahexsha" > .git/refs/broken @@ -22,7 +25,6 @@ echo "ref: refs/tags/multi-link-target2" > .git/refs/heads/multi-link-target1 echo "ref: refs/remotes/origin/multi-link-target3" > .git/refs/tags/multi-link-target2 git rev-parse HEAD > .git/refs/remotes/origin/multi-link-target3 - echo "ref: refs/loop-b" > .git/refs/loop-a echo "ref: refs/loop-a" > .git/refs/loop-b diff --git a/gix-ref/tests/refs/file/store/iter.rs b/gix-ref/tests/refs/file/store/iter.rs index 211332cddb7..2b1e24fd7b7 100644 --- a/gix-ref/tests/refs/file/store/iter.rs +++ b/gix-ref/tests/refs/file/store/iter.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use gix_object::bstr::ByteSlice; use crate::{ @@ -26,7 +28,7 @@ mod with_namespace { let ns_two = gix_ref::namespace::expand("bar")?; let namespaced_refs = store .iter()? - .prefixed(ns_two.to_path())? + .prefixed(ns_two.as_bstr())? .map(Result::unwrap) .map(|r: gix_ref::Reference| r.name) .collect::>(); @@ -45,7 +47,7 @@ mod with_namespace { ); assert_eq!( store - .loose_iter_prefixed(ns_two.to_path())? + .loose_iter_prefixed(ns_two.as_bstr())? .map(Result::unwrap) .map(|r| r.name.into_inner()) .collect::>(), @@ -59,7 +61,7 @@ mod with_namespace { packed .as_ref() .expect("present") - .iter_prefixed(ns_two.as_bstr().into())? + .iter_prefixed(ns_two.as_bstr().to_owned())? .map(Result::unwrap) .map(|r| r.name.to_owned().into_inner()) .collect::>(), @@ -90,7 +92,7 @@ mod with_namespace { assert_eq!( store .iter()? - .prefixed(ns_one.to_path())? + .prefixed(ns_one.as_bstr())? .map(Result::unwrap) .map(|r: gix_ref::Reference| ( r.name.as_bstr().to_owned(), @@ -253,7 +255,7 @@ fn no_packed_available_thus_no_iteration_possible() -> crate::Result { #[test] fn packed_file_iter() -> crate::Result { let store = store_with_packed_refs()?; - assert_eq!(store.open_packed_buffer()?.expect("pack available").iter()?.count(), 9); + assert_eq!(store.open_packed_buffer()?.expect("pack available").iter()?.count(), 11); Ok(()) } @@ -262,7 +264,7 @@ fn loose_iter_with_broken_refs() -> crate::Result { let store = store()?; let mut actual: Vec<_> = store.loose_iter()?.collect(); - assert_eq!(actual.len(), 16); + assert_eq!(actual.len(), 18); actual.sort_by_key(Result::is_err); let first_error = actual .iter() @@ -271,7 +273,7 @@ fn loose_iter_with_broken_refs() -> crate::Result { .expect("there is an error"); assert_eq!( - first_error, 15, + first_error, 17, "there is exactly one invalid item, and it didn't abort the iterator most importantly" ); #[cfg(not(windows))] @@ -299,6 +301,8 @@ fn loose_iter_with_broken_refs() -> crate::Result { "loop-a", "loop-b", "multi-link", + "prefix/feature-suffix", + "prefix/feature/sub/dir/algo", "remotes/origin/HEAD", "remotes/origin/main", "remotes/origin/multi-link-target3", @@ -322,19 +326,50 @@ fn loose_iter_with_prefix_wont_allow_absolute_paths() -> crate::Result { #[cfg(windows)] let abs_path = r"c:\hello"; - match store.loose_iter_prefixed(abs_path.as_ref()) { + match store.loose_iter_prefixed(Cow::Borrowed(abs_path.as_ref())) { Ok(_) => unreachable!("absolute paths aren't allowed"), - Err(err) => assert_eq!(err.to_string(), "prefix must be a relative path, like 'refs/heads'"), + Err(err) => assert_eq!(err.to_string(), "prefix must be a relative path, like 'refs/heads/'"), } Ok(()) } #[test] fn loose_iter_with_prefix() -> crate::Result { + // Test 'refs/heads/' with slash. + let store = store()?; + + let actual = store + .loose_iter_prefixed(b"refs/heads/".as_bstr())? + .collect::, _>>() + .expect("no broken ref in this subset") + .into_iter() + .map(|e| e.name.into_inner()) + .collect::>(); + + assert_eq!( + actual, + vec![ + "refs/heads/A", + "refs/heads/d1", + "refs/heads/dt1", + "refs/heads/main", + "refs/heads/multi-link-target1", + ] + .into_iter() + .map(String::from) + .collect::>(), + "all paths are as expected" + ); + Ok(()) +} + +#[test] +fn loose_iter_with_partial_prefix_dir() -> crate::Result { + // Test 'refs/heads/' without slash. let store = store()?; let actual = store - .loose_iter_prefixed("refs/heads/".as_ref())? + .loose_iter_prefixed(b"refs/heads".as_bstr())? .collect::, _>>() .expect("no broken ref in this subset") .into_iter() @@ -363,7 +398,7 @@ fn loose_iter_with_partial_prefix() -> crate::Result { let store = store()?; let actual = store - .loose_iter_prefixed("refs/heads/d".as_ref())? + .loose_iter_prefixed(b"refs/heads/d".as_bstr())? .collect::, _>>() .expect("no broken ref in this subset") .into_iter() @@ -399,6 +434,8 @@ fn overlay_iter() -> crate::Result { (b"refs/heads/A".as_bstr().to_owned(), Object(c1)), (b"refs/heads/main".into(), Object(c1)), ("refs/heads/newer-as-loose".into(), Object(c2)), + ("refs/prefix/feature-suffix".into(), Object(c1)), + ("refs/prefix/feature/sub/dir/algo".into(), Object(c1)), ( "refs/remotes/origin/HEAD".into(), Symbolic("refs/remotes/origin/main".try_into()?), @@ -505,9 +542,9 @@ fn overlay_iter_with_prefix_wont_allow_absolute_paths() -> crate::Result { #[cfg(windows)] let abs_path = r"c:\hello"; - match store.iter()?.prefixed(abs_path.as_ref()) { + match store.iter()?.prefixed(Cow::Borrowed(abs_path.as_ref())) { Ok(_) => unreachable!("absolute paths aren't allowed"), - Err(err) => assert_eq!(err.to_string(), "prefix must be a relative path, like 'refs/heads'"), + Err(err) => assert_eq!(err.to_string(), "prefix must be a relative path, like 'refs/heads/'"), } Ok(()) } @@ -519,7 +556,7 @@ fn overlay_prefixed_iter() -> crate::Result { let store = store_at("make_packed_ref_repository_for_overlay.sh")?; let ref_names = store .iter()? - .prefixed("refs/heads".as_ref())? + .prefixed(b"refs/heads/".as_bstr())? .map(|r| r.map(|r| (r.name.as_bstr().to_owned(), r.target))) .collect::, _>>()?; let c1 = hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03"); @@ -542,10 +579,62 @@ fn overlay_partial_prefix_iter() -> crate::Result { let store = store_at("make_packed_ref_repository_for_overlay.sh")?; let ref_names = store .iter()? - .prefixed("refs/heads/m".as_ref())? // 'm' is partial + .prefixed(b"refs/heads/m".as_bstr())? // 'm' is partial .map(|r| r.map(|r| (r.name.as_bstr().to_owned(), r.target))) .collect::, _>>()?; let c1 = hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03"); assert_eq!(ref_names, vec![(b"refs/heads/main".as_bstr().to_owned(), Object(c1)),]); Ok(()) } + +#[test] +/// The prefix `refs/d` should match `refs/d1` but not `refs/heads/d1`. +fn overlay_partial_prefix_iter_reproduce_1934() -> crate::Result { + use gix_ref::Target::*; + + let store = store_at("make_ref_repository.sh")?; + let c1 = hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03"); + + let ref_names = store + .iter()? + .prefixed(b"refs/d".as_bstr())? + .map(|r| r.map(|r| (r.name.as_bstr().to_owned(), r.target))) + .collect::, _>>()?; + // Should not match `refs/heads/d1`. + assert_eq!(ref_names, vec![("refs/d1".into(), Object(c1)),]); + Ok(()) +} + +#[test] +fn overlay_partial_prefix_iter_when_prefix_is_dir() -> crate::Result { + // Test 'refs/prefix/' with and without trailing slash. + use gix_ref::Target::*; + + let store = store_at("make_packed_ref_repository_for_overlay.sh")?; + let c1 = hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03"); + + let ref_names = store + .iter()? + .prefixed(b"refs/prefix/feature".as_bstr())? + .map(|r| r.map(|r| (r.name.as_bstr().to_owned(), r.target))) + .collect::, _>>()?; + assert_eq!( + ref_names, + vec![ + ("refs/prefix/feature-suffix".into(), Object(c1)), + ("refs/prefix/feature/sub/dir/algo".into(), Object(c1)), + ] + ); + + let ref_names = store + .iter()? + .prefixed(b"refs/prefix/feature/".as_bstr())? + .map(|r| r.map(|r| (r.name.as_bstr().to_owned(), r.target))) + .collect::, _>>()?; + assert_eq!( + ref_names, + vec![("refs/prefix/feature/sub/dir/algo".into(), Object(c1)),] + ); + + Ok(()) +} diff --git a/gix-ref/tests/refs/file/transaction/prepare_and_commit/create_or_update/mod.rs b/gix-ref/tests/refs/file/transaction/prepare_and_commit/create_or_update/mod.rs index 1ad9bf62199..0b177ec1354 100644 --- a/gix-ref/tests/refs/file/transaction/prepare_and_commit/create_or_update/mod.rs +++ b/gix-ref/tests/refs/file/transaction/prepare_and_commit/create_or_update/mod.rs @@ -791,7 +791,7 @@ fn packed_refs_creation_with_packed_refs_mode_prune_removes_original_loose_refs( assert_eq!( edits.len(), - 9, + 11, "there are a certain amount of loose refs that are packed" ); diff --git a/gix-ref/tests/refs/file/worktree.rs b/gix-ref/tests/refs/file/worktree.rs index 6606af9cc72..d59adb906af 100644 --- a/gix-ref/tests/refs/file/worktree.rs +++ b/gix-ref/tests/refs/file/worktree.rs @@ -191,6 +191,7 @@ mod read_only { mod writable { use gix_lock::acquire::Fail; use gix_ref::{ + bstr::ByteSlice, file::{transaction::PackedRefs, Store}, transaction::{Change, LogChange, PreviousValue, RefEdit}, FullName, FullNameRef, Target, @@ -290,7 +291,7 @@ mod writable { assert_eq!( store .iter()? - .prefixed("refs/stacks/".as_ref())? + .prefixed(b"refs/stacks/".as_bstr())? .map(Result::unwrap) .map(|r| (r.name.to_string(), r.target.to_string())) .collect::>(), @@ -571,7 +572,7 @@ mod writable { assert_eq!( store .iter()? - .prefixed("refs/stacks/".as_ref())? + .prefixed(b"refs/stacks/".as_bstr())? .map(Result::unwrap) .map(|r| (r.name.to_string(), r.target.to_string())) .collect::>(), diff --git a/gix-ref/tests/refs/namespace.rs b/gix-ref/tests/refs/namespace.rs index baaa4dc95d4..70c81d1c7f7 100644 --- a/gix-ref/tests/refs/namespace.rs +++ b/gix-ref/tests/refs/namespace.rs @@ -1,12 +1,16 @@ -use std::path::Path; - #[test] fn into_namespaced_prefix() { assert_eq!( gix_ref::namespace::expand("foo") .unwrap() .into_namespaced_prefix("prefix".as_ref()), - Path::new("refs").join("namespaces").join("foo").join("prefix") + "refs/namespaces/foo/prefix".as_ref(), + ); + assert_eq!( + gix_ref::namespace::expand("foo") + .unwrap() + .into_namespaced_prefix("prefix/".as_ref()), + "refs/namespaces/foo/prefix/".as_ref(), ); } diff --git a/gix-ref/tests/refs/packed/iter.rs b/gix-ref/tests/refs/packed/iter.rs index b22c7c7adba..78db809721a 100644 --- a/gix-ref/tests/refs/packed/iter.rs +++ b/gix-ref/tests/refs/packed/iter.rs @@ -18,7 +18,7 @@ fn packed_refs_with_header() -> crate::Result { let dir = gix_testtools::scripted_fixture_read_only_standalone("make_packed_ref_repository.sh")?; let buf = std::fs::read(dir.join(".git").join("packed-refs"))?; let iter = packed::Iter::new(&buf)?; - assert_eq!(iter.count(), 9, "it finds the right amount of items"); + assert_eq!(iter.count(), 11, "it finds the right amount of items"); Ok(()) } diff --git a/gix/src/open/repository.rs b/gix/src/open/repository.rs index 3bad04d8263..ac61097e5a3 100644 --- a/gix/src/open/repository.rs +++ b/gix/src/open/repository.rs @@ -5,6 +5,7 @@ use std::{borrow::Cow, path::PathBuf}; use super::{Error, Options}; use crate::{ + bstr::BString, config, config::{ cache::interpolate_context, @@ -348,14 +349,13 @@ impl ThreadSafeRepository { .and_then(|prefix| { let _span = gix_trace::detail!("find replacement objects"); let platform = refs.iter().ok()?; - let iter = platform.prefixed(&prefix).ok()?; - let prefix = prefix.to_str()?; + let iter = platform.prefixed(prefix.clone()).ok()?; let replacements = iter .filter_map(Result::ok) .filter_map(|r: gix_ref::Reference| { let target = r.target.try_id()?.to_owned(); let source = - gix_hash::ObjectId::from_hex(r.name.as_bstr().strip_prefix(prefix.as_bytes())?).ok()?; + gix_hash::ObjectId::from_hex(r.name.as_bstr().strip_prefix(prefix.as_slice())?).ok()?; Some((source, target)) }) .collect::>(); @@ -394,7 +394,7 @@ fn replacement_objects_refs_prefix( config: &gix_config::File<'static>, lenient: bool, mut filter_config_section: fn(&gix_config::file::Metadata) -> bool, -) -> Result, Error> { +) -> Result, Error> { let is_disabled = config::shared::is_replace_refs_enabled(config, lenient, filter_config_section) .map_err(config::Error::ConfigBoolean)? .unwrap_or(true); @@ -403,15 +403,15 @@ fn replacement_objects_refs_prefix( return Ok(None); } - let ref_base = gix_path::from_bstr({ + let ref_base = { let key = "gitoxide.objects.replaceRefBase"; debug_assert_eq!(gitoxide::Objects::REPLACE_REF_BASE.logical_name(), key); config .string_filter(key, &mut filter_config_section) .unwrap_or_else(|| Cow::Borrowed("refs/replace/".into())) - }) + } .into_owned(); - Ok(ref_base.into()) + Ok(Some(ref_base)) } fn check_safe_directories( diff --git a/gix/src/reference/iter.rs b/gix/src/reference/iter.rs index 60ded42fef9..576a81a785a 100644 --- a/gix/src/reference/iter.rs +++ b/gix/src/reference/iter.rs @@ -1,8 +1,11 @@ //! #![allow(clippy::empty_docs)] -use std::path::Path; +use std::borrow::Cow; -use gix_ref::file::ReferenceExt; +use gix_ref::{ + bstr::{BStr, ByteSlice}, + file::ReferenceExt, +}; /// A platform to create iterators over references. #[must_use = "Iterators should be obtained from this iterator platform"] @@ -42,11 +45,11 @@ impl Platform<'_> { /// Return an iterator over all references that match the given `prefix`. /// - /// These are of the form `refs/heads` or `refs/remotes/origin`, and must not contain relative paths components like `.` or `..`. + /// These are of the form `refs/heads/` or `refs/remotes/origin`, and must not contain relative paths components like `.` or `..`. // TODO: Create a custom `Path` type that enforces the requirements of git naturally, this type is surprising possibly on windows // and when not using a trailing '/' to signal directories. - pub fn prefixed(&self, prefix: impl AsRef) -> Result, init::Error> { - Ok(Iter::new(self.repo, self.platform.prefixed(prefix.as_ref())?)) + pub fn prefixed<'a>(&self, prefix: impl Into>) -> Result, init::Error> { + Ok(Iter::new(self.repo, self.platform.prefixed(prefix.into())?)) } // TODO: tests @@ -54,7 +57,7 @@ impl Platform<'_> { /// /// They are all prefixed with `refs/tags`. pub fn tags(&self) -> Result, init::Error> { - Ok(Iter::new(self.repo, self.platform.prefixed("refs/tags/".as_ref())?)) + Ok(Iter::new(self.repo, self.platform.prefixed(b"refs/tags/".as_bstr())?)) } // TODO: tests @@ -62,7 +65,7 @@ impl Platform<'_> { /// /// They are all prefixed with `refs/heads`. pub fn local_branches(&self) -> Result, init::Error> { - Ok(Iter::new(self.repo, self.platform.prefixed("refs/heads/".as_ref())?)) + Ok(Iter::new(self.repo, self.platform.prefixed(b"refs/heads/".as_bstr())?)) } // TODO: tests @@ -70,7 +73,10 @@ impl Platform<'_> { /// /// They are all prefixed with `refs/remotes`. pub fn remote_branches(&self) -> Result, init::Error> { - Ok(Iter::new(self.repo, self.platform.prefixed("refs/remotes/".as_ref())?)) + Ok(Iter::new( + self.repo, + self.platform.prefixed(b"refs/remotes/".as_bstr())?, + )) } } diff --git a/gix/tests/gix/repository/reference.rs b/gix/tests/gix/repository/reference.rs index 94e9e8574d2..bd5079aabfc 100644 --- a/gix/tests/gix/repository/reference.rs +++ b/gix/tests/gix/repository/reference.rs @@ -1,5 +1,5 @@ mod set_namespace { - use gix::refs::transaction::PreviousValue; + use gix::{bstr::ByteSlice, refs::transaction::PreviousValue}; use gix_testtools::tempfile; fn easy_repo_rw() -> crate::Result<(gix::Repository, tempfile::TempDir)> { @@ -48,7 +48,7 @@ mod set_namespace { assert_eq!( repo.references()? - .prefixed("refs/tags/")? + .prefixed(b"refs/tags/".as_bstr())? .filter_map(Result::ok) .map(|r| r.name().as_bstr().to_owned()) .collect::>(), @@ -81,6 +81,7 @@ mod set_namespace { } mod iter_references { + use gix::bstr::ByteSlice; use crate::util::hex_to_id; @@ -124,7 +125,7 @@ mod iter_references { let repo = repo()?; assert_eq!( repo.references()? - .prefixed("refs/heads/")? + .prefixed(b"refs/heads/".as_bstr())? .filter_map(Result::ok) .map(|r| ( r.name().as_bstr().to_string(), @@ -155,7 +156,7 @@ mod iter_references { let repo = repo()?; assert_eq!( repo.references()? - .prefixed("refs/heads/")? + .prefixed(b"refs/heads/".as_bstr())? .peeled()? .filter_map(Result::ok) .map(|r| (