[commitgraph] Implement basic commit-graph file verification.

Missing features: 1. It operates on commit-graph files only, so it doesn't verify that commit-graph data matches `git-odb` data. 2. No progress reporting or parallelization. This shouldn't be needed until until we need to check against `git-odb` data. Example output for Linux repo: ``` $ time ./target/release/gixp commit-graph-verify -s ~/src/linux/.git/objects/info number of commits with the given number of parents 0: 4 1: 878988 2: 67800 3: 652 4: 408 5: 382 6: 454 7: 95 8: 65 9: 47 10: 25 11: 26 12: 14 13: 4 14: 3 18: 1 19: 1 20: 1 21: 1 24: 1 27: 1 30: 1 32: 1 66: 1 ->: 948976 longest path length between two commits: 160521 real 0m0.196s user 0m0.180s sys 0m0.016s ```

Conor Davis committed 6 years ago

Commit 25711131 parent 701f33c19 files · +459 −21Browse files

Showing the changes to git-commitgraph/src (8 of 19 files). Show all

Unified · Split

8 files

git-commitgraph/src/file/access.rs +4 −0
git-commitgraph/src/file/commit.rs +4 −0
git-commitgraph/src/file/mod.rs +2 −0
git-commitgraph/src/file/verify.rs +167 −0
git-commitgraph/src/graph/access.rs +7 −5
git-commitgraph/src/graph/mod.rs +1 −0
git-commitgraph/src/graph/verify.rs +158 −0
git-commitgraph/src/lib.rs +3 −0

git-commitgraph/src/file/access.rs+4−0

