fix: loose ref iteration on a repo with missing 'ref/' fails when cre…

…ating the iterator. (#595)

Previously, it would fail on first iteration, making it seem like there
is one reference even though it's just an error stating that the base
cannot be read.

This is clearly worse than making a metadata check on the filesystem,
no matter how unlikely the case.

git-ref/src/store/file/loose/iter.rs

@@ -13,13 +13,24 @@ pub(in crate::store_impl::file) struct SortedLoosePaths {
 }
 
 impl SortedLoosePaths {
-    pub fn at(path: impl AsRef<Path>, base: impl Into<PathBuf>, filename_prefix: Option<BString>) -> Self {
+    pub fn at(
+        path: impl AsRef<Path>,
+        base: impl Into<PathBuf>,
+        filename_prefix: Option<BString>,
+    ) -> std::io::Result<Self> {
+        let path = path.as_ref();
+        if !path.is_dir() {
+            return Err(std::io::Error::new(
+                std::io::ErrorKind::Other,
+                format!("loose reference iteration path does not exist: \"{}\"", path.display()),
+            ));
+        }
         let file_walk = git_features::fs::walkdir_sorted_new(path).into_iter();
-        SortedLoosePaths {
+        Ok(SortedLoosePaths {
             base: base.into(),
             filename_prefix,
             file_walk,
-        }
+        })
     }
 }
 

git-ref/src/store/file/overlay_iter.rs

@@ -253,23 +253,23 @@ impl<'a> IterInfo<'a> {
         }
     }
 
-    fn into_iter(self) -> Peekable<SortedLoosePaths> {
-        match self {
-            IterInfo::Base { base } => SortedLoosePaths::at(base.join("refs"), base, None),
+    fn into_iter(self) -> std::io::Result<Peekable<SortedLoosePaths>> {
+        Ok(match self {
+            IterInfo::Base { base } => SortedLoosePaths::at(base.join("refs"), base, None)?,
             IterInfo::BaseAndIterRoot {
                 base,
                 iter_root,
                 prefix: _,
-            } => SortedLoosePaths::at(iter_root, base, None),
-            IterInfo::PrefixAndBase { base, prefix } => SortedLoosePaths::at(base.join(prefix), base, None),
+            } => SortedLoosePaths::at(iter_root, base, None)?,
+            IterInfo::PrefixAndBase { base, prefix } => SortedLoosePaths::at(base.join(prefix), base, None)?,
             IterInfo::ComputedIterationRoot {
                 iter_root,
                 base,
                 prefix: _,
                 remainder,
-            } => SortedLoosePaths::at(iter_root, base, remainder),
+            } => SortedLoosePaths::at(iter_root, base, remainder)?,
         }
-        .peekable()
+        .peekable())
     }
 
     fn from_prefix(base: &'a Path, prefix: Cow<'a, Path>) -> std::io::Result<Self> {
@@ -397,8 +397,8 @@ impl file::Store {
                 ),
                 None => None,
             },
-            iter_git_dir: git_dir_info.into_iter(),
-            iter_common_dir: common_dir_info.map(IterInfo::into_iter),
+            iter_git_dir: git_dir_info.into_iter()?,
+            iter_common_dir: common_dir_info.map(IterInfo::into_iter).transpose()?,
             buf: Vec::new(),
             namespace: self.namespace.as_ref(),
         })

git-ref/tests/file/transaction/prepare_and_commit/create_or_update.rs

@@ -22,8 +22,9 @@ use crate::file::{
 };
 
 mod collisions {
-    use crate::file::transaction::prepare_and_commit::{create_at, empty_store};
+    use crate::file::transaction::prepare_and_commit::{committer, create_at, empty_store};
     use git_lock::acquire::Fail;
+    use git_ref::file::transaction::PackedRefs;
 
     fn case_sensitive(tmp_dir: &std::path::Path) -> bool {
         std::fs::write(tmp_dir.join("config"), "").expect("can create file once");
@@ -54,6 +55,35 @@ mod collisions {
         }
         Ok(())
     }
+
+    #[test]
+    fn conflicting_creation_into_packed_refs() -> crate::Result {
+        let (_dir, store) = empty_store()?;
+        store
+            .transaction()
+            .packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(
+                Box::new(|_, _| Ok(Some(git_object::Kind::Commit))),
+            ))
+            .prepare(
+                [create_at("refs/a"), create_at("refs/Ab")],
+                Fail::Immediately,
+                Fail::Immediately,
+            )?
+            .commit(committer().to_ref())?;
+
+        assert_eq!(
+            store.cached_packed_buffer()?.expect("created").iter()?.count(),
+            2,
+            "packed-refs can store everything in case-insensitive manner"
+        );
+
+        assert!(
+            store.loose_iter().is_err(),
+            "refs/ directory isn't present as there is no loose ref - it removed every up to the base dir"
+        );
+
+        Ok(())
+    }
 }
 
 #[test]