Skip to content

Commit 8f9a55b

Browse files
committed
More detailed multi-index verification (#279)
1 parent d2bea27 commit 8f9a55b

File tree

3 files changed

+60
-1
lines changed

3 files changed

+60
-1
lines changed

git-pack/src/cache/delta/from_offsets.rs

+5
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,11 @@ impl<T> Tree<T> {
9898
tree.add_root(pack_offset, data)?;
9999
}
100100
RefDelta { base_id } => {
101+
// TODO: ref-deltas can also point to objects (by offset) which we haven't seen yet, so this should really be
102+
// handled maybe by delaying those? Add-Child needs things ordered ascending though, so it's not trivial to do
103+
// and probably requires tham offset-rewriting.
104+
// Note that some packs created by libgit2 generally look like that, see the 'index-bare' repo that is operated on by git2
105+
// and try the 'pack-verify' with the less-time algorithm for reproduction.
101106
resolve_in_pack_id(base_id.as_ref())
102107
.ok_or(Error::UnresolvedRefDelta { id: base_id })
103108
.and_then(|base_pack_offset| {

git-pack/src/index/traverse/with_lookup.rs

+1
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ impl index::File {
117117
))),
118118
);
119119
let mut stats = Vec::with_capacity(entries.len());
120+
progress.set(0);
120121
for index_entry in entries.iter() {
121122
let result = self.decode_and_process_entry(
122123
check,

git-pack/src/multi_index/verify.rs

+54-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ pub mod integrity {
1616
IndexIntegrity(#[from] crate::index::verify::integrity::Error),
1717
#[error(transparent)]
1818
BundleInit(#[from] crate::bundle::init::Error),
19+
#[error("Counted {actual} objects, but expected {expected} as per multi-index")]
20+
UnexpectedObjectCount { actual: usize, expected: usize },
21+
#[error("The object at multi-index entry {index} didn't match the expected oid sort-order or pack-offset")]
22+
OutOfOrder { index: usize },
1923
}
2024

2125
/// Returned by [`multi_index::File::verify_integrity()`][crate::multi_index::File::verify_integrity()].
@@ -89,7 +93,13 @@ impl File {
8993
Some(self.num_indices as usize),
9094
git_features::progress::count("indices"),
9195
);
92-
for index_file_name in &self.index_names {
96+
let mut order_progress = progress.add_child("obtain oid and offset");
97+
order_progress.init(
98+
Some(self.num_objects as usize),
99+
git_features::progress::count("objects"),
100+
);
101+
let mut oids_and_offsets = Vec::with_capacity(self.num_objects as usize);
102+
for (pack_id, index_file_name) in self.index_names.iter().enumerate() {
93103
progress.inc();
94104
let bundle = crate::Bundle::at(parent.join(index_file_name), self.object_hash)
95105
.map_err(integrity::Error::from)
@@ -144,8 +154,51 @@ impl File {
144154
}
145155
})?;
146156
pack_traverse_outcomes.push(pack_traverse_outcome);
157+
158+
oids_and_offsets.extend(
159+
bundle
160+
.index
161+
.iter()
162+
.map(|e| (e.oid, e.pack_offset, pack_id as crate::multi_index::PackIndex)),
163+
);
164+
order_progress.inc_by(bundle.index.num_objects() as usize);
165+
}
166+
167+
let order_start = std::time::Instant::now();
168+
order_progress.set_name("ordering by oid and deduplicating");
169+
order_progress.set(0);
170+
oids_and_offsets.sort_by(|l, r| l.0.cmp(&r.0));
171+
oids_and_offsets.dedup_by(|l, r| l.0.eq(&r.0));
172+
173+
if oids_and_offsets.len() != self.num_objects as usize {
174+
return Err(crate::index::traverse::Error::Processor(
175+
integrity::Error::UnexpectedObjectCount {
176+
actual: oids_and_offsets.len(),
177+
expected: self.num_objects as usize,
178+
},
179+
));
147180
}
148181

182+
order_progress.set_name("comparing oid and pack offset");
183+
for (index, ((loid, lpack_offset, lpack_id), (roid, rpack_offset, rpack_id))) in oids_and_offsets
184+
.into_iter()
185+
.zip(self.iter().map(|e| (e.oid, e.pack_offset, e.pack_index)))
186+
.enumerate()
187+
{
188+
if loid != roid || lpack_offset != rpack_offset || lpack_id != rpack_id {
189+
if loid == roid && lpack_id != rpack_id {
190+
// Right now we can't properly determine which pack would be chosen if objects exists in multiple packs, hence
191+
// our comparison might be off here.
192+
// TODO: check how git does the comparison or get into multi-index writing to be more true to the source.
193+
continue;
194+
}
195+
return Err(crate::index::traverse::Error::Processor(integrity::Error::OutOfOrder {
196+
index,
197+
}));
198+
}
199+
}
200+
order_progress.inc_by(self.num_objects as usize);
201+
order_progress.show_throughput(order_start);
149202
progress.show_throughput(start);
150203

151204
Ok(integrity::Outcome {

0 commit comments

Comments
 (0)