@@ -8,6 +8,10 @@ use std::{ · 7 unmodified lines
8	8
9	9	/// Access
10	10	impl File {
	11	+ pub fn base_graph_count(&self) -> u8 {
	12	+ self.base_graph_count
	13	+ }
	14	+
11	15	/// Returns the commit data for the commit located at the given lexigraphical position.
12	16	///
13	17	/// `pos` must range from 0 to self.num_commits().

git-commitgraph/src/file/commit.rs+4−0

@@ -90,6 +90,10 @@ impl<'a> Commit<'a> { · 89 unmodified lines
90	90	self.iter_parents().next().transpose()
91	91	}
92	92
	93	+ pub fn position(&self) -> file::Position {
	94	+ self.pos
	95	+ }
	96	+
93	97	// Allow the return value to outlive this Commit object, as it only needs to be bound by the
94	98	// lifetime of the parent file.
95	99	pub fn root_tree_id<'b>(&'b self) -> borrowed::Id<'a>

git-commitgraph/src/file/mod.rs+2−0

@@ -2,6 +2,8 @@ · 1 unmodified line
2	2	mod access;
3	3	pub mod commit;
4	4	mod init;
	5	+pub mod verify;
	6	+
5	7	pub use init::Error;
6	8
7	9	pub use commit::Commit;

git-commitgraph/src/file/verify.rs+167−0

@@ -0,0 +1,167 @@
	1	+use crate::{
	2	+ file::{self, File},
	3	+ GENERATION_NUMBER_INFINITY, GENERATION_NUMBER_MAX,
	4	+};
	5	+use bstr::ByteSlice;
	6	+use git_object::{borrowed, owned, SHA1_SIZE};
	7	+use std::cmp::{max, min};
	8	+use std::collections::HashMap;
	9	+use std::convert::TryFrom;
	10	+use std::path::Path;
	11	+
	12	+#[derive(thiserror::Error, Debug)]
	13	+pub enum Error {
	14	+ #[error(transparent)]
	15	+ Commit(#[from] file::commit::Error),
	16	+ #[error("commit at file position {pos} has invalid ID {id}")]
	17	+ CommitId { id: owned::Id, pos: file::Position },
	18	+ #[error("commit at file position {pos} with ID {id} is out of order relative to its predecessor with ID {predecessor_id}")]
	19	+ CommitsOutOfOrder {
	20	+ id: owned::Id,
	21	+ pos: file::Position,
	22	+ predecessor_id: owned::Id,
	23	+ },
	24	+ #[error("commit-graph filename should be {0}")]
	25	+ Filename(String),
	26	+ #[error("commit {id} has invalid generation {generation}")]
	27	+ Generation { generation: u32, id: owned::Id },
	28	+ #[error("checksum mismatch: expected {expected}, got {actual}")]
	29	+ Mismatch { expected: owned::Id, actual: owned::Id },
	30	+ #[error("commit {id} has invalid root tree ID {root_tree_id}")]
	31	+ RootTreeId { id: owned::Id, root_tree_id: owned::Id },
	32	+}
	33	+
	34	+// This is a separate type to let `traverse`'s caller use the same error type for its result and its
	35	+// processor error type while also letting that error type contain file::verify::Error values.
	36	+// Is there a better way? Should the caller's error type just use boxes to avoid recursive type
	37	+// errors?
	38	+#[derive(thiserror::Error, Debug)]
	39	+pub enum EitherError<E1: std::error::Error + 'static, E2: std::error::Error + 'static> {
	40	+ #[error(transparent)]
	41	+ Internal(#[from] E1),
	42	+ // Why can't I use #[from] here? Boo!
	43	+ #[error("{0}")]
	44	+ Processor(#[source] E2),
	45	+}
	46	+
	47	+#[derive(Clone, Debug, Eq, PartialEq)]
	48	+#[cfg_attr(feature = "serde1", derive(serde::Deserialize, serde::Serialize))]
	49	+pub struct Outcome {
	50	+ pub max_generation: u32,
	51	+ pub max_parents: u32,
	52	+ pub min_generation: u32,
	53	+ pub num_commits: u32,
	54	+ pub parent_counts: HashMap<u32, u32>,
	55	+}
	56	+
	57	+impl File {
	58	+ pub fn checksum(&self) -> borrowed::Id<'_> {
	59	+ borrowed::Id::try_from(&self.data[self.data.len() - SHA1_SIZE..]).expect("file to be large enough for a hash")
	60	+ }
	61	+
	62	+ pub fn traverse<'a, E, Processor>(&'a self, mut processor: Processor) -> Result<Outcome, EitherError<Error, E>>
	63	+ where
	64	+ E: std::error::Error + 'static,
	65	+ Processor: FnMut(&file::Commit<'a>) -> Result<(), E>,
	66	+ {
	67	+ self.verify_checksum()?;
	68	+ verify_split_chain_filename_hash(&self.path, self.checksum())?;
	69	+
	70	+ // This probably belongs in borrowed::Id itself?
	71	+ let null_id = borrowed::Id::from(&[0u8; SHA1_SIZE]);
	72	+
	73	+ let mut stats = Outcome {
	74	+ max_generation: 0,
	75	+ max_parents: 0,
	76	+ min_generation: GENERATION_NUMBER_INFINITY,
	77	+ num_commits: self.num_commits(),
	78	+ parent_counts: HashMap::new(),
	79	+ };
	80	+
	81	+ // TODO: Verify self.fan values as we go.
	82	+ let mut prev_id: borrowed::Id<'a> = null_id;
	83	+ for commit in self.iter_commits() {
	84	+ if commit.id() <= prev_id {
	85	+ if commit.id() == null_id {
	86	+ return Err(Error::CommitId {
	87	+ pos: commit.position(),
	88	+ id: commit.id().into(),
	89	+ }
	90	+ .into());
	91	+ }
	92	+ return Err(Error::CommitsOutOfOrder {
	93	+ pos: commit.position(),
	94	+ id: commit.id().into(),
	95	+ predecessor_id: prev_id.into(),
	96	+ }
	97	+ .into());
	98	+ }
	99	+ if commit.root_tree_id() == null_id {
	100	+ return Err(Error::RootTreeId {
	101	+ id: commit.id().into(),
	102	+ root_tree_id: commit.root_tree_id().into(),
	103	+ }
	104	+ .into());
	105	+ }
	106	+ if commit.generation() > GENERATION_NUMBER_MAX {
	107	+ return Err(Error::Generation {
	108	+ generation: commit.generation(),
	109	+ id: commit.id().into(),
	110	+ }
	111	+ .into());
	112	+ }
	113	+
	114	+ processor(&commit).map_err(EitherError::Processor)?;
	115	+
	116	+ stats.max_generation = max(stats.max_generation, commit.generation());
	117	+ stats.min_generation = min(stats.min_generation, commit.generation());
	118	+ let parent_count = commit
	119	+ .iter_parents()
	120	+ .try_fold(0u32, \|acc, pos\| pos.map(\|_\| acc + 1))
	121	+ .map_err(Error::Commit)?;
	122	+ *stats.parent_counts.entry(parent_count).or_insert(0) += 1;
	123	+ prev_id = commit.id();
	124	+ }
	125	+
	126	+ if stats.min_generation == GENERATION_NUMBER_INFINITY {
	127	+ stats.min_generation = 0;
	128	+ }
	129	+
	130	+ Ok(stats)
	131	+ }
	132	+
	133	+ pub fn verify_checksum(&self) -> Result<owned::Id, Error> {
	134	+ // TODO: Use/copy git_odb::hash::bytes_of_file.
	135	+ let data_len_without_trailer = self.data.len() - SHA1_SIZE;
	136	+ let mut hasher = git_features::hash::Sha1::default();
	137	+ hasher.update(&self.data[..data_len_without_trailer]);
	138	+ let actual = owned::Id::new_sha1(hasher.digest());
	139	+
	140	+ let expected = self.checksum();
	141	+ if actual.to_borrowed() == expected {
	142	+ Ok(actual)
	143	+ } else {
	144	+ Err(Error::Mismatch {
	145	+ actual,
	146	+ expected: expected.into(),
	147	+ })
	148	+ }
	149	+ }
	150	+}
	151	+
	152	+/// If the given path's filename matches "graph-{hash}.graph", check that `hash` matches the
	153	+/// expected hash.
	154	+fn verify_split_chain_filename_hash(path: impl AsRef<Path>, expected: borrowed::Id<'_>) -> Result<(), Error> {
	155	+ let path = path.as_ref();
	156	+ path.file_name()
	157	+ .and_then(\|filename\| filename.to_str())
	158	+ .and_then(\|filename\| filename.strip_suffix(".graph"))
	159	+ .and_then(\|stem\| stem.strip_prefix("graph-"))
	160	+ .map_or(Ok(()), \|hex\| match owned::Id::from_40_bytes_in_hex(hex.as_bytes()) {
	161	+ Ok(actual) if actual.to_borrowed() == expected => Ok(()),
	162	+ _ => Err(Error::Filename(format!(
	163	+ "graph-{}.graph",
	164	+ expected.to_sha1_hex().as_bstr()
	165	+ ))),
	166	+ })
	167	+}

git-commitgraph/src/graph/access.rs+7−5

@@ -42,7 +42,7 @@ impl Graph { · 41 unmodified lines
42	42
43	43	/// Access fundamentals
44	44	impl Graph {
45		− fn lookup_by_id(&self, id: borrowed::Id<'_>) -> Option<LookupByIdResult<'_>> {
	45	+ pub(crate) fn lookup_by_id(&self, id: borrowed::Id<'_>) -> Option<LookupByIdResult<'_>> {
46	46	let mut current_file_start = 0;
47	47	for file in &self.files {
48	48	if let Some(lex_pos) = file.lookup(id) {
@@ -57,14 +57,15 @@ impl Graph { · 8 unmodified lines
57	57	None
58	58	}
59	59
60		− fn lookup_by_pos(&self, pos: graph::Position) -> LookupByPositionResult<'_> {
	60	+ pub(crate) fn lookup_by_pos(&self, pos: graph::Position) -> LookupByPositionResult<'_> {
61	61	let mut remaining = pos.0;
62		− for file in &self.files {
	62	+ for (file_index, file) in self.files.iter().enumerate() {
63	63	match remaining.checked_sub(file.num_commits()) {
64	64	Some(v) => remaining = v,
65	65	None => {
66	66	return LookupByPositionResult {
67	67	file,
	68	+ file_index,
68	69	pos: file::Position(remaining),
69	70	}
70	71	}
@@ -75,14 +76,15 @@ impl Graph { · 4 unmodified lines
75	76	}
76	77
77	78	#[derive(Clone)]
78		−struct LookupByIdResult<'a> {
	79	+pub(crate) struct LookupByIdResult<'a> {
79	80	pub file: &'a File,
80	81	pub graph_pos: graph::Position,
81	82	pub file_pos: file::Position,
82	83	}
83	84
84	85	#[derive(Clone)]
85		−struct LookupByPositionResult<'a> {
	86	+pub(crate) struct LookupByPositionResult<'a> {
86	87	pub file: &'a File,
	88	+ pub file_index: usize,
87	89	pub pos: file::Position,
88	90	}

git-commitgraph/src/graph/mod.rs+1−0

@@ -1,6 +1,7 @@
1	1	//! Operations on a complete commit graph.
2	2	mod access;
3	3	mod init;
	4	+pub mod verify;
4	5
5	6	use crate::file::File;
6	7	use std::fmt;

git-commitgraph/src/graph/verify.rs+158−0

@@ -0,0 +1,158 @@
	1	+use crate::{
	2	+ file::{self, commit},
	3	+ graph, Graph, GENERATION_NUMBER_MAX,
	4	+};
	5	+use git_object::owned;
	6	+use std::cmp::{max, min};
	7	+use std::collections::BTreeMap;
	8	+use std::convert::TryInto;
	9	+use std::path::PathBuf;
	10	+
	11	+#[derive(thiserror::Error, Debug)]
	12	+pub enum Error<E: std::error::Error + 'static> {
	13	+ #[error("'{}' should have {expected} base graphs, but claims {actual} base graphs", .path.display())]
	14	+ BaseGraphCount { actual: u8, expected: u8, path: PathBuf },
	15	+ #[error("'{}' base graph at index {index} should have ID {expected} but is {actual}", .path.display())]
	16	+ BaseGraphId {
	17	+ actual: owned::Id,
	18	+ expected: owned::Id,
	19	+ index: u8,
	20	+ path: PathBuf,
	21	+ },
	22	+ #[error(transparent)]
	23	+ Commit(#[from] commit::Error),
	24	+ #[error("{}: {err}", .path.display())]
	25	+ File { err: file::verify::Error, path: PathBuf },
	26	+ #[error("Commit {id}'s generation should be {expected} but is {actual}")]
	27	+ Generation { actual: u32, expected: u32, id: owned::Id },
	28	+ #[error(
	29	+ "Commit {id} has parent position {parent_pos} that is out of range (should be in range 0-{max_valid_pos})"
	30	+ )]
	31	+ ParentOutOfRange {
	32	+ id: owned::Id,
	33	+ max_valid_pos: graph::Position,
	34	+ parent_pos: graph::Position,
	35	+ },
	36	+ #[error("{0}")]
	37	+ Processor(#[source] E),
	38	+ #[error("Commit-graph should be composed of at most 256 files but actually contains {0} files")]
	39	+ TooManyFiles(usize),
	40	+}
	41	+
	42	+#[derive(Clone, Debug, Eq, PartialEq)]
	43	+#[cfg_attr(feature = "serde1", derive(serde::Deserialize, serde::Serialize))]
	44	+pub struct Outcome {
	45	+ pub longest_path_length: Option<u32>,
	46	+ pub num_commits: u32,
	47	+ pub parent_counts: BTreeMap<u32, u32>,
	48	+}
	49	+
	50	+impl Graph {
	51	+ pub fn verify_integrity<E>(
	52	+ &self,
	53	+ mut processor: impl FnMut(&file::Commit<'_>) -> Result<(), E>,
	54	+ ) -> Result<Outcome, Error<E>>
	55	+ where
	56	+ E: std::error::Error + 'static,
	57	+ {
	58	+ if self.files.len() > 256 {
	59	+ // A file in a split chain can only have up to 255 base files.
	60	+ return Err(Error::TooManyFiles(self.files.len()));
	61	+ }
	62	+
	63	+ let mut stats = Outcome {
	64	+ longest_path_length: None,
	65	+ num_commits: 0,
	66	+ parent_counts: BTreeMap::new(),
	67	+ };
	68	+ let mut max_generation = 0u32;
	69	+
	70	+ // TODO: Detect duplicate commit IDs across different files. Not sure how to do this without
	71	+ // a separate loop, e.g. self.iter_sorted_ids().
	72	+
	73	+ let mut file_start_pos = graph::Position(0);
	74	+ for (file_index, file) in self.files.iter().enumerate() {
	75	+ if usize::from(file.base_graph_count()) != file_index {
	76	+ return Err(Error::BaseGraphCount {
	77	+ actual: file.base_graph_count(),
	78	+ expected: file_index
	79	+ .try_into()
	80	+ .expect("files.len() check to protect against this"),
	81	+ path: file.path().to_owned(),
	82	+ });
	83	+ }
	84	+
	85	+ for (base_graph_index, (expected, actual)) in self.files[..file_index]
	86	+ .iter()
	87	+ .map(\|base_file\| base_file.checksum())
	88	+ .zip(file.iter_base_graph_ids())
	89	+ .enumerate()
	90	+ {
	91	+ if actual != expected {
	92	+ return Err(Error::BaseGraphId {
	93	+ actual: actual.into(),
	94	+ expected: expected.into(),
	95	+ index: base_graph_index
	96	+ .try_into()
	97	+ .expect("files.len() check to protect against this"),
	98	+ path: file.path().to_owned(),
	99	+ });
	100	+ }
	101	+ }
	102	+
	103	+ let next_file_start_pos = graph::Position(file_start_pos.0 + file.num_commits());
	104	+ let file_stats = file
	105	+ .traverse(\|commit\| {
	106	+ let mut max_parent_generation = 0u32;
	107	+ for parent_pos in commit.iter_parents() {
	108	+ let parent_pos = parent_pos.map_err(Error::Commit)?;
	109	+ if parent_pos >= next_file_start_pos {
	110	+ return Err(Error::ParentOutOfRange {
	111	+ parent_pos,
	112	+ id: commit.id().into(),
	113	+ max_valid_pos: graph::Position(next_file_start_pos.0 - 1),
	114	+ });
	115	+ }
	116	+ let parent = self.commit_at(parent_pos);
	117	+ max_parent_generation = max(max_parent_generation, parent.generation());
	118	+ }
	119	+
	120	+ // If the max parent generation is GENERATION_NUMBER_MAX, then this commit's
	121	+ // generation should be GENERATION_NUMBER_MAX too.
	122	+ let expected_generation = min(max_parent_generation + 1, GENERATION_NUMBER_MAX);
	123	+ if commit.generation() != expected_generation {
	124	+ return Err(Error::Generation {
	125	+ actual: commit.generation(),
	126	+ expected: expected_generation,
	127	+ id: commit.id().into(),
	128	+ });
	129	+ }
	130	+
	131	+ processor(commit).map_err(Error::Processor)?;
	132	+
	133	+ Ok(())
	134	+ })
	135	+ .map_err(\|e\| match e {
	136	+ file::verify::EitherError::Internal(err) => Error::File {
	137	+ err,
	138	+ path: file.path().to_owned(),
	139	+ },
	140	+ file::verify::EitherError::Processor(e) => e,
	141	+ })?;
	142	+
	143	+ max_generation = max(max_generation, file_stats.max_generation);
	144	+ stats.num_commits += file_stats.num_commits;
	145	+ for (key, value) in file_stats.parent_counts.into_iter() {
	146	+ *stats.parent_counts.entry(key).or_insert(0) += value;
	147	+ }
	148	+ file_start_pos = next_file_start_pos;
	149	+ }
	150	+
	151	+ stats.longest_path_length = if max_generation < GENERATION_NUMBER_MAX {
	152	+ Some(max_generation.saturating_sub(1))
	153	+ } else {
	154	+ None
	155	+ };
	156	+ Ok(stats)
	157	+ }
	158	+}

git-commitgraph/src/lib.rs+3−0

@@ -6,5 +6,8 @@ pub mod graph; · 5 unmodified lines
6	6
7	7	pub use graph::Graph;
8	8
	9	+pub const GENERATION_NUMBER_INFINITY: u32 = 0xffff_ffff;
	10	+pub const GENERATION_NUMBER_MAX: u32 = 0x3fff_ffff;
	11	+
9	12	/// The maximum number of commits that can be stored in a commit graph.
10	13	pub const MAX_COMMITS: u32 = (1 << 30) + (1 << 29) + (1 << 28) - 1;