Skip to content

Commit

Permalink
Add a less thorough and faster way of verifying multi-indices (#279)
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed Jan 1, 2022
1 parent 91e6d38 commit 7517482
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 8 deletions.
61 changes: 54 additions & 7 deletions git-pack/src/multi_index/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,47 @@ impl File {
)
}

/// Similar to [`verify_integrity()`][File::verify_integrity()] but without any deep inspection of objects.
///
/// Instead we only validate the contents of the multi-index itself.
pub fn verify_integrity_fast<P>(
&self,
progress: P,
should_interrupt: &AtomicBool,
) -> Result<(git_hash::ObjectId, P), integrity::Error>
where
P: Progress,
{
self.verify_integrity_inner(progress, should_interrupt, false, integrity::Options::default())
.map_err(|err| match err {
crate::index::traverse::Error::Processor(err) => err,
_ => unreachable!("BUG: no other error type is possible"),
})
.map(|o| (o.actual_index_checksum, o.progress))
}

/// Similar to [`crate::Bundle::verify_integrity()`] but checks all contained indices and their packs.
///
/// Note that it's considered a failure if an index doesn't have a corresponding pack.
#[allow(unused)]
pub fn verify_integrity<C, P, F>(
&self,
progress: P,
should_interrupt: &AtomicBool,
options: integrity::Options<F>,
) -> Result<integrity::Outcome<P>, crate::index::traverse::Error<integrity::Error>>
where
P: Progress,
C: crate::cache::DecodeEntry,
F: Fn() -> C + Send + Clone,
{
self.verify_integrity_inner(progress, should_interrupt, true, options)
}

fn verify_integrity_inner<C, P, F>(
&self,
mut progress: P,
should_interrupt: &AtomicBool,
deep_check: bool,
integrity::Options {
verify_mode,
traversal,
Expand Down Expand Up @@ -191,9 +224,23 @@ impl File {
progress.set_name(index_file_name.display().to_string());
progress.inc();

let bundle = crate::Bundle::at(parent.join(index_file_name), self.object_hash)
.map_err(integrity::Error::from)
.map_err(crate::index::traverse::Error::Processor)?;
let mut bundle = None;
let index;
let index_path = parent.join(index_file_name);
let index = if deep_check {
bundle = crate::Bundle::at(index_path, self.object_hash)
.map_err(integrity::Error::from)
.map_err(crate::index::traverse::Error::Processor)?
.into();
bundle.as_ref().map(|b| &b.index).expect("just set")
} else {
index = Some(
crate::index::File::at(index_path, self.object_hash)
.map_err(|err| integrity::Error::BundleInit(crate::bundle::init::Error::Index(err)))
.map_err(crate::index::traverse::Error::Processor)?,
);
index.as_ref().expect("just set")
};

let slice_end = pack_ids_slice.partition_point(|e| e.0 == pack_id as crate::data::Id);
let multi_index_entries_to_check = &pack_ids_slice[..slice_end];
Expand All @@ -203,10 +250,10 @@ impl File {
for entry_id in multi_index_entries_to_check.iter().map(|e| e.1) {
let oid = self.oid_at_index(entry_id);
let (_, expected_pack_offset) = self.pack_id_and_pack_offset_at_index(entry_id);
let entry_in_bundle_index = bundle.index.lookup(oid).ok_or_else(|| {
let entry_in_bundle_index = index.lookup(oid).ok_or_else(|| {
crate::index::traverse::Error::Processor(integrity::Error::OidNotFound { id: oid.to_owned() })
})?;
let actual_pack_offset = bundle.index.pack_offset_at_index(entry_in_bundle_index);
let actual_pack_offset = index.pack_offset_at_index(entry_in_bundle_index);
if actual_pack_offset != expected_pack_offset {
return Err(crate::index::traverse::Error::Processor(
integrity::Error::PackOffsetMismatch {
Expand All @@ -226,7 +273,7 @@ impl File {
total_objects_checked += multi_index_entries_to_check.len();

progress.set_name("Validating");
{
if let Some(bundle) = bundle {
let progress = progress.add_child(index_file_name.display().to_string());
let crate::bundle::verify::integrity::Outcome {
actual_index_checksum: _,
Expand Down
6 changes: 6 additions & 0 deletions git-pack/tests/pack/multi_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,5 +171,11 @@ mod write {
.actual_index_checksum,
outcome.multi_index_checksum
);

let outcome = file
.verify_integrity_fast(progress::Discard, &AtomicBool::new(false))
.unwrap();

assert_eq!(outcome.0, file.checksum());
}
}
7 changes: 6 additions & 1 deletion gitoxide-core/src/pack/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,12 @@ where
match path.file_name() {
Some(file_name) if file_name == "multi-pack-index" => {
let multi_index = git::odb::pack::multi_index::File::at(path)?;
let res = multi_index.verify_integrity(cache, progress, should_interrupt, git::odb::pack::multi_index::verify::integrity::Options{verify_mode: mode, traversal: algorithm.into(),thread_limit })?;
let res = multi_index.verify_integrity(progress, should_interrupt, git::odb::pack::multi_index::verify::integrity::Options{
verify_mode: mode,
traversal: algorithm.into(),
thread_limit,
make_pack_lookup_cache: cache
})?;
match output_statistics {
Some(OutputFormat::Human) => {
for (index_name, stats) in multi_index.index_names().iter().zip(res.pack_traverse_statistics) {
Expand Down

0 comments on commit 7517482

Please sign in to comment.