diff --git a/gitoxide-core/src/hours/mod.rs b/gitoxide-core/src/hours/mod.rs index 2a814587349..0418ef1f01e 100644 --- a/gitoxide-core/src/hours/mod.rs +++ b/gitoxide-core/src/hours/mod.rs @@ -140,7 +140,7 @@ where let mut skipped_merge_commits = 0; const CHUNK_SIZE: usize = 50; let mut chunk = Vec::with_capacity(CHUNK_SIZE); - let mut commit_iter = commit_id.ancestors(|oid, buf| repo.objects.find_commit_iter(oid, buf)); + let mut commit_iter = commit_id.ancestors(&repo.objects); let mut is_shallow = false; while let Some(c) = commit_iter.next() { progress.inc(); @@ -175,7 +175,7 @@ where } commit_idx += 1; } - Err(gix::traverse::commit::ancestors::Error::FindExisting { .. }) => { + Err(gix::traverse::commit::ancestors::Error::Find { .. }) => { is_shallow = true; break; } diff --git a/gitoxide-core/src/index/checkout.rs b/gitoxide-core/src/index/checkout.rs index f7f31432c7d..635422c6219 100644 --- a/gitoxide-core/src/index/checkout.rs +++ b/gitoxide-core/src/index/checkout.rs @@ -4,7 +4,8 @@ use std::{ }; use anyhow::bail; -use gix::{odb::FindExt, worktree::state::checkout, NestedProgress, Progress}; +use gix::objs::find::Error; +use gix::{worktree::state::checkout, NestedProgress, Progress}; use crate::{ index, @@ -89,20 +90,9 @@ pub fn checkout_exclusive( Some(repo) => gix::worktree::state::checkout( &mut index, dest_directory, - { - let objects = repo.objects.into_arc()?; - move |oid, buf| { - objects.find_blob(oid, buf).ok(); - if empty_files { - // We always want to query the ODB here… - objects.find_blob(oid, buf)?; - buf.clear(); - // …but write nothing - Ok(gix::objs::BlobRef { data: buf }) - } else { - objects.find_blob(oid, buf) - } - } + EmptyOrDb { + empty_files, + db: repo.objects.into_arc()?, }, &files, &bytes, @@ -112,10 +102,7 @@ pub fn checkout_exclusive( None => gix::worktree::state::checkout( &mut index, dest_directory, - |_, buf| { - buf.clear(); - Ok(gix::objs::BlobRef { data: buf }) - }, + Empty, &files, &bytes, should_interrupt, @@ -184,3 +171,41 @@ pub fn checkout_exclusive( } Ok(()) } + +#[derive(Clone)] +struct EmptyOrDb { + empty_files: bool, + db: Find, +} + +impl gix::objs::Find for EmptyOrDb +where + Find: gix::objs::Find, +{ + fn try_find<'a>(&self, id: &gix::oid, buf: &'a mut Vec) -> Result>, Error> { + if self.empty_files { + // We always want to query the ODB here… + let Some(kind) = self.db.try_find(id, buf)?.map(|d| d.kind) else { + return Ok(None); + }; + buf.clear(); + // …but write nothing + Ok(Some(gix::objs::Data { kind, data: buf })) + } else { + self.db.try_find(id, buf) + } + } +} + +#[derive(Clone)] +struct Empty; + +impl gix::objs::Find for Empty { + fn try_find<'a>(&self, _id: &gix::oid, buffer: &'a mut Vec) -> Result>, Error> { + buffer.clear(); + Ok(Some(gix::objs::Data { + kind: gix::object::Kind::Blob, + data: buffer, + })) + } +} diff --git a/gitoxide-core/src/index/mod.rs b/gitoxide-core/src/index/mod.rs index 4b6b50b36ef..0457856dbd6 100644 --- a/gitoxide-core/src/index/mod.rs +++ b/gitoxide-core/src/index/mod.rs @@ -35,7 +35,7 @@ pub fn verify( let file = parse_file(index_path, object_hash)?; file.verify_integrity()?; file.verify_entries()?; - file.verify_extensions(false, gix::index::verify::extensions::no_find)?; + file.verify_extensions(false, gix::objs::find::Never)?; #[cfg_attr(not(feature = "serde"), allow(irrefutable_let_patterns))] if let crate::OutputFormat::Human = format { writeln!(out, "OK").ok(); diff --git a/gitoxide-core/src/pack/create.rs b/gitoxide-core/src/pack/create.rs index 6b3d3d65d8c..f8141e357d6 100644 --- a/gitoxide-core/src/pack/create.rs +++ b/gitoxide-core/src/pack/create.rs @@ -2,13 +2,7 @@ use std::{ffi::OsStr, io, path::Path, str::FromStr, time::Instant}; use anyhow::anyhow; use gix::{ - hash, - hash::ObjectId, - interrupt, - objs::bstr::ByteVec, - odb::{pack, pack::FindExt}, - parallel::InOrderIter, - prelude::Finalize, + hash, hash::ObjectId, interrupt, objs::bstr::ByteVec, odb::pack, parallel::InOrderIter, prelude::Finalize, progress, traverse, Count, NestedProgress, Progress, }; @@ -136,12 +130,9 @@ where .collect::, _>>()?; let handle = repo.objects.into_shared_arc().to_cache_arc(); let iter = Box::new( - traverse::commit::Ancestors::new(tips, traverse::commit::ancestors::State::default(), { - let handle = handle.clone(); - move |oid, buf| handle.find_commit_iter(oid, buf).map(|t| t.0) - }) - .map(|res| res.map_err(|err| Box::new(err) as Box<_>).map(|c| c.id)) - .inspect(move |_| progress.inc()), + traverse::commit::Ancestors::new(tips, traverse::commit::ancestors::State::default(), handle.clone()) + .map(|res| res.map_err(|err| Box::new(err) as Box<_>).map(|c| c.id)) + .inspect(move |_| progress.inc()), ); (handle, iter) } diff --git a/gitoxide-core/src/pack/index.rs b/gitoxide-core/src/pack/index.rs index 814f43d0697..7838f6337fa 100644 --- a/gitoxide-core/src/pack/index.rs +++ b/gitoxide-core/src/pack/index.rs @@ -95,7 +95,7 @@ pub fn from_pack( directory, &mut progress, ctx.should_interrupt, - None, + None::, options, ) } @@ -105,7 +105,7 @@ pub fn from_pack( directory, &mut progress, ctx.should_interrupt, - None, + None::, options, ), } diff --git a/gitoxide-core/src/pack/receive.rs b/gitoxide-core/src/pack/receive.rs index 95f549b1583..785267ea2c5 100644 --- a/gitoxide-core/src/pack/receive.rs +++ b/gitoxide-core/src/pack/receive.rs @@ -396,7 +396,7 @@ fn receive_pack_blocking( directory.take().as_deref(), &mut progress, &ctx.should_interrupt, - None, + None::, options, ) .map_err(|err| io::Error::new(io::ErrorKind::Other, err))?; diff --git a/gitoxide-core/src/query/engine/update.rs b/gitoxide-core/src/query/engine/update.rs index bae1a6d3daa..0e8281cf950 100644 --- a/gitoxide-core/src/query/engine/update.rs +++ b/gitoxide-core/src/query/engine/update.rs @@ -1,3 +1,4 @@ +use std::cell::RefCell; use std::{ convert::Infallible, sync::atomic::{AtomicUsize, Ordering}, @@ -5,11 +6,11 @@ use std::{ }; use anyhow::{anyhow, bail}; +use gix::objs::find::Error; use gix::{ bstr::{BStr, BString, ByteSlice}, features::progress, object::tree::diff::rewrites::CopySource, - odb::FindExt, parallel::{InOrderIter, SequenceId}, prelude::ObjectIdExt, Count, Progress, @@ -150,13 +151,18 @@ pub fn update( }); r }; + + #[derive(Clone)] struct Task { commit: gix::hash::ObjectId, parent_commit: Option, compute_stats: bool, } + + type Packet = (SequenceId, Vec); + let (tx_tree_ids, stat_threads) = { - let (tx, rx) = crossbeam_channel::unbounded::<(SequenceId, Vec)>(); + let (tx, rx) = crossbeam_channel::unbounded::(); let stat_workers = (0..threads) .map(|_| { scope.spawn({ @@ -304,46 +310,94 @@ pub fn update( }; drop(tx_stats); - const CHUNK_SIZE: usize = 50; - let mut chunk = Vec::with_capacity(CHUNK_SIZE); - let mut chunk_id: SequenceId = 0; - let commit_iter = gix::interrupt::Iter::new( - commit_id.ancestors(|oid, buf| -> Result<_, gix::object::find::existing::Error> { - let obj = repo.objects.find(oid, buf)?; - traverse_progress.inc(); - if known_commits.binary_search(&oid.to_owned()).is_err() { + #[derive(Clone)] + struct Db<'a, Find: Clone> { + inner: &'a Find, + progress: &'a dyn gix::progress::Count, + chunk: std::cell::RefCell>, + chunk_id: std::cell::RefCell, + chunk_size: usize, + tx: crossbeam_channel::Sender, + known_commits: &'a [gix::ObjectId], + } + + impl<'a, Find> Db<'a, Find> + where + Find: gix::prelude::Find + Clone, + { + fn new( + inner: &'a Find, + progress: &'a dyn gix::progress::Count, + chunk_size: usize, + tx: crossbeam_channel::Sender, + known_commits: &'a [gix::ObjectId], + ) -> Self { + Self { + inner, + progress, + known_commits, + tx, + chunk_size, + chunk_id: 0.into(), + chunk: RefCell::new(Vec::with_capacity(chunk_size)), + } + } + + fn send_last_chunk(self) { + self.tx.send((self.chunk_id.into_inner(), self.chunk.into_inner())).ok(); + } + } + + impl<'a, Find> gix::prelude::Find for Db<'a, Find> + where + Find: gix::prelude::Find + Clone, + { + fn try_find<'b>(&self, id: &gix::oid, buf: &'b mut Vec) -> Result>, Error> { + let obj = self.inner.try_find(id, buf)?; + let Some(obj) = obj else { return Ok(None) }; + if !obj.kind.is_commit() { + return Ok(None); + } + + self.progress.inc(); + if self.known_commits.binary_search(&id.to_owned()).is_err() { let res = { let mut parents = gix::objs::CommitRefIter::from_bytes(obj.data).parent_ids(); - let res = parents.next().map(|first_parent| (Some(first_parent), oid.to_owned())); + let res = parents.next().map(|first_parent| (Some(first_parent), id.to_owned())); match parents.next() { Some(_) => None, None => res, } }; if let Some((first_parent, commit)) = res { - chunk.push(Task { + self.chunk.borrow_mut().push(Task { parent_commit: first_parent, commit, compute_stats: true, }); } else { - chunk.push(Task { + self.chunk.borrow_mut().push(Task { parent_commit: None, - commit: oid.to_owned(), + commit: id.to_owned(), compute_stats: false, }); } - if chunk.len() == CHUNK_SIZE { - tx_tree_ids - .send((chunk_id, std::mem::replace(&mut chunk, Vec::with_capacity(CHUNK_SIZE)))) + if self.chunk.borrow().len() == self.chunk_size { + self.tx + .send(( + *self.chunk_id.borrow(), + std::mem::replace(&mut self.chunk.borrow_mut(), Vec::with_capacity(self.chunk_size)), + )) .ok(); - chunk_id += 1; + *self.chunk_id.borrow_mut() += 1; } } - Ok(gix::objs::CommitRefIter::from_bytes(obj.data)) - }), - || anyhow!("Cancelled by user"), - ); + Ok(Some(obj)) + } + } + + let db = Db::new(&repo.objects, &traverse_progress, 50, tx_tree_ids, &known_commits); + let commit_iter = gix::interrupt::Iter::new(commit_id.ancestors(&db), || anyhow!("Cancelled by user")); let mut commits = Vec::new(); for c in commit_iter { match c?.map(|c| c.id) { @@ -354,15 +408,14 @@ pub fn update( break; } } - Err(gix::traverse::commit::ancestors::Error::FindExisting { .. }) => { + Err(gix::traverse::commit::ancestors::Error::Find { .. }) => { writeln!(err, "shallow repository - commit history is truncated").ok(); break; } Err(err) => return Err(err.into()), }; } - tx_tree_ids.send((chunk_id, chunk)).ok(); - drop(tx_tree_ids); + db.send_last_chunk(); let saw_new_commits = !commits.is_empty(); if saw_new_commits { traverse_progress.show_throughput(start); diff --git a/gitoxide-core/src/repository/status.rs b/gitoxide-core/src/repository/status.rs index e7cbea20e54..cda8d1d5bd2 100644 --- a/gitoxide-core/src/repository/status.rs +++ b/gitoxide-core/src/repository/status.rs @@ -2,7 +2,6 @@ use crate::OutputFormat; use anyhow::{bail, Context}; use gix::bstr::{BStr, BString}; use gix::index::Entry; -use gix::prelude::FindExt; use gix::Progress; use gix_status::index_as_worktree::traits::FastEq; use gix_status::index_as_worktree::{Change, Conflict, EntryStatus}; @@ -80,10 +79,7 @@ pub fn show( &mut printer, FastEq, Submodule, - { - let odb = repo.objects.clone().into_arc()?; - move |id, buf| odb.find_blob(id, buf) - }, + repo.objects.clone().into_arc()?, &mut progress, pathspec.detach()?, repo.filter_pipeline(Some(gix::hash::ObjectId::empty_tree(repo.object_hash())))? diff --git a/gitoxide-core/src/repository/verify.rs b/gitoxide-core/src/repository/verify.rs index 3f033fae5e4..98b987e4704 100644 --- a/gitoxide-core/src/repository/verify.rs +++ b/gitoxide-core/src/repository/verify.rs @@ -43,11 +43,7 @@ pub fn integrity( if let Some(index) = repo.worktree().map(|wt| wt.index()).transpose()? { index.verify_integrity()?; index.verify_entries()?; - index.verify_extensions(true, { - use gix::odb::FindExt; - let objects = repo.objects; - move |oid, buf: &mut Vec| objects.find_tree_iter(oid, buf).ok() - })?; + index.verify_extensions(true, repo.objects)?; progress.info(format!("Index at '{}' OK", index.path().display())); } match output_statistics { diff --git a/gix-archive/tests/archive.rs b/gix-archive/tests/archive.rs index 3eff24c7570..3e43f755f9b 100644 --- a/gix-archive/tests/archive.rs +++ b/gix-archive/tests/archive.rs @@ -9,7 +9,6 @@ mod from_tree { use gix_archive::Format; use gix_attributes::glob::pattern::Case; use gix_object::tree::EntryMode; - use gix_odb::FindExt; use gix_testtools::bstr::ByteSlice; use gix_worktree::stack::state::attributes::Source; @@ -230,14 +229,11 @@ mod from_tree { let (dir, head_tree, odb, mut cache) = basic()?; let mut stream = gix_worktree_stream::from_tree( head_tree, - { - let odb = odb.clone(); - move |id, buf| odb.find(id, buf) - }, + odb.clone(), noop_pipeline(), move |rela_path, mode, attrs| { cache - .at_entry(rela_path, mode.is_tree().into(), |id, buf| odb.find_blob(id, buf)) + .at_entry(rela_path, mode.is_tree().into(), &odb) .map(|entry| entry.matching_attributes(attrs)) .map(|_| ()) }, diff --git a/gix-diff/src/tree/changes.rs b/gix-diff/src/tree/changes.rs index 16e8f7873ab..9da3bb568b4 100644 --- a/gix-diff/src/tree/changes.rs +++ b/gix-diff/src/tree/changes.rs @@ -1,7 +1,7 @@ use std::{borrow::BorrowMut, collections::VecDeque}; -use gix_hash::{oid, ObjectId}; use gix_object::tree::EntryRef; +use gix_object::FindExt; use crate::{ tree, @@ -12,11 +12,8 @@ use crate::{ #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { - #[error("The object {oid} referenced by the tree or the tree itself was not found in the database")] - FindExisting { - oid: ObjectId, - source: Box, - }, + #[error(transparent)] + Find(#[from] gix_object::find::existing_iter::Error), #[error("The delegate cancelled the operation")] Cancelled, #[error(transparent)] @@ -24,12 +21,12 @@ pub enum Error { } impl<'a> tree::Changes<'a> { - /// Calculate the changes that would need to be applied to `self` to get `other`. + /// Calculate the changes that would need to be applied to `self` to get `other` using `objects` to obtain objects as needed for traversal. /// /// * The `state` maybe owned or mutably borrowed to allow reuses allocated data structures through multiple runs. /// * `locate` is a function `f(object_id, &mut buffer) -> Option` to return a `TreeIter` for the given object id backing /// its data in the given buffer. Returning `None` is unexpected as these trees are obtained during iteration, and in a typical - /// database errors are not expected either which is why the error case is omitted. To allow proper error reporting, [`Error::FindExisting`] + /// database errors are not expected either which is why the error case is omitted. To allow proper error reporting, [`Error::Find`] /// should be converted into a more telling error. /// * `delegate` will receive the computed changes, see the [`Visit`][`tree::Visit`] trait for more information on what to expect. /// @@ -47,16 +44,14 @@ impl<'a> tree::Changes<'a> { /// /// [git_cmp_c]: https://github.com/git/git/blob/311531c9de557d25ac087c1637818bd2aad6eb3a/tree-diff.c#L49:L65 /// [git_cmp_rs]: https://github.com/Byron/gitoxide/blob/a4d5f99c8dc99bf814790928a3bf9649cd99486b/gix-object/src/mutable/tree.rs#L52-L55 - pub fn needed_to_obtain( + pub fn needed_to_obtain( mut self, other: gix_object::TreeRefIter<'_>, mut state: StateMut, - mut find: FindFn, + objects: impl gix_object::Find, delegate: &mut R, ) -> Result<(), Error> where - FindFn: for<'b> FnMut(&oid, &'b mut Vec) -> Result, E>, - E: std::error::Error + Send + Sync + 'static, R: tree::Visit, StateMut: BorrowMut, { @@ -77,28 +72,16 @@ impl<'a> tree::Changes<'a> { match state.trees.pop_front() { Some((None, Some(rhs))) => { delegate.pop_front_tracked_path_and_set_current(); - rhs_entries = peekable(find(&rhs, &mut state.buf2).map_err(|err| Error::FindExisting { - oid: rhs, - source: err.into(), - })?); + rhs_entries = peekable(objects.find_tree_iter(&rhs, &mut state.buf2)?); } Some((Some(lhs), Some(rhs))) => { delegate.pop_front_tracked_path_and_set_current(); - lhs_entries = peekable(find(&lhs, &mut state.buf1).map_err(|err| Error::FindExisting { - oid: lhs, - source: err.into(), - })?); - rhs_entries = peekable(find(&rhs, &mut state.buf2).map_err(|err| Error::FindExisting { - oid: rhs, - source: err.into(), - })?); + lhs_entries = peekable(objects.find_tree_iter(&lhs, &mut state.buf1)?); + rhs_entries = peekable(objects.find_tree_iter(&rhs, &mut state.buf2)?); } Some((Some(lhs), None)) => { delegate.pop_front_tracked_path_and_set_current(); - lhs_entries = peekable(find(&lhs, &mut state.buf1).map_err(|err| Error::FindExisting { - oid: lhs, - source: err.into(), - })?); + lhs_entries = peekable(objects.find_tree_iter(&lhs, &mut state.buf1)?); } Some((None, None)) => unreachable!("BUG: it makes no sense to fill the stack with empties"), None => return Ok(()), diff --git a/gix-diff/tests/tree/mod.rs b/gix-diff/tests/tree/mod.rs index 80fa81ecae3..f853ff259dd 100644 --- a/gix-diff/tests/tree/mod.rs +++ b/gix-diff/tests/tree/mod.rs @@ -61,11 +61,7 @@ mod changes { gix_diff::tree::Changes::from(lhs_tree).needed_to_obtain( rhs_tree, gix_diff::tree::State::default(), - |oid, buf| { - use gix_odb::pack::FindExt; - db.find(oid, buf) - .map(|obj| obj.0.try_into_tree_iter().expect("only called for trees")) - }, + db, &mut recorder, )?; Ok(recorder.records) @@ -108,11 +104,7 @@ mod changes { gix_diff::tree::Changes::from(previous_tree).needed_to_obtain( current_tree, &mut gix_diff::tree::State::default(), - |oid, buf| { - use gix_odb::pack::FindExt; - db.find(oid, buf) - .map(|(obj, _)| obj.try_into_tree_iter().expect("only called for trees")) - }, + db, &mut recorder, )?; Ok(recorder.records) @@ -141,27 +133,24 @@ mod changes { let mut buf = Vec::new(); let head = head_of(db); - commit::Ancestors::new(Some(head), commit::ancestors::State::default(), |oid, buf| { - use gix_odb::FindExt; - db.find_commit_iter(oid, buf) - }) - .collect::, _>>() - .expect("valid iteration") - .into_iter() - .map(|c| { - use gix_odb::FindExt; - ( - db.find_commit(&c.id, &mut buf) - .unwrap() - .message - .trim() - .to_str_lossy() - .into_owned(), - c.id, - ) - }) - .rev() - .collect() + commit::Ancestors::new(Some(head), commit::ancestors::State::default(), &db) + .collect::, _>>() + .expect("valid iteration") + .into_iter() + .map(|c| { + use gix_object::FindExt; + ( + db.find_commit(&c.id, &mut buf) + .unwrap() + .message + .trim() + .to_str_lossy() + .into_owned(), + c.id, + ) + }) + .rev() + .collect() } #[test] diff --git a/gix-filter/tests/pipeline/convert_to_git.rs b/gix-filter/tests/pipeline/convert_to_git.rs index cfa747e2bdd..491c6043d6a 100644 --- a/gix-filter/tests/pipeline/convert_to_git.rs +++ b/gix-filter/tests/pipeline/convert_to_git.rs @@ -53,9 +53,7 @@ fn all_stages_mean_streaming_is_impossible() -> gix_testtools::Result { Path::new("any.txt"), &mut |path, attrs| { cache - .at_entry(path, Some(false), |_oid, _buf| -> Result<_, std::convert::Infallible> { - unreachable!("index access disabled") - }) + .at_entry(path, Some(false), gix_object::find::Never) .expect("cannot fail") .matching_attributes(attrs); }, @@ -84,9 +82,7 @@ fn only_driver_means_streaming_is_possible() -> gix_testtools::Result { Path::new("subdir/doesnot/matter/any.txt"), &mut |path, attrs| { cache - .at_entry(path, Some(false), |_oid, _buf| -> Result<_, std::convert::Infallible> { - unreachable!("index access disabled") - }) + .at_entry(path, Some(false), gix_object::find::Never) .expect("cannot fail") .matching_attributes(attrs); }, @@ -116,9 +112,7 @@ fn no_filter_means_reader_is_returned_unchanged() -> gix_testtools::Result { Path::new("other.txt"), &mut |path, attrs| { cache - .at_entry(path, Some(false), |_oid, _buf| -> Result<_, std::convert::Infallible> { - unreachable!("index access disabled") - }) + .at_entry(path, Some(false), gix_object::find::Never) .expect("cannot fail") .matching_attributes(attrs); }, diff --git a/gix-filter/tests/pipeline/convert_to_worktree.rs b/gix-filter/tests/pipeline/convert_to_worktree.rs index bfc15cd3e2f..8798f5a22c7 100644 --- a/gix-filter/tests/pipeline/convert_to_worktree.rs +++ b/gix-filter/tests/pipeline/convert_to_worktree.rs @@ -21,9 +21,7 @@ fn all_stages() -> gix_testtools::Result { "any.txt".into(), &mut |path, attrs| { cache - .at_entry(path, Some(false), |_oid, _buf| -> Result<_, std::convert::Infallible> { - unreachable!("index access disabled") - }) + .at_entry(path, Some(false), gix_object::find::Never) .expect("cannot fail") .matching_attributes(attrs); }, @@ -56,9 +54,7 @@ fn all_stages_no_filter() -> gix_testtools::Result { "other.txt".into(), &mut |path, attrs| { cache - .at_entry(path, Some(false), |_oid, _buf| -> Result<_, std::convert::Infallible> { - unreachable!("index access disabled") - }) + .at_entry(path, Some(false), gix_object::find::Never) .expect("cannot fail") .matching_attributes(attrs); }, @@ -90,9 +86,7 @@ fn no_filter() -> gix_testtools::Result { "other.txt".into(), &mut |path, attrs| { cache - .at_entry(path, Some(false), |_oid, _buf| -> Result<_, std::convert::Infallible> { - unreachable!("index access disabled") - }) + .at_entry(path, Some(false), gix_object::find::Never) .expect("cannot fail") .matching_attributes(attrs); }, diff --git a/gix-index/src/extension/tree/verify.rs b/gix-index/src/extension/tree/verify.rs index 793f3132500..b0faae2db88 100644 --- a/gix-index/src/extension/tree/verify.rs +++ b/gix-index/src/extension/tree/verify.rs @@ -1,6 +1,7 @@ use std::cmp::Ordering; use bstr::{BString, ByteSlice}; +use gix_object::FindExt; use crate::extension::Tree; @@ -14,8 +15,8 @@ pub enum Error { entry_id: gix_hash::ObjectId, name: BString, }, - #[error("The tree with id {oid} wasn't found in the object database")] - TreeNodeNotFound { oid: gix_hash::ObjectId }, + #[error(transparent)] + TreeNodeNotFound(#[from] gix_object::find::existing_iter::Error), #[error("The tree with id {oid} should have {expected_childcount} children, but its cached representation had {actual_childcount} of them")] TreeNodeChildcountMismatch { oid: gix_hash::ObjectId, @@ -39,20 +40,14 @@ pub enum Error { } impl Tree { - /// - pub fn verify(&self, use_find: bool, mut find: F) -> Result<(), Error> - where - F: for<'a> FnMut(&gix_hash::oid, &'a mut Vec) -> Option>, - { - fn verify_recursive( + /// Validate the correctness of this instance. If `use_objects` is true, then `objects` will be used to access all objects. + pub fn verify(&self, use_objects: bool, objects: impl gix_object::Find) -> Result<(), Error> { + fn verify_recursive( parent_id: gix_hash::ObjectId, children: &[Tree], - mut find_buf: Option<&mut Vec>, - find: &mut F, - ) -> Result, Error> - where - F: for<'a> FnMut(&gix_hash::oid, &'a mut Vec) -> Option>, - { + mut object_buf: Option<&mut Vec>, + objects: &impl gix_object::Find, + ) -> Result, Error> { if children.is_empty() { return Ok(None); } @@ -71,8 +66,8 @@ impl Tree { } prev = Some(child); } - if let Some(buf) = find_buf.as_mut() { - let tree_entries = find(&parent_id, buf).ok_or(Error::TreeNodeNotFound { oid: parent_id })?; + if let Some(buf) = object_buf.as_mut() { + let tree_entries = objects.find_tree_iter(&parent_id, buf)?; let mut num_entries = 0; for entry in tree_entries .filter_map(Result::ok) @@ -99,7 +94,8 @@ impl Tree { for child in children { // This is actually needed here as it's a mut ref, which isn't copy. We do a re-borrow here. #[allow(clippy::needless_option_as_deref)] - let actual_num_entries = verify_recursive(child.id, &child.children, find_buf.as_deref_mut(), find)?; + let actual_num_entries = + verify_recursive(child.id, &child.children, object_buf.as_deref_mut(), objects)?; if let Some((actual, num_entries)) = actual_num_entries.zip(child.num_entries) { if actual > num_entries { return Err(Error::EntriesCount { @@ -120,7 +116,7 @@ impl Tree { } let mut buf = Vec::new(); - let declared_entries = verify_recursive(self.id, &self.children, use_find.then_some(&mut buf), &mut find)?; + let declared_entries = verify_recursive(self.id, &self.children, use_objects.then_some(&mut buf), &objects)?; if let Some((actual, num_entries)) = declared_entries.zip(self.num_entries) { if actual > num_entries { return Err(Error::EntriesCount { diff --git a/gix-index/src/init.rs b/gix-index/src/init.rs index ec09af538f0..b49dac51783 100644 --- a/gix-index/src/init.rs +++ b/gix-index/src/init.rs @@ -4,7 +4,7 @@ mod from_tree { use bstr::{BStr, BString, ByteSlice, ByteVec}; use gix_object::{ tree::{self, EntryMode}, - TreeRefIter, + FindExt, }; use gix_traverse::tree::{breadthfirst, visit::Action, Visit}; @@ -32,19 +32,18 @@ mod from_tree { } } /// Create an index [`State`] by traversing `tree` recursively, accessing sub-trees - /// with `find`. + /// with `objects`. /// /// **No extension data is currently produced**. - pub fn from_tree(tree: &gix_hash::oid, mut find: Find) -> Result + pub fn from_tree(tree: &gix_hash::oid, objects: Find) -> Result where - Find: for<'a> FnMut(&gix_hash::oid, &'a mut Vec) -> Option>, + Find: gix_object::Find, { let _span = gix_features::trace::coarse!("gix_index::State::from_tree()"); let mut buf = Vec::new(); - let root = find(tree, &mut buf).ok_or(breadthfirst::Error::NotFound { oid: tree.into() })?; - + let root = objects.find_tree_iter(tree, &mut buf)?; let mut delegate = CollectEntries::new(); - breadthfirst(root, breadthfirst::State::default(), &mut find, &mut delegate)?; + breadthfirst(root, breadthfirst::State::default(), &objects, &mut delegate)?; let CollectEntries { mut entries, diff --git a/gix-index/src/verify.rs b/gix-index/src/verify.rs index 7782cccbcba..776f13b3c5f 100644 --- a/gix-index/src/verify.rs +++ b/gix-index/src/verify.rs @@ -25,11 +25,6 @@ pub mod entries { pub mod extensions { use crate::extension; - /// An implementation of a `find` function that never finds or returns any object, a no-op. - pub fn no_find<'a>(_: &gix_hash::oid, _: &'a mut Vec) -> Option> { - None - } - /// The error returned by [`State::verify_extensions()`][crate::State::verify_extensions()]. #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] @@ -61,12 +56,9 @@ impl State { Ok(()) } - /// Note: `find` cannot be `Option` as we can't call it with a closure then due to the indirection through `Some`. - pub fn verify_extensions(&self, use_find: bool, find: F) -> Result<(), extensions::Error> - where - F: for<'a> FnMut(&gix_hash::oid, &'a mut Vec) -> Option>, - { - self.tree().map(|t| t.verify(use_find, find)).transpose()?; + /// Note: `objects` cannot be `Option` as we can't call it with a closure then due to the indirection through `Some`. + pub fn verify_extensions(&self, use_find: bool, objects: impl gix_object::Find) -> Result<(), extensions::Error> { + self.tree().map(|t| t.verify(use_find, objects)).transpose()?; // TODO: verify links by running the whole set of tests on the index // - do that once we load it as well, or maybe that's lazy loaded? Too many questions for now. Ok(()) diff --git a/gix-index/tests/index/file/read.rs b/gix-index/tests/index/file/read.rs index ae2b7ace9dc..0745bd8e121 100644 --- a/gix-index/tests/index/file/read.rs +++ b/gix-index/tests/index/file/read.rs @@ -11,9 +11,7 @@ use crate::{hex_to_id, index::Fixture, loose_file_path}; fn verify(index: gix_index::File) -> gix_index::File { index.verify_integrity().unwrap(); index.verify_entries().unwrap(); - index - .verify_extensions(false, gix_index::verify::extensions::no_find) - .unwrap(); + index.verify_extensions(false, gix::objs::find::Never).unwrap(); index } diff --git a/gix-index/tests/index/file/write.rs b/gix-index/tests/index/file/write.rs index b3c01b21de6..a01963c8e31 100644 --- a/gix-index/tests/index/file/write.rs +++ b/gix-index/tests/index/file/write.rs @@ -1,5 +1,5 @@ use filetime::FileTime; -use gix_index::{entry, extension, verify::extensions::no_find, write, write::Options, State, Version}; +use gix_index::{entry, extension, write, write::Options, State, Version}; use crate::index::Fixture::*; @@ -228,7 +228,7 @@ fn compare_states_against_baseline( fn compare_states(actual: &State, actual_version: Version, expected: &State, options: Options, fixture: &str) { actual.verify_entries().expect("valid"); - actual.verify_extensions(false, no_find).expect("valid"); + actual.verify_extensions(false, gix::objs::find::Never).expect("valid"); assert_eq!( actual.version(), diff --git a/gix-index/tests/index/init.rs b/gix-index/tests/index/init.rs index f9aa3c5812f..22595526605 100644 --- a/gix-index/tests/index/init.rs +++ b/gix-index/tests/index/init.rs @@ -1,5 +1,4 @@ -use gix::prelude::FindExt; -use gix_index::{verify::extensions::no_find, State}; +use gix_index::State; use gix_testtools::scripted_fixture_read_only_standalone; #[test] @@ -18,7 +17,7 @@ fn from_tree() -> crate::Result { let tree_id = repo.head_commit()?.tree_id()?; let expected_state = repo.index()?; - let actual_state = State::from_tree(&tree_id, |oid, buf| repo.objects.find_tree_iter(oid, buf).ok())?; + let actual_state = State::from_tree(&tree_id, &repo.objects)?; compare_states(&actual_state, &expected_state, fixture) } @@ -35,7 +34,7 @@ fn new() { fn compare_states(actual: &State, expected: &State, fixture: &str) { actual.verify_entries().expect("valid"); - actual.verify_extensions(false, no_find).expect("valid"); + actual.verify_extensions(false, gix::objs::find::Never).expect("valid"); assert_eq!( actual.entries().len(), diff --git a/gix-negotiate/src/lib.rs b/gix-negotiate/src/lib.rs index 24b76cf690c..bb488816fb3 100644 --- a/gix-negotiate/src/lib.rs +++ b/gix-negotiate/src/lib.rs @@ -144,4 +144,4 @@ pub trait Negotiator { } /// An error that happened during any of the methods on a [`Negotiator`]. -pub type Error = gix_revwalk::graph::lookup::commit::Error; +pub type Error = gix_revwalk::graph::try_lookup_or_insert_default::Error; diff --git a/gix-negotiate/tests/baseline/mod.rs b/gix-negotiate/tests/baseline/mod.rs index f4ba0449f7e..0026ce7cc8d 100644 --- a/gix-negotiate/tests/baseline/mod.rs +++ b/gix-negotiate/tests/baseline/mod.rs @@ -1,8 +1,8 @@ use std::cell::RefCell; use gix_negotiate::Algorithm; +use gix_object::FindExt; use gix_object::{bstr, bstr::ByteSlice}; -use gix_odb::{Find, FindExt}; use gix_ref::{file::ReferenceExt, store::WriteReflog}; #[test] @@ -35,10 +35,7 @@ fn run() -> crate::Result { .iter() .filter_map(|name| { refs.try_find(*name).expect("one tag per commit").map(|mut r| { - r.peel_to_id_in_place(&refs, &mut |id, buf| { - store.try_find(&id, buf).map(|d| d.map(|d| (d.kind, d.data))) - }) - .expect("works"); + r.peel_to_id_in_place(&refs, &store).expect("works"); r.target.into_id() }) }) @@ -59,14 +56,7 @@ fn run() -> crate::Result { let cache = use_cache .then(|| gix_commitgraph::at(store.store_ref().path().join("info")).ok()) .flatten(); - let mut graph = gix_revwalk::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_revwalk::Graph::new(&store, cache); let mut negotiator = algo.into_negotiator(); if debug { eprintln!("ALGO {algo_name} CASE {case}"); diff --git a/gix-object/src/find.rs b/gix-object/src/find.rs new file mode 100644 index 00000000000..271079eb412 --- /dev/null +++ b/gix-object/src/find.rs @@ -0,0 +1,79 @@ +/// The error type returned by the [`Find`](crate::Find) trait. +pub type Error = Box; +/// +pub mod existing { + use gix_hash::ObjectId; + + /// The error returned by the [`find(…)`][crate::FindExt::find()] trait methods. + #[derive(Debug, thiserror::Error)] + #[allow(missing_docs)] + pub enum Error { + #[error(transparent)] + Find(crate::find::Error), + #[error("An object with id {} could not be found", .oid)] + NotFound { oid: ObjectId }, + } +} + +/// +pub mod existing_object { + use gix_hash::ObjectId; + + /// The error returned by the various [`find_*()`][crate::FindExt::find_commit()] trait methods. + #[derive(Debug, thiserror::Error)] + #[allow(missing_docs)] + pub enum Error { + #[error(transparent)] + Find(crate::find::Error), + #[error("Could not decode object at {oid}")] + Decode { + oid: ObjectId, + source: crate::decode::Error, + }, + #[error("An object with id {oid} could not be found")] + NotFound { oid: ObjectId }, + #[error("Expected object of kind {expected} but got {actual} at {oid}")] + ObjectKind { + oid: ObjectId, + actual: crate::Kind, + expected: crate::Kind, + }, + } +} + +/// +pub mod existing_iter { + use gix_hash::ObjectId; + + /// The error returned by the various [`find_*_iter()`][crate::FindExt::find_commit_iter()] trait methods. + #[derive(Debug, thiserror::Error)] + #[allow(missing_docs)] + pub enum Error { + #[error(transparent)] + Find(crate::find::Error), + #[error("An object with id {oid} could not be found")] + NotFound { oid: ObjectId }, + #[error("Expected object of kind {expected} but got {actual} at {oid}")] + ObjectKind { + oid: ObjectId, + actual: crate::Kind, + expected: crate::Kind, + }, + } +} + +/// An implementation of all traits that never fails, but also never finds anything. +#[derive(Debug, Copy, Clone)] +pub struct Never; + +impl super::Find for Never { + fn try_find<'a>(&self, _id: &gix_hash::oid, _buffer: &'a mut Vec) -> Result>, Error> { + Ok(None) + } +} + +impl super::Exists for Never { + fn exists(&self, _id: &gix_hash::oid) -> bool { + false + } +} diff --git a/gix-object/src/lib.rs b/gix-object/src/lib.rs index a07502917e6..845ded59634 100644 --- a/gix-object/src/lib.rs +++ b/gix-object/src/lib.rs @@ -30,8 +30,11 @@ mod blob; /// pub mod data; +/// +pub mod find; + mod traits; -pub use traits::WriteTo; +pub use traits::{Exists, Find, FindExt, WriteTo}; pub mod encode; pub(crate) mod parse; @@ -50,7 +53,7 @@ pub enum Kind { Tag, } /// A chunk of any [`data`][BlobRef::data]. -#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] +#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct BlobRef<'a> { /// The bytes themselves. @@ -222,7 +225,7 @@ pub struct TreeRef<'a> { } /// A directory snapshot containing files (blobs), directories (trees) and submodules (commits), lazily evaluated. -#[derive(Default, PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] +#[derive(Default, PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)] pub struct TreeRefIter<'a> { /// The directories and files contained in this tree. data: &'a [u8], @@ -246,7 +249,7 @@ impl Tree { } /// A borrowed object using a slice as backing buffer, or in other words a bytes buffer that knows the kind of object it represents. -#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] +#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)] pub struct Data<'a> { /// kind of object pub kind: Kind, diff --git a/gix-object/src/traits.rs b/gix-object/src/traits.rs index ce0463c988c..d48bc195d1a 100644 --- a/gix-object/src/traits.rs +++ b/gix-object/src/traits.rs @@ -41,3 +41,203 @@ where ::size(self) } } + +mod find { + use crate::find; + + /// Check if an object is present in an object store. + pub trait Exists { + /// Returns `true` if the object exists in the database. + fn exists(&self, id: &gix_hash::oid) -> bool; + } + + /// Find an object in the object store. + /// + /// ## Notes + /// + /// Find effectively needs [generic associated types][issue] to allow a trait for the returned object type. + /// Until then, we will have to make due with explicit types and give them the potentially added features we want. + /// + /// [issue]: https://github.com/rust-lang/rust/issues/44265 + pub trait Find { + /// Find an object matching `id` in the database while placing its raw, possibly encoded data into `buffer`. + /// + /// Returns `Some` object if it was present in the database, or the error that occurred during lookup or object + /// retrieval. + fn try_find<'a>( + &self, + id: &gix_hash::oid, + buffer: &'a mut Vec, + ) -> Result>, find::Error>; + } + + mod _impls { + use std::{ops::Deref, rc::Rc, sync::Arc}; + + use crate::Data; + use gix_hash::oid; + + impl crate::Exists for &T + where + T: crate::Exists, + { + fn exists(&self, id: &oid) -> bool { + (*self).exists(id) + } + } + + impl crate::Find for &T + where + T: crate::Find, + { + fn try_find<'a>(&self, id: &oid, buffer: &'a mut Vec) -> Result>, crate::find::Error> { + (*self).try_find(id, buffer) + } + } + + impl crate::Exists for Box + where + T: crate::Exists, + { + fn exists(&self, id: &oid) -> bool { + self.deref().exists(id) + } + } + + impl crate::Exists for Rc + where + T: crate::Exists, + { + fn exists(&self, id: &oid) -> bool { + self.deref().exists(id) + } + } + + impl crate::Find for Rc + where + T: crate::Find, + { + fn try_find<'a>(&self, id: &oid, buffer: &'a mut Vec) -> Result>, crate::find::Error> { + self.deref().try_find(id, buffer) + } + } + + impl crate::Find for Box + where + T: crate::Find, + { + fn try_find<'a>(&self, id: &oid, buffer: &'a mut Vec) -> Result>, crate::find::Error> { + self.deref().try_find(id, buffer) + } + } + + impl crate::Exists for Arc + where + T: crate::Exists, + { + fn exists(&self, id: &oid) -> bool { + self.deref().exists(id) + } + } + + impl crate::Find for Arc + where + T: crate::Find, + { + fn try_find<'a>(&self, id: &oid, buffer: &'a mut Vec) -> Result>, crate::find::Error> { + self.deref().try_find(id, buffer) + } + } + } + + mod ext { + use crate::{BlobRef, CommitRef, CommitRefIter, Kind, ObjectRef, TagRef, TagRefIter, TreeRef, TreeRefIter}; + + use crate::find; + + macro_rules! make_obj_lookup { + ($method:ident, $object_variant:path, $object_kind:path, $object_type:ty) => { + /// Like [`find(…)`][Self::find()], but flattens the `Result>` into a single `Result` making a non-existing object an error + /// while returning the desired object type. + fn $method<'a>( + &self, + id: &gix_hash::oid, + buffer: &'a mut Vec, + ) -> Result<$object_type, find::existing_object::Error> { + self.try_find(id, buffer) + .map_err(find::existing_object::Error::Find)? + .ok_or_else(|| find::existing_object::Error::NotFound { + oid: id.as_ref().to_owned(), + }) + .and_then(|o| { + o.decode() + .map_err(|err| find::existing_object::Error::Decode { + source: err, + oid: id.as_ref().to_owned(), + }) + }) + .and_then(|o| match o { + $object_variant(o) => return Ok(o), + o => Err(find::existing_object::Error::ObjectKind { + oid: id.as_ref().to_owned(), + actual: o.kind(), + expected: $object_kind, + }), + }) + } + }; + } + + macro_rules! make_iter_lookup { + ($method:ident, $object_kind:path, $object_type:ty, $into_iter:tt) => { + /// Like [`find(…)`][Self::find()], but flattens the `Result>` into a single `Result` making a non-existing object an error + /// while returning the desired iterator type. + fn $method<'a>( + &self, + id: &gix_hash::oid, + buffer: &'a mut Vec, + ) -> Result<$object_type, find::existing_iter::Error> { + self.try_find(id, buffer) + .map_err(find::existing_iter::Error::Find)? + .ok_or_else(|| find::existing_iter::Error::NotFound { + oid: id.as_ref().to_owned(), + }) + .and_then(|o| { + o.$into_iter() + .ok_or_else(|| find::existing_iter::Error::ObjectKind { + oid: id.as_ref().to_owned(), + actual: o.kind, + expected: $object_kind, + }) + }) + } + }; + } + + /// An extension trait with convenience functions. + pub trait FindExt: super::Find { + /// Like [`try_find(…)`][super::Find::try_find()], but flattens the `Result>` into a single `Result` making a non-existing object an error. + fn find<'a>( + &self, + id: &gix_hash::oid, + buffer: &'a mut Vec, + ) -> Result, find::existing::Error> { + self.try_find(id, buffer) + .map_err(find::existing::Error::Find)? + .ok_or_else(|| find::existing::Error::NotFound { oid: id.to_owned() }) + } + + make_obj_lookup!(find_commit, ObjectRef::Commit, Kind::Commit, CommitRef<'a>); + make_obj_lookup!(find_tree, ObjectRef::Tree, Kind::Tree, TreeRef<'a>); + make_obj_lookup!(find_tag, ObjectRef::Tag, Kind::Tag, TagRef<'a>); + make_obj_lookup!(find_blob, ObjectRef::Blob, Kind::Blob, BlobRef<'a>); + make_iter_lookup!(find_commit_iter, Kind::Commit, CommitRefIter<'a>, try_into_commit_iter); + make_iter_lookup!(find_tree_iter, Kind::Tree, TreeRefIter<'a>, try_into_tree_iter); + make_iter_lookup!(find_tag_iter, Kind::Tag, TagRefIter<'a>, try_into_tag_iter); + } + + impl FindExt for T {} + } + pub use ext::FindExt; +} +pub use find::*; diff --git a/gix-odb/src/cache.rs b/gix-odb/src/cache.rs index 87c8da4ed33..aaa99756301 100644 --- a/gix-odb/src/cache.rs +++ b/gix-odb/src/cache.rs @@ -150,16 +150,21 @@ mod impls { } } - impl crate::Find for Cache + impl gix_object::Find for Cache where S: gix_pack::Find, { - fn contains(&self, id: &oid) -> bool { - self.inner.contains(id) + fn try_find<'a>(&self, id: &oid, buffer: &'a mut Vec) -> Result>, gix_object::find::Error> { + gix_pack::Find::try_find(self, id, buffer).map(|t| t.map(|t| t.0)) } + } - fn try_find<'a>(&self, id: &oid, buffer: &'a mut Vec) -> Result>, crate::find::Error> { - gix_pack::Find::try_find(self, id, buffer).map(|t| t.map(|t| t.0)) + impl gix_object::Exists for Cache + where + S: gix_pack::Find, + { + fn exists(&self, id: &oid) -> bool { + self.inner.contains(id) } } @@ -167,7 +172,7 @@ mod impls { where S: crate::Header, { - fn try_header(&self, id: &oid) -> Result, crate::find::Error> { + fn try_header(&self, id: &oid) -> Result, gix_object::find::Error> { self.inner.try_header(id) } } @@ -184,7 +189,7 @@ mod impls { &self, id: &oid, buffer: &'a mut Vec, - ) -> Result, Option)>, crate::find::Error> { + ) -> Result, Option)>, gix_object::find::Error> { match self.pack_cache.as_ref().map(RefCell::borrow_mut) { Some(mut pack_cache) => self.try_find_cached(id, buffer, pack_cache.deref_mut()), None => self.try_find_cached(id, buffer, &mut gix_pack::cache::Never), @@ -196,7 +201,7 @@ mod impls { id: &oid, buffer: &'a mut Vec, pack_cache: &mut dyn gix_pack::cache::DecodeEntry, - ) -> Result, Option)>, crate::find::Error> { + ) -> Result, Option)>, gix_object::find::Error> { if let Some(mut obj_cache) = self.object_cache.as_ref().map(RefCell::borrow_mut) { if let Some(kind) = obj_cache.get(&id.as_ref().to_owned(), buffer) { return Ok(Some((Data::new(kind, buffer), None))); diff --git a/gix-odb/src/find.rs b/gix-odb/src/find.rs index 196845c4d7c..1ee1fa8eddb 100644 --- a/gix-odb/src/find.rs +++ b/gix-odb/src/find.rs @@ -1,56 +1,3 @@ -/// The error type returned by the [`Find`](crate::Find) and [`Header`](crate::Header) traits. -pub type Error = Box; -/// -pub mod existing { - use gix_hash::ObjectId; - - /// The error returned by the [`find(…)`][crate::FindExt::find()] trait methods. - #[derive(Debug, thiserror::Error)] - #[allow(missing_docs)] - pub enum Error { - #[error(transparent)] - Find(crate::find::Error), - #[error("An object with id {} could not be found", .oid)] - NotFound { oid: ObjectId }, - } -} - -/// -pub mod existing_object { - use gix_hash::ObjectId; - - /// The error returned by the various [`find_*()`][crate::FindExt::find_commit()] trait methods. - #[derive(Debug, thiserror::Error)] - #[allow(missing_docs)] - pub enum Error { - #[error(transparent)] - Find(crate::find::Error), - #[error(transparent)] - Decode(gix_object::decode::Error), - #[error("An object with id {oid} could not be found")] - NotFound { oid: ObjectId }, - #[error("Expected object of kind {expected}")] - ObjectKind { expected: gix_object::Kind }, - } -} - -/// -pub mod existing_iter { - use gix_hash::ObjectId; - - /// The error returned by the various [`find_*_iter()`][crate::FindExt::find_commit_iter()] trait methods. - #[derive(Debug, thiserror::Error)] - #[allow(missing_docs)] - pub enum Error { - #[error(transparent)] - Find(crate::find::Error), - #[error("An object with id {oid} could not be found")] - NotFound { oid: ObjectId }, - #[error("Expected object of kind {expected}")] - ObjectKind { expected: gix_object::Kind }, - } -} - /// An object header informing about object properties, without it being fully decoded in the process. #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] pub enum Header { diff --git a/gix-odb/src/lib.rs b/gix-odb/src/lib.rs index a6aa4caee75..45dd3f569e1 100644 --- a/gix-odb/src/lib.rs +++ b/gix-odb/src/lib.rs @@ -74,7 +74,7 @@ pub mod find; /// An object database equivalent to `/dev/null`, dropping all objects stored into it. mod traits; -pub use traits::{Find, FindExt, Header, HeaderExt, Write}; +pub use traits::{Header, HeaderExt, Write}; /// pub mod write { diff --git a/gix-odb/src/store_impls/dynamic/find.rs b/gix-odb/src/store_impls/dynamic/find.rs index b4cd25aeb24..b57b5fe830e 100644 --- a/gix-odb/src/store_impls/dynamic/find.rs +++ b/gix-odb/src/store_impls/dynamic/find.rs @@ -77,7 +77,8 @@ pub(crate) mod error { pub use error::Error; use gix_features::zlib; -use crate::{store::types::PackId, Find}; +use crate::store::types::PackId; +use gix_object::{Exists, Find}; impl super::Handle where @@ -346,7 +347,7 @@ where id: &gix_hash::oid, buffer: &'a mut Vec, pack_cache: &mut dyn DecodeEntry, - ) -> Result, Option)>, gix_pack::find::Error> { + ) -> Result, Option)>, gix_object::find::Error> { let mut snapshot = self.snapshot.borrow_mut(); let mut inflate = self.inflate.borrow_mut(); self.try_find_cached_inner(id, buffer, &mut inflate, pack_cache, &mut snapshot, None) @@ -503,15 +504,21 @@ where S: Deref + Clone, Self: gix_pack::Find, { - fn contains(&self, id: &gix_hash::oid) -> bool { - gix_pack::Find::contains(self, id) - } - fn try_find<'a>( &self, id: &gix_hash::oid, buffer: &'a mut Vec, - ) -> Result>, crate::find::Error> { + ) -> Result>, gix_object::find::Error> { gix_pack::Find::try_find(self, id, buffer).map(|t| t.map(|t| t.0)) } } + +impl Exists for super::Handle +where + S: Deref + Clone, + Self: gix_pack::Find, +{ + fn exists(&self, id: &gix_hash::oid) -> bool { + gix_pack::Find::contains(self, id) + } +} diff --git a/gix-odb/src/store_impls/dynamic/header.rs b/gix-odb/src/store_impls/dynamic/header.rs index 05ff7cf64af..42fd51c85ba 100644 --- a/gix-odb/src/store_impls/dynamic/header.rs +++ b/gix-odb/src/store_impls/dynamic/header.rs @@ -182,7 +182,7 @@ impl crate::Header for super::Handle where S: Deref + Clone, { - fn try_header(&self, id: &oid) -> Result, crate::find::Error> { + fn try_header(&self, id: &oid) -> Result, gix_object::find::Error> { let mut snapshot = self.snapshot.borrow_mut(); let mut inflate = self.inflate.borrow_mut(); self.try_header_inner(id, &mut inflate, &mut snapshot, None) diff --git a/gix-odb/src/store_impls/dynamic/prefix.rs b/gix-odb/src/store_impls/dynamic/prefix.rs index 58a49416b19..78c5e688494 100644 --- a/gix-odb/src/store_impls/dynamic/prefix.rs +++ b/gix-odb/src/store_impls/dynamic/prefix.rs @@ -1,9 +1,7 @@ use std::{collections::HashSet, ops::Deref}; -use crate::{ - store::{load_index, Handle}, - Find, -}; +use crate::store::{load_index, Handle}; +use gix_object::Exists; /// pub mod lookup { @@ -107,7 +105,7 @@ where ) -> Result, disambiguate::Error> { let max_hex_len = candidate.id().kind().len_in_hex(); if candidate.hex_len() == max_hex_len { - return Ok(self.contains(candidate.id()).then(|| candidate.to_prefix())); + return Ok(self.exists(candidate.id()).then(|| candidate.to_prefix())); } while candidate.hex_len() != max_hex_len { diff --git a/gix-odb/src/traits.rs b/gix-odb/src/traits.rs index 91d66c42d33..5f688b110ec 100644 --- a/gix-odb/src/traits.rs +++ b/gix-odb/src/traits.rs @@ -1,3 +1,4 @@ +use crate::find; use std::io; use gix_object::WriteTo; @@ -25,40 +26,17 @@ pub trait Write { ) -> Result; } -/// Describe how object can be located in an object store. -/// -/// ## Notes -/// -/// Find effectively needs [generic associated types][issue] to allow a trait for the returned object type. -/// Until then, we will have to make due with explicit types and give them the potentially added features we want. -/// -/// [issue]: https://github.com/rust-lang/rust/issues/44265 -pub trait Find { - /// Returns true if the object exists in the database. - fn contains(&self, id: &gix_hash::oid) -> bool; - - /// Find an object matching `id` in the database while placing its raw, possibly encoded data into `buffer`. - /// - /// Returns `Some` object if it was present in the database, or the error that occurred during lookup or object - /// retrieval. - fn try_find<'a>( - &self, - id: &gix_hash::oid, - buffer: &'a mut Vec, - ) -> Result>, find::Error>; -} - /// A way to obtain object properties without fully decoding it. pub trait Header { /// Try to read the header of the object associated with `id` or return `None` if it could not be found. - fn try_header(&self, id: &gix_hash::oid) -> Result, find::Error>; + fn try_header(&self, id: &gix_hash::oid) -> Result, gix_object::find::Error>; } mod _impls { use std::{io::Read, ops::Deref, rc::Rc, sync::Arc}; use gix_hash::{oid, ObjectId}; - use gix_object::{Data, Kind, WriteTo}; + use gix_object::{Kind, WriteTo}; use crate::find::Header; @@ -113,164 +91,47 @@ mod _impls { } } - impl crate::Find for &T - where - T: crate::Find, - { - fn contains(&self, id: &oid) -> bool { - (*self).contains(id) - } - - fn try_find<'a>(&self, id: &oid, buffer: &'a mut Vec) -> Result>, crate::find::Error> { - (*self).try_find(id, buffer) - } - } - impl crate::Header for &T where T: crate::Header, { - fn try_header(&self, id: &oid) -> Result, crate::find::Error> { + fn try_header(&self, id: &oid) -> Result, gix_object::find::Error> { (*self).try_header(id) } } - impl crate::Find for Rc - where - T: crate::Find, - { - fn contains(&self, id: &oid) -> bool { - self.deref().contains(id) - } - - fn try_find<'a>(&self, id: &oid, buffer: &'a mut Vec) -> Result>, crate::find::Error> { - self.deref().try_find(id, buffer) - } - } - impl crate::Header for Rc where T: crate::Header, { - fn try_header(&self, id: &oid) -> Result, crate::find::Error> { + fn try_header(&self, id: &oid) -> Result, gix_object::find::Error> { self.deref().try_header(id) } } - impl crate::Find for Arc - where - T: crate::Find, - { - fn contains(&self, id: &oid) -> bool { - self.deref().contains(id) - } - - fn try_find<'a>(&self, id: &oid, buffer: &'a mut Vec) -> Result>, crate::find::Error> { - self.deref().try_find(id, buffer) - } - } - impl crate::Header for Arc where T: crate::Header, { - fn try_header(&self, id: &oid) -> Result, crate::find::Error> { + fn try_header(&self, id: &oid) -> Result, gix_object::find::Error> { self.deref().try_header(id) } } } mod ext { - use gix_object::{BlobRef, CommitRef, CommitRefIter, Kind, ObjectRef, TagRef, TagRefIter, TreeRef, TreeRefIter}; - use crate::find; - - macro_rules! make_obj_lookup { - ($method:ident, $object_variant:path, $object_kind:path, $object_type:ty) => { - /// Like [`find(…)`][Self::find()], but flattens the `Result>` into a single `Result` making a non-existing object an error - /// while returning the desired object type. - fn $method<'a>( - &self, - id: &gix_hash::oid, - buffer: &'a mut Vec, - ) -> Result<$object_type, find::existing_object::Error> { - self.try_find(id, buffer) - .map_err(find::existing_object::Error::Find)? - .ok_or_else(|| find::existing_object::Error::NotFound { - oid: id.as_ref().to_owned(), - }) - .and_then(|o| o.decode().map_err(find::existing_object::Error::Decode)) - .and_then(|o| match o { - $object_variant(o) => return Ok(o), - _other => Err(find::existing_object::Error::ObjectKind { - expected: $object_kind, - }), - }) - } - }; - } - - macro_rules! make_iter_lookup { - ($method:ident, $object_kind:path, $object_type:ty, $into_iter:tt) => { - /// Like [`find(…)`][Self::find()], but flattens the `Result>` into a single `Result` making a non-existing object an error - /// while returning the desired iterator type. - fn $method<'a>( - &self, - id: &gix_hash::oid, - buffer: &'a mut Vec, - ) -> Result<$object_type, find::existing_iter::Error> { - self.try_find(id, buffer) - .map_err(find::existing_iter::Error::Find)? - .ok_or_else(|| find::existing_iter::Error::NotFound { - oid: id.as_ref().to_owned(), - }) - .and_then(|o| { - o.$into_iter() - .ok_or_else(|| find::existing_iter::Error::ObjectKind { - expected: $object_kind, - }) - }) - } - }; - } - /// An extension trait with convenience functions. pub trait HeaderExt: super::Header { /// Like [`try_header(…)`][super::Header::try_header()], but flattens the `Result>` into a single `Result` making a non-existing object an error. - fn header(&self, id: impl AsRef) -> Result { + fn header(&self, id: impl AsRef) -> Result { let id = id.as_ref(); self.try_header(id) - .map_err(find::existing::Error::Find)? - .ok_or_else(|| find::existing::Error::NotFound { oid: id.to_owned() }) + .map_err(gix_object::find::existing::Error::Find)? + .ok_or_else(|| gix_object::find::existing::Error::NotFound { oid: id.to_owned() }) } } impl HeaderExt for T {} - - /// An extension trait with convenience functions. - pub trait FindExt: super::Find { - /// Like [`try_find(…)`][super::Find::try_find()], but flattens the `Result>` into a single `Result` making a non-existing object an error. - fn find<'a>( - &self, - id: &gix_hash::oid, - buffer: &'a mut Vec, - ) -> Result, find::existing::Error> { - self.try_find(id, buffer) - .map_err(find::existing::Error::Find)? - .ok_or_else(|| find::existing::Error::NotFound { oid: id.to_owned() }) - } - - make_obj_lookup!(find_commit, ObjectRef::Commit, Kind::Commit, CommitRef<'a>); - make_obj_lookup!(find_tree, ObjectRef::Tree, Kind::Tree, TreeRef<'a>); - make_obj_lookup!(find_tag, ObjectRef::Tag, Kind::Tag, TagRef<'a>); - make_obj_lookup!(find_blob, ObjectRef::Blob, Kind::Blob, BlobRef<'a>); - make_iter_lookup!(find_commit_iter, Kind::Commit, CommitRefIter<'a>, try_into_commit_iter); - make_iter_lookup!(find_tree_iter, Kind::Tree, TreeRefIter<'a>, try_into_tree_iter); - make_iter_lookup!(find_tag_iter, Kind::Tag, TagRefIter<'a>, try_into_tag_iter); - } - - impl FindExt for T {} } -pub use ext::{FindExt, HeaderExt}; - -use crate::find; +pub use ext::HeaderExt; diff --git a/gix-odb/tests/odb/find/mod.rs b/gix-odb/tests/odb/find/mod.rs index 022b8f9b72d..3a93a03f1af 100644 --- a/gix-odb/tests/odb/find/mod.rs +++ b/gix-odb/tests/odb/find/mod.rs @@ -6,7 +6,7 @@ fn db() -> gix_odb::Handle { use crate::hex_to_id; -fn can_find(db: impl gix_odb::Find, hex_id: &str) { +fn can_find(db: impl gix_object::Find, hex_id: &str) { let mut buf = vec![]; assert!(db .try_find(&hex_to_id(hex_id), &mut buf) diff --git a/gix-odb/tests/odb/regression/mod.rs b/gix-odb/tests/odb/regression/mod.rs index 6a7989c44c1..1b8f5de0b0a 100644 --- a/gix-odb/tests/odb/regression/mod.rs +++ b/gix-odb/tests/odb/regression/mod.rs @@ -1,5 +1,5 @@ mod repo_with_small_packs { - use gix_odb::Find; + use gix_object::Find; use crate::odb::{db_small_packs, hex_to_id}; diff --git a/gix-odb/tests/odb/store/compound.rs b/gix-odb/tests/odb/store/compound.rs index 5498f17b56c..1b0a1617e32 100644 --- a/gix-odb/tests/odb/store/compound.rs +++ b/gix-odb/tests/odb/store/compound.rs @@ -3,7 +3,7 @@ //! tests o the general store itself, so they can possibly be removed at some point. mod locate { - use gix_odb::Find; + use gix_object::Find; use crate::{hex_to_id, odb::db}; diff --git a/gix-odb/tests/odb/store/dynamic.rs b/gix-odb/tests/odb/store/dynamic.rs index 875697d3f6a..87caddd9682 100644 --- a/gix-odb/tests/odb/store/dynamic.rs +++ b/gix-odb/tests/odb/store/dynamic.rs @@ -1,7 +1,8 @@ use std::process::Command; use gix_hash::ObjectId; -use gix_odb::{store, store::iter::Ordering, Find, FindExt, Header, Write}; +use gix_object::{Exists, FindExt}; +use gix_odb::{store, store::iter::Ordering, Header, Write}; use gix_testtools::fixture_path_standalone; use crate::{hex_to_id, odb::db}; @@ -64,7 +65,7 @@ fn multi_index_access() -> crate::Result { let mut buf = Vec::new(); for oid in handle.iter()?.with_ordering(order) { let oid = oid?; - assert!(handle.contains(&oid)); + assert!(handle.exists(&oid)); let obj = handle.find(&oid, &mut buf)?; let hdr = handle.try_header(&oid)?.expect("exists"); assert_eq!(hdr.kind(), obj.kind); @@ -92,7 +93,7 @@ fn multi_index_access() -> crate::Result { ); let non_existing_to_trigger_refresh = hex_to_id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"); - handle.contains(&non_existing_to_trigger_refresh); + handle.exists(&non_existing_to_trigger_refresh); assert_eq!( handle.store_ref().metrics(), @@ -115,7 +116,7 @@ fn multi_index_access() -> crate::Result { handle.store_ref().path().join("pack/multi-pack-index"), filetime::FileTime::now(), )?; - handle.contains(&non_existing_to_trigger_refresh); + handle.exists(&non_existing_to_trigger_refresh); assert_eq!( handle.store_ref().metrics(), @@ -181,7 +182,7 @@ fn multi_index_keep_open() -> crate::Result { handle.store_ref().path().join("pack/multi-pack-index"), filetime::FileTime::now(), )?; - gix_odb::Find::contains(&handle, &non_existing_to_trigger_refresh); + gix_object::Exists::exists(&handle, &non_existing_to_trigger_refresh); assert_eq!( handle.store_ref().metrics(), @@ -324,7 +325,7 @@ fn object_replacement() -> crate::Result { fn contains() { let handle = db(); - assert!(handle.contains(&hex_to_id("37d4e6c5c48ba0d245164c4e10d5f41140cab980"))); // loose object + assert!(handle.exists(&hex_to_id("37d4e6c5c48ba0d245164c4e10d5f41140cab980"))); // loose object assert_eq!( handle.store_ref().metrics(), gix_odb::store::Metrics { @@ -343,7 +344,7 @@ fn contains() { ); // pack c043, the biggest one - assert!(handle.contains(&hex_to_id("dd25c539efbb0ab018caa4cda2d133285634e9b5"))); + assert!(handle.exists(&hex_to_id("dd25c539efbb0ab018caa4cda2d133285634e9b5"))); assert_eq!( handle.store_ref().metrics(), @@ -366,7 +367,7 @@ fn contains() { // The new handle should make no difference. #[allow(clippy::redundant_clone)] let mut new_handle = handle.clone(); - assert!(new_handle.contains(&hex_to_id("501b297447a8255d3533c6858bb692575cdefaa0"))); + assert!(new_handle.exists(&hex_to_id("501b297447a8255d3533c6858bb692575cdefaa0"))); assert_eq!( new_handle.store_ref().metrics(), gix_odb::store::Metrics { @@ -384,7 +385,7 @@ fn contains() { "when asking for an object in the smallest pack, all in between packs are also loaded." ); - assert!(!new_handle.contains(&hex_to_id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))); + assert!(!new_handle.exists(&hex_to_id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))); assert_eq!( new_handle.store_ref().metrics(), gix_odb::store::Metrics { @@ -403,7 +404,7 @@ fn contains() { ); new_handle.refresh_never(); - assert!(!new_handle.contains(&hex_to_id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))); + assert!(!new_handle.exists(&hex_to_id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))); assert_eq!( new_handle.store_ref().metrics(), gix_odb::store::Metrics { @@ -430,7 +431,7 @@ fn lookup() { fn can_locate(db: &gix_odb::Handle, hex_id: &str) { let id = hex_to_id(hex_id); - assert!(db.contains(&id)); + assert!(db.exists(&id)); let mut buf = Vec::new(); let obj = db.find(&id, &mut buf).expect("exists"); @@ -486,7 +487,7 @@ fn lookup() { handle.refresh_mode(), store::RefreshMode::AfterAllIndicesLoaded )); - assert!(!handle.contains(&hex_to_id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))); + assert!(!handle.exists(&hex_to_id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))); all_loaded.num_refreshes += 1; assert_eq!( @@ -497,7 +498,7 @@ fn lookup() { handle.refresh_never(); let previous_refresh_count = all_loaded.num_refreshes; - assert!(!handle.contains(&hex_to_id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))); + assert!(!handle.exists(&hex_to_id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))); assert_eq!( handle.store_ref().metrics().num_refreshes, previous_refresh_count, @@ -733,7 +734,7 @@ mod lookup_prefix { #[test] fn missing_objects_triggers_everything_is_loaded() { let handle = db(); - assert!(!handle.contains(&hex_to_id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))); + assert!(!handle.exists(&hex_to_id("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))); assert_eq!( handle.store_ref().metrics(), @@ -787,7 +788,7 @@ fn iterate_over_a_bunch_of_loose_and_packed_objects() -> crate::Result { ); assert_eq!(iter.count(), 146, "it sees the correct amount of objects"); for id in db.iter()? { - assert!(db.contains(&id?), "each object exists"); + assert!(db.exists(&id?), "each object exists"); } } Ok(()) diff --git a/gix-odb/tests/odb/store/linked.rs b/gix-odb/tests/odb/store/linked.rs index 8753ece7453..18bf3c05ea2 100644 --- a/gix-odb/tests/odb/store/linked.rs +++ b/gix-odb/tests/odb/store/linked.rs @@ -55,7 +55,7 @@ mod locate { mod init { use gix_hash::ObjectId; - use gix_odb::Find; + use gix_object::Exists; use crate::odb::{alternate::alternate, db}; @@ -64,7 +64,7 @@ mod init { let tmp = gix_testtools::tempfile::TempDir::new()?; let (object_path, _linked_object_path) = alternate(tmp.path().join("a"), tmp.path().join("b"))?; let db = gix_odb::at(object_path.clone())?; - db.contains(&ObjectId::null(gix_hash::Kind::Sha1)); // trigger load + db.exists(&ObjectId::null(gix_hash::Kind::Sha1)); // trigger load assert_eq!(db.store_ref().metrics().loose_dbs, 2); assert_eq!(db.iter()?.count(), 0, "the test locations are actually empty"); @@ -76,7 +76,7 @@ mod init { fn a_db_without_alternates() -> crate::Result { let tmp = gix_testtools::tempfile::TempDir::new()?; let db = gix_odb::at(tmp.path())?; - db.contains(&ObjectId::null(gix_hash::Kind::Sha1)); // trigger load + db.exists(&ObjectId::null(gix_hash::Kind::Sha1)); // trigger load assert_eq!(db.store_ref().metrics().loose_dbs, 1); assert_eq!(db.store_ref().path(), tmp.path()); Ok(()) @@ -85,7 +85,7 @@ mod init { #[test] fn has_packs() { let db = db(); - db.contains(&ObjectId::null(gix_hash::Kind::Sha1)); // trigger load + db.exists(&ObjectId::null(gix_hash::Kind::Sha1)); // trigger load assert_eq!(db.store_ref().metrics().known_packs, 3); } } diff --git a/gix-pack/src/bundle/write/mod.rs b/gix-pack/src/bundle/write/mod.rs index 8983cb92f91..9ddd1b9d3a5 100644 --- a/gix-pack/src/bundle/write/mod.rs +++ b/gix-pack/src/bundle/write/mod.rs @@ -21,10 +21,6 @@ pub use types::{Options, Outcome}; use crate::bundle::write::types::SharedTempFile; -type ThinPackLookupFn = Box FnMut(gix_hash::ObjectId, &'a mut Vec) -> Option>>; -type ThinPackLookupFnSend = - Box FnMut(gix_hash::ObjectId, &'a mut Vec) -> Option> + Send + 'static>; - /// The progress ids used in [`write_to_directory()`][crate::Bundle::write_to_directory()]. /// /// Use this information to selectively extract the progress of interest in case the parent application has custom visualization. @@ -54,7 +50,7 @@ impl crate::Bundle { /// /// * `progress` provides detailed progress information which can be discarded with [`gix_features::progress::Discard`]. /// * `should_interrupt` is checked regularly and when true, the whole operation will stop. - /// * `thin_pack_base_object_lookup_fn` If set, we expect to see a thin-pack with objects that reference their base object by object id which is + /// * `thin_pack_base_object_lookup` If set, we expect to see a thin-pack with objects that reference their base object by object id which is /// expected to exist in the object database the bundle is contained within. /// `options` further configure how the task is performed. /// @@ -68,7 +64,7 @@ impl crate::Bundle { directory: Option<&Path>, progress: &mut dyn DynNestedProgress, should_interrupt: &AtomicBool, - thin_pack_base_object_lookup_fn: Option, + thin_pack_base_object_lookup: Option, options: Options, ) -> Result { let _span = gix_features::trace::coarse!("gix_pack::Bundle::write_to_directory()"); @@ -90,8 +86,8 @@ impl crate::Bundle { let (pack_entries_iter, pack_version): ( Box>>, _, - ) = match thin_pack_base_object_lookup_fn { - Some(thin_pack_lookup_fn) => { + ) = match thin_pack_base_object_lookup { + Some(thin_pack_lookup) => { let pack = interrupt::Read { inner: pack, should_interrupt, @@ -104,7 +100,7 @@ impl crate::Bundle { data::input::EntryDataMode::KeepAndCrc32, object_hash, )?, - thin_pack_lookup_fn, + thin_pack_lookup, ); let pack_version = pack_entries_iter.inner.version(); let pack_entries_iter = data::input::EntriesToBytesIter::new( @@ -178,7 +174,7 @@ impl crate::Bundle { directory: Option>, progress: &mut dyn DynNestedProgress, should_interrupt: &'static AtomicBool, - thin_pack_base_object_lookup_fn: Option, + thin_pack_base_object_lookup: Option, options: Options, ) -> Result { let _span = gix_features::trace::coarse!("gix_pack::Bundle::write_to_directory_eagerly()"); @@ -198,8 +194,8 @@ impl crate::Bundle { let (pack_entries_iter, pack_version): ( Box> + Send + 'static>, _, - ) = match thin_pack_base_object_lookup_fn { - Some(thin_pack_lookup_fn) => { + ) = match thin_pack_base_object_lookup { + Some(thin_pack_lookup) => { let pack = interrupt::Read { inner: pack, should_interrupt, @@ -212,7 +208,7 @@ impl crate::Bundle { data::input::EntryDataMode::KeepAndCrc32, object_hash, )?, - thin_pack_lookup_fn, + thin_pack_lookup, ); let pack_kind = pack_entries_iter.inner.version(); (Box::new(pack_entries_iter), pack_kind) diff --git a/gix-pack/src/data/input/lookup_ref_delta_objects.rs b/gix-pack/src/data/input/lookup_ref_delta_objects.rs index 60f686d3a34..f6036e20f0e 100644 --- a/gix-pack/src/data/input/lookup_ref_delta_objects.rs +++ b/gix-pack/src/data/input/lookup_ref_delta_objects.rs @@ -5,10 +5,10 @@ use gix_hash::ObjectId; use crate::data::{entry::Header, input}; /// An iterator to resolve thin packs on the fly. -pub struct LookupRefDeltaObjectsIter { +pub struct LookupRefDeltaObjectsIter { /// The inner iterator whose entries we will resolve. pub inner: I, - lookup: LFn, + lookup: Find, /// The cached delta to provide next time we are called, it's the delta to go with the base we just resolved in its place. next_delta: Option, /// Fuse to stop iteration after first missing object. @@ -21,14 +21,14 @@ pub struct LookupRefDeltaObjectsIter { buf: Vec, } -impl LookupRefDeltaObjectsIter +impl LookupRefDeltaObjectsIter where I: Iterator>, - LFn: for<'a> FnMut(ObjectId, &'a mut Vec) -> Option>, + Find: gix_object::Find, { /// Create a new instance wrapping `iter` and using `lookup` as function to retrieve objects that will serve as bases /// for ref deltas seen while traversing `iter`. - pub fn new(iter: I, lookup: LFn) -> Self { + pub fn new(iter: I, lookup: Find) -> Self { LookupRefDeltaObjectsIter { inner: iter, lookup, @@ -75,10 +75,10 @@ where } } -impl Iterator for LookupRefDeltaObjectsIter +impl Iterator for LookupRefDeltaObjectsIter where I: Iterator>, - LFn: for<'a> FnMut(ObjectId, &'a mut Vec) -> Option>, + Find: gix_object::Find, { type Item = Result; @@ -94,7 +94,7 @@ where Header::RefDelta { base_id } => { match self.inserted_entry_length_at_offset.iter().rfind(|e| e.oid == base_id) { None => { - let base_entry = match (self.lookup)(base_id, &mut self.buf) { + let base_entry = match self.lookup.try_find(&base_id, &mut self.buf).ok()? { Some(obj) => { let current_pack_offset = entry.pack_offset; let mut entry = match input::Entry::from_data_obj(&obj, 0) { diff --git a/gix-pack/src/data/output/count/objects/mod.rs b/gix-pack/src/data/output/count/objects/mod.rs index 24810577c88..f40cf29c97f 100644 --- a/gix-pack/src/data/output/count/objects/mod.rs +++ b/gix-pack/src/data/output/count/objects/mod.rs @@ -114,10 +114,11 @@ pub fn objects_unthreaded( } mod expand { + use std::cell::RefCell; use std::sync::atomic::{AtomicBool, Ordering}; use gix_hash::{oid, ObjectId}; - use gix_object::{CommitRefIter, TagRefIter}; + use gix_object::{CommitRefIter, Data, TagRefIter}; use super::{ tree, @@ -206,24 +207,15 @@ mod expand { let objects_ref = if parent_commit_ids.is_empty() { traverse_delegate.clear(); + let objects = ExpandedCountingObjects::new(db, out, objects); gix_traverse::tree::breadthfirst( current_tree_iter, &mut tree_traversal_state, - |oid, buf| { - stats.decoded_objects += 1; - match db.find(oid, buf).ok() { - Some((obj, location)) => { - objects.fetch_add(1, Ordering::Relaxed); - stats.expanded_objects += 1; - out.push(output::Count::from_data(oid, location)); - obj.try_into_tree_iter() - } - None => None, - } - }, + &objects, &mut traverse_delegate, ) .map_err(Error::TreeTraverse)?; + out = objects.dissolve(stats); &traverse_delegate.non_trees } else { for commit_id in &parent_commit_ids { @@ -252,17 +244,16 @@ mod expand { }; changes_delegate.clear(); + let objects = CountingObjects::new(db); gix_diff::tree::Changes::from(Some(parent_tree)) .needed_to_obtain( - current_tree_iter.clone(), + current_tree_iter, &mut tree_diff_state, - |oid, buf| { - stats.decoded_objects += 1; - db.find_tree_iter(oid, buf).map(|t| t.0) - }, + &objects, &mut changes_delegate, ) .map_err(Error::TreeChanges)?; + stats.decoded_objects += objects.into_count(); } &changes_delegate.objects }; @@ -283,24 +274,17 @@ mod expand { match obj.0.kind { Tree => { traverse_delegate.clear(); - gix_traverse::tree::breadthfirst( - gix_object::TreeRefIter::from_bytes(obj.0.data), - &mut tree_traversal_state, - |oid, buf| { - stats.decoded_objects += 1; - match db.find(oid, buf).ok() { - Some((obj, location)) => { - objects.fetch_add(1, Ordering::Relaxed); - stats.expanded_objects += 1; - out.push(output::Count::from_data(oid, location)); - obj.try_into_tree_iter() - } - None => None, - } - }, - &mut traverse_delegate, - ) - .map_err(Error::TreeTraverse)?; + { + let objects = ExpandedCountingObjects::new(db, out, objects); + gix_traverse::tree::breadthfirst( + gix_object::TreeRefIter::from_bytes(obj.0.data), + &mut tree_traversal_state, + &objects, + &mut traverse_delegate, + ) + .map_err(Error::TreeTraverse)?; + out = objects.dissolve(stats); + } for id in &traverse_delegate.non_trees { out.push(id_to_count(db, buf1, id, objects, stats, allow_pack_lookups)); } @@ -374,4 +358,76 @@ mod expand { }, } } + + struct CountingObjects<'a> { + decoded_objects: std::cell::RefCell, + objects: &'a dyn crate::Find, + } + + impl<'a> CountingObjects<'a> { + fn new(objects: &'a dyn crate::Find) -> Self { + Self { + decoded_objects: Default::default(), + objects, + } + } + + fn into_count(self) -> usize { + self.decoded_objects.into_inner() + } + } + + impl gix_object::Find for CountingObjects<'_> { + fn try_find<'a>(&self, id: &oid, buffer: &'a mut Vec) -> Result>, gix_object::find::Error> { + let res = Ok(self.objects.try_find(id, buffer)?.map(|t| t.0)); + *self.decoded_objects.borrow_mut() += 1; + res + } + } + + struct ExpandedCountingObjects<'a> { + decoded_objects: std::cell::RefCell, + expanded_objects: std::cell::RefCell, + out: std::cell::RefCell>, + objects_count: &'a gix_features::progress::AtomicStep, + objects: &'a dyn crate::Find, + } + + impl<'a> ExpandedCountingObjects<'a> { + fn new( + objects: &'a dyn crate::Find, + out: Vec, + objects_count: &'a gix_features::progress::AtomicStep, + ) -> Self { + Self { + decoded_objects: Default::default(), + expanded_objects: Default::default(), + out: RefCell::new(out), + objects_count, + objects, + } + } + + fn dissolve(self, stats: &mut Outcome) -> Vec { + stats.decoded_objects += self.decoded_objects.into_inner(); + stats.expanded_objects += self.expanded_objects.into_inner(); + self.out.into_inner() + } + } + + impl gix_object::Find for ExpandedCountingObjects<'_> { + fn try_find<'a>(&self, id: &oid, buffer: &'a mut Vec) -> Result>, gix_object::find::Error> { + let maybe_obj = self.objects.try_find(id, buffer)?; + *self.decoded_objects.borrow_mut() += 1; + match maybe_obj { + None => Ok(None), + Some((obj, location)) => { + self.objects_count.fetch_add(1, Ordering::Relaxed); + *self.expanded_objects.borrow_mut() += 1; + self.out.borrow_mut().push(output::Count::from_data(id, location)); + Ok(Some(obj)) + } + } + } + } } diff --git a/gix-pack/src/data/output/count/objects/types.rs b/gix-pack/src/data/output/count/objects/types.rs index 4b9ecea20a1..15e39501828 100644 --- a/gix-pack/src/data/output/count/objects/types.rs +++ b/gix-pack/src/data/output/count/objects/types.rs @@ -84,7 +84,7 @@ pub enum Error { #[error(transparent)] CommitDecode(gix_object::decode::Error), #[error(transparent)] - FindExisting(#[from] crate::find::existing::Error), + FindExisting(#[from] gix_object::find::existing::Error), #[error(transparent)] InputIteration(Box), #[error(transparent)] diff --git a/gix-pack/src/data/output/entry/iter_from_counts.rs b/gix-pack/src/data/output/entry/iter_from_counts.rs index 2bebf5b2007..dbe7060b6f4 100644 --- a/gix-pack/src/data/output/entry/iter_from_counts.rs +++ b/gix-pack/src/data/output/entry/iter_from_counts.rs @@ -204,7 +204,7 @@ pub(crate) mod function { stats.objects_copied_from_pack += 1; entry } - None => match db.try_find(&count.id, buf).map_err(Error::FindExisting)? { + None => match db.try_find(&count.id, buf).map_err(Error::Find)? { Some((obj, _location)) => { stats.decoded_and_recompressed_objects += 1; output::Entry::from_data(count, &obj) @@ -216,7 +216,7 @@ pub(crate) mod function { }, } } - None => match db.try_find(&count.id, buf).map_err(Error::FindExisting)? { + None => match db.try_find(&count.id, buf).map_err(Error::Find)? { Some((obj, _location)) => { stats.decoded_and_recompressed_objects += 1; output::Entry::from_data(count, &obj) @@ -397,7 +397,7 @@ mod types { #[allow(missing_docs)] pub enum Error { #[error(transparent)] - FindExisting(crate::find::Error), + Find(gix_object::find::Error), #[error(transparent)] NewEntry(#[from] entry::Error), } diff --git a/gix-pack/src/find.rs b/gix-pack/src/find.rs index b049d4d7829..efc9513edbb 100644 --- a/gix-pack/src/find.rs +++ b/gix-pack/src/find.rs @@ -1,57 +1,3 @@ -/// The error returned by methods of the [Find](crate::Find) trait. -pub type Error = Box; - -/// -pub mod existing { - use gix_hash::ObjectId; - - /// The error returned by the [`find(…)`](crate::FindExt::find()) trait methods. - #[derive(Debug, thiserror::Error)] - #[allow(missing_docs)] - pub enum Error { - #[error(transparent)] - Find(crate::find::Error), - #[error("An object with id {} could not be found", .oid)] - NotFound { oid: ObjectId }, - } -} - -/// -pub mod existing_object { - use gix_hash::ObjectId; - - /// The error returned by the various [`find_*`](crate::FindExt::find_commit()) trait methods. - #[derive(Debug, thiserror::Error)] - #[allow(missing_docs)] - pub enum Error { - #[error(transparent)] - Find(crate::find::Error), - #[error(transparent)] - Decode(gix_object::decode::Error), - #[error("An object with id {} could not be found", .oid)] - NotFound { oid: ObjectId }, - #[error("Expected object of kind {} something else", .expected)] - ObjectKind { expected: gix_object::Kind }, - } -} - -/// -pub mod existing_iter { - use gix_hash::ObjectId; - - /// The error returned by the various [`find_*`](crate::FindExt::find_commit()) trait methods. - #[derive(Debug, thiserror::Error)] - #[allow(missing_docs)] - pub enum Error { - #[error(transparent)] - Find(crate::find::Error), - #[error("An object with id {} could not be found", .oid)] - NotFound { oid: ObjectId }, - #[error("Expected object of kind {} something else", .expected)] - ObjectKind { expected: gix_object::Kind }, - } -} - /// An Entry in a pack providing access to its data. /// /// Its commonly retrieved by reading from a pack index file followed by a read from a pack data file. diff --git a/gix-pack/src/find_traits.rs b/gix-pack/src/find_traits.rs index 7c4821d8147..787ed5b635e 100644 --- a/gix-pack/src/find_traits.rs +++ b/gix-pack/src/find_traits.rs @@ -25,7 +25,7 @@ pub trait Find { &self, id: &gix_hash::oid, buffer: &'a mut Vec, - ) -> Result, Option)>, find::Error> { + ) -> Result, Option)>, gix_object::find::Error> { self.try_find_cached(id, buffer, &mut crate::cache::Never) } @@ -40,7 +40,7 @@ pub trait Find { id: &gix_hash::oid, buffer: &'a mut Vec, pack_cache: &mut dyn crate::cache::DecodeEntry, - ) -> Result, Option)>, find::Error>; + ) -> Result, Option)>, gix_object::find::Error>; /// Find the packs location where an object with `id` can be found in the database, or `None` if there is no pack /// holding the object. @@ -66,8 +66,6 @@ pub trait Find { mod ext { use gix_object::{BlobRef, CommitRef, CommitRefIter, Kind, ObjectRef, TagRef, TagRefIter, TreeRef, TreeRefIter}; - use crate::find; - macro_rules! make_obj_lookup { ($method:ident, $object_variant:path, $object_kind:path, $object_type:ty) => { /// Like [`find(…)`][Self::find()], but flattens the `Result>` into a single `Result` making a non-existing object an error @@ -76,21 +74,27 @@ mod ext { &self, id: &gix_hash::oid, buffer: &'a mut Vec, - ) -> Result<($object_type, Option), find::existing_object::Error> { + ) -> Result<($object_type, Option), gix_object::find::existing_object::Error> + { let id = id.as_ref(); self.try_find(id, buffer) - .map_err(find::existing_object::Error::Find)? - .ok_or_else(|| find::existing_object::Error::NotFound { + .map_err(gix_object::find::existing_object::Error::Find)? + .ok_or_else(|| gix_object::find::existing_object::Error::NotFound { oid: id.as_ref().to_owned(), }) .and_then(|(o, l)| { o.decode() - .map_err(find::existing_object::Error::Decode) + .map_err(|err| gix_object::find::existing_object::Error::Decode { + source: err, + oid: id.to_owned(), + }) .map(|o| (o, l)) }) .and_then(|(o, l)| match o { $object_variant(o) => return Ok((o, l)), - _other => Err(find::existing_object::Error::ObjectKind { + o => Err(gix_object::find::existing_object::Error::ObjectKind { + oid: id.to_owned(), + actual: o.kind(), expected: $object_kind, }), }) @@ -106,16 +110,18 @@ mod ext { &self, id: &gix_hash::oid, buffer: &'a mut Vec, - ) -> Result<($object_type, Option), find::existing_iter::Error> { + ) -> Result<($object_type, Option), gix_object::find::existing_iter::Error> { let id = id.as_ref(); self.try_find(id, buffer) - .map_err(find::existing_iter::Error::Find)? - .ok_or_else(|| find::existing_iter::Error::NotFound { + .map_err(gix_object::find::existing_iter::Error::Find)? + .ok_or_else(|| gix_object::find::existing_iter::Error::NotFound { oid: id.as_ref().to_owned(), }) .and_then(|(o, l)| { o.$into_iter() - .ok_or_else(|| find::existing_iter::Error::ObjectKind { + .ok_or_else(|| gix_object::find::existing_iter::Error::ObjectKind { + oid: id.to_owned(), + actual: o.kind, expected: $object_kind, }) .map(|i| (i, l)) @@ -131,10 +137,11 @@ mod ext { &self, id: &gix_hash::oid, buffer: &'a mut Vec, - ) -> Result<(gix_object::Data<'a>, Option), find::existing::Error> { + ) -> Result<(gix_object::Data<'a>, Option), gix_object::find::existing::Error> + { self.try_find(id, buffer) - .map_err(find::existing::Error::Find)? - .ok_or_else(|| find::existing::Error::NotFound { + .map_err(gix_object::find::existing::Error::Find)? + .ok_or_else(|| gix_object::find::existing::Error::NotFound { oid: id.as_ref().to_owned(), }) } @@ -172,7 +179,7 @@ mod find_impls { id: &oid, buffer: &'a mut Vec, pack_cache: &mut dyn crate::cache::DecodeEntry, - ) -> Result, Option)>, crate::find::Error> { + ) -> Result, Option)>, gix_object::find::Error> { (*self).try_find_cached(id, buffer, pack_cache) } @@ -202,7 +209,7 @@ mod find_impls { id: &oid, buffer: &'a mut Vec, pack_cache: &mut dyn crate::cache::DecodeEntry, - ) -> Result, Option)>, find::Error> { + ) -> Result, Option)>, gix_object::find::Error> { self.deref().try_find_cached(id, buffer, pack_cache) } @@ -232,7 +239,7 @@ mod find_impls { id: &oid, buffer: &'a mut Vec, pack_cache: &mut dyn crate::cache::DecodeEntry, - ) -> Result, Option)>, find::Error> { + ) -> Result, Option)>, gix_object::find::Error> { self.deref().try_find_cached(id, buffer, pack_cache) } @@ -262,7 +269,7 @@ mod find_impls { id: &oid, buffer: &'a mut Vec, pack_cache: &mut dyn crate::cache::DecodeEntry, - ) -> Result, Option)>, find::Error> { + ) -> Result, Option)>, gix_object::find::Error> { self.deref().try_find_cached(id, buffer, pack_cache) } diff --git a/gix-pack/tests/pack/bundle.rs b/gix-pack/tests/pack/bundle.rs index a68a7b8b1b7..4c8951d5057 100644 --- a/gix-pack/tests/pack/bundle.rs +++ b/gix-pack/tests/pack/bundle.rs @@ -170,7 +170,7 @@ mod write_to_directory { directory, &mut progress::Discard, &SHOULD_INTERRUPT, - None, + None::, pack::bundle::write::Options { thread_limit: None, iteration_mode: pack::data::input::Mode::Verify, diff --git a/gix-pack/tests/pack/data/input.rs b/gix-pack/tests/pack/data/input.rs index c698f2ccfa3..14c14c54cf4 100644 --- a/gix-pack/tests/pack/data/input.rs +++ b/gix-pack/tests/pack/data/input.rs @@ -1,6 +1,9 @@ mod lookup_ref_delta_objects { - use gix_hash::ObjectId; + use gix_hash::{oid, ObjectId}; + use gix_object::find::Error; + use gix_object::Data; use gix_pack::data::{entry::Header, input, input::LookupRefDeltaObjectsIter}; + use std::sync::atomic::{AtomicUsize, Ordering}; use crate::pack::hex_to_id; @@ -60,13 +63,42 @@ mod lookup_ref_delta_objects { entries.into_iter().map(Ok) } + struct FindData<'a> { + calls: &'a AtomicUsize, + data: Option<&'a [u8]>, + } + + impl<'a> FindData<'a> { + fn new(data: impl Into>, calls: &'a AtomicUsize) -> Self { + FindData { + data: data.into(), + calls, + } + } + } + + impl gix_object::Find for FindData<'_> { + fn try_find<'a>(&self, _id: &oid, buf: &'a mut Vec) -> Result>, Error> { + self.calls.fetch_add(1, Ordering::Relaxed); + if let Some(data) = self.data { + buf.resize(data.len(), 0); + buf.copy_from_slice(data); + Ok(Some(gix_object::Data { + kind: gix_object::Kind::Blob, + data: buf.as_slice(), + })) + } else { + Ok(None) + } + } + } + #[test] fn only_ref_deltas_are_handled() -> crate::Result { let input = compute_offsets(vec![entry(base(), D_A), entry(delta_ofs(100), D_B)]); let expected = input.clone(); - let actual = - LookupRefDeltaObjectsIter::new(into_results_iter(input), |_, _| unreachable!("not going to be called")) - .collect::, _>>()?; + let actual = LookupRefDeltaObjectsIter::new(into_results_iter(input), gix_object::find::Never) + .collect::, _>>()?; assert_eq!(actual, expected, "it won't change the input at all"); validate_pack_offsets(&actual); Ok(()) @@ -86,23 +118,16 @@ mod lookup_ref_delta_objects { let fifth = entry(delta_ref(second_id), D_A); let input = compute_offsets(vec![first, second, third_entry, fourth_entry, fifth]); - let mut calls = 0; + let calls = AtomicUsize::default(); let input_entries = into_results_iter(input); let actual_size = input_entries.size_hint(); - let iter = LookupRefDeltaObjectsIter::new(input_entries, |_oid, buf| { - calls += 1; - buf.resize(inserted_data.len(), 0); - buf.copy_from_slice(inserted_data); - Some(gix_object::Data { - kind: gix_object::Kind::Blob, - data: buf.as_slice(), - }) - }); + let db = FindData::new(inserted_data, &calls); + let iter = LookupRefDeltaObjectsIter::new(input_entries, &db); assert_eq!(iter.size_hint(), (actual_size.0, actual_size.1.map(|s| s * 2)), "size hints are estimated and the upper bound reflects the worst-case scenario for the amount of possible objects"); let actual = iter.collect::, _>>().unwrap(); - assert_eq!(calls, 2, "there is only two objects to insert"); + assert_eq!(calls.load(Ordering::Relaxed), 2, "there is only two objects to insert"); assert_eq!(actual.len(), 7, "two object was inserted"); assert_eq!(&actual[0], &inserted, "first object is inserted one"); @@ -150,13 +175,10 @@ mod lookup_ref_delta_objects { #[test] fn lookup_errors_trigger_a_fuse_and_stop_iteration() { let input = vec![entry(delta_ref(gix_hash::Kind::Sha1.null()), D_A), entry(base(), D_B)]; - let mut calls = 0; - let mut result = LookupRefDeltaObjectsIter::new(into_results_iter(input), |_, _| { - calls += 1; - None - }) - .collect::>(); - assert_eq!(calls, 1, "it tries to lookup the object"); + let calls = AtomicUsize::default(); + let db = FindData::new(None, &calls); + let mut result = LookupRefDeltaObjectsIter::new(into_results_iter(input), &db).collect::>(); + assert_eq!(calls.load(Ordering::Relaxed), 1, "it tries to lookup the object"); assert_eq!(result.len(), 1, "the error stops iteration"); assert!(matches!( result.pop().expect("one"), @@ -182,8 +204,7 @@ mod lookup_ref_delta_objects { }), Ok(entry(base(), D_B)), ]; - let actual = LookupRefDeltaObjectsIter::new(input.into_iter(), |_, _| unreachable!("won't be called")) - .collect::>(); + let actual = LookupRefDeltaObjectsIter::new(input.into_iter(), gix_object::find::Never).collect::>(); for (actual, expected) in actual.into_iter().zip(expected.into_iter()) { assert_eq!(format!("{actual:?}"), format!("{expected:?}")); } diff --git a/gix-pack/tests/pack/data/output/count_and_entries.rs b/gix-pack/tests/pack/data/output/count_and_entries.rs index 9ccc8ebe32c..a98f02d387e 100644 --- a/gix-pack/tests/pack/data/output/count_and_entries.rs +++ b/gix-pack/tests/pack/data/output/count_and_entries.rs @@ -241,13 +241,10 @@ fn traversals() -> crate::Result { .copied() { let head = hex_to_id("dfcb5e39ac6eb30179808bbab721e8a28ce1b52e"); - let mut commits = commit::Ancestors::new(Some(head), commit::ancestors::State::default(), { - let db = db.clone(); - move |oid, buf| db.find_commit_iter(oid, buf).map(|t| t.0) - }) - .map(Result::unwrap) - .map(|c| c.id) - .collect::>(); + let mut commits = commit::Ancestors::new(Some(head), commit::ancestors::State::default(), db.clone()) + .map(Result::unwrap) + .map(|c| c.id) + .collect::>(); if let Some(take) = take { commits.resize(take, gix_hash::Kind::Sha1.null()); } @@ -392,7 +389,7 @@ fn write_and_verify( Some(tmp_dir.path()), &mut progress::Discard, &should_interrupt, - Some(Box::new(move |oid, buf| db.find(&oid, buf).ok().map(|t| t.0))), + Some(&db), pack::bundle::write::Options::default(), )? .data_path diff --git a/gix-ref/src/peel.rs b/gix-ref/src/peel.rs index 47a231df095..b119a907334 100644 --- a/gix-ref/src/peel.rs +++ b/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, -) -> Result, Box> { - 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), + 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 }, } diff --git a/gix-ref/src/store/file/raw_ext.rs b/gix-ref/src/store/file/raw_ext.rs index 0aa598351f7..ede518827fd 100644 --- a/gix-ref/src/store/file/raw_ext.rs +++ b/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, - ) -> Result, Box> - + '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; - /// 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; @@ -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 { 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 { 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| { diff --git a/gix-ref/src/store/file/transaction/mod.rs b/gix-ref/src/store/file/transaction/mod.rs index c3368c1b6cd..c1675803bde 100644 --- a/gix-ref/src/store/file/transaction/mod.rs +++ b/gix-ref/src/store/file/transaction/mod.rs @@ -8,16 +8,6 @@ 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, - ) -> Result, Box> - + 'a; - /// How to handle packed refs during a transaction #[derive(Default)] pub enum PackedRefs<'a> { @@ -25,11 +15,11 @@ pub enum PackedRefs<'a> { #[default] DeletionsOnly, /// Propagate deletions as well as updates to references which are peeled, that is contain an object id - DeletionsAndNonSymbolicUpdates(Box>), + DeletionsAndNonSymbolicUpdates(Box), /// 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>), + DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(Box), } #[derive(Debug)] diff --git a/gix-ref/src/store/file/transaction/prepare.rs b/gix-ref/src/store/file/transaction/prepare.rs index eb802f10f1d..34807325856 100644 --- a/gix-ref/src/store/file/transaction/prepare.rs +++ b/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)?, }); } } diff --git a/gix-ref/src/store/packed/transaction.rs b/gix-ref/src/store/packed/transaction.rs index e2955f8e069..41a6cdc9b95 100644 --- a/gix-ref/src/store/packed/transaction.rs +++ b/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, - find: &mut FindObjectFn<'_>, + objects: &dyn gix_object::Find, ) -> Result { 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(|_| { diff --git a/gix-ref/tests/file/mod.rs b/gix-ref/tests/file/mod.rs index 3d76e537a2e..c92c5b6c278 100644 --- a/gix-ref/tests/file/mod.rs +++ b/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, + ) -> Result>, gix_object::find::Error> { + Ok(Some(gix_object::Data { + kind: gix_object::Kind::Commit, + data: &[], + })) + } +} + mod log; mod reference; mod store; diff --git a/gix-ref/tests/file/reference.rs b/gix-ref/tests/file/reference.rs index 0b69c984bd9..3761b067f28 100644 --- a/gix-ref/tests/file/reference.rs +++ b/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,12 +119,12 @@ 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" ); @@ -132,10 +132,7 @@ mod peel { 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"); diff --git a/gix-ref/tests/file/transaction/prepare_and_commit/create_or_update/collisions.rs b/gix-ref/tests/file/transaction/prepare_and_commit/create_or_update/collisions.rs index bba4e6f614e..586547e5434 100644 --- a/gix-ref/tests/file/transaction/prepare_and_commit/create_or_update/collisions.rs +++ b/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( [ diff --git a/gix-ref/tests/file/transaction/prepare_and_commit/create_or_update/mod.rs b/gix-ref/tests/file/transaction/prepare_and_commit/create_or_update/mod.rs index c34434d856e..a1d89269f92 100644 --- a/gix-ref/tests/file/transaction/prepare_and_commit/create_or_update/mod.rs +++ b/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_odb::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!( diff --git a/gix-ref/tests/file/worktree.rs b/gix-ref/tests/file/worktree.rs index 8a76664cca1..21000bd82f2 100644 --- a/gix-ref/tests/file/worktree.rs +++ b/gix-ref/tests/file/worktree.rs @@ -1,6 +1,5 @@ use std::{cmp::Ordering, path::PathBuf}; -use gix_odb::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 diff --git a/gix-revision/tests/describe/mod.rs b/gix-revision/tests/describe/mod.rs index c66ce86bf11..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_odb::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..692b53c3258 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,30 @@ 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> { + 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 +281,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 +349,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. diff --git a/gix-status/src/index_as_worktree/function.rs b/gix-status/src/index_as_worktree/function.rs index b8ef0c7e4c5..7e1b9c86465 100644 --- a/gix-status/src/index_as_worktree/function.rs +++ b/gix-status/src/index_as_worktree/function.rs @@ -6,6 +6,7 @@ use bstr::BStr; use filetime::FileTime; use gix_features::parallel::{in_parallel_if, Reduce}; use gix_filter::pipeline::convert::ToGitOutcome; +use gix_object::FindExt; use crate::index_as_worktree::traits::read_data::Stream; use crate::index_as_worktree::{Conflict, EntryStatus}; @@ -31,6 +32,7 @@ use crate::{ /// `should_interrupt` can be used to stop all processing. /// `filter` is used to convert worktree files back to their internal git representation. For this to be correct, /// [`Options::attributes`] must be configured as well. +/// `objects` is used to access the version of an object in the object database for direct comparison. /// /// **It's important to note that the `index` should have its [timestamp updated](gix_index::State::set_timestamp()) with a timestamp /// from just before making this call *if* [entries were updated](Outcome::entries_to_update)** @@ -45,13 +47,13 @@ use crate::{ /// Thus some care has to be taken to do the right thing when letting the index match the worktree by evaluating the changes observed /// by the `collector`. #[allow(clippy::too_many_arguments)] -pub fn index_as_worktree<'index, T, U, Find, E1, E2>( +pub fn index_as_worktree<'index, T, U, Find, E>( index: &'index gix_index::State, worktree: &Path, collector: &mut impl VisitEntry<'index, ContentChange = T, SubmoduleStatus = U>, compare: impl CompareBlobs + Send + Clone, - submodule: impl SubmoduleStatus + Send + Clone, - find: Find, + submodule: impl SubmoduleStatus + Send + Clone, + objects: Find, progress: &mut dyn gix_features::progress::Progress, pathspec: impl Pathspec + Send + Clone, filter: gix_filter::Pipeline, @@ -61,9 +63,8 @@ pub fn index_as_worktree<'index, T, U, Find, E1, E2>( where T: Send, U: Send, - E1: std::error::Error + Send + Sync + 'static, - E2: std::error::Error + Send + Sync + 'static, - Find: for<'a> FnMut(&gix_hash::oid, &'a mut Vec) -> Result, E1> + Send + Clone, + E: std::error::Error + Send + Sync + 'static, + Find: gix_object::Find + Send + Clone, { // the order is absolutely critical here we use the old timestamp to detect racy index entries // (modified at or after the last index update) during the index update we then set those @@ -135,7 +136,7 @@ where }, compare, submodule, - find, + objects, pathspec, ) } @@ -151,7 +152,7 @@ where ), thread_limit, new_state, - |(entry_offset, chunk_entries), (state, blobdiff, submdule, find, pathspec)| { + |(entry_offset, chunk_entries), (state, blobdiff, submdule, objects, pathspec)| { let all_entries = index.entries(); let mut out = Vec::new(); let mut idx = 0; @@ -181,7 +182,7 @@ where pathspec, blobdiff, submdule, - find, + objects, &mut idx, ); idx += 1; @@ -244,21 +245,20 @@ type StatusResult<'index, T, U> = Result<(&'index gix_index::Entry, usize, &'ind impl<'index> State<'_, 'index> { #[allow(clippy::too_many_arguments)] - fn process( + fn process( &mut self, entries: &'index [gix_index::Entry], entry: &'index gix_index::Entry, entry_index: usize, pathspec: &mut impl Pathspec, diff: &mut impl CompareBlobs, - submodule: &mut impl SubmoduleStatus, - find: &mut Find, + submodule: &mut impl SubmoduleStatus, + objects: &Find, outer_entry_index: &mut usize, ) -> Option> where - E1: std::error::Error + Send + Sync + 'static, - E2: std::error::Error + Send + Sync + 'static, - Find: for<'a> FnMut(&gix_hash::oid, &'a mut Vec) -> Result, E1>, + E: std::error::Error + Send + Sync + 'static, + Find: gix_object::Find, { if entry.flags.intersects( gix_index::entry::Flags::UPTODATE @@ -282,7 +282,7 @@ impl<'index> State<'_, 'index> { }), ) } else { - self.compute_status(entry, path, diff, submodule, find) + self.compute_status(entry, path, diff, submodule, objects) }; match status { Ok(None) => None, @@ -330,18 +330,17 @@ impl<'index> State<'_, 'index> { /// which is a constant. /// /// Adapted from [here](https://github.com/Byron/gitoxide/pull/805#discussion_r1164676777). - fn compute_status( + fn compute_status( &mut self, entry: &gix_index::Entry, rela_path: &BStr, diff: &mut impl CompareBlobs, - submodule: &mut impl SubmoduleStatus, - find: &mut Find, + submodule: &mut impl SubmoduleStatus, + objects: &Find, ) -> Result>, Error> where - E1: std::error::Error + Send + Sync + 'static, - E2: std::error::Error + Send + Sync + 'static, - Find: for<'a> FnMut(&gix_hash::oid, &'a mut Vec) -> Result, E1>, + E: std::error::Error + Send + Sync + 'static, + Find: gix_object::Find, { let worktree_path = match self.path_stack.verified_path(gix_path::from_bstr(rela_path).as_ref()) { Ok(path) => path, @@ -423,7 +422,7 @@ impl<'index> State<'_, 'index> { attr_stack: &mut self.attr_stack, options: self.options, id: &entry.id, - find, + objects, worktree_reads: self.worktree_reads, worktree_bytes: self.worktree_bytes, odb_reads: self.odb_reads, @@ -478,10 +477,9 @@ impl<'index, T, U, C: VisitEntry<'index, ContentChange = T, SubmoduleStatus = U> } } -struct ReadDataImpl<'a, Find, E> +struct ReadDataImpl<'a, Find> where - E: std::error::Error + Send + Sync + 'static, - Find: for<'b> FnMut(&gix_hash::oid, &'b mut Vec) -> Result, E>, + Find: gix_object::Find, { buf: &'a mut Vec, path: &'a Path, @@ -492,29 +490,26 @@ where attr_stack: &'a mut gix_worktree::Stack, options: &'a Options, id: &'a gix_hash::oid, - find: Find, + objects: Find, worktree_bytes: &'a AtomicU64, worktree_reads: &'a AtomicUsize, odb_bytes: &'a AtomicU64, odb_reads: &'a AtomicUsize, } -impl<'a, Find, E> traits::ReadData<'a> for ReadDataImpl<'a, Find, E> +impl<'a, Find> traits::ReadData<'a> for ReadDataImpl<'a, Find> where - E: std::error::Error + Send + Sync + 'static, - Find: for<'b> FnMut(&gix_hash::oid, &'b mut Vec) -> Result, E>, + Find: gix_object::Find, { - fn read_blob(mut self) -> Result<&'a [u8], Error> { - (self.find)(self.id, self.buf) - .map(|b| { - self.odb_reads.fetch_add(1, Ordering::Relaxed); - self.odb_bytes.fetch_add(b.data.len() as u64, Ordering::Relaxed); - b.data - }) - .map_err(move |err| Error::Find(Box::new(err))) + fn read_blob(self) -> Result<&'a [u8], Error> { + Ok(self.objects.find_blob(self.id, self.buf).map(|b| { + self.odb_reads.fetch_add(1, Ordering::Relaxed); + self.odb_bytes.fetch_add(b.data.len() as u64, Ordering::Relaxed); + b.data + })?) } - fn stream_worktree_file(mut self) -> Result, Error> { + fn stream_worktree_file(self) -> Result, Error> { self.buf.clear(); // symlinks are only stored as actual symlinks if the FS supports it otherwise they are just // normal files with their content equal to the linked path (so can be read normally) @@ -534,7 +529,7 @@ where } } else { self.buf.clear(); - let platform = self.attr_stack.at_entry(self.rela_path, Some(false), &mut self.find)?; + let platform = self.attr_stack.at_entry(self.rela_path, Some(false), &self.objects)?; let file = std::fs::File::open(self.path)?; let out = self .filter @@ -544,11 +539,7 @@ where &mut |_path, attrs| { platform.matching_attributes(attrs); }, - &mut |buf| { - (self.find)(self.id, buf) - .map(|_| Some(())) - .map_err(|err| Box::new(err) as Box) - }, + &mut |buf| Ok(self.objects.find_blob(self.id, buf).map(|_| Some(()))?), ) .map_err(|err| io::Error::new(io::ErrorKind::Other, err))?; let len = match out { diff --git a/gix-status/src/index_as_worktree/types.rs b/gix-status/src/index_as_worktree/types.rs index daad11f27ad..0f253f553de 100644 --- a/gix-status/src/index_as_worktree/types.rs +++ b/gix-status/src/index_as_worktree/types.rs @@ -11,7 +11,7 @@ pub enum Error { #[error("IO error while writing blob or reading file metadata or changing filetype")] Io(#[from] std::io::Error), #[error("Failed to obtain blob from object database")] - Find(#[source] Box), + Find(#[from] gix_object::find::existing_object::Error), #[error("Could not determine status for submodule at '{rela_path}'")] SubmoduleStatus { rela_path: BString, diff --git a/gix-status/tests/status/index_as_worktree.rs b/gix-status/tests/status/index_as_worktree.rs index 49c264d9b95..e0ba96ebd07 100644 --- a/gix-status/tests/status/index_as_worktree.rs +++ b/gix-status/tests/status/index_as_worktree.rs @@ -102,7 +102,7 @@ fn fixture_filtered_detailed( &mut recorder, FastEq, SubmoduleStatusMock { dirty: submodule_dirty }, - |_, _| Ok::<_, std::convert::Infallible>(gix_object::BlobRef { data: &[] }), + gix_object::find::Never, &mut gix_features::progress::Discard, Pathspec(search), Default::default(), @@ -615,7 +615,7 @@ fn racy_git() { &mut recorder, counter.clone(), SubmoduleStatusMock { dirty: false }, - |_, _| Err(std::io::Error::new(std::io::ErrorKind::Other, "no odb access expected")), + gix_object::find::Never, &mut gix_features::progress::Discard, Pathspec::default(), Default::default(), @@ -654,7 +654,7 @@ fn racy_git() { &mut recorder, counter, SubmoduleStatusMock { dirty: false }, - |_, _| Err(std::io::Error::new(std::io::ErrorKind::Other, "no odb access expected")), + gix_object::find::Never, &mut gix_features::progress::Discard, Pathspec::default(), Default::default(), diff --git a/gix-traverse/src/commit.rs b/gix-traverse/src/commit.rs index 29f5947b37b..4bf49d2a0b4 100644 --- a/gix-traverse/src/commit.rs +++ b/gix-traverse/src/commit.rs @@ -1,8 +1,9 @@ +use gix_object::FindExt; use smallvec::SmallVec; /// An iterator over the ancestors one or more starting commits pub struct Ancestors { - find: Find, + objects: Find, cache: Option, predicate: Predicate, state: StateMut, @@ -95,7 +96,7 @@ pub mod ancestors { use gix_date::SecondsSinceUnixEpoch; use gix_hash::{oid, ObjectId}; use gix_hashtable::HashSet; - use gix_object::CommitRefIter; + use gix_object::{CommitRefIter, FindExt}; use smallvec::SmallVec; use crate::commit::{collect_parents, Ancestors, Either, Info, ParentIds, Parents, Sorting}; @@ -104,11 +105,8 @@ pub mod ancestors { #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { - #[error("The commit {oid} could not be found")] - FindExisting { - oid: ObjectId, - source: Box, - }, + #[error(transparent)] + Find(#[from] gix_object::find::existing_iter::Error), #[error(transparent)] ObjectDecode(#[from] gix_object::decode::Error), } @@ -147,11 +145,10 @@ pub mod ancestors { } /// Builder - impl Ancestors + impl Ancestors where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Result, E>, + Find: gix_object::Find, StateMut: BorrowMut, - E: std::error::Error + Send + Sync + 'static, { /// Set the sorting method, either topological or by author date pub fn sorting(mut self, sorting: Sorting) -> Result { @@ -164,11 +161,7 @@ pub mod ancestors { let cutoff_time = self.sorting.cutoff_time(); let state = self.state.borrow_mut(); for commit_id in state.next.drain(..) { - let commit_iter = - (self.find)(&commit_id, &mut state.buf).map_err(|err| Error::FindExisting { - oid: commit_id, - source: err.into(), - })?; + let commit_iter = self.objects.find_commit_iter(&commit_id, &mut state.buf)?; let time = commit_iter.committer()?.time.seconds; match cutoff_time { Some(cutoff_time) if time >= cutoff_time => { @@ -214,11 +207,10 @@ pub mod ancestors { } /// Initialization - impl Ancestors bool, StateMut> + impl Ancestors bool, StateMut> where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Result, E>, + Find: gix_object::Find, StateMut: BorrowMut, - E: std::error::Error + Send + Sync + 'static, { /// Create a new instance. /// @@ -236,12 +228,11 @@ pub mod ancestors { } /// Initialization - impl Ancestors + impl Ancestors where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Result, E>, + Find: gix_object::Find, Predicate: FnMut(&oid) -> bool, StateMut: BorrowMut, - E: std::error::Error + Send + Sync + 'static, { /// Create a new instance with commit filtering enabled. /// @@ -274,7 +265,7 @@ pub mod ancestors { } } Self { - find, + objects: find, cache: None, predicate, state, @@ -299,12 +290,11 @@ pub mod ancestors { } } - impl Iterator for Ancestors + impl Iterator for Ancestors where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Result, E>, + Find: gix_object::Find, Predicate: FnMut(&oid) -> bool, StateMut: BorrowMut, - E: std::error::Error + Send + Sync + 'static, { type Item = Result; @@ -334,12 +324,11 @@ pub mod ancestors { } /// Utilities - impl Ancestors + impl Ancestors where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Result, E>, + Find: gix_object::Find, Predicate: FnMut(&oid) -> bool, StateMut: BorrowMut, - E: std::error::Error + Send + Sync + 'static, { fn next_by_commit_date( &mut self, @@ -349,7 +338,7 @@ pub mod ancestors { let (commit_time, oid) = state.queue.pop()?; let mut parents: ParentIds = Default::default(); - match super::find(self.cache.as_ref(), &mut self.find, &oid, &mut state.buf) { + match super::find(self.cache.as_ref(), &self.objects, &oid, &mut state.buf) { Ok(Either::CachedCommit(commit)) => { if !collect_parents(&mut state.parent_ids, self.cache.as_ref(), commit.iter_parents()) { // drop corrupt caches and try again with ODB @@ -380,7 +369,7 @@ pub mod ancestors { continue; } - let parent = (self.find)(id.as_ref(), &mut state.parents_buf).ok(); + let parent = self.objects.find_commit_iter(id.as_ref(), &mut state.parents_buf).ok(); let parent_commit_time = parent .and_then(|parent| parent.committer().ok().map(|committer| committer.time.seconds)) .unwrap_or_default(); @@ -395,12 +384,7 @@ pub mod ancestors { } } } - Err(err) => { - return Some(Err(Error::FindExisting { - oid, - source: err.into(), - })) - } + Err(err) => return Some(Err(err.into())), } Some(Ok(Info { id: oid, @@ -411,18 +395,17 @@ pub mod ancestors { } /// Utilities - impl Ancestors + impl Ancestors where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Result, E>, + Find: gix_object::Find, Predicate: FnMut(&oid) -> bool, StateMut: BorrowMut, - E: std::error::Error + Send + Sync + 'static, { fn next_by_topology(&mut self) -> Option> { let state = self.state.borrow_mut(); let oid = state.next.pop_front()?; let mut parents: ParentIds = Default::default(); - match super::find(self.cache.as_ref(), &mut self.find, &oid, &mut state.buf) { + match super::find(self.cache.as_ref(), &self.objects, &oid, &mut state.buf) { Ok(Either::CachedCommit(commit)) => { if !collect_parents(&mut state.parent_ids, self.cache.as_ref(), commit.iter_parents()) { // drop corrupt caches and try again with ODB @@ -460,12 +443,7 @@ pub mod ancestors { } } } - Err(err) => { - return Some(Err(Error::FindExisting { - oid, - source: err.into(), - })) - } + Err(err) => return Some(Err(err.into())), } Some(Ok(Info { id: oid, @@ -503,18 +481,17 @@ fn collect_parents( true } -fn find<'cache, 'buf, Find, E>( +fn find<'cache, 'buf, Find>( cache: Option<&'cache gix_commitgraph::Graph>, - mut find: Find, + objects: Find, id: &gix_hash::oid, buf: &'buf mut Vec, -) -> Result, E> +) -> Result, gix_object::find::existing_iter::Error> where - Find: for<'a> FnMut(&gix_hash::oid, &'a mut Vec) -> Result, E>, - E: std::error::Error + Send + Sync + 'static, + Find: gix_object::Find, { match cache.and_then(|cache| cache.commit_by_id(id).map(Either::CachedCommit)) { Some(c) => Ok(c), - None => find(id, buf).map(Either::CommitRefIter), + None => objects.find_commit_iter(id, buf).map(Either::CommitRefIter), } } diff --git a/gix-traverse/src/tree/breadthfirst.rs b/gix-traverse/src/tree/breadthfirst.rs index 115b9a396ac..a6c0a4c780f 100644 --- a/gix-traverse/src/tree/breadthfirst.rs +++ b/gix-traverse/src/tree/breadthfirst.rs @@ -6,8 +6,8 @@ use gix_hash::ObjectId; #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { - #[error("The tree {oid} could not be found")] - NotFound { oid: ObjectId }, + #[error(transparent)] + Find(#[from] gix_object::find::existing_iter::Error), #[error("The delegate cancelled the operation")] Cancelled, #[error(transparent)] @@ -31,8 +31,7 @@ impl State { pub(crate) mod impl_ { use std::borrow::BorrowMut; - use gix_hash::oid; - use gix_object::{tree::EntryMode, TreeRefIter}; + use gix_object::{tree::EntryMode, FindExt, TreeRefIter}; use super::{Error, State}; use crate::tree::Visit; @@ -46,17 +45,17 @@ pub(crate) mod impl_ { /// * `find` - a way to lookup new object data during traversal by their `ObjectId`, writing their data into buffer and returning /// an iterator over entries if the object is present and is a tree. Caching should be implemented within this function /// as needed. The return value is `Option` which degenerates all error information. Not finding a commit should also - /// be considered an errors as all objects in the tree DAG should be present in the database. Hence [`Error::NotFound`] should + /// be considered an errors as all objects in the tree DAG should be present in the database. Hence [`Error::Find`] should /// be escalated into a more specific error if its encountered by the caller. /// * `delegate` - A way to observe entries and control the iteration while allowing the optimizer to let you pay only for what you use. pub fn traverse( root: TreeRefIter<'_>, mut state: StateMut, - mut find: Find, + objects: Find, delegate: &mut V, ) -> Result<(), Error> where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Option>, + Find: gix_object::Find, StateMut: BorrowMut, V: Visit, { @@ -95,10 +94,7 @@ pub(crate) mod impl_ { match state.next.pop_front() { Some(oid) => { delegate.pop_front_tracked_path_and_set_current(); - match find(&oid, &mut state.buf) { - Some(tree_iter) => tree = tree_iter, - None => return Err(Error::NotFound { oid: oid.to_owned() }), - } + tree = objects.find_tree_iter(&oid, &mut state.buf)?; } None => break Ok(()), } diff --git a/gix-traverse/tests/commit/mod.rs b/gix-traverse/tests/commit/mod.rs index 2b6c20460ca..6d9fac3e9c8 100644 --- a/gix-traverse/tests/commit/mod.rs +++ b/gix-traverse/tests/commit/mod.rs @@ -1,6 +1,5 @@ mod ancestor { use gix_hash::{oid, ObjectId}; - use gix_odb::pack::FindExt; use gix_traverse::commit; use crate::hex_to_id; @@ -68,7 +67,7 @@ mod ancestor { let oids = commit::Ancestors::filtered( tips.clone(), commit::ancestors::State::default(), - |oid, buf| store.find_commit_iter(oid, buf).map(|t| t.0), + &store, predicate.clone(), ) .sorting(self.sorting)? @@ -86,14 +85,12 @@ mod ancestor { let (store, tips, expected) = self.setup()?; for use_commitgraph in [false, true] { - let oids = commit::Ancestors::new(tips.clone(), commit::ancestors::State::default(), |oid, buf| { - store.find_commit_iter(oid, buf).map(|t| t.0) - }) - .sorting(self.sorting)? - .parents(self.mode) - .commit_graph(self.setup_commitgraph(store.store_ref(), use_commitgraph)) - .map(|res| res.map(|info| info.id)) - .collect::, _>>()?; + let oids = commit::Ancestors::new(tips.clone(), commit::ancestors::State::default(), &store) + .sorting(self.sorting)? + .parents(self.mode) + .commit_graph(self.setup_commitgraph(store.store_ref(), use_commitgraph)) + .map(|res| res.map(|info| info.id)) + .collect::, _>>()?; assert_eq!(oids, expected); } Ok(()) @@ -346,7 +343,6 @@ mod ancestor { /// Some dates adjusted to be a year apart, but still 'c1' and 'c2' with the same date. mod adjusted_dates { - use gix_odb::FindExt; use gix_traverse::commit::{ancestors, Ancestors, Parents, Sorting}; use crate::{commit::ancestor::TraversalAssertion, hex_to_id}; @@ -403,7 +399,7 @@ mod ancestor { let iter = Ancestors::new( Some(hex_to_id("9902e3c3e8f0c569b4ab295ddf473e6de763e1e7" /* c2 */)), ancestors::State::default(), - move |oid, buf| store.find_commit_iter(oid, buf), + &store, ) .sorting(Sorting::ByCommitTimeNewestFirstCutoffOlderThan { seconds: 978393600, // =2001-01-02 00:00:00 +0000 diff --git a/gix-traverse/tests/tree/mod.rs b/gix-traverse/tests/tree/mod.rs index 2e05fe4d1e7..50efc62fe3a 100644 --- a/gix-traverse/tests/tree/mod.rs +++ b/gix-traverse/tests/tree/mod.rs @@ -24,7 +24,7 @@ fn breadth_first_full_path() -> crate::Result<()> { db.find_tree_iter(&commit.tree_id().expect("a tree is available in a commit"), &mut buf2)? .0, tree::breadthfirst::State::default(), - |oid, buf| db.find_tree_iter(oid, buf).ok().map(|t| t.0), + &db, &mut recorder, )?; @@ -111,7 +111,7 @@ fn breadth_first_filename_only() -> crate::Result<()> { db.find_tree_iter(&commit.tree_id().expect("a tree is available in a commit"), &mut buf2)? .0, tree::breadthfirst::State::default(), - |oid, buf| db.find_tree_iter(oid, buf).ok().map(|t| t.0), + &db, &mut recorder, )?; @@ -138,7 +138,7 @@ fn breadth_first_no_location() -> crate::Result<()> { db.find_tree_iter(&commit.tree_id().expect("a tree is available in a commit"), &mut buf2)? .0, tree::breadthfirst::State::default(), - |oid, buf| db.find_tree_iter(oid, buf).ok().map(|t| t.0), + &db, &mut recorder, )?; diff --git a/gix-worktree-state/src/checkout/chunk.rs b/gix-worktree-state/src/checkout/chunk.rs index 5e5a3c0fb28..af473f93f78 100644 --- a/gix-worktree-state/src/checkout/chunk.rs +++ b/gix-worktree-state/src/checkout/chunk.rs @@ -4,29 +4,22 @@ use std::{ }; use bstr::{BStr, BString}; -use gix_hash::oid; use gix_worktree::Stack; use crate::{checkout, checkout::entry}; mod reduce { - use std::marker::PhantomData; - use crate::checkout; - pub struct Reduce<'entry, E> { + pub struct Reduce<'entry> { pub aggregate: super::Outcome<'entry>, - pub marker: PhantomData, } - impl<'entry, E> gix_features::parallel::Reduce for Reduce<'entry, E> - where - E: std::error::Error + Send + Sync + 'static, - { - type Input = Result, checkout::Error>; + impl<'entry> gix_features::parallel::Reduce for Reduce<'entry> { + type Input = Result, checkout::Error>; type FeedProduce = (); type Output = super::Outcome<'entry>; - type Error = checkout::Error; + type Error = checkout::Error; fn feed(&mut self, item: Self::Input) -> Result { let item = item?; @@ -78,7 +71,7 @@ pub struct Outcome<'a> { #[derive(Clone)] pub struct Context { - pub find: Find, + pub objects: Find, pub path_cache: Stack, pub filters: gix_filter::Pipeline, pub buf: Vec, @@ -106,16 +99,15 @@ impl From<&checkout::Options> for Options { } } -pub fn process<'entry, Find, E>( +pub fn process<'entry, Find>( entries_with_paths: impl Iterator, files: &AtomicUsize, bytes: &AtomicUsize, delayed_filter_results: &mut Vec>, ctx: &mut Context, -) -> Result, checkout::Error> +) -> Result, checkout::Error> where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Result, E> + Clone, - E: std::error::Error + Send + Sync + 'static, + Find: gix_object::Find + Clone, { let mut delayed_symlinks = Vec::new(); let mut collisions = Vec::new(); @@ -162,16 +154,15 @@ where }) } -pub fn process_delayed_filter_results( +pub fn process_delayed_filter_results( mut delayed_filter_results: Vec>, files: &AtomicUsize, bytes: &AtomicUsize, out: &mut Outcome<'_>, ctx: &mut Context, -) -> Result<(), checkout::Error> +) -> Result<(), checkout::Error> where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Result, E> + Clone, - E: std::error::Error + Send + Sync + 'static, + Find: gix_object::Find + Clone, { let Options { destination_is_initially_empty, @@ -288,7 +279,7 @@ where } } -pub fn checkout_entry_handle_result<'entry, Find, E>( +pub fn checkout_entry_handle_result<'entry, Find>( entry: &'entry mut gix_index::Entry, entry_path: &'entry BStr, errors: &mut Vec, @@ -296,22 +287,21 @@ pub fn checkout_entry_handle_result<'entry, Find, E>( files: &AtomicUsize, bytes: &AtomicUsize, Context { - find, + objects, path_cache, filters, buf, options, }: &mut Context, -) -> Result, checkout::Error> +) -> Result, checkout::Error> where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Result, E> + Clone, - E: std::error::Error + Send + Sync + 'static, + Find: gix_object::Find + Clone, { let res = entry::checkout( entry, entry_path, entry::Context { - find, + objects, path_cache, filters, buf, diff --git a/gix-worktree-state/src/checkout/entry.rs b/gix-worktree-state/src/checkout/entry.rs index c19a65ed03b..b913c3bbda3 100644 --- a/gix-worktree-state/src/checkout/entry.rs +++ b/gix-worktree-state/src/checkout/entry.rs @@ -6,13 +6,13 @@ use std::{ use bstr::BStr; use gix_filter::{driver::apply::MaybeDelayed, pipeline::convert::ToWorktreeOutcome}; -use gix_hash::oid; use gix_index::{entry::Stat, Entry}; +use gix_object::FindExt; use gix_worktree::Stack; use io_close::Close; pub struct Context<'a, Find> { - pub find: &'a mut Find, + pub objects: &'a mut Find, pub path_cache: &'a mut Stack, pub filters: &'a mut gix_filter::Pipeline, pub buf: &'a mut Vec, @@ -53,11 +53,11 @@ impl Outcome<'_> { } #[cfg_attr(not(unix), allow(unused_variables))] -pub fn checkout<'entry, Find, E>( +pub fn checkout<'entry, Find>( entry: &'entry mut Entry, entry_path: &'entry BStr, Context { - find, + objects, filters, path_cache, buf, @@ -73,25 +73,25 @@ pub fn checkout<'entry, Find, E>( filter_process_delay, .. }: crate::checkout::chunk::Options, -) -> Result, crate::checkout::Error> +) -> Result, crate::checkout::Error> where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Result, E>, - E: std::error::Error + Send + Sync + 'static, + Find: gix_object::Find, { let dest_relative = gix_path::try_from_bstr(entry_path).map_err(|_| crate::checkout::Error::IllformedUtf8 { path: entry_path.to_owned(), })?; let is_dir = Some(entry.mode == gix_index::entry::Mode::COMMIT || entry.mode == gix_index::entry::Mode::DIR); - let path_cache = path_cache.at_path(dest_relative, is_dir, &mut *find)?; + let path_cache = path_cache.at_path(dest_relative, is_dir, &*objects)?; let dest = path_cache.path(); let object_size = match entry.mode { gix_index::entry::Mode::FILE | gix_index::entry::Mode::FILE_EXECUTABLE => { - let obj = find(&entry.id, buf).map_err(|err| crate::checkout::Error::Find { - err, - oid: entry.id, - path: dest.to_path_buf(), - })?; + let obj = (*objects) + .find_blob(&entry.id, buf) + .map_err(|err| crate::checkout::Error::Find { + err, + path: dest.to_path_buf(), + })?; let filtered = filters.convert_to_worktree( obj.data, @@ -140,11 +140,12 @@ where num_bytes } gix_index::entry::Mode::SYMLINK => { - let obj = find(&entry.id, buf).map_err(|err| crate::checkout::Error::Find { - err, - oid: entry.id, - path: dest.to_path_buf(), - })?; + let obj = (*objects) + .find_blob(&entry.id, buf) + .map_err(|err| crate::checkout::Error::Find { + err, + path: dest.to_path_buf(), + })?; let symlink_destination = gix_path::try_from_byte_slice(obj.data) .map_err(|_| crate::checkout::Error::IllformedUtf8 { path: obj.data.into() })?; @@ -269,14 +270,11 @@ pub(crate) fn open_file( /// Close `file` and store its stats in `entry`, possibly setting `file` executable depending on `set_executable_after_creation`. #[cfg_attr(windows, allow(unused_variables))] -pub(crate) fn finalize_entry( +pub(crate) fn finalize_entry( entry: &mut gix_index::Entry, file: std::fs::File, set_executable_after_creation: Option<&Path>, -) -> Result<(), crate::checkout::Error> -where - E: std::error::Error + Send + Sync + 'static, -{ +) -> Result<(), crate::checkout::Error> { // For possibly existing, overwritten files, we must change the file mode explicitly. #[cfg(unix)] if let Some(path) = set_executable_after_creation { diff --git a/gix-worktree-state/src/checkout/function.rs b/gix-worktree-state/src/checkout/function.rs index 8b0ea923b55..9046af47110 100644 --- a/gix-worktree-state/src/checkout/function.rs +++ b/gix-worktree-state/src/checkout/function.rs @@ -1,12 +1,11 @@ use std::sync::atomic::AtomicBool; use gix_features::{interrupt, parallel::in_parallel_with_finalize}; -use gix_hash::oid; use gix_worktree::{stack, Stack}; use crate::checkout::chunk; -/// Checkout the entire `index` into `dir`, and resolve objects found in index entries with `find` to write their content to their +/// Checkout the entire `index` into `dir`, and resolve objects found in index entries with `objects` to write their content to their /// respective path in `dir`. /// Use `files` to count each fully checked out file, and count the amount written `bytes`. If `should_interrupt` is `true`, the /// operation will abort. @@ -17,39 +16,37 @@ use crate::checkout::chunk; /// Note that interruption still produce an `Ok(…)` value, so the caller should look at `should_interrupt` to communicate the outcome. /// #[allow(clippy::too_many_arguments)] -pub fn checkout( +pub fn checkout( index: &mut gix_index::State, dir: impl Into, - find: Find, + objects: Find, files: &dyn gix_features::progress::Count, bytes: &dyn gix_features::progress::Count, should_interrupt: &AtomicBool, options: crate::checkout::Options, -) -> Result> +) -> Result where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Result, E> + Send + Clone, - E: std::error::Error + Send + Sync + 'static, + Find: gix_object::Find + Send + Clone, { let paths = index.take_path_backing(); - let res = checkout_inner(index, &paths, dir, find, files, bytes, should_interrupt, options); + let res = checkout_inner(index, &paths, dir, objects, files, bytes, should_interrupt, options); index.return_path_backing(paths); res } #[allow(clippy::too_many_arguments)] -fn checkout_inner( +fn checkout_inner( index: &mut gix_index::State, paths: &gix_index::PathStorage, dir: impl Into, - find: Find, + objects: Find, files: &dyn gix_features::progress::Count, bytes: &dyn gix_features::progress::Count, should_interrupt: &AtomicBool, mut options: crate::checkout::Options, -) -> Result> +) -> Result where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Result, E> + Send + Clone, - E: std::error::Error + Send + Sync + 'static, + Find: gix_object::Find + Send + Clone, { let num_files = files.counter(); let num_bytes = bytes.counter(); @@ -72,7 +69,7 @@ where paths, ), filters: options.filters, - find, + objects, }; let chunk::Outcome { @@ -123,7 +120,6 @@ where }, chunk::Reduce { aggregate: Default::default(), - marker: Default::default(), }, )? }; diff --git a/gix-worktree-state/src/checkout/mod.rs b/gix-worktree-state/src/checkout/mod.rs index cddc597a84b..0206e7e3408 100644 --- a/gix-worktree-state/src/checkout/mod.rs +++ b/gix-worktree-state/src/checkout/mod.rs @@ -71,18 +71,17 @@ pub struct Options { /// The error returned by the [checkout()][crate::checkout()] function. #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] -pub enum Error { +pub enum Error { #[error("Could not convert path to UTF8: {}", .path)] IllformedUtf8 { path: BString }, #[error("The clock was off when reading file related metadata after updating a file on disk")] Time(#[from] std::time::SystemTimeError), #[error("IO error while writing blob or reading file metadata or changing filetype")] Io(#[from] std::io::Error), - #[error("object {} for checkout at {} could not be retrieved from object database", .oid.to_hex(), .path.display())] + #[error("object for checkout at {} could not be retrieved from object database", .path.display())] Find { #[source] - err: E, - oid: gix_hash::ObjectId, + err: gix_object::find::existing_object::Error, path: std::path::PathBuf, }, #[error(transparent)] diff --git a/gix-worktree-state/tests/state/checkout.rs b/gix-worktree-state/tests/state/checkout.rs index 6622f55d064..6dbbcba5ba9 100644 --- a/gix-worktree-state/tests/state/checkout.rs +++ b/gix-worktree-state/tests/state/checkout.rs @@ -9,7 +9,7 @@ use std::{ use gix_features::progress; use gix_object::bstr::ByteSlice; -use gix_odb::FindExt; +use gix_object::Data; use gix_testtools::tempfile::TempDir; use gix_worktree_state::checkout::Collision; use once_cell::sync::Lazy; @@ -487,7 +487,7 @@ fn checkout_index_in_tmp_dir( fn checkout_index_in_tmp_dir_opts( opts: gix_worktree_state::checkout::Options, name: &str, - mut allow_return_object: impl FnMut(&gix_hash::oid) -> bool + Send + Clone, + allow_return_object: impl FnMut(&gix_hash::oid) -> bool + Send + Clone, prep_dest: impl Fn(&Path) -> std::io::Result<()>, ) -> crate::Result<(PathBuf, TempDir, gix_index::File, gix_worktree_state::checkout::Outcome)> { let source_tree = fixture_path(name); @@ -497,16 +497,38 @@ fn checkout_index_in_tmp_dir_opts( let destination = gix_testtools::tempfile::tempdir_in(std::env::current_dir()?)?; prep_dest(destination.path()).expect("preparation must succeed"); + #[derive(Clone)] + struct MaybeFind { + allow: std::cell::RefCell, + objects: Find, + } + + impl gix_object::Find for MaybeFind + where + Allow: FnMut(&gix_hash::oid) -> bool + Send + Clone, + Find: gix_object::Find + Send + Clone, + { + fn try_find<'a>( + &self, + id: &gix_hash::oid, + buf: &'a mut Vec, + ) -> Result>, gix_object::find::Error> { + if (self.allow.borrow_mut())(id) { + self.objects.try_find(id, buf) + } else { + Ok(None) + } + } + } + + let db = MaybeFind { + allow: allow_return_object.into(), + objects: odb, + }; let outcome = gix_worktree_state::checkout( &mut index, destination.path(), - move |oid, buf| { - if allow_return_object(oid) { - odb.find_blob(oid, buf) - } else { - Err(gix_odb::find::existing_object::Error::NotFound { oid: oid.to_owned() }) - } - }, + db, &progress::Discard, &progress::Discard, &AtomicBool::default(), diff --git a/gix-worktree-stream/src/entry.rs b/gix-worktree-stream/src/entry.rs index 4d7314db259..b8ef2c5afcc 100644 --- a/gix-worktree-stream/src/entry.rs +++ b/gix-worktree-stream/src/entry.rs @@ -13,8 +13,10 @@ use crate::{protocol, Entry, Stream}; pub enum Error { #[error(transparent)] Io(#[from] std::io::Error), - #[error("Could not find a blob or tree for archival")] - Find(#[source] Box), + #[error("Could not find a tree's leaf, typically a blob")] + Find(#[from] gix_object::find::existing::Error), + #[error("Could not find a tree to traverse")] + FindTree(#[from] gix_object::find::existing_iter::Error), #[error("Could not query attributes for path \"{path}\"")] Attributes { path: BString, diff --git a/gix-worktree-stream/src/from_tree/mod.rs b/gix-worktree-stream/src/from_tree/mod.rs index e59892b6b33..0afd354ef67 100644 --- a/gix-worktree-stream/src/from_tree/mod.rs +++ b/gix-worktree-stream/src/from_tree/mod.rs @@ -1,10 +1,11 @@ use std::io::Write; use gix_object::bstr::BStr; +use gix_object::FindExt; use crate::{entry, entry::Error, protocol, AdditionalEntry, SharedErrorSlot, Stream}; -/// Use `find` to traverse `tree` and fetch the contained blobs to return as [`Stream`], which makes them queryable +/// Use `objects` to traverse `tree` and fetch the contained blobs to return as [`Stream`], which makes them queryable /// on demand with support for streaming each entry. /// /// `pipeline` is used to convert blobs to their worktree representation, and `attributes` is used to read @@ -32,18 +33,17 @@ use crate::{entry, entry::Error, protocol, AdditionalEntry, SharedErrorSlot, Str /// ### Limitations /// /// * `export-subst` is not support, as it requires the entire formatting engine of `git log`. -pub fn from_tree( +pub fn from_tree( tree: gix_hash::ObjectId, - find: Find, + objects: Find, pipeline: gix_filter::Pipeline, - attributes: impl FnMut(&BStr, gix_object::tree::EntryMode, &mut gix_attributes::search::Outcome) -> Result<(), E2> + attributes: impl FnMut(&BStr, gix_object::tree::EntryMode, &mut gix_attributes::search::Outcome) -> Result<(), E> + Send + 'static, ) -> Stream where - Find: for<'a> FnMut(&gix_hash::oid, &'a mut Vec) -> Result, E1> + Clone + Send + 'static, - E1: std::error::Error + Send + Sync + 'static, - E2: std::error::Error + Send + Sync + 'static, + Find: gix_object::Find + Clone + Send + 'static, + E: std::error::Error + Send + Sync + 'static, { let (stream, mut write, additional_entries) = Stream::new(); std::thread::spawn({ @@ -51,7 +51,7 @@ where move || { if let Err(err) = run( tree, - find, + objects, pipeline, attributes, &mut write, @@ -76,11 +76,11 @@ where stream } -fn run( +fn run( tree: gix_hash::ObjectId, - mut find: Find, + objects: Find, mut pipeline: gix_filter::Pipeline, - mut attributes: impl FnMut(&BStr, gix_object::tree::EntryMode, &mut gix_attributes::search::Outcome) -> Result<(), E2> + mut attributes: impl FnMut(&BStr, gix_object::tree::EntryMode, &mut gix_attributes::search::Outcome) -> Result<(), E> + Send + 'static, out: &mut gix_features::io::pipe::Writer, @@ -88,16 +88,14 @@ fn run( additional_entries: std::sync::mpsc::Receiver, ) -> Result<(), Error> where - Find: for<'a> FnMut(&gix_hash::oid, &'a mut Vec) -> Result, E1> + Clone + Send + 'static, - E1: std::error::Error + Send + Sync + 'static, - E2: std::error::Error + Send + Sync + 'static, + Find: gix_object::Find + Clone, + E: std::error::Error + Send + Sync + 'static, { let mut buf = Vec::new(); - let obj = find(tree.as_ref(), &mut buf).map_err(|err| Error::Find(Box::new(err)))?; + let tree_iter = objects.find_tree_iter(tree.as_ref(), &mut buf)?; if pipeline.driver_context_mut().treeish.is_none() { pipeline.driver_context_mut().treeish = Some(tree); } - let tree = gix_object::TreeRefIter::from_bytes(obj.data); let mut attrs = gix_attributes::search::Outcome::default(); attrs.initialize_with_selection(&Default::default(), Some("export-ignore")); @@ -106,10 +104,7 @@ where err, pipeline, attrs, - find: { - let mut find = find.clone(); - move |a: &gix_hash::oid, b: &mut Vec| find(a, b).map_err(|err| Error::Find(Box::new(err))) - }, + objects: objects.clone(), fetch_attributes: move |a: &BStr, b: gix_object::tree::EntryMode, c: &mut gix_attributes::search::Outcome| { attributes(a, b, c).map_err(|err| Error::Attributes { source: Box::new(err), @@ -121,13 +116,9 @@ where buf: Vec::with_capacity(1024), }; gix_traverse::tree::breadthfirst( - tree, + tree_iter, gix_traverse::tree::breadthfirst::State::default(), - |id, buf| { - find(id, buf) - .map(|obj| gix_object::TreeRefIter::from_bytes(obj.data)) - .ok() - }, + &objects, &mut dlg, )?; diff --git a/gix-worktree-stream/src/from_tree/traverse.rs b/gix-worktree-stream/src/from_tree/traverse.rs index df242b304e9..378054c5d22 100644 --- a/gix-worktree-stream/src/from_tree/traverse.rs +++ b/gix-worktree-stream/src/from_tree/traverse.rs @@ -3,15 +3,15 @@ use std::{collections::VecDeque, io::Write}; use gix_filter::{driver::apply::MaybeDelayed, pipeline::convert::ToWorktreeOutcome}; use gix_object::{ bstr::{BStr, BString, ByteSlice, ByteVec}, - tree, + tree, FindExt, }; use gix_traverse::tree::{visit::Action, Visit}; use crate::{entry::Error, protocol, SharedErrorSlot}; -pub struct Delegate<'a, AttributesFn, FindFn> +pub struct Delegate<'a, AttributesFn, Find> where - FindFn: for<'b> FnMut(&gix_hash::oid, &'b mut Vec) -> Result, Error> + 'static, + Find: gix_object::Find, { pub(crate) out: &'a mut gix_features::io::pipe::Writer, pub(crate) err: SharedErrorSlot, @@ -20,13 +20,13 @@ where pub(crate) pipeline: gix_filter::Pipeline, pub(crate) attrs: gix_attributes::search::Outcome, pub(crate) fetch_attributes: AttributesFn, - pub(crate) find: FindFn, + pub(crate) objects: Find, pub(crate) buf: Vec, } -impl Delegate<'_, AttributesFn, FindFn> +impl Delegate<'_, AttributesFn, Find> where - FindFn: for<'b> FnMut(&gix_hash::oid, &'b mut Vec) -> Result, Error> + 'static, + Find: gix_object::Find, AttributesFn: FnMut(&BStr, gix_object::tree::EntryMode, &mut gix_attributes::search::Outcome) -> Result<(), Error> + 'static, { @@ -62,7 +62,7 @@ where if self.ignore_state().is_set() { return Ok(Action::Continue); } - (self.find)(entry.oid, &mut self.buf)?; + self.objects.find(entry.oid, &mut self.buf)?; self.pipeline.driver_context_mut().blob = Some(entry.oid.into()); let converted = self.pipeline.convert_to_worktree( @@ -99,9 +99,9 @@ where } } -impl Visit for Delegate<'_, AttributesFn, FindFn> +impl Visit for Delegate<'_, AttributesFn, Find> where - FindFn: for<'a> FnMut(&gix_hash::oid, &'a mut Vec) -> Result, Error> + 'static, + Find: gix_object::Find, AttributesFn: FnMut(&BStr, gix_object::tree::EntryMode, &mut gix_attributes::search::Outcome) -> Result<(), Error> + 'static, { diff --git a/gix-worktree-stream/tests/stream.rs b/gix-worktree-stream/tests/stream.rs index 22561bb7076..d9452281757 100644 --- a/gix-worktree-stream/tests/stream.rs +++ b/gix-worktree-stream/tests/stream.rs @@ -12,34 +12,45 @@ mod from_tree { }; use gix_attributes::glob::pattern::Case; + use gix_hash::oid; + use gix_object::Data; use gix_object::{bstr::ByteSlice, tree::EntryMode}; - use gix_odb::FindExt; use gix_testtools::once_cell::sync::Lazy; use gix_worktree::stack::state::attributes::Source; use crate::hex_to_id; + #[derive(Clone)] + struct FailObjectRetrieval; + + impl gix_object::Find for FailObjectRetrieval { + fn try_find<'a>( + &self, + _id: &oid, + _buffer: &'a mut Vec, + ) -> Result>, gix_object::find::Error> { + Err(Box::new(Error::new(ErrorKind::Other, "object retrieval failed"))) + } + } + #[test] fn can_receive_err_if_root_is_not_found() { let mut stream = gix_worktree_stream::from_tree( gix_hash::Kind::Sha1.null(), - |_, _| Err(Error::new(ErrorKind::Other, "object retrieval failed")), + FailObjectRetrieval, mutating_pipeline(false), |_, _, _| -> Result<_, Infallible> { unreachable!("must not be called") }, ); let err = stream.next_entry().unwrap_err(); - assert_eq!(err.to_string(), "Could not find a blob or tree for archival"); + assert_eq!(err.to_string(), "Could not find a tree to traverse"); } #[test] fn can_receive_err_if_attribute_not_found() -> gix_testtools::Result { let (_dir, head_tree, odb, _cache) = basic()?; - let mut stream = gix_worktree_stream::from_tree( - head_tree, - move |id, buf| odb.find(id, buf), - mutating_pipeline(false), - |_, _, _| Err(Error::new(ErrorKind::Other, "attribute retrieval failed")), - ); + let mut stream = gix_worktree_stream::from_tree(head_tree, odb, mutating_pipeline(false), |_, _, _| { + Err(Error::new(ErrorKind::Other, "attribute retrieval failed")) + }); let err = stream.next_entry().unwrap_err(); assert_eq!( err.to_string(), @@ -53,14 +64,11 @@ mod from_tree { let (dir, head_tree, odb, mut cache) = basic()?; let mut stream = gix_worktree_stream::from_tree( head_tree, - { - let odb = odb.clone(); - move |id, buf| odb.find(id, buf) - }, + odb.clone(), mutating_pipeline(true), move |rela_path, mode, attrs| { cache - .at_entry(rela_path, mode.is_tree().into(), |id, buf| odb.find_blob(id, buf)) + .at_entry(rela_path, mode.is_tree().into(), &odb) .map(|entry| entry.matching_attributes(attrs)) .map(|_| ()) }, @@ -214,14 +222,11 @@ mod from_tree { let (_dir, head_tree, odb, mut cache) = basic()?; let mut stream = gix_worktree_stream::from_tree( head_tree, - { - let odb = odb.clone(); - move |id, buf| odb.find(id, buf) - }, + odb.clone(), mutating_pipeline(false), move |rela_path, mode, attrs| { cache - .at_entry(rela_path, mode.is_tree().into(), |id, buf| odb.find_blob(id, buf)) + .at_entry(rela_path, mode.is_tree().into(), &odb) .map(|entry| entry.matching_attributes(attrs)) .map(|_| ()) }, diff --git a/gix-worktree/src/stack/delegate.rs b/gix-worktree/src/stack/delegate.rs index 8e48057dea7..1234346c5de 100644 --- a/gix-worktree/src/stack/delegate.rs +++ b/gix-worktree/src/stack/delegate.rs @@ -18,19 +18,13 @@ pub struct Statistics { pub pop_directory: usize, } -pub(crate) type FindFn<'a> = dyn for<'b> FnMut( - &gix_hash::oid, - &'b mut Vec, - ) -> Result, Box> - + 'a; - pub(crate) struct StackDelegate<'a, 'find> { pub state: &'a mut State, pub buf: &'a mut Vec, #[cfg_attr(not(feature = "attributes"), allow(dead_code))] pub is_dir: bool, pub id_mappings: &'a Vec, - pub find: &'find mut FindFn<'find>, + pub objects: &'find dyn gix_object::Find, pub case: gix_glob::pattern::Case, pub statistics: &'a mut super::Statistics, } @@ -63,7 +57,7 @@ impl<'a, 'find> gix_fs::stack::Delegate for StackDelegate<'a, 'find> { rela_dir, self.buf, self.id_mappings, - self.find, + self.objects, &mut self.statistics.attributes, )?; } @@ -75,7 +69,7 @@ impl<'a, 'find> gix_fs::stack::Delegate for StackDelegate<'a, 'find> { rela_dir, self.buf, self.id_mappings, - &mut self.find, + self.objects, &mut self.statistics.attributes, )?; ignore.push_directory( @@ -84,7 +78,7 @@ impl<'a, 'find> gix_fs::stack::Delegate for StackDelegate<'a, 'find> { rela_dir, self.buf, self.id_mappings, - &mut self.find, + self.objects, self.case, &mut self.statistics.ignore, )? @@ -95,7 +89,7 @@ impl<'a, 'find> gix_fs::stack::Delegate for StackDelegate<'a, 'find> { rela_dir, self.buf, self.id_mappings, - &mut self.find, + self.objects, self.case, &mut self.statistics.ignore, )?, diff --git a/gix-worktree/src/stack/mod.rs b/gix-worktree/src/stack/mod.rs index 228467f5bee..387acf2acff 100644 --- a/gix-worktree/src/stack/mod.rs +++ b/gix-worktree/src/stack/mod.rs @@ -2,7 +2,6 @@ use std::path::{Path, PathBuf}; use bstr::{BStr, ByteSlice}; -use gix_hash::oid; use super::Stack; use crate::PathIdMapping; @@ -106,18 +105,17 @@ impl Stack { /// symlinks are in that path. /// Unless `is_dir` is known with `Some(…)`, then `relative` points to a directory itself in which case the entire resulting /// path is created as directory. If it's not known it is assumed to be a file. - /// `find` maybe used to lookup objects from an [id mapping][crate::stack::State::id_mappings_from_index()], with mappnigs + /// `objects` maybe used to lookup objects from an [id mapping][crate::stack::State::id_mappings_from_index()], with mappnigs /// /// Provide access to cached information for that `relative` path via the returned platform. - pub fn at_path( + pub fn at_path( &mut self, relative: impl AsRef, is_dir: Option, - mut find: Find, + objects: Find, ) -> std::io::Result> where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Result, E>, - E: std::error::Error + Send + Sync + 'static, + Find: gix_object::Find, { self.statistics.platforms += 1; let mut delegate = StackDelegate { @@ -125,7 +123,7 @@ impl Stack { buf: &mut self.buf, is_dir: is_dir.unwrap_or(false), id_mappings: &self.id_mappings, - find: &mut |oid, buf| Ok(find(oid, buf).map_err(Box::new)?), + objects: &objects, case: self.case, statistics: &mut self.statistics, }; @@ -136,7 +134,7 @@ impl Stack { /// Obtain a platform for lookups from a repo-`relative` path, typically obtained from an index entry. `is_dir` should reflect /// whether it's a directory or not, or left at `None` if unknown. - /// `find` maybe used to lookup objects from an [id mapping][crate::stack::State::id_mappings_from_index()]. + /// `objects` maybe used to lookup objects from an [id mapping][crate::stack::State::id_mappings_from_index()]. /// All effects are similar to [`at_path()`][Self::at_path()]. /// /// If `relative` ends with `/` and `is_dir` is `None`, it is automatically assumed to be a directory. @@ -144,15 +142,14 @@ impl Stack { /// ### Panics /// /// on illformed UTF8 in `relative` - pub fn at_entry<'r, Find, E>( + pub fn at_entry<'r, Find>( &mut self, relative: impl Into<&'r BStr>, is_dir: Option, - find: Find, + objects: Find, ) -> std::io::Result> where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Result, E>, - E: std::error::Error + Send + Sync + 'static, + Find: gix_object::Find, { let relative = relative.into(); let relative_path = gix_path::from_bstr(relative); @@ -160,7 +157,7 @@ impl Stack { self.at_path( relative_path, is_dir.or_else(|| relative.ends_with_str("/").then_some(true)), - find, + objects, ) } } diff --git a/gix-worktree/src/stack/state/attributes.rs b/gix-worktree/src/stack/state/attributes.rs index d49de1288c1..04ad8b5c712 100644 --- a/gix-worktree/src/stack/state/attributes.rs +++ b/gix-worktree/src/stack/state/attributes.rs @@ -2,8 +2,8 @@ use std::path::{Path, PathBuf}; use bstr::{BStr, ByteSlice}; use gix_glob::pattern::Case; +use gix_object::FindExt; -use crate::stack::delegate::FindFn; use crate::{ stack::state::{AttributeMatchGroup, Attributes}, PathIdMapping, Stack, @@ -95,7 +95,7 @@ impl Attributes { rela_dir: &BStr, buf: &mut Vec, id_mappings: &[PathIdMapping], - find: &mut FindFn<'_>, + objects: &dyn gix_object::Find, stats: &mut Statistics, ) -> std::io::Result<()> { let attr_path_relative = @@ -109,7 +109,8 @@ impl Attributes { match self.source { Source::IdMapping | Source::IdMappingThenWorktree => { if let Ok(idx) = attr_file_in_index { - let blob = find(&id_mappings[idx].1, buf) + let blob = objects + .find_blob(&id_mappings[idx].1, buf) .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?; let attr_path = gix_path::from_bstring(attr_path_relative.into_owned()); self.stack.add_patterns_buffer( @@ -147,7 +148,8 @@ impl Attributes { stats.pattern_files += usize::from(added); stats.tried_pattern_files += 1; if let Some(idx) = attr_file_in_index.ok().filter(|_| !added) { - let blob = find(&id_mappings[idx].1, buf) + let blob = objects + .find_blob(&id_mappings[idx].1, buf) .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?; let attr_path = gix_path::from_bstring(attr_path_relative.into_owned()); self.stack.add_patterns_buffer( diff --git a/gix-worktree/src/stack/state/ignore.rs b/gix-worktree/src/stack/state/ignore.rs index e2a2d5a3d99..370459053b8 100644 --- a/gix-worktree/src/stack/state/ignore.rs +++ b/gix-worktree/src/stack/state/ignore.rs @@ -2,8 +2,8 @@ use std::path::Path; use bstr::{BStr, ByteSlice}; use gix_glob::pattern::Case; +use gix_object::FindExt; -use crate::stack::delegate::FindFn; use crate::{ stack::state::{Ignore, IgnoreMatchGroup}, PathIdMapping, @@ -164,7 +164,7 @@ impl Ignore { rela_dir: &BStr, buf: &mut Vec, id_mappings: &[PathIdMapping], - find: &mut FindFn<'_>, + objects: &dyn gix_object::Find, case: Case, stats: &mut Statistics, ) -> std::io::Result<()> { @@ -177,7 +177,8 @@ impl Ignore { Source::IdMapping => { match ignore_file_in_index { Ok(idx) => { - let ignore_blob = find(&id_mappings[idx].1, buf) + let ignore_blob = objects + .find_blob(&id_mappings[idx].1, buf) .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?; let ignore_path = gix_path::from_bstring(ignore_path_relative.into_owned()); self.stack @@ -204,7 +205,8 @@ impl Ignore { if !added { match ignore_file_in_index { Ok(idx) => { - let ignore_blob = find(&id_mappings[idx].1, buf) + let ignore_blob = objects + .find_blob(&id_mappings[idx].1, buf) .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?; let ignore_path = gix_path::from_bstring(ignore_path_relative.into_owned()); self.stack diff --git a/gix-worktree/tests/worktree/stack/attributes.rs b/gix-worktree/tests/worktree/stack/attributes.rs index a563a51468e..d595a9e2860 100644 --- a/gix-worktree/tests/worktree/stack/attributes.rs +++ b/gix-worktree/tests/worktree/stack/attributes.rs @@ -29,9 +29,7 @@ fn baseline() -> crate::Result { let mut actual = cache.attribute_matches(); let input = std::fs::read(base.join("baseline"))?; for (rela_path, expected) in (baseline::Expectations { lines: input.lines() }) { - let entry = cache.at_entry(rela_path, None, |_, _| -> Result<_, std::convert::Infallible> { - unreachable!("we provide not id-mappings") - })?; + let entry = cache.at_entry(rela_path, None, gix_object::find::Never)?; let has_match = entry.matching_attributes(&mut actual); assert_eq!( @@ -52,9 +50,7 @@ fn baseline() -> crate::Result { let mut actual = cache.selected_attribute_matches(["info", "test"]); let input = std::fs::read(base.join("baseline.selected"))?; for (rela_path, expected) in (baseline::Expectations { lines: input.lines() }) { - let entry = cache.at_entry(rela_path, None, |_, _| -> Result<_, std::convert::Infallible> { - unreachable!("we provide not id-mappings") - })?; + let entry = cache.at_entry(rela_path, None, gix_object::find::Never)?; let has_match = entry.matching_attributes(&mut actual); assert_eq!( diff --git a/gix-worktree/tests/worktree/stack/create_directory.rs b/gix-worktree/tests/worktree/stack/create_directory.rs index 7d21c550394..4ae15cf70f6 100644 --- a/gix-worktree/tests/worktree/stack/create_directory.rs +++ b/gix-worktree/tests/worktree/stack/create_directory.rs @@ -3,11 +3,6 @@ use std::path::Path; use gix_testtools::tempfile::{tempdir, TempDir}; use gix_worktree::{stack, Stack}; -#[allow(clippy::ptr_arg)] -fn panic_on_find<'buf>(_oid: &gix_hash::oid, _buf: &'buf mut Vec) -> std::io::Result> { - unreachable!("find should not be called") -} - #[test] fn root_is_assumed_to_exist_and_files_in_root_do_not_create_directory() -> crate::Result { let dir = tempdir()?; @@ -20,7 +15,7 @@ fn root_is_assumed_to_exist_and_files_in_root_do_not_create_directory() -> crate ); assert_eq!(cache.statistics().delegate.num_mkdir_calls, 0); - let path = cache.at_path("hello", Some(false), panic_on_find)?.path(); + let path = cache.at_path("hello", Some(false), gix_object::find::Never)?.path(); assert!(!path.parent().unwrap().exists(), "prefix itself is never created"); assert_eq!(cache.statistics().delegate.num_mkdir_calls, 0); Ok(()) @@ -38,7 +33,7 @@ fn directory_paths_are_created_in_full() { ("link", None), ] { let path = cache - .at_path(Path::new("dir").join(name), *is_dir, panic_on_find) + .at_path(Path::new("dir").join(name), *is_dir, gix_object::find::Never) .unwrap() .path(); assert!(path.parent().unwrap().is_dir(), "dir exists"); @@ -52,7 +47,7 @@ fn existing_directories_are_fine() -> crate::Result { let (mut cache, tmp) = new_cache(); std::fs::create_dir(tmp.path().join("dir"))?; - let path = cache.at_path("dir/file", Some(false), panic_on_find)?.path(); + let path = cache.at_path("dir/file", Some(false), gix_object::find::Never)?.path(); assert!(path.parent().unwrap().is_dir(), "directory is still present"); assert!(!path.exists(), "it won't create the file"); assert_eq!(cache.statistics().delegate.num_mkdir_calls, 1); @@ -77,7 +72,7 @@ fn symlinks_or_files_in_path_are_forbidden_or_unlinked_when_forced() -> crate::R let relative_path = format!("{dirname}/file"); assert_eq!( cache - .at_path(&relative_path, Some(false), panic_on_find) + .at_path(&relative_path, Some(false), gix_object::find::Never) .unwrap_err() .kind(), std::io::ErrorKind::AlreadyExists @@ -97,7 +92,9 @@ fn symlinks_or_files_in_path_are_forbidden_or_unlinked_when_forced() -> crate::R *unlink_on_collision = true; } let relative_path = format!("{dirname}/file"); - let path = cache.at_path(&relative_path, Some(false), panic_on_find)?.path(); + let path = cache + .at_path(&relative_path, Some(false), gix_object::find::Never)? + .path(); assert!(path.parent().unwrap().is_dir(), "directory was forcefully created"); assert!(!path.exists()); } diff --git a/gix-worktree/tests/worktree/stack/ignore.rs b/gix-worktree/tests/worktree/stack/ignore.rs index b367cd05c3a..6dbcecb5cb8 100644 --- a/gix-worktree/tests/worktree/stack/ignore.rs +++ b/gix-worktree/tests/worktree/stack/ignore.rs @@ -1,5 +1,4 @@ use bstr::{BStr, ByteSlice}; -use gix_odb::FindExt; use gix_worktree::{stack::state::ignore::Source, Stack}; use crate::hex_to_id; @@ -51,16 +50,22 @@ fn exclude_by_dir_is_handled_just_like_git() { let expectations = IgnoreExpectations { lines: baseline.lines(), }; + struct FindError; + impl gix_object::Find for FindError { + fn try_find<'a>( + &self, + _id: &gix_hash::oid, + _buffer: &'a mut Vec, + ) -> Result>, gix_object::find::Error> { + Err(std::io::Error::new(std::io::ErrorKind::Other, "unreachable").into()) + } + } for (relative_entry, source_and_line) in expectations { let (source, line, expected_pattern) = source_and_line.expect("every value is matched"); let relative_path = gix_path::from_byte_slice(relative_entry); let is_dir = dir.join(relative_path).metadata().ok().map(|m| m.is_dir()); - let platform = cache - .at_entry(relative_entry, is_dir, |_oid, _buf| { - Err(std::io::Error::new(std::io::ErrorKind::Other, "unreachable")) - }) - .unwrap(); + let platform = cache.at_entry(relative_entry, is_dir, FindError).unwrap(); let match_ = platform.matching_exclude_pattern().expect("match all values"); let _is_excluded = platform.is_excluded(); assert_eq!( @@ -119,7 +124,7 @@ fn check_against_baseline() -> crate::Result { let relative_path = gix_path::from_byte_slice(relative_entry); let is_dir = worktree_dir.join(relative_path).metadata().ok().map(|m| m.is_dir()); - let platform = cache.at_entry(relative_entry, is_dir, |oid, buf| odb.find_blob(oid, buf))?; + let platform = cache.at_entry(relative_entry, is_dir, &odb)?; let match_ = platform.matching_exclude_pattern(); let is_excluded = platform.is_excluded(); diff --git a/gix/src/attribute_stack.rs b/gix/src/attribute_stack.rs index 1aaca0f2b35..d0495c9b21c 100644 --- a/gix/src/attribute_stack.rs +++ b/gix/src/attribute_stack.rs @@ -1,7 +1,6 @@ use crate::bstr::BStr; use crate::types::AttributeStack; use crate::Repository; -use gix_odb::FindExt; use std::ops::{Deref, DerefMut}; /// Lifecycle @@ -46,8 +45,7 @@ impl<'repo> AttributeStack<'repo> { relative: impl AsRef, is_dir: Option, ) -> std::io::Result> { - self.inner - .at_path(relative, is_dir, |id, buf| self.repo.objects.find_blob(id, buf)) + self.inner.at_path(relative, is_dir, &self.repo.objects) } /// Obtain a platform for lookups from a repo-`relative` path, typically obtained from an index entry. `is_dir` should reflect @@ -63,7 +61,6 @@ impl<'repo> AttributeStack<'repo> { relative: impl Into<&'r BStr>, is_dir: Option, ) -> std::io::Result> { - self.inner - .at_entry(relative, is_dir, |id, buf| self.repo.objects.find_blob(id, buf)) + self.inner.at_entry(relative, is_dir, &self.repo.objects) } } diff --git a/gix/src/clone/checkout.rs b/gix/src/clone/checkout.rs index 664f6e96d38..84a3fedbddb 100644 --- a/gix/src/clone/checkout.rs +++ b/gix/src/clone/checkout.rs @@ -4,8 +4,6 @@ use crate::{clone::PrepareCheckout, Repository}; pub mod main_worktree { use std::{path::PathBuf, sync::atomic::AtomicBool}; - use gix_odb::FindExt; - use crate::{clone::PrepareCheckout, Progress, Repository}; /// The error returned by [`PrepareCheckout::main_worktree()`]. @@ -26,7 +24,7 @@ pub mod main_worktree { #[error(transparent)] CheckoutOptions(#[from] crate::config::checkout_options::Error), #[error(transparent)] - IndexCheckout(#[from] gix_worktree_state::checkout::Error), + IndexCheckout(#[from] gix_worktree_state::checkout::Error), #[error("Failed to reopen object database as Arc (only if thread-safety wasn't compiled in)")] OpenArcOdb(#[from] std::io::Error), #[error("The HEAD reference could not be located")] @@ -96,11 +94,10 @@ pub mod main_worktree { )) } }; - let index = gix_index::State::from_tree(&root_tree, |oid, buf| repo.objects.find_tree_iter(oid, buf).ok()) - .map_err(|err| Error::IndexFromTree { - id: root_tree, - source: err, - })?; + let index = gix_index::State::from_tree(&root_tree, &repo.objects).map_err(|err| Error::IndexFromTree { + id: root_tree, + source: err, + })?; let mut index = gix_index::File::from_state(index, repo.index_path()); let mut opts = repo @@ -118,10 +115,7 @@ pub mod main_worktree { let outcome = gix_worktree_state::checkout( &mut index, workdir, - { - let objects = repo.objects.clone().into_arc()?; - move |oid, buf| objects.find_blob(oid, buf) - }, + repo.objects.clone().into_arc()?, &files, &bytes, should_interrupt, diff --git a/gix/src/clone/fetch/util.rs b/gix/src/clone/fetch/util.rs index db9bc0a1c8f..627201301de 100644 --- a/gix/src/clone/fetch/util.rs +++ b/gix/src/clone/fetch/util.rs @@ -1,6 +1,5 @@ use std::{borrow::Cow, convert::TryInto, io::Write}; -use gix_odb::Find; use gix_ref::{ transaction::{LogChange, RefLog}, FullNameRef, @@ -107,7 +106,7 @@ pub fn update_head( repo.refs .transaction() .packed_refs(gix_ref::file::transaction::PackedRefs::DeletionsAndNonSymbolicUpdates( - Box::new(|oid, buf| repo.objects.try_find(&oid, buf).map(|obj| obj.map(|obj| obj.kind))), + Box::new(&repo.objects), )) .prepare( { diff --git a/gix/src/commit.rs b/gix/src/commit.rs index 2cc8226f5a4..ce5dee4d6f7 100644 --- a/gix/src/commit.rs +++ b/gix/src/commit.rs @@ -28,7 +28,6 @@ pub mod describe { use gix_hash::ObjectId; use gix_hashtable::HashMap; - use gix_odb::Find; use crate::{bstr::BStr, ext::ObjectIdExt, Repository}; @@ -199,12 +198,7 @@ pub mod describe { pub fn try_resolve(&self) -> Result>, Error> { // TODO: dirty suffix with respective dirty-detection let mut graph = gix_revwalk::Graph::new( - |id, buf| { - self.repo - .objects - .try_find(id, buf) - .map(|r| r.and_then(gix_object::Data::try_into_commit_iter)) - }, + &self.repo.objects, gix_commitgraph::Graph::from_info_dir(self.repo.objects.store_ref().path().join("info").as_ref()).ok(), ); let outcome = gix_revision::describe( diff --git a/gix/src/ext/object_id.rs b/gix/src/ext/object_id.rs index a4515022b9d..d4d9467664b 100644 --- a/gix/src/ext/object_id.rs +++ b/gix/src/ext/object_id.rs @@ -8,10 +8,9 @@ pub type AncestorsIter = Ancestors bool, ances /// An extension trait to add functionality to [`ObjectId`]s. pub trait ObjectIdExt: Sealed { /// Create an iterator over the ancestry of the commits reachable from this id, which must be a commit. - fn ancestors(self, find: Find) -> AncestorsIter + fn ancestors(self, find: Find) -> AncestorsIter where - Find: for<'a> FnMut(&gix_hash::oid, &'a mut Vec) -> Result, E>, - E: std::error::Error + Send + Sync + 'static; + Find: gix_object::Find; /// Infuse this object id `repo` access. fn attach(self, repo: &crate::Repository) -> crate::Id<'_>; @@ -20,10 +19,9 @@ pub trait ObjectIdExt: Sealed { impl Sealed for ObjectId {} impl ObjectIdExt for ObjectId { - fn ancestors(self, find: Find) -> AncestorsIter + fn ancestors(self, find: Find) -> AncestorsIter where - Find: for<'a> FnMut(&gix_hash::oid, &'a mut Vec) -> Result, E>, - E: std::error::Error + Send + Sync + 'static, + Find: gix_object::Find, { Ancestors::new(Some(self), ancestors::State::default(), find) } diff --git a/gix/src/ext/tree.rs b/gix/src/ext/tree.rs index 56b832b84f4..9aacc9d5808 100644 --- a/gix/src/ext/tree.rs +++ b/gix/src/ext/tree.rs @@ -1,6 +1,5 @@ use std::borrow::BorrowMut; -use gix_hash::oid; use gix_object::TreeRefIter; use gix_traverse::tree::breadthfirst; @@ -16,11 +15,11 @@ pub trait TreeIterExt: Sealed { fn traverse( &self, state: StateMut, - find: Find, + objects: Find, delegate: &mut V, ) -> Result<(), breadthfirst::Error> where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Option>, + Find: gix_object::Find, StateMut: BorrowMut, V: gix_traverse::tree::Visit; } @@ -31,15 +30,15 @@ impl<'d> TreeIterExt for TreeRefIter<'d> { fn traverse( &self, state: StateMut, - find: Find, + objects: Find, delegate: &mut V, ) -> Result<(), breadthfirst::Error> where - Find: for<'a> FnMut(&oid, &'a mut Vec) -> Option>, + Find: gix_object::Find, StateMut: BorrowMut, V: gix_traverse::tree::Visit, { - breadthfirst(self.clone(), state, find, delegate) + breadthfirst(*self, state, objects, delegate) } } diff --git a/gix/src/filter.rs b/gix/src/filter.rs index b106840a79a..c3fda49eb23 100644 --- a/gix/src/filter.rs +++ b/gix/src/filter.rs @@ -2,7 +2,7 @@ use std::borrow::Cow; pub use gix_filter as plumbing; -use gix_odb::{Find, FindExt}; +use gix_object::Find; use crate::{ bstr::BStr, @@ -141,16 +141,14 @@ impl<'repo> Pipeline<'repo> { where R: std::io::Read, { - let entry = self - .cache - .at_path(rela_path, Some(false), |id, buf| self.repo.objects.find_blob(id, buf))?; + let entry = self.cache.at_path(rela_path, Some(false), &self.repo.objects)?; Ok(self.inner.convert_to_git( src, rela_path, &mut |_, attrs| { entry.matching_attributes(attrs); }, - &mut |buf| -> Result<_, gix_odb::find::Error> { + &mut |buf| -> Result<_, gix_object::find::Error> { let entry = match index.entry_by_path(gix_path::into_bstr(rela_path).as_ref()) { None => return Ok(None), Some(entry) => entry, @@ -175,9 +173,7 @@ impl<'repo> Pipeline<'repo> { can_delay: gix_filter::driver::apply::Delay, ) -> Result, pipeline::convert_to_worktree::Error> { - let entry = self - .cache - .at_entry(rela_path, Some(false), |id, buf| self.repo.objects.find_blob(id, buf))?; + let entry = self.cache.at_entry(rela_path, Some(false), &self.repo.objects)?; Ok(self.inner.convert_to_worktree( src, rela_path, diff --git a/gix/src/object/errors.rs b/gix/src/object/errors.rs index 92789b6cbe5..db81daacb23 100644 --- a/gix/src/object/errors.rs +++ b/gix/src/object/errors.rs @@ -20,12 +20,12 @@ pub mod find { /// Indicate that an error occurred when trying to find an object. #[derive(Debug, thiserror::Error)] #[error(transparent)] - pub struct Error(#[from] pub gix_odb::find::Error); + pub struct Error(#[from] pub gix_object::find::Error); /// pub mod existing { /// An object could not be found in the database, or an error occurred when trying to obtain it. - pub type Error = gix_odb::find::existing::Error; + pub type Error = gix_object::find::existing::Error; } } @@ -34,5 +34,5 @@ pub mod write { /// An error to indicate writing to the loose object store failed. #[derive(Debug, thiserror::Error)] #[error(transparent)] - pub struct Error(#[from] pub gix_odb::find::Error); + pub struct Error(#[from] pub gix_object::find::Error); } diff --git a/gix/src/object/tree/diff/for_each.rs b/gix/src/object/tree/diff/for_each.rs index 3932f902788..cd9c60f547d 100644 --- a/gix/src/object/tree/diff/for_each.rs +++ b/gix/src/object/tree/diff/for_each.rs @@ -1,5 +1,4 @@ use gix_object::TreeRefIter; -use gix_odb::FindExt; use super::{change, Action, Change, Platform}; use crate::{ @@ -61,7 +60,7 @@ impl<'a, 'old> Platform<'a, 'old> { match gix_diff::tree::Changes::from(TreeRefIter::from_bytes(&self.lhs.data)).needed_to_obtain( TreeRefIter::from_bytes(&other.data), &mut self.state, - |oid, buf| repo.objects.find_tree_iter(oid, buf), + &repo.objects, &mut delegate, ) { Ok(()) => { diff --git a/gix/src/object/tree/mod.rs b/gix/src/object/tree/mod.rs index e4dac24f8f5..75c2d9a9825 100644 --- a/gix/src/object/tree/mod.rs +++ b/gix/src/object/tree/mod.rs @@ -1,8 +1,8 @@ use gix_hash::ObjectId; use gix_macros::momo; pub use gix_object::tree::EntryMode; +use gix_object::FindExt; use gix_object::{bstr::BStr, TreeRefIter}; -use gix_odb::FindExt; use crate::{object::find, Id, ObjectDetached, Tree}; diff --git a/gix/src/object/tree/traverse.rs b/gix/src/object/tree/traverse.rs index 974df6b0db9..f2f3ff817f8 100644 --- a/gix/src/object/tree/traverse.rs +++ b/gix/src/object/tree/traverse.rs @@ -1,5 +1,3 @@ -use gix_odb::FindExt; - use crate::Tree; /// Traversal @@ -52,11 +50,6 @@ impl<'a, 'repo> Platform<'a, 'repo> { { let root = gix_object::TreeRefIter::from_bytes(&self.root.data); let state = gix_traverse::tree::breadthfirst::State::default(); - gix_traverse::tree::breadthfirst( - root, - state, - |oid, buf| self.root.repo.objects.find_tree_iter(oid, buf).ok(), - delegate, - ) + gix_traverse::tree::breadthfirst(root, state, &self.root.repo.objects, delegate) } } diff --git a/gix/src/pathspec.rs b/gix/src/pathspec.rs index 235a91d7630..a56ad1c321e 100644 --- a/gix/src/pathspec.rs +++ b/gix/src/pathspec.rs @@ -1,6 +1,5 @@ //! Pathspec plumbing and abstractions use gix_macros::momo; -use gix_odb::FindExt; pub use gix_pathspec::*; use crate::{bstr::BStr, AttributeStack, Pathspec, PathspecDetached, Repository}; @@ -122,9 +121,7 @@ impl<'repo> Pathspec<'repo> { let stack = self.stack.as_mut().expect("initialized in advance"); stack .set_case(case) - .at_entry(relative_path, Some(is_dir), |id, buf| { - self.repo.objects.find_blob(id, buf) - }) + .at_entry(relative_path, Some(is_dir), &self.repo.objects) .map_or(false, |platform| platform.matching_attributes(out)) }, ) @@ -180,7 +177,7 @@ impl PathspecDetached { let stack = self.stack.as_mut().expect("initialized in advance"); stack .set_case(case) - .at_entry(relative_path, Some(is_dir), |id, buf| self.odb.find_blob(id, buf)) + .at_entry(relative_path, Some(is_dir), &self.odb) .map_or(false, |platform| platform.matching_attributes(out)) }, ) diff --git a/gix/src/prelude.rs b/gix/src/prelude.rs index 36fbfc7b186..8a3e01124c2 100644 --- a/gix/src/prelude.rs +++ b/gix/src/prelude.rs @@ -1,4 +1,5 @@ pub use gix_features::parallel::reduce::Finalize; -pub use gix_odb::{Find, FindExt, Header, HeaderExt, Write}; +pub use gix_object::{Find, FindExt}; +pub use gix_odb::{Header, HeaderExt, Write}; pub use crate::ext::*; diff --git a/gix/src/reference/iter.rs b/gix/src/reference/iter.rs index a79a74743c7..604a8ac4b05 100644 --- a/gix/src/reference/iter.rs +++ b/gix/src/reference/iter.rs @@ -2,7 +2,6 @@ use std::path::Path; use gix_macros::momo; -use gix_odb::pack::Find; use gix_ref::file::ReferenceExt; /// A platform to create iterators over references. @@ -96,15 +95,10 @@ impl<'r> Iterator for Iter<'r> { res.map_err(|err| Box::new(err) as Box) .and_then(|mut r| { if self.peel { - let handle = &self.repo; - r.peel_to_id_in_place(&handle.refs, &mut |oid, buf| { - handle - .objects - .try_find(oid.as_ref(), buf) - .map(|po| po.map(|(o, _l)| (o.kind, o.data))) - }) - .map_err(|err| Box::new(err) as Box) - .map(|_| r) + let repo = &self.repo; + r.peel_to_id_in_place(&repo.refs, &repo.objects) + .map_err(|err| Box::new(err) as Box) + .map(|_| r) } else { Ok(r) } diff --git a/gix/src/reference/mod.rs b/gix/src/reference/mod.rs index 2d32f595d7a..ebdbf66a717 100644 --- a/gix/src/reference/mod.rs +++ b/gix/src/reference/mod.rs @@ -1,6 +1,5 @@ //! -use gix_odb::pack::Find; use gix_ref::file::ReferenceExt; use crate::{Id, Reference}; @@ -69,13 +68,8 @@ impl<'repo> Reference<'repo> { /// /// This is useful to learn where this reference is ultimately pointing to. pub fn peel_to_id_in_place(&mut self) -> Result, peel::Error> { - let repo = &self.repo; - let oid = self.inner.peel_to_id_in_place(&repo.refs, &mut |oid, buf| { - repo.objects - .try_find(&oid, buf) - .map(|po| po.map(|(o, _l)| (o.kind, o.data))) - })?; - Ok(Id::from_id(oid, repo)) + let oid = self.inner.peel_to_id_in_place(&self.repo.refs, &self.repo.objects)?; + Ok(Id::from_id(oid, self.repo)) } /// Similar to [`peel_to_id_in_place()`][Reference::peel_to_id_in_place()], but consumes this instance. diff --git a/gix/src/remote/connection/fetch/negotiate.rs b/gix/src/remote/connection/fetch/negotiate.rs index 92a141f6fa2..f5b6a031c4c 100644 --- a/gix/src/remote/connection/fetch/negotiate.rs +++ b/gix/src/remote/connection/fetch/negotiate.rs @@ -16,7 +16,7 @@ pub enum Error { #[error("We were unable to figure out what objects the server should send after {rounds} round(s)")] NegotiationFailed { rounds: usize }, #[error(transparent)] - LookupCommitInGraph(#[from] gix_revwalk::graph::lookup::commit::Error), + LookupCommitInGraph(#[from] gix_revwalk::graph::try_lookup_or_insert_default::Error), #[error(transparent)] InitRefsIterator(#[from] crate::reference::iter::init::Error), #[error(transparent)] diff --git a/gix/src/remote/connection/fetch/receive_pack.rs b/gix/src/remote/connection/fetch/receive_pack.rs index cff2d0f40d1..26e71f4ca36 100644 --- a/gix/src/remote/connection/fetch/receive_pack.rs +++ b/gix/src/remote/connection/fetch/receive_pack.rs @@ -3,7 +3,7 @@ use std::{ sync::atomic::{AtomicBool, Ordering}, }; -use gix_odb::{store::RefreshMode, FindExt}; +use gix_odb::store::RefreshMode; use gix_protocol::{ fetch::Arguments, transport::{client::Transport, packetline::read::ProgressAction}, @@ -277,7 +277,7 @@ where should_interrupt, Some(Box::new({ let repo = repo.clone(); - move |oid, buf| repo.objects.find(&oid, buf).ok() + repo.objects })), options, )?; diff --git a/gix/src/remote/connection/fetch/update_refs/mod.rs b/gix/src/remote/connection/fetch/update_refs/mod.rs index 3d6fb18bdb2..c487e7f5c2c 100644 --- a/gix/src/remote/connection/fetch/update_refs/mod.rs +++ b/gix/src/remote/connection/fetch/update_refs/mod.rs @@ -1,7 +1,7 @@ #![allow(clippy::result_large_err)] use std::{collections::BTreeMap, convert::TryInto, path::PathBuf}; -use gix_odb::{Find, FindExt}; +use gix_object::Exists; use gix_ref::{ transaction::{Change, LogChange, PreviousValue, RefEdit, RefLog}, Target, TargetRef, @@ -96,8 +96,8 @@ pub(crate) fn update( ) { // `None` only if unborn. let remote_id = remote.as_id(); - if matches!(dry_run, fetch::DryRun::No) && !remote_id.map_or(true, |id| repo.objects.contains(id)) { - if let Some(remote_id) = remote_id.filter(|id| !repo.objects.contains(id)) { + if matches!(dry_run, fetch::DryRun::No) && !remote_id.map_or(true, |id| repo.objects.exists(id)) { + if let Some(remote_id) = remote_id.filter(|id| !repo.objects.exists(id)) { let update = if is_implicit_tag { Mode::ImplicitTagNotSentByRemote.into() } else { @@ -159,7 +159,7 @@ pub(crate) fn update( }).and_then(|local_commit_time| remote_id .to_owned() - .ancestors(|id, buf| repo.objects.find_commit_iter(id, buf)) + .ancestors(&repo.objects) .sorting( gix_traverse::commit::Sorting::ByCommitTimeNewestFirstCutoffOlderThan { seconds: local_commit_time @@ -325,11 +325,7 @@ pub(crate) fn update( .packed_refs( match write_packed_refs { fetch::WritePackedRefs::Only => { - gix_ref::file::transaction::PackedRefs::DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(Box::new(|oid, buf| { - repo.objects - .try_find(&oid, buf) - .map(|obj| obj.map(|obj| obj.kind)) - }))}, + gix_ref::file::transaction::PackedRefs::DeletionsAndNonSymbolicUpdatesRemoveLooseSourceReference(Box::new(&repo.objects))}, fetch::WritePackedRefs::Never => gix_ref::file::transaction::PackedRefs::DeletionsOnly } ) diff --git a/gix/src/repository/graph.rs b/gix/src/repository/graph.rs index f4f2b18cc69..7d59589ed3d 100644 --- a/gix/src/repository/graph.rs +++ b/gix/src/repository/graph.rs @@ -1,5 +1,3 @@ -use gix_odb::Find; - impl crate::Repository { /// Create a graph data-structure capable of accelerating graph traversals and storing state of type `T` with each commit /// it encountered. @@ -16,11 +14,7 @@ impl crate::Repository { /// of the commit walk. pub fn revision_graph(&self) -> gix_revwalk::Graph<'_, T> { gix_revwalk::Graph::new( - |id, buf| { - self.objects - .try_find(id, buf) - .map(|r| r.and_then(gix_object::Data::try_into_commit_iter)) - }, + &self.objects, self.config .may_use_commit_graph() .unwrap_or(true) diff --git a/gix/src/repository/index.rs b/gix/src/repository/index.rs index 59666fc5f10..85a1a664bb0 100644 --- a/gix/src/repository/index.rs +++ b/gix/src/repository/index.rs @@ -1,5 +1,3 @@ -use gix_odb::FindExt; - use crate::{config::cache::util::ApplyLeniencyDefault, repository::IndexPersistedOrInMemory, worktree}; /// Index access @@ -113,7 +111,7 @@ impl crate::Repository { tree: &gix_hash::oid, ) -> Result { Ok(gix_index::File::from_state( - gix_index::State::from_tree(tree, |oid, buf| self.objects.find_tree_iter(oid, buf).ok())?, + gix_index::State::from_tree(tree, &self.objects)?, self.git_dir().join("index"), )) } diff --git a/gix/src/repository/object.rs b/gix/src/repository/object.rs index 0b894939fa7..77f188badf8 100644 --- a/gix/src/repository/object.rs +++ b/gix/src/repository/object.rs @@ -3,7 +3,8 @@ use std::{convert::TryInto, ops::DerefMut}; use gix_hash::ObjectId; use gix_macros::momo; -use gix_odb::{Find, FindExt, Header, HeaderExt, Write}; +use gix_object::{Exists, Find, FindExt}; +use gix_odb::{Header, HeaderExt, Write}; use gix_ref::{ transaction::{LogChange, PreviousValue, RefLog}, FullName, @@ -70,7 +71,7 @@ impl crate::Repository { if id == ObjectId::empty_tree(self.object_hash()) { true } else { - self.objects.contains(id) + self.objects.exists(id) } } @@ -140,7 +141,7 @@ impl crate::Repository { fn write_object_inner(&self, buf: &[u8], kind: gix_object::Kind) -> Result, object::write::Error> { let oid = gix_object::compute_hash(self.object_hash(), kind, buf); - if self.objects.contains(&oid) { + if self.objects.exists(&oid) { return Ok(oid.attach(self)); } @@ -158,7 +159,7 @@ impl crate::Repository { pub fn write_blob(&self, bytes: impl AsRef<[u8]>) -> Result, object::write::Error> { let bytes = bytes.as_ref(); let oid = gix_object::compute_hash(self.object_hash(), gix_object::Kind::Blob, bytes); - if self.objects.contains(&oid) { + if self.objects.exists(&oid) { return Ok(oid.attach(self)); } self.objects @@ -185,7 +186,7 @@ impl crate::Repository { fn write_blob_stream_inner(&self, buf: &[u8]) -> Result, object::write::Error> { let oid = gix_object::compute_hash(self.object_hash(), gix_object::Kind::Blob, buf); - if self.objects.contains(&oid) { + if self.objects.exists(&oid) { return Ok(oid.attach(self)); } diff --git a/gix/src/repository/worktree.rs b/gix/src/repository/worktree.rs index cc6f0bf7322..9e6edeea751 100644 --- a/gix/src/repository/worktree.rs +++ b/gix/src/repository/worktree.rs @@ -62,7 +62,7 @@ impl crate::Repository { &self, id: impl Into, ) -> Result<(gix_worktree_stream::Stream, gix_index::File), crate::repository::worktree_stream::Error> { - use gix_odb::{FindExt, HeaderExt}; + use gix_odb::HeaderExt; let id = id.into(); let header = self.objects.header(id)?; if !header.kind().is_tree() { @@ -84,13 +84,10 @@ impl crate::Repository { let objects = self.objects.clone().into_arc().expect("TBD error handling"); let stream = gix_worktree_stream::from_tree( id, - { - let objects = objects.clone(); - move |id, buf| objects.find(id, buf) - }, + objects.clone(), pipeline, move |path, mode, attrs| -> std::io::Result<()> { - let entry = cache.at_entry(path, Some(mode.is_tree()), |id, buf| objects.find_blob(id, buf))?; + let entry = cache.at_entry(path, Some(mode.is_tree()), &objects)?; entry.matching_attributes(attrs); Ok(()) }, diff --git a/gix/src/revision/walk.rs b/gix/src/revision/walk.rs index 5e76ad89833..19d15d569ab 100644 --- a/gix/src/revision/walk.rs +++ b/gix/src/revision/walk.rs @@ -1,5 +1,5 @@ use gix_hash::ObjectId; -use gix_odb::FindExt; +use gix_object::FindExt; use crate::{ext::ObjectIdExt, revision, Repository}; @@ -169,7 +169,7 @@ impl<'repo> Platform<'repo> { gix_traverse::commit::Ancestors::filtered( tips, gix_traverse::commit::ancestors::State::default(), - move |oid, buf| repo.objects.find_commit_iter(oid, buf), + &repo.objects, { // Note that specific shallow handling for commit-graphs isn't needed as these contain // all information there is, and exclude shallow parents to be structurally consistent. diff --git a/gix/src/submodule/mod.rs b/gix/src/submodule/mod.rs index 52c5938fc28..14a054e8b49 100644 --- a/gix/src/submodule/mod.rs +++ b/gix/src/submodule/mod.rs @@ -7,7 +7,6 @@ use std::{ path::PathBuf, }; -use gix_odb::FindExt; pub use gix_submodule::*; use crate::{bstr::BStr, repository::IndexPersistedOrInMemory, Repository, Submodule}; @@ -147,9 +146,7 @@ impl<'repo> Submodule<'repo> { &mut |relative_path, case, is_dir, out| { attributes .set_case(case) - .at_entry(relative_path, Some(is_dir), |id, buf| { - self.state.repo.objects.find_blob(id, buf) - }) + .at_entry(relative_path, Some(is_dir), &self.state.repo.objects) .map_or(false, |platform| platform.matching_attributes(out)) } })?; diff --git a/gix/tests/clone/mod.rs b/gix/tests/clone/mod.rs index e594ff5d6a9..fb2736676a3 100644 --- a/gix/tests/clone/mod.rs +++ b/gix/tests/clone/mod.rs @@ -326,7 +326,7 @@ mod blocking_io { match out.status { gix::remote::fetch::Status::Change { update_refs, .. } => { for edit in &update_refs.edits { - use gix_odb::Find; + use gix_object::Exists; match edit.change.new_value().expect("always set/no deletion") { TargetRef::Symbolic(referent) => { assert!( @@ -335,7 +335,7 @@ mod blocking_io { ) } TargetRef::Peeled(id) => { - assert!(repo.objects.contains(id), "part of the fetched pack"); + assert!(repo.objects.exists(id), "part of the fetched pack"); } } let r = repo