diff --git a/gix-revision/tests/describe/mod.rs b/gix-revision/tests/describe/mod.rs index 0b1851417ba..d72e839a31e 100644 --- a/gix-revision/tests/describe/mod.rs +++ b/gix-revision/tests/describe/mod.rs @@ -1,7 +1,6 @@ use std::{borrow::Cow, path::PathBuf}; use gix_object::bstr::ByteSlice; -use gix_object::Find; use gix_revision::{ describe, describe::{Error, Outcome}, @@ -23,14 +22,7 @@ fn run_test( let cache = use_commitgraph .then(|| gix_commitgraph::Graph::from_info_dir(&store.store_ref().path().join("info")).ok()) .flatten(); - let mut graph = gix_revision::Graph::new( - |id, buf| { - store - .try_find(id, buf) - .map(|r| r.and_then(gix_object::Data::try_into_commit_iter)) - }, - cache, - ); + let mut graph = gix_revision::Graph::new(&store, cache); run_assertions( gix_revision::describe(&commit_id, &mut graph, options(commit_id)), commit_id, diff --git a/gix-revwalk/src/graph/errors.rs b/gix-revwalk/src/graph/errors.rs deleted file mode 100644 index a2d849fbf43..00000000000 --- a/gix-revwalk/src/graph/errors.rs +++ /dev/null @@ -1,53 +0,0 @@ -/// -pub mod lookup { - /// The error returned by [`try_lookup()`][crate::Graph::try_lookup()]. - #[derive(Debug, thiserror::Error)] - #[error("There was an error looking up a commit")] - pub struct Error { - #[from] - source: Box, - } - - /// - pub mod commit { - /// The error returned by [`try_lookup_or_insert_commit()`][crate::Graph::try_lookup_or_insert_commit()]. - #[derive(Debug, thiserror::Error)] - #[allow(missing_docs)] - pub enum Error { - #[error(transparent)] - Find(#[from] crate::graph::lookup::Error), - #[error(transparent)] - ToOwned(#[from] crate::graph::commit::to_owned::Error), - } - } - - /// - pub mod existing { - /// The error returned by [`lookup()`][crate::Graph::lookup()]. - #[derive(Debug, thiserror::Error)] - #[allow(missing_docs)] - pub enum Error { - #[error(transparent)] - Find(#[from] super::Error), - #[error("Commit could not be found")] - Missing, - } - } -} - -/// -pub mod insert_parents { - use crate::graph::{commit::iter_parents, lookup}; - - /// The error returned by [`insert_parents()`][crate::Graph::insert_parents()]. - #[derive(Debug, thiserror::Error)] - #[allow(missing_docs)] - pub enum Error { - #[error(transparent)] - Lookup(#[from] lookup::existing::Error), - #[error("A commit could not be decoded during traversal")] - Decode(#[from] gix_object::decode::Error), - #[error(transparent)] - Parent(#[from] iter_parents::Error), - } -} diff --git a/gix-revwalk/src/graph/mod.rs b/gix-revwalk/src/graph/mod.rs index 619a2bf7f35..8cd62a7506a 100644 --- a/gix-revwalk/src/graph/mod.rs +++ b/gix-revwalk/src/graph/mod.rs @@ -5,14 +5,47 @@ use smallvec::SmallVec; use crate::Graph; -/// A mapping between an object id and arbitrary data, and produced when calling [`Graph::detach`]. +/// A mapping between an object id and arbitrary data, and produced when calling [`Graph::detach()`]. pub type IdMap = gix_hashtable::HashMap; /// pub mod commit; -mod errors; -pub use errors::{insert_parents, lookup}; +mod errors { + /// + pub mod insert_parents { + use crate::graph::commit::iter_parents; + + /// The error returned by [`insert_parents()`](crate::Graph::insert_parents()). + #[derive(Debug, thiserror::Error)] + #[allow(missing_docs)] + pub enum Error { + #[error(transparent)] + Lookup(#[from] gix_object::find::existing_iter::Error), + #[error("A commit could not be decoded during traversal")] + Decode(#[from] gix_object::decode::Error), + #[error(transparent)] + Parent(#[from] iter_parents::Error), + } + } + + /// + pub mod try_lookup_or_insert_default { + use crate::graph::commit::to_owned; + + /// The error returned by [`try_lookup_or_insert_default()`](crate::Graph::try_lookup_or_insert_default()). + #[derive(Debug, thiserror::Error)] + #[allow(missing_docs)] + pub enum Error { + #[error(transparent)] + Lookup(#[from] gix_object::find::existing_iter::Error), + #[error(transparent)] + ToOwned(#[from] to_owned::Error), + } + } +} +pub use errors::{insert_parents, try_lookup_or_insert_default}; + use gix_date::SecondsSinceUnixEpoch; /// The generation away from the HEAD of graph, useful to limit algorithms by topological depth as well. @@ -36,7 +69,7 @@ impl<'find, T: Default> Graph<'find, T> { &mut self, id: gix_hash::ObjectId, update_data: impl FnOnce(&mut T), - ) -> Result>, lookup::Error> { + ) -> Result>, try_lookup_or_insert_default::Error> { self.try_lookup_or_insert_default(id, T::default, update_data) } } @@ -85,9 +118,7 @@ impl<'find, T> Graph<'find, T> { let parent_id = parent_id?; match self.map.entry(parent_id) { gix_hashtable::hash_map::Entry::Vacant(entry) => { - let parent = match try_lookup(&parent_id, &mut self.find, self.cache.as_ref(), &mut self.parent_buf) - .map_err(|err| insert_parents::Error::Lookup(lookup::existing::Error::Find(err)))? - { + let parent = match try_lookup(&parent_id, &*self.find, self.cache.as_ref(), &mut self.parent_buf)? { Some(p) => p, None => continue, // skip missing objects, this is due to shallow clones for instance. }; @@ -114,7 +145,7 @@ impl<'find, T> Graph<'find, T> { /// Initialization impl<'find, T> Graph<'find, T> { - /// Create a new instance with `find` to retrieve commits and optionally `cache` to accelerate commit access. + /// Create a new instance with `objects` to retrieve commits and optionally `cache` to accelerate commit access. /// /// ### Performance /// @@ -122,18 +153,9 @@ impl<'find, T> Graph<'find, T> { /// most recently used commits. /// Furthermore, **none-existing commits should not trigger the pack-db to be refreshed.** Otherwise, performance may be sub-optimal /// in shallow repositories as running into non-existing commits will trigger a refresh of the `packs` directory. - pub fn new(find: Find, cache: impl Into>) -> Self - where - Find: for<'a> FnMut( - &gix_hash::oid, - &'a mut Vec, - ) -> Result< - Option>, - Box, - > + 'find, - { + pub fn new(objects: impl gix_object::Find + 'find, cache: impl Into>) -> Self { Graph { - find: Box::new(find), + find: Box::new(objects), cache: cache.into(), map: gix_hashtable::HashMap::default(), buf: Vec::new(), @@ -154,10 +176,10 @@ impl<'find, T> Graph<'find, Commit> { id: gix_hash::ObjectId, new_data: impl FnOnce() -> T, update_data: impl FnOnce(&mut T), - ) -> Result>, lookup::commit::Error> { + ) -> Result>, try_lookup_or_insert_default::Error> { match self.map.entry(id) { gix_hashtable::hash_map::Entry::Vacant(entry) => { - let res = try_lookup(&id, &mut self.find, self.cache.as_ref(), &mut self.buf)?; + let res = try_lookup(&id, &*self.find, self.cache.as_ref(), &mut self.buf)?; let commit = match res { None => return Ok(None), Some(commit) => commit, @@ -176,8 +198,8 @@ impl<'find, T> Graph<'find, Commit> { /// commit access impl<'find, T: Default> Graph<'find, Commit> { - /// Lookup `id` without failing if the commit doesn't exist, and assure that `id` is inserted into our set - /// with a commit and default data assigned. + /// Lookup `id` without failing if the commit doesn't exist or `id` isn't a commit, + /// and assure that `id` is inserted into our set with a commit and default data assigned. /// `update_data(data)` gets run either on existing or on new data. /// /// Note that none of the data updates happen if `id` didn't exist. @@ -188,14 +210,15 @@ impl<'find, T: Default> Graph<'find, Commit> { &mut self, id: gix_hash::ObjectId, update_data: impl FnOnce(&mut T), - ) -> Result>, lookup::commit::Error> { + ) -> Result>, try_lookup_or_insert_default::Error> { self.try_lookup_or_insert_commit_default(id, T::default, update_data) } } /// Lazy commit access impl<'find, T> Graph<'find, T> { - /// Lookup `id` without failing if the commit doesn't exist, and assure that `id` is inserted into our set + /// Lookup `id` without failing if the commit doesn't exist or `id` isn't a commit, + /// and assure that `id` is inserted into our set /// with a `default` value assigned to it. /// `update_data(data)` gets run either on existing or no new data. /// Return the commit when done. @@ -209,8 +232,8 @@ impl<'find, T> Graph<'find, T> { id: gix_hash::ObjectId, default: impl FnOnce() -> T, update_data: impl FnOnce(&mut T), - ) -> Result>, lookup::Error> { - let res = try_lookup(&id, &mut self.find, self.cache.as_ref(), &mut self.buf)?; + ) -> Result>, try_lookup_or_insert_default::Error> { + let res = try_lookup(&id, &*self.find, self.cache.as_ref(), &mut self.buf)?; Ok(res.map(|commit| { match self.map.entry(id) { gix_hashtable::hash_map::Entry::Vacant(entry) => { @@ -226,25 +249,31 @@ impl<'find, T> Graph<'find, T> { })) } - /// Try to lookup `id` and return a handle to it for accessing its data, but don't fail if the commit doesn't exist. + /// Try to lookup `id` and return a handle to it for accessing its data, but don't fail if the commit doesn't exist + /// or isn't a commit. /// /// It's possible that commits don't exist if the repository is shallow. - pub fn try_lookup(&mut self, id: &gix_hash::oid) -> Result>, lookup::Error> { - try_lookup(id, &mut self.find, self.cache.as_ref(), &mut self.buf) + pub fn try_lookup( + &mut self, + id: &gix_hash::oid, + ) -> Result>, gix_object::find::existing_iter::Error> { + try_lookup(id, &*self.find, self.cache.as_ref(), &mut self.buf) } - /// Lookup `id` and return a handle to it, or fail if it doesn't exist. - pub fn lookup(&mut self, id: &gix_hash::oid) -> Result, lookup::existing::Error> { - self.try_lookup(id)?.ok_or(lookup::existing::Error::Missing) + /// Lookup `id` and return a handle to it, or fail if it doesn't exist or is no commit. + pub fn lookup(&mut self, id: &gix_hash::oid) -> Result, gix_object::find::existing_iter::Error> { + Ok(self + .try_lookup(id)? + .ok_or(gix_object::find::existing_iter::Error::NotFound { oid: id.to_owned() })?) } } fn try_lookup<'graph>( id: &gix_hash::oid, - find: &mut Box>, + objects: &dyn gix_object::Find, cache: Option<&'graph gix_commitgraph::Graph>, buf: &'graph mut Vec, -) -> Result>, lookup::Error> { +) -> Result>, gix_object::find::existing_iter::Error> { if let Some(cache) = cache { if let Some(pos) = cache.lookup(id) { return Ok(Some(LazyCommit { @@ -253,12 +282,17 @@ fn try_lookup<'graph>( } } #[allow(clippy::manual_map)] - Ok(match find(id, buf)? { - Some(_) => Some(LazyCommit { - backing: Either::Left(buf), - }), - None => None, - }) + Ok( + match objects + .try_find(id, buf) + .map_err(gix_object::find::existing_iter::Error::Find)? + { + Some(data) => data.kind.is_commit().then_some(LazyCommit { + backing: Either::Left(buf), + }), + None => None, + }, + ) } impl<'a, 'find, T> Index<&'a gix_hash::oid> for Graph<'find, T> { @@ -316,13 +350,6 @@ pub struct LazyCommit<'graph> { backing: Either<&'graph [u8], (&'graph gix_commitgraph::Graph, gix_commitgraph::Position)>, } -pub(crate) type FindFn<'find> = dyn for<'a> FnMut( - &gix_hash::oid, - &'a mut Vec, - ) - -> Result>, Box> - + 'find; - enum Either { Left(T), Right(U), diff --git a/gix-revwalk/src/lib.rs b/gix-revwalk/src/lib.rs index 9348d9e162a..67bfc6dea6b 100644 --- a/gix-revwalk/src/lib.rs +++ b/gix-revwalk/src/lib.rs @@ -27,7 +27,7 @@ /// everything related to commit traversal in our own hashmap. pub struct Graph<'find, T> { /// A way to resolve a commit from the object database. - find: Box>, + find: Box, /// A way to speedup commit access, essentially a multi-file commit database. cache: Option, /// The set of cached commits that we have seen once, along with data associated with them.