Skip to content

Commit

Permalink
change!: Use new gix-object::Find trait
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed Nov 4, 2023
1 parent 806ea47 commit 1165de0
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 31 deletions.
32 changes: 14 additions & 18 deletions gix-index/src/extension/tree/verify.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::cmp::Ordering;

use bstr::{BString, ByteSlice};
use gix_object::FindExt;

use crate::extension::Tree;

Expand All @@ -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,
Expand All @@ -39,20 +40,14 @@ pub enum Error {
}

impl Tree {
///
pub fn verify<F>(&self, use_find: bool, mut find: F) -> Result<(), Error>
where
F: for<'a> FnMut(&gix_hash::oid, &'a mut Vec<u8>) -> Option<gix_object::TreeRefIter<'a>>,
{
fn verify_recursive<F>(
/// 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<u8>>,
find: &mut F,
) -> Result<Option<u32>, Error>
where
F: for<'a> FnMut(&gix_hash::oid, &'a mut Vec<u8>) -> Option<gix_object::TreeRefIter<'a>>,
{
mut object_buf: Option<&mut Vec<u8>>,
objects: &impl gix_object::Find,
) -> Result<Option<u32>, Error> {
if children.is_empty() {
return Ok(None);
}
Expand All @@ -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)
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
13 changes: 6 additions & 7 deletions gix-index/src/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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<Find>(tree: &gix_hash::oid, mut find: Find) -> Result<Self, breadthfirst::Error>
pub fn from_tree<Find>(tree: &gix_hash::oid, objects: Find) -> Result<Self, breadthfirst::Error>
where
Find: for<'a> FnMut(&gix_hash::oid, &'a mut Vec<u8>) -> Option<TreeRefIter<'a>>,
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::Find { 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,
Expand Down
9 changes: 3 additions & 6 deletions gix-index/src/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,9 @@ impl State {
Ok(())
}

/// Note: `find` cannot be `Option<F>` as we can't call it with a closure then due to the indirection through `Some`.
pub fn verify_extensions<F>(&self, use_find: bool, find: F) -> Result<(), extensions::Error>
where
F: for<'a> FnMut(&gix_hash::oid, &'a mut Vec<u8>) -> Option<gix_object::TreeRefIter<'a>>,
{
self.tree().map(|t| t.verify(use_find, find)).transpose()?;
/// Note: `objects` cannot be `Option<F>` 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(())
Expand Down

0 comments on commit 1165de0

Please sign in to comment.