From 210256932a338038adb55c5475d8f90560aa4c12 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 27 Aug 2021 11:17:42 +0800 Subject: [PATCH 1/5] [pack #179] add changelog --- git-pack/CHANGELOG.md | 3 +++ git-pack/Cargo.toml | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 git-pack/CHANGELOG.md diff --git a/git-pack/CHANGELOG.md b/git-pack/CHANGELOG.md new file mode 100644 index 00000000000..95f9b40c135 --- /dev/null +++ b/git-pack/CHANGELOG.md @@ -0,0 +1,3 @@ +### 0.9.0 (2021-08-??) + +- TBD diff --git a/git-pack/Cargo.toml b/git-pack/Cargo.toml index 402a0a4bae7..3b2c9079eb4 100644 --- a/git-pack/Cargo.toml +++ b/git-pack/Cargo.toml @@ -6,7 +6,7 @@ authors = ["Sebastian Thiel "] license = "MIT/Apache-2.0" description = "Implements git packs and related data structures" edition = "2018" -include = ["src/**/*"] +include = ["src/**/*", "CHANGELOG.md"] [lib] doctest = false From 620d8a54db5cd8367ec85c8b837cab710c509e3e Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 27 Aug 2021 11:35:58 +0800 Subject: [PATCH 2/5] [pack #179] refactor --- git-pack/CHANGELOG.md | 5 +- git-pack/src/data/output/count/mod.rs | 17 +-- git-pack/src/data/output/entry/mod.rs | 22 +-- git-pack/src/data/output/mod.rs | 37 ++++- git-pack/src/find.rs | 206 -------------------------- git-pack/src/find_traits.rs | 205 +++++++++++++++++++++++++ git-pack/src/lib.rs | 5 +- 7 files changed, 249 insertions(+), 248 deletions(-) create mode 100644 git-pack/src/find_traits.rs diff --git a/git-pack/CHANGELOG.md b/git-pack/CHANGELOG.md index 95f9b40c135..597e1c19c24 100644 --- a/git-pack/CHANGELOG.md +++ b/git-pack/CHANGELOG.md @@ -1,3 +1,6 @@ ### 0.9.0 (2021-08-??) -- TBD +- **renames / moves** + - `find::Find` and `find::FindExt` only in `Find` and `FindExt` (not in `find` anymore) + - `data::output::count::Count` -> `data::output::Count` + - `data::output::entry::Entry` -> `data::output::Entry` diff --git a/git-pack/src/data/output/count/mod.rs b/git-pack/src/data/output/count/mod.rs index 1706c5ae62e..ab4adc59c61 100644 --- a/git-pack/src/data/output/count/mod.rs +++ b/git-pack/src/data/output/count/mod.rs @@ -1,19 +1,6 @@ -use git_hash::ObjectId; - use crate::data; - -/// An item representing a future Entry in the leanest way possible. -/// -/// One can expect to have one of these in memory when building big objects, so smaller is better here. -/// They should contain everything of importance to generate a pack as fast as possible. -#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] -#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] -pub struct Count { - /// The hash of the object to write - pub id: ObjectId, - /// A way to locate a pack entry in the object database, only available if the object is in a pack. - pub entry_pack_location: PackLocation, -} +use crate::data::output::Count; +use git_hash::ObjectId; /// Specifies how the pack location was handled during counting #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] diff --git a/git-pack/src/data/output/entry/mod.rs b/git-pack/src/data/output/entry/mod.rs index 043787cf8a1..f799284f749 100644 --- a/git-pack/src/data/output/entry/mod.rs +++ b/git-pack/src/data/output/entry/mod.rs @@ -9,24 +9,6 @@ use crate::{data, data::output, find}; pub mod iter_from_counts; pub use iter_from_counts::iter_from_counts; -/// An entry to be written to a file. -/// -/// Some of these will be in-flight and in memory while waiting to be written. Memory requirements depend on the amount of compressed -/// data they hold. -#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] -#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] -pub struct Entry { - /// The hash of the object to write - pub id: ObjectId, - /// The kind of entry represented by `data`. It's used alongside with it to complete the pack entry - /// at rest or in transit. - pub kind: Kind, - /// The size in bytes needed once `data` gets decompressed - pub decompressed_size: usize, - /// The compressed data right behind the header - pub compressed_data: Vec, -} - /// The kind of pack entry to be written #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)] #[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] @@ -62,8 +44,8 @@ pub enum Error { impl output::Entry { /// An object which can be identified as invalid easily which happens if objects didn't exist even if they were referred to. - pub fn invalid() -> Entry { - Entry { + pub fn invalid() -> output::Entry { + output::Entry { id: ObjectId::null_sha1(), kind: Kind::Base(git_object::Kind::Blob), decompressed_size: 0, diff --git a/git-pack/src/data/output/mod.rs b/git-pack/src/data/output/mod.rs index 7bd3f141c4b..bae9342d1f2 100644 --- a/git-pack/src/data/output/mod.rs +++ b/git-pack/src/data/output/mod.rs @@ -1,12 +1,41 @@ +use git_hash::ObjectId; + /// pub mod count; -#[doc(inline)] -pub use count::Count; + +/// An item representing a future Entry in the leanest way possible. +/// +/// One can expect to have one of these in memory when building big objects, so smaller is better here. +/// They should contain everything of importance to generate a pack as fast as possible. +#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] +#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] +pub struct Count { + /// The hash of the object to write + pub id: ObjectId, + /// A way to locate a pack entry in the object database, only available if the object is in a pack. + pub entry_pack_location: count::PackLocation, +} + +/// An entry to be written to a file. +/// +/// Some of these will be in-flight and in memory while waiting to be written. Memory requirements depend on the amount of compressed +/// data they hold. +#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] +#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] +pub struct Entry { + /// The hash of the object to write + pub id: ObjectId, + /// The kind of entry represented by `data`. It's used alongside with it to complete the pack entry + /// at rest or in transit. + pub kind: entry::Kind, + /// The size in bytes needed once `data` gets decompressed + pub decompressed_size: usize, + /// The compressed data right behind the header + pub compressed_data: Vec, +} /// pub mod entry; -#[doc(inline)] -pub use entry::Entry; /// pub mod bytes; diff --git a/git-pack/src/find.rs b/git-pack/src/find.rs index 595a10380c3..c83ba319bbe 100644 --- a/git-pack/src/find.rs +++ b/git-pack/src/find.rs @@ -1,145 +1,3 @@ -use crate::data; - -/// Describe how object can be located in an object store with built-in facilities to supports packs specifically. -/// -/// ## Notes -/// -/// Locate 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 { - /// The error returned by [`find()`][Find::find()] - type Error: std::error::Error + 'static; - - /// Find an object matching `id` in the database while placing its raw, undecoded data into `buffer`. - /// A `pack_cache` can be used to speed up subsequent lookups, set it to [`crate::cache::Never`] if the - /// workload isn't suitable for caching. - /// - /// Returns `Some` object if it was present in the database, or the error that occurred during lookup or object - /// retrieval. - fn find<'a>( - &self, - id: impl AsRef, - buffer: &'a mut Vec, - pack_cache: &mut impl crate::cache::DecodeEntry, - ) -> Result>, Self::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. - /// - /// _Note_ that the object database may have no notion of packs and thus always returns `None`. - fn location_by_oid(&self, id: impl AsRef, buf: &mut Vec) -> Option; - - /// Find the bundle matching `pack_id`, or `None` if there is no such pack. - /// - /// _Note_ that the object database may have no notion of packs and thus always returns `None`. - fn bundle_by_pack_id(&self, pack_id: u32) -> Option<&crate::Bundle>; - - /// Return the [`Entry`] for `location` if it is backed by a pack. - /// - /// Note that this is only in the interest of avoiding duplicate work during pack generation. - /// Pack locations can be obtained from a [`data::Object`]. - /// - /// # Notes - /// - /// Custom implementations might be interested in providing their own meta-data with `object`, - /// which currently isn't possible as the `Locate` trait requires GATs to work like that. - fn entry_by_location(&self, location: &crate::bundle::Location) -> Option>; -} - -mod ext { - use git_object::{BlobRef, CommitRef, CommitRefIter, Kind, ObjectRef, TagRef, TreeRef, TreeRefIter}; - - use crate::{data, find}; - - macro_rules! make_obj_lookup { - ($method:ident, $object_variant:path, $object_kind:path, $object_type:ty) => { - /// Like [`find_existing(…)`][Self::find_existing()], 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: impl AsRef, - buffer: &'a mut Vec, - pack_cache: &mut impl crate::cache::DecodeEntry, - ) -> Result<$object_type, find::existing_object::Error> { - let id = id.as_ref(); - self.find(id, buffer, pack_cache) - .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_existing(…)`][Self::find_existing()], 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: impl AsRef, - buffer: &'a mut Vec, - pack_cache: &mut impl crate::cache::DecodeEntry, - ) -> Result<$object_type, find::existing_iter::Error> { - let id = id.as_ref(); - self.find(id, buffer, pack_cache) - .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 FindExt: super::Find { - /// Like [`find(…)`][super::Find::find()], but flattens the `Result>` into a single `Result` making a non-existing object an error. - fn find_existing<'a>( - &self, - id: impl AsRef, - buffer: &'a mut Vec, - pack_cache: &mut impl crate::cache::DecodeEntry, - ) -> Result, find::existing::Error> { - let id = id.as_ref(); - self.find(id, buffer, pack_cache) - .map_err(find::existing::Error::Find)? - .ok_or_else(|| find::existing::Error::NotFound { - oid: id.as_ref().to_owned(), - }) - } - - make_obj_lookup!(find_existing_commit, ObjectRef::Commit, Kind::Commit, CommitRef<'a>); - make_obj_lookup!(find_existing_tree, ObjectRef::Tree, Kind::Tree, TreeRef<'a>); - make_obj_lookup!(find_existing_tag, ObjectRef::Tag, Kind::Tag, TagRef<'a>); - make_obj_lookup!(find_existing_blob, ObjectRef::Blob, Kind::Blob, BlobRef<'a>); - make_iter_lookup!( - find_existing_commit_iter, - Kind::Blob, - CommitRefIter<'a>, - into_commit_iter - ); - make_iter_lookup!(find_existing_tree_iter, Kind::Tree, TreeRefIter<'a>, into_tree_iter); - } - - impl FindExt for T {} -} -pub use ext::FindExt; - /// pub mod existing { use git_hash::ObjectId; @@ -205,67 +63,3 @@ pub struct Entry<'a> { /// The version of the pack file containing `data` pub version: crate::data::Version, } - -mod find_impls { - use std::ops::Deref; - - use git_hash::oid; - - use crate::{bundle::Location, data::Object, find::Entry, Bundle}; - - impl super::Find for std::sync::Arc - where - T: super::Find, - { - type Error = T::Error; - - fn find<'a>( - &self, - id: impl AsRef, - buffer: &'a mut Vec, - pack_cache: &mut impl crate::cache::DecodeEntry, - ) -> Result>, Self::Error> { - self.deref().find(id, buffer, pack_cache) - } - - fn location_by_oid(&self, id: impl AsRef, buf: &mut Vec) -> Option { - self.deref().location_by_oid(id, buf) - } - - fn bundle_by_pack_id(&self, pack_id: u32) -> Option<&Bundle> { - self.deref().bundle_by_pack_id(pack_id) - } - - fn entry_by_location(&self, object: &crate::bundle::Location) -> Option> { - self.deref().entry_by_location(object) - } - } - - impl super::Find for Box - where - T: super::Find, - { - type Error = T::Error; - - fn find<'a>( - &self, - id: impl AsRef, - buffer: &'a mut Vec, - pack_cache: &mut impl crate::cache::DecodeEntry, - ) -> Result>, Self::Error> { - self.deref().find(id, buffer, pack_cache) - } - - fn location_by_oid(&self, id: impl AsRef, buf: &mut Vec) -> Option { - self.deref().location_by_oid(id, buf) - } - - fn bundle_by_pack_id(&self, pack_id: u32) -> Option<&Bundle> { - self.deref().bundle_by_pack_id(pack_id) - } - - fn entry_by_location(&self, location: &crate::bundle::Location) -> Option> { - self.deref().entry_by_location(location) - } - } -} diff --git a/git-pack/src/find_traits.rs b/git-pack/src/find_traits.rs new file mode 100644 index 00000000000..ba65595638a --- /dev/null +++ b/git-pack/src/find_traits.rs @@ -0,0 +1,205 @@ +use crate::{data, find}; + +/// Describe how object can be located in an object store with built-in facilities to supports packs specifically. +/// +/// ## Notes +/// +/// Locate 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 { + /// The error returned by [`find()`][Find::find()] + type Error: std::error::Error + 'static; + + /// Find an object matching `id` in the database while placing its raw, undecoded data into `buffer`. + /// A `pack_cache` can be used to speed up subsequent lookups, set it to [`crate::cache::Never`] if the + /// workload isn't suitable for caching. + /// + /// Returns `Some` object if it was present in the database, or the error that occurred during lookup or object + /// retrieval. + fn find<'a>( + &self, + id: impl AsRef, + buffer: &'a mut Vec, + pack_cache: &mut impl crate::cache::DecodeEntry, + ) -> Result>, Self::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. + /// + /// _Note_ that the object database may have no notion of packs and thus always returns `None`. + fn location_by_oid(&self, id: impl AsRef, buf: &mut Vec) -> Option; + + /// Find the bundle matching `pack_id`, or `None` if there is no such pack. + /// + /// _Note_ that the object database may have no notion of packs and thus always returns `None`. + fn bundle_by_pack_id(&self, pack_id: u32) -> Option<&crate::Bundle>; + + /// Return the [`find::Entry`] for `location` if it is backed by a pack. + /// + /// Note that this is only in the interest of avoiding duplicate work during pack generation. + /// Pack locations can be obtained from a [`data::Object`]. + /// + /// # Notes + /// + /// Custom implementations might be interested in providing their own meta-data with `object`, + /// which currently isn't possible as the `Locate` trait requires GATs to work like that. + fn entry_by_location(&self, location: &crate::bundle::Location) -> Option>; +} + +mod ext { + use git_object::{BlobRef, CommitRef, CommitRefIter, Kind, ObjectRef, TagRef, TreeRef, TreeRefIter}; + + use crate::{data, find}; + + macro_rules! make_obj_lookup { + ($method:ident, $object_variant:path, $object_kind:path, $object_type:ty) => { + /// Like [`find_existing(…)`][Self::find_existing()], 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: impl AsRef, + buffer: &'a mut Vec, + pack_cache: &mut impl crate::cache::DecodeEntry, + ) -> Result<$object_type, find::existing_object::Error> { + let id = id.as_ref(); + self.find(id, buffer, pack_cache) + .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_existing(…)`][Self::find_existing()], 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: impl AsRef, + buffer: &'a mut Vec, + pack_cache: &mut impl crate::cache::DecodeEntry, + ) -> Result<$object_type, find::existing_iter::Error> { + let id = id.as_ref(); + self.find(id, buffer, pack_cache) + .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 FindExt: super::Find { + /// Like [`find(…)`][super::Find::find()], but flattens the `Result>` into a single `Result` making a non-existing object an error. + fn find_existing<'a>( + &self, + id: impl AsRef, + buffer: &'a mut Vec, + pack_cache: &mut impl crate::cache::DecodeEntry, + ) -> Result, find::existing::Error> { + let id = id.as_ref(); + self.find(id, buffer, pack_cache) + .map_err(find::existing::Error::Find)? + .ok_or_else(|| find::existing::Error::NotFound { + oid: id.as_ref().to_owned(), + }) + } + + make_obj_lookup!(find_existing_commit, ObjectRef::Commit, Kind::Commit, CommitRef<'a>); + make_obj_lookup!(find_existing_tree, ObjectRef::Tree, Kind::Tree, TreeRef<'a>); + make_obj_lookup!(find_existing_tag, ObjectRef::Tag, Kind::Tag, TagRef<'a>); + make_obj_lookup!(find_existing_blob, ObjectRef::Blob, Kind::Blob, BlobRef<'a>); + make_iter_lookup!( + find_existing_commit_iter, + Kind::Blob, + CommitRefIter<'a>, + into_commit_iter + ); + make_iter_lookup!(find_existing_tree_iter, Kind::Tree, TreeRefIter<'a>, into_tree_iter); + } + + impl FindExt for T {} +} +pub use ext::FindExt; + +mod find_impls { + use std::ops::Deref; + + use git_hash::oid; + + use crate::{bundle::Location, data::Object, find, Bundle}; + + impl super::Find for std::sync::Arc + where + T: super::Find, + { + type Error = T::Error; + + fn find<'a>( + &self, + id: impl AsRef, + buffer: &'a mut Vec, + pack_cache: &mut impl crate::cache::DecodeEntry, + ) -> Result>, Self::Error> { + self.deref().find(id, buffer, pack_cache) + } + + fn location_by_oid(&self, id: impl AsRef, buf: &mut Vec) -> Option { + self.deref().location_by_oid(id, buf) + } + + fn bundle_by_pack_id(&self, pack_id: u32) -> Option<&Bundle> { + self.deref().bundle_by_pack_id(pack_id) + } + + fn entry_by_location(&self, object: &crate::bundle::Location) -> Option> { + self.deref().entry_by_location(object) + } + } + + impl super::Find for Box + where + T: super::Find, + { + type Error = T::Error; + + fn find<'a>( + &self, + id: impl AsRef, + buffer: &'a mut Vec, + pack_cache: &mut impl crate::cache::DecodeEntry, + ) -> Result>, Self::Error> { + self.deref().find(id, buffer, pack_cache) + } + + fn location_by_oid(&self, id: impl AsRef, buf: &mut Vec) -> Option { + self.deref().location_by_oid(id, buf) + } + + fn bundle_by_pack_id(&self, pack_id: u32) -> Option<&Bundle> { + self.deref().bundle_by_pack_id(pack_id) + } + + fn entry_by_location(&self, location: &crate::bundle::Location) -> Option> { + self.deref().entry_by_location(location) + } + } +} diff --git a/git-pack/src/lib.rs b/git-pack/src/lib.rs index 6d612351ab2..b15df436e15 100644 --- a/git-pack/src/lib.rs +++ b/git-pack/src/lib.rs @@ -19,8 +19,9 @@ pub use crate::bundle::Bundle; /// pub mod find; -#[doc(inline)] -pub use find::{Find, FindExt}; + +mod find_traits; +pub use find_traits::{Find, FindExt}; /// pub mod cache; From ab6554b0cd5838f1ea4e82f6b5019798288076fa Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 27 Aug 2021 11:46:18 +0800 Subject: [PATCH 3/5] [pack #179] refactor --- experiments/diffing/src/main.rs | 7 +--- experiments/object-access/src/main.rs | 4 +- experiments/traversal/src/main.rs | 23 ++++------- git-diff/tests/visit/mod.rs | 18 ++++----- git-odb/src/store/linked/find.rs | 6 +-- git-odb/tests/odb/store/linked/mod.rs | 2 +- git-pack/CHANGELOG.md | 3 ++ git-pack/src/data/output/count/objects.rs | 20 +++++----- .../src/data/output/entry/iter_from_counts.rs | 4 +- git-pack/src/find_traits.rs | 40 +++++++++---------- .../pack/data/output/count_and_entries.rs | 6 +-- git-ref/tests/file/reference.rs | 2 +- git-repository/src/easy/ext/object.rs | 4 +- git-repository/src/easy/reference.rs | 2 +- git-traverse/tests/commit/mod.rs | 4 +- git-traverse/tests/tree/mod.rs | 6 +-- gitoxide-core/src/hours.rs | 4 +- gitoxide-core/src/pack/create.rs | 4 +- 18 files changed, 74 insertions(+), 85 deletions(-) diff --git a/experiments/diffing/src/main.rs b/experiments/diffing/src/main.rs index d2e99d0951a..2d657d1107b 100644 --- a/experiments/diffing/src/main.rs +++ b/experiments/diffing/src/main.rs @@ -39,10 +39,7 @@ fn main() -> anyhow::Result<()> { let start = Instant::now(); let all_commits = commit_id - .ancestors_iter(|oid, buf| { - db.find_existing_commit_iter(oid, buf, &mut odb::pack::cache::Never) - .ok() - }) + .ancestors_iter(|oid, buf| db.find_commit_iter(oid, buf, &mut odb::pack::cache::Never).ok()) .collect::, _>>()?; let num_diffs = all_commits.len(); let elapsed = start.elapsed(); @@ -77,7 +74,7 @@ fn main() -> anyhow::Result<()> { Some(odb::data::Object::new(*kind, buf)) } None => { - let obj = db.find_existing(oid, buf, pack_cache).ok(); + let obj = db.find(oid, buf, pack_cache).ok(); if let Some(ref obj) = obj { obj_cache.insert( oid, diff --git a/experiments/object-access/src/main.rs b/experiments/object-access/src/main.rs index d0422676dfb..bf774a5b4d2 100644 --- a/experiments/object-access/src/main.rs +++ b/experiments/object-access/src/main.rs @@ -173,7 +173,7 @@ where let mut bytes = 0u64; let mut cache = new_cache(); for hash in hashes { - let obj = repo.odb.find_existing(hash, &mut buf, &mut cache)?; + let obj = repo.odb.find(hash, &mut buf, &mut cache)?; bytes += obj.data.len() as u64; } Ok(bytes) @@ -200,7 +200,7 @@ where |(buf, cache), hash| { match mode { AccessMode::ObjectData => { - let obj = repo.odb.find_existing(hash, buf, cache)?; + let obj = repo.odb.find(hash, buf, cache)?; bytes.fetch_add(obj.data.len() as u64, std::sync::atomic::Ordering::Relaxed); } AccessMode::ObjectExists => { diff --git a/experiments/traversal/src/main.rs b/experiments/traversal/src/main.rs index f919b599149..c3079b55923 100644 --- a/experiments/traversal/src/main.rs +++ b/experiments/traversal/src/main.rs @@ -37,10 +37,7 @@ fn main() -> anyhow::Result<()> { let start = Instant::now(); let all_commits = commit_id - .ancestors_iter(|oid, buf| { - db.find_existing_commit_iter(oid, buf, &mut odb::pack::cache::Never) - .ok() - }) + .ancestors_iter(|oid, buf| db.find_commit_iter(oid, buf, &mut odb::pack::cache::Never).ok()) .collect::, _>>()?; let elapsed = start.elapsed(); println!( @@ -149,7 +146,7 @@ where C: odb::pack::cache::DecodeEntry, { let mut cache = new_cache(); - let ancestors = tip.ancestors_iter(|oid, buf| db.find_existing_commit_iter(oid, buf, &mut cache).ok()); + let ancestors = tip.ancestors_iter(|oid, buf| db.find_commit_iter(oid, buf, &mut cache).ok()); let mut commits = 0; for commit_id in ancestors { let _ = commit_id?; @@ -211,18 +208,14 @@ where for commit in commits { let tree_id = db - .find(commit, &mut buf, &mut cache)? + .try_find(commit, &mut buf, &mut cache)? .and_then(|o| o.into_commit_iter().and_then(|mut c| c.tree_id())) .expect("commit as starting point"); let mut count = Count { entries: 0, seen }; - db.find_existing_tree_iter(tree_id, &mut buf2, &mut cache)?.traverse( + db.find_tree_iter(tree_id, &mut buf2, &mut cache)?.traverse( &mut state, - |oid, buf| { - db.find_existing(oid, buf, &mut cache) - .ok() - .and_then(|o| o.into_tree_iter()) - }, + |oid, buf| db.find(oid, buf, &mut cache).ok().and_then(|o| o.into_tree_iter()), &mut count, )?; entries += count.entries as u64; @@ -278,13 +271,13 @@ where }, |(count, buf, buf2, cache, state), commit| { let tid = db - .find_existing_commit_iter(commit, buf, cache)? + .find_commit_iter(commit, buf, cache)? .tree_id() .expect("commit as starting point"); count.entries = 0; - db.find_existing_tree_iter(tid, buf2, cache)?.traverse( + db.find_tree_iter(tid, buf2, cache)?.traverse( state, - |oid, buf| db.find_existing_tree_iter(oid, buf, cache).ok(), + |oid, buf| db.find_tree_iter(oid, buf, cache).ok(), count, )?; entries.fetch_add(count.entries as u64, std::sync::atomic::Ordering::Relaxed); diff --git a/git-diff/tests/visit/mod.rs b/git-diff/tests/visit/mod.rs index bd41eb5448a..23faff0f47f 100644 --- a/git-diff/tests/visit/mod.rs +++ b/git-diff/tests/visit/mod.rs @@ -24,7 +24,7 @@ mod changes { buf: &'a mut Vec, ) -> crate::Result> { let tree_id = db - .find(commit, buf, &mut pack::cache::Never)? + .try_find(commit, buf, &mut pack::cache::Never)? .ok_or_else(|| format!("start commit {:?} to be present", commit))? .decode()? .into_commit() @@ -32,7 +32,7 @@ mod changes { .tree(); Ok(db - .find(tree_id, buf, &mut pack::cache::Never)? + .try_find(tree_id, buf, &mut pack::cache::Never)? .expect("main tree present") .into_tree_iter() .expect("id to be a tree")) @@ -50,7 +50,7 @@ mod changes { rhs_tree, git_diff::tree::State::default(), |oid, buf| { - db.find(oid, buf, &mut pack::cache::Never) + db.try_find(oid, buf, &mut pack::cache::Never) .ok() .flatten() .and_then(|obj| obj.into_tree_iter()) @@ -64,7 +64,7 @@ mod changes { let mut buf = Vec::new(); let (main_tree_id, parent_commit_id) = { let commit = db - .find(commit_id, &mut buf, &mut pack::cache::Never)? + .try_find(commit_id, &mut buf, &mut pack::cache::Never)? .ok_or_else(|| format!("start commit {:?} to be present", commit_id))? .decode()? .into_commit() @@ -76,18 +76,18 @@ mod changes { }) }; let current_tree = db - .find(main_tree_id, &mut buf, &mut pack::cache::Never)? + .try_find(main_tree_id, &mut buf, &mut pack::cache::Never)? .expect("main tree present") .into_tree_iter() .expect("id to be a tree"); let mut buf2 = Vec::new(); let previous_tree: Option<_> = { parent_commit_id - .and_then(|id| db.find(id, &mut buf2, &mut pack::cache::Never).ok().flatten()) + .and_then(|id| db.try_find(id, &mut buf2, &mut pack::cache::Never).ok().flatten()) .and_then(|c| c.decode().ok()) .and_then(|c| c.into_commit()) .map(|c| c.tree()) - .and_then(|tree| db.find(tree, &mut buf2, &mut pack::cache::Never).ok().flatten()) + .and_then(|tree| db.try_find(tree, &mut buf2, &mut pack::cache::Never).ok().flatten()) .and_then(|tree| tree.into_tree_iter()) }; @@ -96,7 +96,7 @@ mod changes { current_tree, &mut git_diff::tree::State::default(), |oid, buf| { - db.find(oid, buf, &mut pack::cache::Never) + db.try_find(oid, buf, &mut pack::cache::Never) .ok() .flatten() .and_then(|obj| obj.into_tree_iter()) @@ -130,7 +130,7 @@ mod changes { let head = head_of(db); commit::Ancestors::new(Some(head), commit::ancestors::State::default(), |oid, buf| { - db.find(oid, buf, &mut pack::cache::Never) + db.try_find(oid, buf, &mut pack::cache::Never) .ok() .flatten() .and_then(|o| o.into_commit_iter()) diff --git a/git-odb/src/store/linked/find.rs b/git-odb/src/store/linked/find.rs index f9c03724ea1..c8900e5595f 100644 --- a/git-odb/src/store/linked/find.rs +++ b/git-odb/src/store/linked/find.rs @@ -25,7 +25,7 @@ impl linked::Store { impl crate::Find for linked::Store { type Error = compound::find::Error; - fn find<'a>( + fn try_find<'a>( &self, id: impl AsRef, buffer: &'a mut Vec, @@ -107,13 +107,13 @@ impl crate::Find for linked::Store { impl crate::Find for &linked::Store { type Error = compound::find::Error; - fn find<'a>( + fn try_find<'a>( &self, id: impl AsRef, buffer: &'a mut Vec, pack_cache: &mut impl pack::cache::DecodeEntry, ) -> Result>, Self::Error> { - (*self).find(id, buffer, pack_cache) + (*self).try_find(id, buffer, pack_cache) } fn location_by_oid(&self, id: impl AsRef, buf: &mut Vec) -> Option { diff --git a/git-odb/tests/odb/store/linked/mod.rs b/git-odb/tests/odb/store/linked/mod.rs index 58606393979..a0e82d15676 100644 --- a/git-odb/tests/odb/store/linked/mod.rs +++ b/git-odb/tests/odb/store/linked/mod.rs @@ -40,7 +40,7 @@ mod locate { fn can_locate(db: &Store, hex_id: &str) { let mut buf = vec![]; assert!(db - .find(hex_to_id(hex_id), &mut buf, &mut pack::cache::Never) + .try_find(hex_to_id(hex_id), &mut buf, &mut pack::cache::Never) .expect("no read error") .is_some()); } diff --git a/git-pack/CHANGELOG.md b/git-pack/CHANGELOG.md index 597e1c19c24..d9b0b3ad15c 100644 --- a/git-pack/CHANGELOG.md +++ b/git-pack/CHANGELOG.md @@ -4,3 +4,6 @@ - `find::Find` and `find::FindExt` only in `Find` and `FindExt` (not in `find` anymore) - `data::output::count::Count` -> `data::output::Count` - `data::output::entry::Entry` -> `data::output::Entry` + - `Find::find_existing_*` -> `Find::find_*` + - `Find::find_existing_*` -> `Find::find_*` + - `Find::find()-> `Find::try_find()` diff --git a/git-pack/src/data/output/count/objects.rs b/git-pack/src/data/output/count/objects.rs index 6c5cddb9d64..c358533261d 100644 --- a/git-pack/src/data/output/count/objects.rs +++ b/git-pack/src/data/output/count/objects.rs @@ -166,7 +166,7 @@ where } let id = id.map(|oid| oid.into()).map_err(Error::InputIteration)?; - let obj = db.find_existing(id, buf1, cache)?; + let obj = db.find(id, buf1, cache)?; stats.input_objects += 1; match input_object_expansion { TreeAdditionsComparedToAncestor => { @@ -182,7 +182,7 @@ where id = TagRefIter::from_bytes(obj.data) .target_id() .expect("every tag has a target"); - obj = db.find_existing(id, buf1, cache)?; + obj = db.find(id, buf1, cache)?; stats.expanded_objects += 1; continue; } @@ -200,7 +200,7 @@ where Err(err) => return Err(Error::CommitDecode(err)), } } - let obj = db.find_existing(tree_id, buf1, cache)?; + let obj = db.find(tree_id, buf1, cache)?; push_obj_count_unique(&mut out, seen_objs, &tree_id, &obj, progress, stats, true); git_object::TreeRefIter::from_bytes(obj.data) }; @@ -212,7 +212,7 @@ where &mut tree_traversal_state, |oid, buf| { stats.decoded_objects += 1; - match db.find_existing(oid, buf, cache).ok() { + match db.find(oid, buf, cache).ok() { Some(obj) => { progress.inc(); stats.expanded_objects += 1; @@ -229,7 +229,7 @@ where } else { for commit_id in &parent_commit_ids { let parent_tree_id = { - let parent_commit_obj = db.find_existing(commit_id, buf2, cache)?; + let parent_commit_obj = db.find(commit_id, buf2, cache)?; push_obj_count_unique( &mut out, @@ -245,7 +245,7 @@ where .expect("every commit has a tree") }; let parent_tree = { - let parent_tree_obj = db.find_existing(parent_tree_id, buf2, cache)?; + let parent_tree_obj = db.find(parent_tree_id, buf2, cache)?; push_obj_count_unique( &mut out, seen_objs, @@ -265,7 +265,7 @@ where &mut tree_diff_state, |oid, buf| { stats.decoded_objects += 1; - db.find_existing_tree_iter(oid, buf, cache).ok() + db.find_tree_iter(oid, buf, cache).ok() }, &mut changes_delegate, ) @@ -295,7 +295,7 @@ where &mut tree_traversal_state, |oid, buf| { stats.decoded_objects += 1; - match db.find_existing(oid, buf, cache).ok() { + match db.find(oid, buf, cache).ok() { Some(obj) => { progress.inc(); stats.expanded_objects += 1; @@ -318,7 +318,7 @@ where .tree_id() .expect("every commit has a tree"); stats.expanded_objects += 1; - obj = db.find_existing(id, buf1, cache)?; + obj = db.find(id, buf1, cache)?; continue; } Blob => break, @@ -327,7 +327,7 @@ where .target_id() .expect("every tag has a target"); stats.expanded_objects += 1; - obj = db.find_existing(id, buf1, cache)?; + obj = db.find(id, buf1, cache)?; continue; } } diff --git a/git-pack/src/data/output/entry/iter_from_counts.rs b/git-pack/src/data/output/entry/iter_from_counts.rs index 87ac7b20bdc..859f3b9ae01 100644 --- a/git-pack/src/data/output/entry/iter_from_counts.rs +++ b/git-pack/src/data/output/entry/iter_from_counts.rs @@ -217,7 +217,7 @@ where stats.objects_copied_from_pack += 1; entry } - None => match db.find(count.id, buf, cache).map_err(Error::FindExisting)? { + None => match db.try_find(count.id, buf, cache).map_err(Error::FindExisting)? { Some(obj) => { stats.decoded_and_recompressed_objects += 1; output::Entry::from_data(count, &obj) @@ -229,7 +229,7 @@ where }, } } - None => match db.find(count.id, buf, cache).map_err(Error::FindExisting)? { + None => match db.try_find(count.id, buf, cache).map_err(Error::FindExisting)? { Some(obj) => { stats.decoded_and_recompressed_objects += 1; output::Entry::from_data(count, &obj) diff --git a/git-pack/src/find_traits.rs b/git-pack/src/find_traits.rs index ba65595638a..72396ed0fd4 100644 --- a/git-pack/src/find_traits.rs +++ b/git-pack/src/find_traits.rs @@ -18,7 +18,7 @@ pub trait Find { /// /// Returns `Some` object if it was present in the database, or the error that occurred during lookup or object /// retrieval. - fn find<'a>( + fn try_find<'a>( &self, id: impl AsRef, buffer: &'a mut Vec, @@ -49,13 +49,13 @@ pub trait Find { } mod ext { - use git_object::{BlobRef, CommitRef, CommitRefIter, Kind, ObjectRef, TagRef, TreeRef, TreeRefIter}; + use git_object::{BlobRef, CommitRef, CommitRefIter, Kind, ObjectRef, TagRef, TagRefIter, TreeRef, TreeRefIter}; use crate::{data, find}; macro_rules! make_obj_lookup { ($method:ident, $object_variant:path, $object_kind:path, $object_type:ty) => { - /// Like [`find_existing(…)`][Self::find_existing()], but flattens the `Result>` into a single `Result` making a non-existing object an error + /// 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, @@ -64,7 +64,7 @@ mod ext { pack_cache: &mut impl crate::cache::DecodeEntry, ) -> Result<$object_type, find::existing_object::Error> { let id = id.as_ref(); - self.find(id, buffer, pack_cache) + self.try_find(id, buffer, pack_cache) .map_err(find::existing_object::Error::Find)? .ok_or_else(|| find::existing_object::Error::NotFound { oid: id.as_ref().to_owned(), @@ -91,7 +91,7 @@ mod ext { pack_cache: &mut impl crate::cache::DecodeEntry, ) -> Result<$object_type, find::existing_iter::Error> { let id = id.as_ref(); - self.find(id, buffer, pack_cache) + self.try_find(id, buffer, pack_cache) .map_err(find::existing_iter::Error::Find)? .ok_or_else(|| find::existing_iter::Error::NotFound { oid: id.as_ref().to_owned(), @@ -109,31 +109,27 @@ mod ext { /// An extension trait with convenience functions. pub trait FindExt: super::Find { /// Like [`find(…)`][super::Find::find()], but flattens the `Result>` into a single `Result` making a non-existing object an error. - fn find_existing<'a>( + fn find<'a>( &self, id: impl AsRef, buffer: &'a mut Vec, pack_cache: &mut impl crate::cache::DecodeEntry, ) -> Result, find::existing::Error> { let id = id.as_ref(); - self.find(id, buffer, pack_cache) + self.try_find(id, buffer, pack_cache) .map_err(find::existing::Error::Find)? .ok_or_else(|| find::existing::Error::NotFound { oid: id.as_ref().to_owned(), }) } - make_obj_lookup!(find_existing_commit, ObjectRef::Commit, Kind::Commit, CommitRef<'a>); - make_obj_lookup!(find_existing_tree, ObjectRef::Tree, Kind::Tree, TreeRef<'a>); - make_obj_lookup!(find_existing_tag, ObjectRef::Tag, Kind::Tag, TagRef<'a>); - make_obj_lookup!(find_existing_blob, ObjectRef::Blob, Kind::Blob, BlobRef<'a>); - make_iter_lookup!( - find_existing_commit_iter, - Kind::Blob, - CommitRefIter<'a>, - into_commit_iter - ); - make_iter_lookup!(find_existing_tree_iter, Kind::Tree, TreeRefIter<'a>, into_tree_iter); + 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::Blob, CommitRefIter<'a>, into_commit_iter); + make_iter_lookup!(find_tree_iter, Kind::Tree, TreeRefIter<'a>, into_tree_iter); + make_iter_lookup!(find_tag_iter, Kind::Tag, TagRefIter<'a>, into_tag_iter); } impl FindExt for T {} @@ -153,13 +149,13 @@ mod find_impls { { type Error = T::Error; - fn find<'a>( + fn try_find<'a>( &self, id: impl AsRef, buffer: &'a mut Vec, pack_cache: &mut impl crate::cache::DecodeEntry, ) -> Result>, Self::Error> { - self.deref().find(id, buffer, pack_cache) + self.deref().try_find(id, buffer, pack_cache) } fn location_by_oid(&self, id: impl AsRef, buf: &mut Vec) -> Option { @@ -181,13 +177,13 @@ mod find_impls { { type Error = T::Error; - fn find<'a>( + fn try_find<'a>( &self, id: impl AsRef, buffer: &'a mut Vec, pack_cache: &mut impl crate::cache::DecodeEntry, ) -> Result>, Self::Error> { - self.deref().find(id, buffer, pack_cache) + self.deref().try_find(id, buffer, pack_cache) } fn location_by_oid(&self, id: impl AsRef, buf: &mut Vec) -> Option { diff --git a/git-pack/tests/pack/data/output/count_and_entries.rs b/git-pack/tests/pack/data/output/count_and_entries.rs index b6fd700bbe5..c1dfcc75720 100644 --- a/git-pack/tests/pack/data/output/count_and_entries.rs +++ b/git-pack/tests/pack/data/output/count_and_entries.rs @@ -234,7 +234,7 @@ fn traversals() -> crate::Result { let head = hex_to_id("dfcb5e39ac6eb30179808bbab721e8a28ce1b52e"); let mut commits = commit::Ancestors::new(Some(head), commit::ancestors::State::default(), { let db = Arc::clone(&db); - move |oid, buf| db.find_existing_commit_iter(oid, buf, &mut pack::cache::Never).ok() + move |oid, buf| db.find_commit_iter(oid, buf, &mut pack::cache::Never).ok() }) .map(Result::unwrap) .collect::>(); @@ -265,7 +265,7 @@ fn traversals() -> crate::Result { )?; let actual_count = counts.iter().fold(ObjectCount::default(), |mut c, e| { let mut buf = Vec::new(); - if let Some(obj) = db.find_existing(e.id, &mut buf, &mut pack::cache::Never).ok() { + if let Some(obj) = db.find(e.id, &mut buf, &mut pack::cache::Never).ok() { c.add(obj.kind); } c @@ -369,7 +369,7 @@ fn write_and_verify( progress::Discard, &should_interrupt, Some(Box::new(move |oid, buf| { - db.find_existing(oid, buf, &mut git_pack::cache::Never).ok() + db.find(oid, buf, &mut git_pack::cache::Never).ok() })), pack::bundle::write::Options::default(), )? diff --git a/git-ref/tests/file/reference.rs b/git-ref/tests/file/reference.rs index 49fe74f9ff0..0e481625c5e 100644 --- a/git-ref/tests/file/reference.rs +++ b/git-ref/tests/file/reference.rs @@ -145,7 +145,7 @@ mod peel { let odb = git_odb::linked::Store::at(store.base.join("objects"))?; assert_eq!( r.peel_to_id_in_place(&store, None, |oid, buf| { - odb.find(oid, buf, &mut git_odb::pack::cache::Never) + odb.try_find(oid, buf, &mut git_odb::pack::cache::Never) .map(|obj| obj.map(|obj| (obj.kind, obj.data))) })?, commit, diff --git a/git-repository/src/easy/ext/object.rs b/git-repository/src/easy/ext/object.rs index 8c090f99606..595fa826099 100644 --- a/git-repository/src/easy/ext/object.rs +++ b/git-repository/src/easy/ext/object.rs @@ -19,7 +19,7 @@ pub fn find_object( let obj = access .repo()? .odb - .find_existing(&id, &mut buf, state.try_borrow_mut_pack_cache()?.deref_mut())?; + .find(&id, &mut buf, state.try_borrow_mut_pack_cache()?.deref_mut())?; obj.kind }; @@ -35,7 +35,7 @@ pub fn try_find_object( access .repo()? .odb - .find( + .try_find( &id, state.try_borrow_mut_buf()?.deref_mut(), state.try_borrow_mut_pack_cache()?.deref_mut(), diff --git a/git-repository/src/easy/reference.rs b/git-repository/src/easy/reference.rs index 0f24baea7dc..cdaec73bd19 100644 --- a/git-repository/src/easy/reference.rs +++ b/git-repository/src/easy/reference.rs @@ -135,7 +135,7 @@ where state.assure_packed_refs_uptodate(&repo.refs)?.as_ref(), |oid, buf| { repo.odb - .find(oid, buf, pack_cache.deref_mut()) + .try_find(oid, buf, pack_cache.deref_mut()) .map(|po| po.map(|o| (o.kind, o.data))) }, )? diff --git a/git-traverse/tests/commit/mod.rs b/git-traverse/tests/commit/mod.rs index 20404ed1251..2cda5f6dcf4 100644 --- a/git-traverse/tests/commit/mod.rs +++ b/git-traverse/tests/commit/mod.rs @@ -19,7 +19,7 @@ mod ancestor { commit::Ancestors::filtered( tips, commit::ancestors::State::default(), - move |oid, buf| db.find_existing_commit_iter(oid, buf, &mut pack::cache::Never).ok(), + move |oid, buf| db.find_commit_iter(oid, buf, &mut pack::cache::Never).ok(), predicate, ) } @@ -44,7 +44,7 @@ mod ancestor { ) -> impl Iterator> { let db = db().expect("db instantiation works as its definitely valid"); commit::Ancestors::new(tips, commit::ancestors::State::default(), move |oid, buf| { - db.find_existing_commit_iter(oid, buf, &mut pack::cache::Never).ok() + db.find_commit_iter(oid, buf, &mut pack::cache::Never).ok() }) } diff --git a/git-traverse/tests/tree/mod.rs b/git-traverse/tests/tree/mod.rs index 135ff6ce116..4e8172d4a92 100644 --- a/git-traverse/tests/tree/mod.rs +++ b/git-traverse/tests/tree/mod.rs @@ -14,20 +14,20 @@ fn basic_nesting() -> crate::Result<()> { let db = db()?; let mut buf = Vec::new(); let mut buf2 = Vec::new(); - let mut commit = db.find_existing_commit_iter( + let mut commit = db.find_commit_iter( hex_to_id("85df34aa34848b8138b2b3dcff5fb5c2b734e0ce"), &mut buf, &mut pack::cache::Never, )?; let mut recorder = tree::Recorder::default(); git_traverse::tree::breadthfirst( - db.find_existing_tree_iter( + db.find_tree_iter( commit.tree_id().expect("a tree is available in a commit"), &mut buf2, &mut pack::cache::Never, )?, tree::breadthfirst::State::default(), - |oid, buf| db.find_existing_tree_iter(oid, buf, &mut pack::cache::Never).ok(), + |oid, buf| db.find_tree_iter(oid, buf, &mut pack::cache::Never).ok(), &mut recorder, )?; diff --git a/gitoxide-core/src/hours.rs b/gitoxide-core/src/hours.rs index b13462d2215..a37b81e7649 100644 --- a/gitoxide-core/src/hours.rs +++ b/gitoxide-core/src/hours.rs @@ -52,7 +52,7 @@ where .find(refname.to_string_lossy().as_ref(), packed.as_ref())? .peel_to_id_in_place(&repo.refs, packed.as_ref(), |oid, buf| { repo.odb - .find(oid, buf, &mut pack::cache::Never) + .try_find(oid, buf, &mut pack::cache::Never) .map(|obj| obj.map(|obj| (obj.kind, obj.data))) })? .to_owned(); @@ -66,7 +66,7 @@ where for c in interrupt::Iter::new( commit_id.ancestors_iter(|oid, buf| { progress.inc(); - repo.odb.find_existing(oid, buf, &mut pack_cache).ok().map(|o| { + repo.odb.find(oid, buf, &mut pack_cache).ok().map(|o| { commits.push(o.data.to_owned()); objs::CommitRefIter::from_bytes(o.data) }) diff --git a/gitoxide-core/src/pack/create.rs b/gitoxide-core/src/pack/create.rs index f6530734ee0..b574f7e097b 100644 --- a/gitoxide-core/src/pack/create.rs +++ b/gitoxide-core/src/pack/create.rs @@ -122,7 +122,7 @@ where .and_then(|mut r| { r.peel_to_id_in_place(refs, packed.as_ref(), |oid, buf| { repo.odb - .find(oid, buf, &mut pack::cache::Never) + .try_find(oid, buf, &mut pack::cache::Never) .map(|obj| obj.map(|obj| (obj.kind, obj.data))) }) .map(|oid| oid.to_owned()) @@ -140,7 +140,7 @@ where // TODO: Easy-based traversal traverse::commit::Ancestors::new(tips, traverse::commit::ancestors::State::default(), { let db = Arc::clone(&db); - move |oid, buf| db.find_existing_commit_iter(oid, buf, &mut pack::cache::Never).ok() + move |oid, buf| db.find_commit_iter(oid, buf, &mut pack::cache::Never).ok() }) .map(|res| res.map_err(Into::into)) .inspect(move |_| progress.inc()), From 7ad7a4428d0e38f2ff776f7efab6996505d2bba2 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 27 Aug 2021 11:48:18 +0800 Subject: [PATCH 4/5] [pack #179] fix docs --- git-pack/src/find.rs | 6 +++--- git-pack/src/find_traits.rs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/git-pack/src/find.rs b/git-pack/src/find.rs index c83ba319bbe..080e098a156 100644 --- a/git-pack/src/find.rs +++ b/git-pack/src/find.rs @@ -2,7 +2,7 @@ pub mod existing { use git_hash::ObjectId; - /// The error returned by the [`find_existing(…)`][crate::FindExt::find_existing()] trait methods. + /// The error returned by the [`find(…)`][crate::FindExt::find()] trait methods. #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { @@ -17,7 +17,7 @@ pub mod existing { pub mod existing_object { use git_hash::ObjectId; - /// The error returned by the various [`find_existing_*`][crate::FindExt::find_existing_commit()] trait methods. + /// The error returned by the various [`find_*`][crate::FindExt::find_commit()] trait methods. #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { @@ -36,7 +36,7 @@ pub mod existing_object { pub mod existing_iter { use git_hash::ObjectId; - /// The error returned by the various [`find_existing_*`][crate::FindExt::find_existing_commit()] trait methods. + /// The error returned by the various [`find_*`][crate::FindExt::find_commit()] trait methods. #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { diff --git a/git-pack/src/find_traits.rs b/git-pack/src/find_traits.rs index 72396ed0fd4..3c766b106cf 100644 --- a/git-pack/src/find_traits.rs +++ b/git-pack/src/find_traits.rs @@ -9,7 +9,7 @@ use crate::{data, find}; /// /// [issue]: https://github.com/rust-lang/rust/issues/44265 pub trait Find { - /// The error returned by [`find()`][Find::find()] + /// The error returned by [`try_find()`][Find::try_find()] type Error: std::error::Error + 'static; /// Find an object matching `id` in the database while placing its raw, undecoded data into `buffer`. @@ -82,7 +82,7 @@ mod ext { macro_rules! make_iter_lookup { ($method:ident, $object_kind:path, $object_type:ty, $into_iter:tt) => { - /// Like [`find_existing(…)`][Self::find_existing()], but flattens the `Result>` into a single `Result` making a non-existing object an error + /// 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, @@ -108,7 +108,7 @@ mod ext { /// An extension trait with convenience functions. pub trait FindExt: super::Find { - /// Like [`find(…)`][super::Find::find()], but flattens the `Result>` into a single `Result` making a non-existing object an error. + /// 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: impl AsRef, From 420dca29bccca6e7d759880d8342f23b33eead0d Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 27 Aug 2021 15:14:09 +0800 Subject: [PATCH 5/5] [pack #179] refactor bundle --- etc/check-package-size.sh | 2 +- git-odb/src/store/compound/init.rs | 2 +- git-pack/CHANGELOG.md | 5 ++ git-pack/src/bundle/init.rs | 50 ++++++++++++++ git-pack/src/bundle/mod.rs | 104 +++++++---------------------- git-pack/src/bundle/write/types.rs | 2 +- git-pack/src/lib.rs | 8 ++- git-pack/src/loose.rs | 5 +- gitoxide-core/src/pack/receive.rs | 12 +--- 9 files changed, 97 insertions(+), 93 deletions(-) create mode 100644 git-pack/src/bundle/init.rs diff --git a/etc/check-package-size.sh b/etc/check-package-size.sh index d1f0c73e2e3..15975ce8974 100755 --- a/etc/check-package-size.sh +++ b/etc/check-package-size.sh @@ -30,7 +30,7 @@ indent cargo diet -n --package-size-limit 25KB (enter git-validate && indent cargo diet -n --package-size-limit 5KB) (enter git-object && indent cargo diet -n --package-size-limit 20KB) (enter git-commitgraph && indent cargo diet -n --package-size-limit 15KB) -(enter git-pack && indent cargo diet -n --package-size-limit 70KB) +(enter git-pack && indent cargo diet -n --package-size-limit 75KB) (enter git-odb && indent cargo diet -n --package-size-limit 15KB) (enter git-protocol && indent cargo diet -n --package-size-limit 25KB) (enter git-packetline && indent cargo diet -n --package-size-limit 15KB) diff --git a/git-odb/src/store/compound/init.rs b/git-odb/src/store/compound/init.rs index 5532f243d8f..dfc09e1bf64 100644 --- a/git-odb/src/store/compound/init.rs +++ b/git-odb/src/store/compound/init.rs @@ -12,7 +12,7 @@ pub enum Error { #[error("The objects directory at '{0}' is not an accessible directory")] Inaccessible(PathBuf), #[error(transparent)] - Pack(#[from] pack::bundle::Error), + Pack(#[from] pack::bundle::init::Error), #[error(transparent)] Alternate(#[from] Box), } diff --git a/git-pack/CHANGELOG.md b/git-pack/CHANGELOG.md index d9b0b3ad15c..3a16f49ebe8 100644 --- a/git-pack/CHANGELOG.md +++ b/git-pack/CHANGELOG.md @@ -7,3 +7,8 @@ - `Find::find_existing_*` -> `Find::find_*` - `Find::find_existing_*` -> `Find::find_*` - `Find::find()-> `Find::try_find()` + - `bundle::Bundle` -> `Bundle` + - `bundle::Error` -> `bundle::init::Error` + +* **new methods** + - `Find::find_tag_iter()` diff --git a/git-pack/src/bundle/init.rs b/git-pack/src/bundle/init.rs new file mode 100644 index 00000000000..e6e627c15b3 --- /dev/null +++ b/git-pack/src/bundle/init.rs @@ -0,0 +1,50 @@ +use crate::Bundle; +use std::{ + convert::TryFrom, + path::{Path, PathBuf}, +}; + +/// Returned by [`Bundle::at()`] +#[derive(thiserror::Error, Debug)] +#[allow(missing_docs)] +pub enum Error { + #[error("An 'idx' extension is expected of an index file: '{0}'")] + InvalidPath(PathBuf), + #[error(transparent)] + Pack(#[from] crate::data::header::decode::Error), + #[error(transparent)] + Index(#[from] crate::index::init::Error), +} + +/// Initialization +impl Bundle { + /// Create a `Bundle` from `path`, which is either a pack file _(*.pack)_ or an index file _(*.idx)_. + /// + /// The corresponding complementary file is expected to be present. + /// Also available via [`Bundle::try_from()`]. + pub fn at(path: impl AsRef) -> Result { + Self::try_from(path.as_ref()) + } +} + +impl TryFrom<&Path> for Bundle { + type Error = Error; + + fn try_from(path: &Path) -> Result { + let ext = path + .extension() + .and_then(|e| e.to_str()) + .ok_or_else(|| Error::InvalidPath(path.to_owned()))?; + Ok(match ext { + "idx" => Self { + index: crate::index::File::at(path)?, + pack: crate::data::File::at(path.with_extension("pack"))?, + }, + "pack" => Self { + pack: crate::data::File::at(path)?, + index: crate::index::File::at(path.with_extension("idx"))?, + }, + _ => return Err(Error::InvalidPath(path.to_owned())), + }) + } +} diff --git a/git-pack/src/bundle/mod.rs b/git-pack/src/bundle/mod.rs index 4534e98ef5f..684b0b27392 100644 --- a/git-pack/src/bundle/mod.rs +++ b/git-pack/src/bundle/mod.rs @@ -1,7 +1,26 @@ -use std::{ - convert::TryFrom, - path::{Path, PathBuf}, -}; +/// A way to uniquely identify the location of an object within a pack bundle +#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] +#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] +pub struct Location { + /// The id of the pack containing the object //TODO: this should probably at least by a typedef or even an opaque type + pub pack_id: u32, + /// The index at which the object can be found in the index file corresponding to the `pack_id`. + pub index_file_id: u32, + /// The size of the entry of disk so that the range of bytes of the entry is `pack_offset..pack_offset + entry_size`. + pub entry_size: usize, + /// The start of the entry in the pack identified by `pack_id`. + pub pack_offset: u64, +} + +impl Location { + /// Compute a range suitable for lookup in pack data using the [`entry_slice()`][crate::data::File::entry_slice()] method. + pub fn entry_range(&self, pack_offset: u64) -> crate::data::EntryRange { + pack_offset..pack_offset + self.entry_size as u64 + } +} + +/// +pub mod init; mod find; /// @@ -10,9 +29,10 @@ pub mod write; mod verify { use std::sync::{atomic::AtomicBool, Arc}; + use crate::Bundle; use git_features::progress::Progress; - impl super::Bundle { + impl Bundle { /// Similar to [`crate::index::File::verify_integrity()`] but more convenient to call as the presence of the /// pack file is a given. pub fn verify_integrity( @@ -40,77 +60,3 @@ mod verify { } } } - -/// Returned by [`Bundle::at()`] -#[derive(thiserror::Error, Debug)] -#[allow(missing_docs)] -pub enum Error { - #[error("An 'idx' extension is expected of an index file: '{0}'")] - InvalidPath(PathBuf), - #[error(transparent)] - Pack(#[from] crate::data::header::decode::Error), - #[error(transparent)] - Index(#[from] crate::index::init::Error), -} - -/// A way to uniquely identify the location of an object within a pack bundle -#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] -#[cfg_attr(feature = "serde1", derive(serde::Serialize, serde::Deserialize))] -pub struct Location { - /// The id of the pack containing the object //TODO: this should probably at least by a typedef or even an opaque type - pub pack_id: u32, - /// The index at which the object can be found in the index file corresponding to the `pack_id`. - pub index_file_id: u32, - /// The size of the entry of disk so that the range of bytes of the entry is `pack_offset..pack_offset + entry_size`. - pub entry_size: usize, - /// The start of the entry in the pack identified by `pack_id`. - pub pack_offset: u64, -} - -impl Location { - /// Compute a range suitable for lookup in pack data using the [`entry_slice()`][crate::data::File::entry_slice()] method. - pub fn entry_range(&self, pack_offset: u64) -> crate::data::EntryRange { - pack_offset..pack_offset + self.entry_size as u64 - } -} - -/// A bundle of pack data and the corresponding pack index -pub struct Bundle { - /// The pack file corresponding to `index` - pub pack: crate::data::File, - /// The index file corresponding to `pack` - pub index: crate::index::File, -} - -/// Initialization -impl Bundle { - /// Create a `Bundle` from `path`, which is either a pack file _(*.pack)_ or an index file _(*.idx)_. - /// - /// The corresponding complementary file is expected to be present. - /// Also available via [`Bundle::try_from()`]. - pub fn at(path: impl AsRef) -> Result { - Self::try_from(path.as_ref()) - } -} - -impl TryFrom<&Path> for Bundle { - type Error = Error; - - fn try_from(path: &Path) -> Result { - let ext = path - .extension() - .and_then(|e| e.to_str()) - .ok_or_else(|| Error::InvalidPath(path.to_owned()))?; - Ok(match ext { - "idx" => Self { - index: crate::index::File::at(path)?, - pack: crate::data::File::at(path.with_extension("pack"))?, - }, - "pack" => Self { - pack: crate::data::File::at(path)?, - index: crate::index::File::at(path.with_extension("idx"))?, - }, - _ => return Err(Error::InvalidPath(path.to_owned())), - }) - } -} diff --git a/git-pack/src/bundle/write/types.rs b/git-pack/src/bundle/write/types.rs index 075382eeb5a..69f75d7fea1 100644 --- a/git-pack/src/bundle/write/types.rs +++ b/git-pack/src/bundle/write/types.rs @@ -43,7 +43,7 @@ pub struct Outcome { impl Outcome { /// Instantiate a bundle from the newly written index and data file that are represented by this `Outcome` - pub fn to_bundle(&self) -> Option> { + pub fn to_bundle(&self) -> Option> { self.index_path.as_ref().map(crate::Bundle::at) } } diff --git a/git-pack/src/lib.rs b/git-pack/src/lib.rs index b15df436e15..bf04b174250 100644 --- a/git-pack/src/lib.rs +++ b/git-pack/src/lib.rs @@ -15,7 +15,13 @@ /// pub mod bundle; -pub use crate::bundle::Bundle; +/// A bundle of pack data and the corresponding pack index +pub struct Bundle { + /// The pack file corresponding to `index` + pub pack: crate::data::File, + /// The index file corresponding to `pack` + pub index: crate::index::File, +} /// pub mod find; diff --git a/git-pack/src/loose.rs b/git-pack/src/loose.rs index 915acd7414b..b4d9f244675 100644 --- a/git-pack/src/loose.rs +++ b/git-pack/src/loose.rs @@ -2,7 +2,10 @@ pub mod object { /// pub mod header { - //! loose object header encoding and decoding + //! loose object header encoding and decoding. + //! + //! Note that these are still relevant for packs as they are part of the computed hash of any git object, packed or not. + //! It just so happened that loose objects where the first ones used for implementing an object database. use byteorder::WriteBytesExt; use git_object as object; diff --git a/gitoxide-core/src/pack/receive.rs b/gitoxide-core/src/pack/receive.rs index 78b5f4c362f..1ab73a943cc 100644 --- a/gitoxide-core/src/pack/receive.rs +++ b/gitoxide-core/src/pack/receive.rs @@ -301,15 +301,9 @@ fn receive_pack_blocking( index_kind: pack::index::Version::V2, iteration_mode: pack::data::input::Mode::Verify, }; - let outcome = pack::bundle::Bundle::write_to_directory( - input, - directory.take(), - progress, - &ctx.should_interrupt, - None, - options, - ) - .map_err(|err| io::Error::new(io::ErrorKind::Other, err))?; + let outcome = + pack::Bundle::write_to_directory(input, directory.take(), progress, &ctx.should_interrupt, None, options) + .map_err(|err| io::Error::new(io::ErrorKind::Other, err))?; if let Some(directory) = refs_directory.take() { write_raw_refs(refs, directory)?;