change!: use gix-object::Find trait

Author: Byron

gix-ref/src/peel.rs

@@ -1,12 +1,3 @@
-/// A function for use in [`crate::file::ReferenceExt::peel_to_id_in_place()`] to indicate no peeling should happen.
-#[allow(clippy::type_complexity)]
-pub fn none(
-    _id: gix_hash::ObjectId,
-    #[allow(clippy::ptr_arg)] _buf: &mut Vec<u8>,
-) -> Result<Option<(gix_object::Kind, &[u8])>, Box<dyn std::error::Error + Send + Sync>> {
-    Ok(Some((gix_object::Kind::Commit, &[])))
-}
-
 ///
 pub mod to_id {
     use std::path::PathBuf;
@@ -26,7 +17,7 @@ pub mod to_id {
         #[error("Refusing to follow more than {max_depth} levels of indirection")]
         DepthLimitExceeded { max_depth: usize },
         #[error("An error occurred when trying to resolve an object a reference points to")]
-        Find(#[from] Box<dyn std::error::Error + Send + Sync + 'static>),
+        Find(#[from] gix_object::find::Error),
         #[error("Object {oid} as referred to by {name:?} could not be found")]
         NotFound { oid: gix_hash::ObjectId, name: BString },
     }

gix-ref/src/store/file/raw_ext.rs

@@ -12,12 +12,6 @@ use crate::{
 pub trait Sealed {}
 impl Sealed for crate::Reference {}
 
-pub type FindObjectFn<'a> = dyn FnMut(
-        gix_hash::ObjectId,
-        &mut Vec<u8>,
-    ) -> Result<Option<(gix_object::Kind, &[u8])>, Box<dyn std::error::Error + Send + Sync>>
-    + 'a;
-
 /// A trait to extend [Reference][crate::Reference] with functionality requiring a [file::Store].
 pub trait ReferenceExt: Sealed {
     /// A step towards obtaining forward or reverse iterators on reference logs.
@@ -26,18 +20,22 @@ pub trait ReferenceExt: Sealed {
     /// For details, see [`Reference::log_exists()`].
     fn log_exists(&self, store: &file::Store) -> bool;
 
-    /// For details, see [`Reference::peel_to_id_in_place()`].
+    /// Follow all symbolic targets this reference might point to and peel the underlying object
+    /// to the end of the chain, and return it, using `objects` to access them.
+    ///
+    /// This is useful to learn where this reference is ultimately pointing to.
     fn peel_to_id_in_place(
         &mut self,
         store: &file::Store,
-        find: &mut FindObjectFn<'_>,
+        objects: &dyn gix_object::Find,
     ) -> Result<ObjectId, peel::to_id::Error>;
 
-    /// For details, see [`Reference::peel_to_id_in_place()`], with support for a known stable packed buffer.
+    /// Like [`ReferenceExt::peel_to_id_in_place()`], but with support for a known stable packed buffer
+    /// to use for resolving symbolic links.
     fn peel_to_id_in_place_packed(
         &mut self,
         store: &file::Store,
-        find: &mut FindObjectFn<'_>,
+        objects: &dyn gix_object::Find,
         packed: Option<&packed::Buffer>,
     ) -> Result<ObjectId, peel::to_id::Error>;
 
@@ -75,18 +73,18 @@ impl ReferenceExt for Reference {
     fn peel_to_id_in_place(
         &mut self,
         store: &file::Store,
-        find: &mut FindObjectFn<'_>,
+        objects: &dyn gix_object::Find,
     ) -> Result<ObjectId, peel::to_id::Error> {
         let packed = store.assure_packed_refs_uptodate().map_err(|err| {
             peel::to_id::Error::Follow(file::find::existing::Error::Find(file::find::Error::PackedOpen(err)))
         })?;
-        self.peel_to_id_in_place_packed(store, find, packed.as_ref().map(|b| &***b))
+        self.peel_to_id_in_place_packed(store, objects, packed.as_ref().map(|b| &***b))
     }
 
     fn peel_to_id_in_place_packed(
         &mut self,
         store: &file::Store,
-        find: &mut FindObjectFn<'_>,
+        objects: &dyn gix_object::Find,
         packed: Option<&packed::Buffer>,
     ) -> Result<ObjectId, peel::to_id::Error> {
         match self.peeled {
@@ -118,10 +116,13 @@ impl ReferenceExt for Reference {
                 let mut buf = Vec::new();
                 let mut oid = self.target.try_id().expect("peeled ref").to_owned();
                 let peeled_id = loop {
-                    let (kind, data) = find(oid, &mut buf)?.ok_or_else(|| peel::to_id::Error::NotFound {
-                        oid,
-                        name: self.name.0.clone(),
-                    })?;
+                    let gix_object::Data { kind, data } =
+                        objects
+                            .try_find(&oid, &mut buf)?
+                            .ok_or_else(|| peel::to_id::Error::NotFound {
+                                oid,
+                                name: self.name.0.clone(),
+                            })?;
                     match kind {
                         gix_object::Kind::Tag => {
                             oid = gix_object::TagRefIter::from_bytes(data).target_id().map_err(|_err| {

gix-ref/src/store/file/transaction/mod.rs

@@ -8,28 +8,18 @@ use crate::{
     transaction::RefEdit,
 };
 
-/// A function receiving an object id to resolve, returning its decompressed bytes,
-/// used to obtain the peeled object ids for storage in packed-refs files.
-///
-/// Resolution means to follow tag objects until the end of the chain.
-pub type FindObjectFn<'a> = dyn FnMut(
-        gix_hash::ObjectId,
-        &mut Vec<u8>,
-    ) -> Result<Option<gix_object::Kind>, Box<dyn std::error::Error + Send + Sync + 'static>>
-    + 'a;
-
 /// How to handle packed refs during a transaction
 #[derive(Default)]
 pub enum PackedRefs<'a> {
     /// Only propagate deletions of references. This is the default
     #[default]
     DeletionsOnly,
     /// Propagate deletions as well as updates to references which are peeled, that is contain an object id
-    DeletionsAndNonSymbolicUpdates(Box<FindObjectFn<'a>>),
+    DeletionsAndNonSymbolicUpdates(Box<dyn gix_object::Find + 'a>),
     /// Propagate deletions as well as updates to references which are peeled, that is contain an object id. Furthermore delete the
     /// reference which is originally updated if it exists. If it doesn't, the new value will be written into the packed ref right away.
     /// Note that this doesn't affect symbolic references at all, which can't be placed into packed refs.
-    DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(Box<FindObjectFn<'a>>),
+    DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(Box<dyn gix_object::Find + 'a>),
 }
 
 #[derive(Debug)]

gix-ref/src/store/file/transaction/prepare.rs

@@ -338,12 +338,10 @@ impl<'s, 'p> Transaction<'s, 'p> {
                     self.packed_transaction = Some(match &mut self.packed_refs {
                         PackedRefs::DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(f)
                         | PackedRefs::DeletionsAndNonSymbolicUpdates(f) => {
-                            transaction.prepare(&mut edits_for_packed_transaction.into_iter(), f)?
+                            transaction.prepare(&mut edits_for_packed_transaction.into_iter(), &**f)?
                         }
                         PackedRefs::DeletionsOnly => transaction
-                            .prepare(&mut edits_for_packed_transaction.into_iter(), &mut |_, _| {
-                                unreachable!("BUG: deletions never trigger object lookups")
-                            })?,
+                            .prepare(&mut edits_for_packed_transaction.into_iter(), &gix_object::find::Never)?,
                     });
                 }
             }

gix-ref/src/store/packed/transaction.rs

@@ -2,7 +2,7 @@ use std::{fmt::Formatter, io::Write};
 
 use crate::{
     file,
-    store_impl::{file::transaction::FindObjectFn, packed, packed::Edit},
+    store_impl::{packed, packed::Edit},
     transaction::{Change, RefEdit},
     Target,
 };
@@ -44,10 +44,11 @@ impl packed::Transaction {
 /// Lifecycle
 impl packed::Transaction {
     /// Prepare the transaction by checking all edits for applicability.
+    /// Use `objects` to access objects for the purpose of peeling them - this is only used if packed-refs are involved.
     pub fn prepare(
         mut self,
         edits: &mut dyn Iterator<Item = RefEdit>,
-        find: &mut FindObjectFn<'_>,
+        objects: &dyn gix_object::Find,
     ) -> Result<Self, prepare::Error> {
         assert!(self.edits.is_none(), "BUG: cannot call prepare(…) more than once");
         let buffer = &self.buffer;
@@ -76,7 +77,7 @@ impl packed::Transaction {
             {
                 let mut next_id = new;
                 edit.peeled = loop {
-                    let kind = find(next_id, &mut buf)?;
+                    let kind = objects.try_find(&next_id, &mut buf)?.map(|d| d.kind);
                     match kind {
                         Some(gix_object::Kind::Tag) => {
                             next_id = gix_object::TagRefIter::from_bytes(&buf).target_id().map_err(|_| {

gix-ref/tests/file/mod.rs

@@ -31,6 +31,20 @@ fn store_writable(name: &str) -> crate::Result<(gix_testtools::tempfile::TempDir
     ))
 }
 
+struct EmptyCommit;
+impl gix_object::Find for EmptyCommit {
+    fn try_find<'a>(
+        &self,
+        _id: &gix_hash::oid,
+        _buffer: &'a mut Vec<u8>,
+    ) -> Result<Option<gix_object::Data<'a>>, gix_object::find::Error> {
+        Ok(Some(gix_object::Data {
+            kind: gix_object::Kind::Commit,
+            data: &[],
+        }))
+    }
+}
+
 mod log;
 mod reference;
 mod store;

gix-ref/tests/file/reference.rs

@@ -47,9 +47,9 @@ mod reflog {
 }
 
 mod peel {
-    use gix_odb::pack::Find;
-    use gix_ref::{file::ReferenceExt, peel, Reference};
+    use gix_ref::{file::ReferenceExt, Reference};
 
+    use crate::file::EmptyCommit;
     use crate::{file, file::store_with_packed_refs, hex_to_id};
 
     #[test]
@@ -77,11 +77,11 @@ mod peel {
         let store = store_with_packed_refs()?;
         let mut head: Reference = store.find_loose("HEAD")?.into();
         let expected = hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03");
-        assert_eq!(head.peel_to_id_in_place(&store, &mut peel::none)?, expected);
+        assert_eq!(head.peel_to_id_in_place(&store, &EmptyCommit)?, expected);
         assert_eq!(head.target.try_id().map(ToOwned::to_owned), Some(expected));
 
         let mut head = store.find("dt1")?;
-        assert_eq!(head.peel_to_id_in_place(&store, &mut peel::none)?, expected);
+        assert_eq!(head.peel_to_id_in_place(&store, &gix_object::find::Never)?, expected);
         assert_eq!(head.target.into_id(), expected);
         Ok(())
     }
@@ -119,23 +119,20 @@ mod peel {
         assert_eq!(r.kind(), gix_ref::Kind::Symbolic, "there is something to peel");
 
         let commit = hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03");
-        assert_eq!(r.peel_to_id_in_place(&store, &mut peel::none)?, commit);
+        assert_eq!(r.peel_to_id_in_place(&store, &EmptyCommit)?, commit);
         assert_eq!(r.name.as_bstr(), "refs/remotes/origin/multi-link-target3");
 
         let mut r: Reference = store.find_loose("dt1")?.into();
         assert_eq!(
-            r.peel_to_id_in_place(&store, &mut peel::none)?,
+            r.peel_to_id_in_place(&store, &EmptyCommit)?,
             hex_to_id("4c3f4cce493d7beb45012e478021b5f65295e5a3"),
             "points to a tag object without actual object lookup"
         );
 
         let odb = gix_odb::at(store.git_dir().join("objects"))?;
         let mut r: Reference = store.find_loose("dt1")?.into();
         assert_eq!(
-            r.peel_to_id_in_place(&store, &mut |oid, buf| {
-                odb.try_find(&oid, buf)
-                    .map(|obj| obj.map(|(obj, _)| (obj.kind, obj.data)))
-            })?,
+            r.peel_to_id_in_place(&store, &odb)?,
             commit,
             "points to the commit with lookup"
         );
@@ -151,7 +148,7 @@ mod peel {
         assert_eq!(r.name.as_bstr(), "refs/loop-a");
 
         assert!(matches!(
-            r.peel_to_id_in_place(&store, &mut peel::none).unwrap_err(),
+            r.peel_to_id_in_place(&store, &gix_object::find::Never).unwrap_err(),
             gix_ref::peel::to_id::Error::Cycle { .. }
         ));
         assert_eq!(r.name.as_bstr(), "refs/loop-a", "the ref is not changed on error");

gix-ref/tests/file/transaction/prepare_and_commit/create_or_update/collisions.rs

@@ -7,6 +7,7 @@ use gix_ref::{
     Target,
 };
 
+use crate::file::EmptyCommit;
 use crate::{
     file::transaction::prepare_and_commit::{committer, create_at, create_symbolic_at, delete_at, empty_store},
     hex_to_id,
@@ -69,14 +70,14 @@ fn packed_refs_lock_is_mandatory_for_multiple_ongoing_transactions_even_if_one_d
     let _t1 = store
         .transaction()
         .packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(
-            Box::new(|_, _| Ok(Some(gix_object::Kind::Commit))),
+            Box::new(EmptyCommit),
         ))
         .prepare([create_at(ref_name)], Fail::Immediately, Fail::Immediately)?;
 
     let t2res = store
         .transaction()
         .prepare([delete_at(ref_name)], Fail::Immediately, Fail::Immediately);
-    assert_eq!(&t2res.unwrap_err().to_string()[..51], "The lock for the packed-ref file could not be obtai", "if packed-refs are about to be created, other transactions always acquire a packed-refs lock as to not miss anything");
+    assert_eq!(&t2res.unwrap_err().to_string()[..54], "The lock for the packed-ref file could not be obtained", "if packed-refs are about to be created, other transactions always acquire a packed-refs lock as to not miss anything");
     Ok(())
 }
 
@@ -86,7 +87,7 @@ fn conflicting_creation_into_packed_refs() -> crate::Result {
     store
         .transaction()
         .packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(
-            Box::new(|_, _| Ok(Some(gix_object::Kind::Commit))),
+            Box::new(EmptyCommit),
         ))
         .prepare(
             [
@@ -118,7 +119,7 @@ fn conflicting_creation_into_packed_refs() -> crate::Result {
     store
         .transaction()
         .packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(
-            Box::new(|_, _| Ok(Some(gix_object::Kind::Commit))),
+            Box::new(EmptyCommit),
         ))
         .prepare(
             [

gix-ref/tests/file/transaction/prepare_and_commit/create_or_update/mod.rs

@@ -3,7 +3,6 @@ use std::convert::TryInto;
 use gix_hash::ObjectId;
 use gix_lock::acquire::Fail;
 use gix_object::bstr::{BString, ByteSlice};
-use gix_object::Find;
 use gix_ref::{
     file::{
         transaction::{self, PackedRefs},
@@ -14,6 +13,7 @@ use gix_ref::{
     Target,
 };
 
+use crate::file::EmptyCommit;
 use crate::{
     file::{
         store_with_packed_refs, store_writable,
@@ -713,7 +713,7 @@ fn packed_refs_creation_with_packed_refs_mode_prune_removes_original_loose_refs(
     let edits = store
         .transaction()
         .packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(
-            Box::new(move |oid, buf| odb.try_find(&oid, buf).map(|obj| obj.map(|obj| obj.kind))),
+            Box::new(odb),
         ))
         .prepare(
             store
@@ -782,9 +782,7 @@ fn packed_refs_creation_with_packed_refs_mode_leave_keeps_original_loose_refs()
 
     let edits = store
         .transaction()
-        .packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdates(Box::new(|_, _| {
-            Ok(Some(gix_object::Kind::Commit))
-        })))
+        .packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdates(Box::new(EmptyCommit)))
         .prepare(edits, Fail::Immediately, Fail::Immediately)?
         .commit(committer().to_ref())?;
     assert_eq!(

gix-ref/tests/file/worktree.rs

@@ -1,6 +1,5 @@
 use std::{cmp::Ordering, path::PathBuf};
 
-use gix_object::Find;
 use gix_ref::{file::ReferenceExt, Reference};
 use gix_testtools::Creation;
 
@@ -61,13 +60,7 @@ fn into_peel(
     store: &gix_ref::file::Store,
     odb: gix_odb::Handle,
 ) -> impl Fn(gix_ref::Reference) -> gix_hash::ObjectId + '_ {
-    move |mut r: gix_ref::Reference| {
-        r.peel_to_id_in_place(store, &mut |id, buf| {
-            let data = odb.try_find(&id, buf)?;
-            Ok(data.map(|d| (d.kind, d.data)))
-        })
-        .unwrap()
-    }
+    move |mut r: gix_ref::Reference| r.peel_to_id_in_place(store, &odb).unwrap()
 }
 
 enum Mode {
@@ -205,6 +198,7 @@ mod writable {
         FullName, FullNameRef, Target,
     };
 
+    use crate::file::EmptyCommit;
     use crate::{
         file::{
             transaction::prepare_and_commit::committer,
@@ -232,9 +226,7 @@ mod writable {
             let (store, _odb, _tmp) = main_store(packed, Mode::Write)?;
             let mut t = store.transaction();
             if packed {
-                t = t.packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdates(Box::new(|_, _| {
-                    Ok(Some(gix_object::Kind::Commit))
-                })));
+                t = t.packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdates(Box::new(EmptyCommit)));
             }
 
             let edits = t
@@ -514,9 +506,7 @@ mod writable {
 
             let mut t = store.transaction();
             if packed {
-                t = t.packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdates(Box::new(|_, _| {
-                    Ok(Some(gix_object::Kind::Commit))
-                })));
+                t = t.packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdates(Box::new(EmptyCommit)));
             }
 
             let edits = t