Merge pull request #2507 from GitoxideLabs/improvements

Various Fixes

Author: Byron

gix-object/src/tree/mod.rs

@@ -111,7 +111,7 @@ impl EntryMode {
         }
 
         // Happy path: space is at index 6
-        let space_pos = if i.get(6) == Some(&b' ') {
+        let space_pos = if i.get(6) == Some(&b' ') && i.get(5) != Some(&b' ') {
             for b in i.iter().take(6) {
                 let b = b.wrapping_sub(b'0') as u16;
                 // Not a pure octal input.

gix-object/tests/object/tree/iter.rs

@@ -76,6 +76,23 @@ fn everything() -> crate::Result {
     Ok(())
 }
 
+#[test]
+fn leading_space_in_tree_name() -> crate::Result {
+    let oid = hex_to_id("4d5fcadc293a348e88f777dc0920f11e7d71441c");
+    let mut buf = b"40000  leading space\0".to_vec();
+    buf.extend_from_slice(oid.as_bytes());
+
+    assert_eq!(
+        TreeRefIter::from_bytes(&buf).collect::<Result<Vec<_>, _>>()?,
+        vec![EntryRef {
+            mode: tree::EntryKind::Tree.into(),
+            filename: b" leading space".as_bstr(),
+            oid: oid.as_ref(),
+        }]
+    );
+    Ok(())
+}
+
 mod lookup_entry {
     use gix_object::tree::EntryKind;
     use utils::entry;

gix-ref/src/fullname.rs

@@ -166,9 +166,6 @@ impl Category<'_> {
     /// the `short_name` to create a valid fully qualified [reference name](FullName).
     ///
     /// If `short_name` already contains the prefix that it would receive (and is thus a full name), no duplication will occur.
-    ///
-    /// Note that [`Self::LocalBranch`] rejects the exact name `refs/heads/HEAD`, matching Git's branch-specific rules.
-    /// Generic full reference handling accepts that name, however, as it is a valid reference outside of local-branch creation.
     pub fn to_full_name<'a>(&self, short_name: impl Into<&'a BStr>) -> Result<FullName, crate::name::Error> {
         let mut out: BString = self.prefix().into();
         let short_name = short_name.into();
@@ -189,16 +186,12 @@ impl Category<'_> {
             | Category::PseudoRef
             | Category::MainPseudoRef => short_name,
         };
-        let full_name = if out.is_empty() || !partial_name.starts_with(&out) {
+        if out.is_empty() || !partial_name.starts_with(&out) {
             out.extend_from_slice(partial_name);
-            FullName::try_from(out)?
+            FullName::try_from(out)
         } else {
-            FullName::try_from(partial_name.as_bstr())?
-        };
-        if matches!(self, Category::LocalBranch) && full_name.as_bstr() == b"refs/heads/HEAD".as_bstr() {
-            return Err(gix_validate::reference::name::Error::Reserved { name: full_name.into() });
+            FullName::try_from(partial_name.as_bstr())
         }
-        Ok(full_name)
     }
 }
 

gix-ref/tests/refs/fullname.rs

@@ -137,19 +137,16 @@ fn to_full_name() -> gix_testtools::Result {
 }
 
 #[test]
-fn local_branch_head_is_reserved() -> gix_testtools::Result {
-    assert!(matches!(
-        Category::LocalBranch.to_full_name("HEAD"),
-        Err(gix_validate::reference::name::Error::Reserved { name }) if name == "refs/heads/HEAD"
-    ));
-    assert!(matches!(
-        Category::LocalBranch.to_full_name("refs/heads/HEAD"),
-        Err(gix_validate::reference::name::Error::Reserved { name }) if name == "refs/heads/HEAD"
-    ));
+fn local_branch_head_is_representable_as_full_ref_name() -> gix_testtools::Result {
     assert_eq!(
-        FullName::try_from("refs/heads/HEAD")?.as_bstr(),
+        Category::LocalBranch.to_full_name("HEAD")?.as_bstr(),
         "refs/heads/HEAD",
-        "generic full-name parsing still accepts this name so remote refs remain representable"
+        "generic full-name construction accepts names that are invalid only in branch-specific contexts"
+    );
+    assert_eq!(
+        Category::LocalBranch.to_full_name("refs/heads/HEAD")?.as_bstr(),
+        "refs/heads/HEAD",
+        "fully qualified names keep their category prefix de-duplicated"
     );
     Ok(())
 }

gix/src/init.rs

@@ -7,7 +7,11 @@ use gix_ref::{
     Category, FullName, Target,
 };
 
-use crate::{bstr::BString, config::tree::Init, ThreadSafeRepository};
+use crate::{
+    bstr::{BString, ByteSlice},
+    config::tree::Init,
+    ThreadSafeRepository,
+};
 
 /// The name of the branch to use if non is configured via git configuration.
 ///
@@ -75,12 +79,17 @@ impl ThreadSafeRepository {
             .string(Init::DEFAULT_BRANCH)
             .unwrap_or_else(|| Cow::Borrowed(DEFAULT_BRANCH_NAME.into()));
         if branch_name.as_ref() != DEFAULT_BRANCH_NAME {
+            let configured_branch_name = branch_name.into_owned();
             let sym_ref: FullName = Category::LocalBranch
-                .to_full_name(branch_name.as_ref())
+                .to_full_name(configured_branch_name.as_bstr())
                 .map_err(|err| Error::InvalidBranchName {
-                    name: branch_name.into_owned(),
+                    name: configured_branch_name.clone(),
                     source: err,
                 })?;
+            gix_validate::reference::branch_name(sym_ref.as_bstr()).map_err(|err| Error::InvalidBranchName {
+                name: configured_branch_name,
+                source: err,
+            })?;
             let mut repo = repo.to_thread_local();
             let prev_write_reflog = repo.refs.write_reflog;
             repo.refs.write_reflog = WriteReflog::Disable;

gix/src/repository/reference.rs

@@ -73,7 +73,7 @@ impl crate::Repository {
     ///
     /// Note that this accepts any valid full reference name, including `refs/heads/HEAD`.
     /// Git rejects creating a local branch with that name in branch-specific code paths, but this API operates on generic
-    /// references instead. Branch names can be validated with [`gix_ref::Category::to_full_name()`].
+    /// references instead. Branch names can be validated with [`gix_validate::reference::branch_name()`].
     ///
     /// The newly created Reference is returned.
     pub fn reference<Name, E>(

gix/tests/gix/init.rs

@@ -121,6 +121,30 @@ mod non_bare {
         Ok(())
     }
 
+    #[test]
+    fn init_bare_rejects_reserved_fully_qualified_branch_name() -> crate::Result {
+        let tmp = tempfile::tempdir()?;
+        let err = gix::ThreadSafeRepository::init_opts(
+            tmp.path(),
+            gix::create::Kind::Bare,
+            gix::create::Options::default(),
+            gix::open::Options::isolated().config_overrides([
+                "user.name=a",
+                "user.email=b",
+                "init.defaultBranch=refs/heads/HEAD",
+            ]),
+        )
+        .unwrap_err();
+        assert!(matches!(
+            err,
+            gix::init::Error::InvalidBranchName {
+                name,
+                source: gix_validate::reference::name::Error::Reserved { name: reserved }
+            } if name == "refs/heads/HEAD" && reserved == "refs/heads/HEAD"
+        ));
+        Ok(())
+    }
+
     #[test]
     fn init_into_empty_directory_creates_a_dot_git_dir() -> crate::Result {
         let tmp = tempfile::tempdir()?;