From aacf0f05a909e5b7d9ffd5623ef9833e0465be93 Mon Sep 17 00:00:00 2001 From: Conor Davis Date: Thu, 1 Oct 2020 22:03:05 -0500 Subject: [PATCH 1/9] [commitgraph] Stub out commit-graph-verify plumbing command. Commit graphs are optional, so commit-graph plumbing is guarded by a `commitgraph` feature that is enabled by default. --- Cargo.lock | 1 + gitoxide-core/Cargo.toml | 1 + gitoxide-core/src/commitgraph/mod.rs | 1 + gitoxide-core/src/commitgraph/verify.rs | 36 +++++++++++++++++++++++++ gitoxide-core/src/lib.rs | 1 + src/plumbing/lean/main.rs | 12 +++++++++ src/plumbing/lean/options.rs | 9 +++++++ src/plumbing/pretty/main.rs | 11 ++++++++ src/plumbing/pretty/options.rs | 8 ++++++ 9 files changed, 80 insertions(+) create mode 100644 gitoxide-core/src/commitgraph/mod.rs create mode 100644 gitoxide-core/src/commitgraph/verify.rs diff --git a/Cargo.lock b/Cargo.lock index df030b424fa..20d4d0ca48d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -868,6 +868,7 @@ version = "0.4.1" dependencies = [ "anyhow", "bytesize", + "git-commitgraph", "git-features", "git-object", "git-odb", diff --git a/gitoxide-core/Cargo.toml b/gitoxide-core/Cargo.toml index fa7d277cd13..850b4ccff05 100644 --- a/gitoxide-core/Cargo.toml +++ b/gitoxide-core/Cargo.toml @@ -28,3 +28,4 @@ anyhow = "1.0.31" quick-error = "2.0.0" bytesize = "1.0.1" serde_json = { version = "1.0.56", optional = true } +git-commitgraph = { version = "^0.1.2", path = "../git-commitgraph" } diff --git a/gitoxide-core/src/commitgraph/mod.rs b/gitoxide-core/src/commitgraph/mod.rs new file mode 100644 index 00000000000..a8118c56ae7 --- /dev/null +++ b/gitoxide-core/src/commitgraph/mod.rs @@ -0,0 +1 @@ +pub mod verify; diff --git a/gitoxide-core/src/commitgraph/verify.rs b/gitoxide-core/src/commitgraph/verify.rs new file mode 100644 index 00000000000..d44beaf80b4 --- /dev/null +++ b/gitoxide-core/src/commitgraph/verify.rs @@ -0,0 +1,36 @@ +use anyhow::{Context as AnyhowContext, Result}; +use std::{io, path::Path}; + +/// A general purpose context for many operations provided here +pub struct Context { + /// A stream to which to output operation results + pub out: W1, + /// A stream to which to errors + pub err: W2, +} + +impl Default for Context, Vec> { + fn default() -> Self { + Context { + out: Vec::new(), + err: Vec::new(), + } + } +} + +pub fn graph_or_file( + path: impl AsRef, + Context { + out: mut _out, + err: mut _err, + }: Context, +) -> Result<()> +where + W1: io::Write, + W2: io::Write, +{ + // TODO: Handle `path` being objects/info, objects/info/commit-graph, + // or objects/info/commit-graphs/graph-xyz.graph. + let _g = git_commitgraph::Graph::from_info_dir(path).with_context(|| "Could not open commit graph")?; + Err(anyhow::Error::msg("not implemented")) +} diff --git a/gitoxide-core/src/lib.rs b/gitoxide-core/src/lib.rs index 2f7ed60ce13..7528c63a5fc 100644 --- a/gitoxide-core/src/lib.rs +++ b/gitoxide-core/src/lib.rs @@ -37,6 +37,7 @@ impl FromStr for OutputFormat { mod protocol; pub use protocol::Protocol; +pub mod commitgraph; pub mod pack; pub mod remote; pub mod repository; diff --git a/src/plumbing/lean/main.rs b/src/plumbing/lean/main.rs index 0f6d0169da4..13c894243e1 100644 --- a/src/plumbing/lean/main.rs +++ b/src/plumbing/lean/main.rs @@ -140,5 +140,17 @@ pub fn main() -> Result<()> { ) .map(|_| ()) } + SubCommands::CommitGraphVerify(CommitGraphVerify { path }) => { + use self::core::commitgraph::verify; + + verify::graph_or_file( + path, + verify::Context { + out: stdout(), + err: stderr(), + }, + ) + .map(|_| ()) + } } } diff --git a/src/plumbing/lean/options.rs b/src/plumbing/lean/options.rs index 6b6b26798fd..c8bfb93ead0 100644 --- a/src/plumbing/lean/options.rs +++ b/src/plumbing/lean/options.rs @@ -32,6 +32,7 @@ pub enum SubCommands { IndexFromPack(IndexFromPack), RemoteRefList(RemoteRefList), PackReceive(PackReceive), + CommitGraphVerify(CommitGraphVerify), } /// Create an index from a packfile. @@ -188,3 +189,11 @@ pub struct PackVerify { #[argh(positional)] pub path: PathBuf, } + +/// Verify a commit graph +#[derive(FromArgs, PartialEq, Debug)] +#[argh(subcommand, name = "commit-graph-verify")] +pub struct CommitGraphVerify { + #[argh(positional)] + pub path: PathBuf, +} diff --git a/src/plumbing/pretty/main.rs b/src/plumbing/pretty/main.rs index 514ad354a39..1d8bd009353 100644 --- a/src/plumbing/pretty/main.rs +++ b/src/plumbing/pretty/main.rs @@ -256,6 +256,17 @@ pub fn main() -> Result<()> { }, ) .map(|_| ()), + Subcommands::CommitGraphVerify { path } => prepare_and_run( + "commit-graph-verify", + verbose, + progress, + progress_keep_open, + None, + move |_progress, out, err| { + core::commitgraph::verify::graph_or_file(path, core::commitgraph::verify::Context { out, err }) + }, + ) + .map(|_| ()), }?; Ok(()) } diff --git a/src/plumbing/pretty/options.rs b/src/plumbing/pretty/options.rs index 1c18e591155..91c4ecf16ad 100644 --- a/src/plumbing/pretty/options.rs +++ b/src/plumbing/pretty/options.rs @@ -186,4 +186,12 @@ pub enum Subcommands { #[clap(parse(from_os_str))] path: PathBuf, }, + /// Verify the integrity of a commit graph + #[clap(setting = AppSettings::ColoredHelp)] + #[clap(setting = AppSettings::DisableVersion)] + CommitGraphVerify { + /// The 'objects/info/' dir, 'objects/info/commit-graphs' dir, or 'objects/info/commit-graph' file to validate. + #[clap(parse(from_os_str))] + path: PathBuf, + }, } From c8b1f74328965708e38a689b865660ad36f22ecb Mon Sep 17 00:00:00 2001 From: Conor Davis Date: Sat, 10 Oct 2020 13:40:00 -0500 Subject: [PATCH 2/9] [commitgraph] Use `thiserror` instead of `quick_error`. It doesn't look like `quick_error` supports type parameters on error enums, and I want to use type parameters for verification errors. --- Cargo.lock | 2 +- git-commitgraph/Cargo.toml | 2 +- git-commitgraph/src/file/commit.rs | 36 ++----- git-commitgraph/src/file/init.rs | 164 ++++++++++++++--------------- git-commitgraph/src/file/mod.rs | 1 - git-commitgraph/src/graph/init.rs | 87 ++++++++------- 6 files changed, 139 insertions(+), 153 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 20d4d0ca48d..ceda5e6b692 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -699,8 +699,8 @@ dependencies = [ "byteorder", "filebuffer", "git-object", - "quick-error 2.0.0", "tempfile", + "thiserror", ] [[package]] diff --git a/git-commitgraph/Cargo.toml b/git-commitgraph/Cargo.toml index 27a32e896fd..5bd0c76c82e 100644 --- a/git-commitgraph/Cargo.toml +++ b/git-commitgraph/Cargo.toml @@ -20,7 +20,7 @@ git-object = { version = "^0.4.0", path = "../git-object" } bstr = { version = "0.2.13", default-features = false, features = ["std"] } byteorder = "1.2.3" filebuffer = "0.4.0" -quick-error = "2.0.0" +thiserror = "1.0.20" [dev-dependencies] tempfile = "3.1.0" diff --git a/git-commitgraph/src/file/commit.rs b/git-commitgraph/src/file/commit.rs index 88cd148cfeb..4bdd347a9a9 100644 --- a/git-commitgraph/src/file/commit.rs +++ b/git-commitgraph/src/file/commit.rs @@ -4,38 +4,22 @@ use crate::{ }; use byteorder::{BigEndian, ByteOrder}; use git_object::{borrowed, owned, SHA1_SIZE}; -use quick_error::quick_error; use std::{ convert::{TryFrom, TryInto}, fmt::{Debug, Formatter}, slice::Chunks, }; -quick_error! { - #[derive(Debug)] - pub enum Error { - ExtraEdgesListOverflow(commit: owned::Id) { - display( - "commit {}'s extra edges overflows the commit-graph file's extra edges list", - commit, - ) - } - FirstParentIsExtraEdgeIndex(commit: owned::Id) { - display( - "commit {}'s first parent is an extra edge index, which is invalid", - commit, - ) - } - MissingExtraEdgesList(commit: owned::Id) { - display( - "commit {} has extra edges, but commit-graph file has no extra edges list", - commit, - ) - } - SecondParentWithoutFirstParent(commit: owned::Id) { - display("commit {} has a second parent but not a first parent", commit) - } - } +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error("commit {0}'s extra edges overflows the commit-graph file's extra edges list")] + ExtraEdgesListOverflow(owned::Id), + #[error("commit {0}'s first parent is an extra edge index, which is invalid")] + FirstParentIsExtraEdgeIndex(owned::Id), + #[error("commit {0} has extra edges, but commit-graph file has no extra edges list")] + MissingExtraEdgesList(owned::Id), + #[error("commit {0} has a second parent but not a first parent")] + SecondParentWithoutFirstParent(owned::Id), } // Note that git's commit-graph-format.txt as of v2.28.0 gives an incorrect value 0x0700_0000 for diff --git a/git-commitgraph/src/file/init.rs b/git-commitgraph/src/file/init.rs index a31e5338c1e..f8a0d20fe90 100644 --- a/git-commitgraph/src/file/init.rs +++ b/git-commitgraph/src/file/init.rs @@ -3,7 +3,6 @@ use bstr::ByteSlice; use byteorder::{BigEndian, ByteOrder}; use filebuffer::FileBuffer; use git_object::SHA1_SIZE; -use quick_error::quick_error; use std::{ convert::{TryFrom, TryInto}, ops::Range, @@ -12,53 +11,38 @@ use std::{ type ChunkId = [u8; 4]; -quick_error! { - #[derive(Debug)] - pub enum Error { - BaseGraphMismatch(from_header: u8, from_chunk: u32) { - display( - "Commit-graph {} chunk contains {} base graphs, but commit-graph file header claims {} base graphs", - BASE_GRAPHS_LIST_CHUNK_ID.as_bstr(), - from_chunk, - from_header, - ) - } - CommitCountMismatch(chunk1_id: ChunkId, chunk1_commits: u32, chunk2_id: ChunkId, chunk2_commits: u32) { - display( - "Commit-graph {:?} chunk contains {} commits, but {:?} chunk contains {} commits", - chunk1_id.as_bstr(), - chunk1_commits, - chunk2_id.as_bstr(), - chunk2_commits, - ) - } - Corrupt(msg: String) { - display("{}", msg) - } - DuplicateChunk(id: ChunkId) { - display("Commit-graph file contains multiple {:?} chunks", id.as_bstr()) - } - // This error case is disabled, as git allows extra garbage in the extra edges list. - // ExtraEdgesOverflow { - // display("The last entry in commit-graph's extended edges list does is not marked as being terminal") - // } - InvalidChunkSize(id: ChunkId, msg: String) { - display("Commit-graph chunk {:?} has invalid size: {}", id.as_bstr(), msg) - } - Io(err: std::io::Error, path: std::path::PathBuf) { - display("Could not open commit-graph file at '{}'", path.display()) - source(err) - } - MissingChunk(id: ChunkId) { - display("Missing required chunk {:?}", id.as_bstr()) - } - UnsupportedHashVersion(version: u8) { - display("Commit-graph file uses unsupported hash version: {}", version) - } - UnsupportedVersion(version: u8) { - display("Unsupported commit-graph file version: {}", version) - } - } +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error("Commit-graph {:?} chunk contains {from_chunk} base graphs, but commit-graph file header claims {from_header} base graphs", BASE_GRAPHS_LIST_CHUNK_ID.as_bstr())] + BaseGraphMismatch { from_header: u8, from_chunk: u32 }, + #[error("Commit-graph {:?} chunk contains {chunk1_commits} commits, but {:?} chunk contains {chunk2_commits} commits", .chunk1_id.as_bstr(), .chunk2_id.as_bstr())] + CommitCountMismatch { + chunk1_id: ChunkId, + chunk1_commits: u32, + chunk2_id: ChunkId, + chunk2_commits: u32, + }, + #[error("{0}")] + Corrupt(String), + #[error("Commit-graph file contains multiple {:?} chunks", .0.as_bstr())] + DuplicateChunk(ChunkId), + // This error case is disabled, as git allows extra garbage in the extra edges list? + // #[error("The last entry in commit-graph's extended edges list does is not marked as being terminal")] + // ExtraEdgesOverflow, + #[error("Commit-graph chunk {:?} has invalid size: {msg}", .id.as_bstr())] + InvalidChunkSize { id: ChunkId, msg: String }, + #[error("Could not open commit-graph file at '{}'", .path.display())] + Io { + #[source] + err: std::io::Error, + path: std::path::PathBuf, + }, + #[error("Missing required chunk {:?}", .0.as_bstr())] + MissingChunk(ChunkId), + #[error("Commit-graph file uses unsupported hash version: {0}")] + UnsupportedHashVersion(u8), + #[error("Unsupported commit-graph file version: {0}")] + UnsupportedVersion(u8), } const CHUNK_LOOKUP_SIZE: usize = 12; @@ -85,7 +69,10 @@ impl TryFrom<&Path> for File { type Error = Error; fn try_from(path: &Path) -> Result { - let data = FileBuffer::open(path).map_err(|e| Error::Io(e, path.to_owned()))?; + let data = FileBuffer::open(path).map_err(|e| Error::Io { + err: e, + path: path.to_owned(), + })?; let data_size = data.len(); if data_size < MIN_FILE_SIZE { return Err(Error::Corrupt( @@ -164,14 +151,18 @@ impl TryFrom<&Path> for File { .expect("an offset small enough to fit a usize"); ofs += 8; - let chunk_size: usize = next_chunk_offset - .checked_sub(chunk_offset) - .ok_or_else(|| Error::InvalidChunkSize(chunk_id, "size is negative".to_string()))?; + let chunk_size: usize = + next_chunk_offset + .checked_sub(chunk_offset) + .ok_or_else(|| Error::InvalidChunkSize { + id: chunk_id, + msg: "size is negative".to_string(), + })?; if next_chunk_offset >= data_size { - return Err(Error::InvalidChunkSize( - chunk_id, - "chunk extends beyond end of file".to_string(), - )); + return Err(Error::InvalidChunkSize { + id: chunk_id, + msg: "chunk extends beyond end of file".to_string(), + }); } match chunk_id { @@ -180,14 +171,17 @@ impl TryFrom<&Path> for File { return Err(Error::DuplicateChunk(chunk_id)); } if chunk_size % SHA1_SIZE != 0 { - return Err(Error::InvalidChunkSize( - chunk_id, - format!("chunk size {} is not a multiple of {}", chunk_size, SHA1_SIZE), - )); + return Err(Error::InvalidChunkSize { + id: chunk_id, + msg: format!("chunk size {} is not a multiple of {}", chunk_size, SHA1_SIZE), + }); } let chunk_base_graph_count = (chunk_size / SHA1_SIZE) as u32; if chunk_base_graph_count != base_graph_count as u32 { - return Err(Error::BaseGraphMismatch(base_graph_count, chunk_base_graph_count)); + return Err(Error::BaseGraphMismatch { + from_chunk: chunk_base_graph_count, + from_header: base_graph_count, + }); } base_graphs_list_offset = Some(chunk_offset); } @@ -196,13 +190,13 @@ impl TryFrom<&Path> for File { return Err(Error::DuplicateChunk(chunk_id)); } if chunk_size % COMMIT_DATA_ENTRY_SIZE != 0 { - return Err(Error::InvalidChunkSize( - chunk_id, - format!( + return Err(Error::InvalidChunkSize { + id: chunk_id, + msg: format!( "chunk size {} is not a multiple of {}", chunk_size, COMMIT_DATA_ENTRY_SIZE ), - )); + }); } commit_data_offset = Some(chunk_offset); commit_data_count = (chunk_size / COMMIT_DATA_ENTRY_SIZE) as u32; @@ -223,10 +217,10 @@ impl TryFrom<&Path> for File { } let expected_size = 4 * FAN_LEN; if chunk_size != expected_size { - return Err(Error::InvalidChunkSize( - chunk_id, - format!("expected chunk length {}, got {}", expected_size, chunk_size), - )); + return Err(Error::InvalidChunkSize { + id: chunk_id, + msg: format!("expected chunk length {}, got {}", expected_size, chunk_size), + }); } fan_offset = Some(chunk_offset); } @@ -235,13 +229,13 @@ impl TryFrom<&Path> for File { return Err(Error::DuplicateChunk(chunk_id)); } if chunk_size % OID_LOOKUP_ENTRY_SIZE != 0 { - return Err(Error::InvalidChunkSize( - chunk_id, - format!( + return Err(Error::InvalidChunkSize { + id: chunk_id, + msg: format!( "chunk size {} is not a multiple of {}", chunk_size, OID_LOOKUP_ENTRY_SIZE ), - )); + }); } oid_lookup_offset = Some(chunk_offset); oid_lookup_count = (chunk_size / OID_LOOKUP_ENTRY_SIZE) as u32; @@ -277,20 +271,20 @@ impl TryFrom<&Path> for File { let (fan, _) = read_fan(&data[fan_offset..]); if oid_lookup_count != fan[255] { - return Err(Error::CommitCountMismatch( - OID_FAN_CHUNK_ID, - fan[255], - OID_LOOKUP_CHUNK_ID, - oid_lookup_count, - )); + return Err(Error::CommitCountMismatch { + chunk1_id: OID_FAN_CHUNK_ID, + chunk1_commits: fan[255], + chunk2_id: OID_LOOKUP_CHUNK_ID, + chunk2_commits: oid_lookup_count, + }); } if commit_data_count != fan[255] { - return Err(Error::CommitCountMismatch( - OID_FAN_CHUNK_ID, - fan[255], - COMMIT_DATA_CHUNK_ID, - commit_data_count, - )); + return Err(Error::CommitCountMismatch { + chunk1_id: OID_FAN_CHUNK_ID, + chunk1_commits: fan[255], + chunk2_id: COMMIT_DATA_CHUNK_ID, + chunk2_commits: commit_data_count, + }); } Ok(File { base_graph_count, diff --git a/git-commitgraph/src/file/mod.rs b/git-commitgraph/src/file/mod.rs index 3441390e91e..844e62349e5 100644 --- a/git-commitgraph/src/file/mod.rs +++ b/git-commitgraph/src/file/mod.rs @@ -1,7 +1,6 @@ //! Operations on a single commit-graph file. mod access; pub mod commit; - mod init; pub use init::Error; diff --git a/git-commitgraph/src/graph/init.rs b/git-commitgraph/src/graph/init.rs index 4e6817cd31a..46e5daac491 100644 --- a/git-commitgraph/src/graph/init.rs +++ b/git-commitgraph/src/graph/init.rs @@ -3,40 +3,37 @@ use crate::{ Graph, MAX_COMMITS, }; use git_object::HashKind; -use quick_error::quick_error; use std::{ io::{BufRead, BufReader}, path::{Path, PathBuf}, }; -quick_error! { - #[derive(Debug)] - pub enum Error { - File(err: file::Error, path: PathBuf) { - display("{}", path.display()) - source(err) - } - HashVersionMismatch(path1: PathBuf, hash1: HashKind, path2: PathBuf, hash2: HashKind) { - display( - "Commit-graph files mismatch: '{}' uses hash {:?}, but '{}' uses hash {:?}", - path1.display(), - hash1, - path2.display(), - hash2, - ) - } - Io(err: std::io::Error, path: PathBuf) { - display("Could not open commit-graph file at '{}'", path.display()) - source(err) - } - TooManyCommits(num_commits: u64) { - display( - "Commit-graph files contain {} commits altogether, but only {} commits are allowed", - num_commits, - MAX_COMMITS, - ) - } - } +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error("{}", .path.display())] + File { + #[source] + err: file::Error, + path: PathBuf, + }, + #[error("Commit-graph files mismatch: '{}' uses hash {hash1:?}, but '{}' uses hash {hash2:?}", .path1.display(), .path2.display())] + HashVersionMismatch { + path1: PathBuf, + hash1: HashKind, + path2: PathBuf, + hash2: HashKind, + }, + #[error("Could not open commit-graph file at '{}'", path.display())] + Io { + #[source] + err: std::io::Error, + path: PathBuf, + }, + #[error( + "Commit-graph files contain {0} commits altogether, but only {} commits are allowed", + MAX_COMMITS + )] + TooManyCommits(u64), } /// Instantiate a `Graph` from various sources @@ -48,19 +45,31 @@ impl Graph { pub fn from_single_file(info_dir: impl AsRef) -> Result { let single_graph_file = info_dir.as_ref().join("commit-graph"); - let file = File::at(&single_graph_file).map_err(|e| Error::File(e, single_graph_file.clone()))?; + let file = File::at(&single_graph_file).map_err(|e| Error::File { + err: e, + path: single_graph_file.clone(), + })?; Self::new(vec![file]) } pub fn from_split_chain(commit_graphs_dir: impl AsRef) -> Result { let commit_graphs_dir = commit_graphs_dir.as_ref(); let chain_file_path = commit_graphs_dir.join("commit-graph-chain"); - let chain_file = std::fs::File::open(&chain_file_path).map_err(|e| Error::Io(e, chain_file_path.clone()))?; + let chain_file = std::fs::File::open(&chain_file_path).map_err(|e| Error::Io { + err: e, + path: chain_file_path.clone(), + })?; let mut files = Vec::new(); for line in BufReader::new(chain_file).lines() { - let hash = line.map_err(|e| Error::Io(e, chain_file_path.clone()))?; + let hash = line.map_err(|e| Error::Io { + err: e, + path: chain_file_path.clone(), + })?; let graph_file_path = commit_graphs_dir.join(format!("graph-{}.graph", hash)); - files.push(File::at(&graph_file_path).map_err(|e| Error::File(e, graph_file_path.clone()))?); + files.push(File::at(&graph_file_path).map_err(|e| Error::File { + err: e, + path: graph_file_path.clone(), + })?); } Self::new(files) } @@ -75,12 +84,12 @@ impl Graph { let f1 = &window[0]; let f2 = &window[1]; if f1.hash_kind() != f2.hash_kind() { - return Err(Error::HashVersionMismatch( - f1.path().to_owned(), - f1.hash_kind(), - f2.path().to_owned(), - f2.hash_kind(), - )); + return Err(Error::HashVersionMismatch { + path1: f1.path().to_owned(), + hash1: f1.hash_kind(), + path2: f2.path().to_owned(), + hash2: f2.hash_kind(), + }); } } From 1b738ac0719ec20b24982d148a386d63ec4dc2d6 Mon Sep 17 00:00:00 2001 From: Conor Davis Date: Sat, 10 Oct 2020 20:48:00 -0500 Subject: [PATCH 3/9] [commitgraph] Validate trailer section when parsing files. --- git-commitgraph/src/file/init.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/git-commitgraph/src/file/init.rs b/git-commitgraph/src/file/init.rs index f8a0d20fe90..7a62870ff27 100644 --- a/git-commitgraph/src/file/init.rs +++ b/git-commitgraph/src/file/init.rs @@ -39,6 +39,8 @@ pub enum Error { }, #[error("Missing required chunk {:?}", .0.as_bstr())] MissingChunk(ChunkId), + #[error("{0}")] + Trailer(String), #[error("Commit-graph file uses unsupported hash version: {0}")] UnsupportedHashVersion(u8), #[error("Unsupported commit-graph file version: {0}")] @@ -47,7 +49,8 @@ pub enum Error { const CHUNK_LOOKUP_SIZE: usize = 12; const HEADER_LEN: usize = 8; -const MIN_FILE_SIZE: usize = HEADER_LEN + ((MIN_CHUNKS + 1) * CHUNK_LOOKUP_SIZE); +const TRAILER_LEN: usize = SHA1_SIZE; +const MIN_FILE_SIZE: usize = HEADER_LEN + ((MIN_CHUNKS + 1) * CHUNK_LOOKUP_SIZE) + TRAILER_LEN; const OID_LOOKUP_ENTRY_SIZE: usize = SHA1_SIZE; // Required chunks: OIDF, OIDL, CDAT @@ -262,6 +265,14 @@ impl TryFrom<&Path> for File { ))); } + let actual_trailer_len = data_size.saturating_sub(chunk_offset); + if actual_trailer_len != TRAILER_LEN { + return Err(Error::Trailer(format!( + "Expected commit-graph trailer to contain {} bytes, got {}", + TRAILER_LEN, actual_trailer_len + ))); + } + let fan_offset = fan_offset.ok_or_else(|| Error::MissingChunk(OID_FAN_CHUNK_ID))?; let oid_lookup_offset = oid_lookup_offset.ok_or_else(|| Error::MissingChunk(OID_LOOKUP_CHUNK_ID))?; let commit_data_offset = commit_data_offset.ok_or_else(|| Error::MissingChunk(COMMIT_DATA_CHUNK_ID))?; From a783052d0cc2d3c9fa1dda3ea77286a79690d2c1 Mon Sep 17 00:00:00 2001 From: Conor Davis Date: Sat, 10 Oct 2020 14:28:09 -0500 Subject: [PATCH 4/9] [commitgraph] Add `Graph::at` constructor. This constructor tries to accept as many commit-graph-looking paths as possible, such as `.git/objects/info`, `.git/objects/info/commit-graph`, or `.git/objects/info/commit-graphs`. --- git-commitgraph/src/graph/init.rs | 57 ++++++++++++++++++------- gitoxide-core/src/commitgraph/verify.rs | 3 +- 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/git-commitgraph/src/graph/init.rs b/git-commitgraph/src/graph/init.rs index 46e5daac491..569ecb8ba2f 100644 --- a/git-commitgraph/src/graph/init.rs +++ b/git-commitgraph/src/graph/init.rs @@ -4,6 +4,7 @@ use crate::{ }; use git_object::HashKind; use std::{ + convert::TryFrom, io::{BufRead, BufReader}, path::{Path, PathBuf}, }; @@ -23,7 +24,9 @@ pub enum Error { path2: PathBuf, hash2: HashKind, }, - #[error("Could not open commit-graph file at '{}'", path.display())] + #[error("Did not find any files that look like commit graphs at '{}'", .0.display())] + InvalidPath(PathBuf), + #[error("Could not open commit-graph file at '{}'", .path.display())] Io { #[source] err: std::io::Error, @@ -38,22 +41,12 @@ pub enum Error { /// Instantiate a `Graph` from various sources impl Graph { - pub fn from_info_dir(info_dir: impl AsRef) -> Result { - Self::from_single_file(info_dir.as_ref()) - .or_else(|_| Self::from_split_chain(info_dir.as_ref().join("commit-graphs"))) - } - - pub fn from_single_file(info_dir: impl AsRef) -> Result { - let single_graph_file = info_dir.as_ref().join("commit-graph"); - let file = File::at(&single_graph_file).map_err(|e| Error::File { - err: e, - path: single_graph_file.clone(), - })?; - Self::new(vec![file]) + pub fn at(path: impl AsRef) -> Result { + Self::try_from(path.as_ref()) } - pub fn from_split_chain(commit_graphs_dir: impl AsRef) -> Result { - let commit_graphs_dir = commit_graphs_dir.as_ref(); + pub fn from_commit_graphs_dir(path: impl AsRef) -> Result { + let commit_graphs_dir = path.as_ref(); let chain_file_path = commit_graphs_dir.join("commit-graph-chain"); let chain_file = std::fs::File::open(&chain_file_path).map_err(|e| Error::Io { err: e, @@ -74,6 +67,20 @@ impl Graph { Self::new(files) } + pub fn from_file(path: impl AsRef) -> Result { + let path = path.as_ref(); + let file = File::at(path).map_err(|e| Error::File { + err: e, + path: path.to_owned(), + })?; + Self::new(vec![file]) + } + + pub fn from_info_dir(info_dir: impl AsRef) -> Result { + Self::from_file(info_dir.as_ref().join("commit-graph")) + .or_else(|_| Self::from_commit_graphs_dir(info_dir.as_ref().join("commit-graphs"))) + } + pub fn new(files: Vec) -> Result { let num_commits: u64 = files.iter().map(|f| f.num_commits() as u64).sum(); if num_commits > MAX_COMMITS as u64 { @@ -96,3 +103,23 @@ impl Graph { Ok(Self { files }) } } + +impl TryFrom<&Path> for Graph { + type Error = Error; + + fn try_from(path: &Path) -> Result { + if path.is_file() { + // Assume we are looking at `.git/objects/info/commit-graph` or + // `.git/objects/info/commit-graphs/graph-*.graph`. + Self::from_file(path) + } else if path.is_dir() { + if path.join("commit-graph-chain").is_file() { + Self::from_commit_graphs_dir(path) + } else { + Self::from_info_dir(path) + } + } else { + Err(Error::InvalidPath(path.to_owned())) + } + } +} diff --git a/gitoxide-core/src/commitgraph/verify.rs b/gitoxide-core/src/commitgraph/verify.rs index d44beaf80b4..8bb2df8ecd7 100644 --- a/gitoxide-core/src/commitgraph/verify.rs +++ b/gitoxide-core/src/commitgraph/verify.rs @@ -1,4 +1,5 @@ use anyhow::{Context as AnyhowContext, Result}; +use git_commitgraph::Graph; use std::{io, path::Path}; /// A general purpose context for many operations provided here @@ -31,6 +32,6 @@ where { // TODO: Handle `path` being objects/info, objects/info/commit-graph, // or objects/info/commit-graphs/graph-xyz.graph. - let _g = git_commitgraph::Graph::from_info_dir(path).with_context(|| "Could not open commit graph")?; + let _g = Graph::at(path).with_context(|| "Could not open commit graph")?; Err(anyhow::Error::msg("not implemented")) } From 5b067808a793e3515c0c12cf95c11b57beaa8d09 Mon Sep 17 00:00:00 2001 From: Conor Davis Date: Sat, 10 Oct 2020 19:49:35 -0500 Subject: [PATCH 5/9] [commitgraph] Tweak `File::iter_base_graph_ids` implementation. * It doesn't use evil `as`. * It's a bit less code. * It should be a tiny bit faster. * It is slightly more strict in that it will panic if the base graphs list is not evenly divisible into hashes. The file parser's error checking should ensure that this panic never happens, but maybe maybe maybe... --- git-commitgraph/src/file/access.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/git-commitgraph/src/file/access.rs b/git-commitgraph/src/file/access.rs index 5626784e7a0..e5142f9a2d1 100644 --- a/git-commitgraph/src/file/access.rs +++ b/git-commitgraph/src/file/access.rs @@ -42,12 +42,10 @@ impl File { } pub fn iter_base_graph_ids(&self) -> impl Iterator> { - let base_graphs_list = match self.base_graphs_list_offset { - Some(v) => &self.data[v..v + (SHA1_SIZE * self.base_graph_count as usize)], - None => &[], - }; + let start = self.base_graphs_list_offset.unwrap_or(0); + let base_graphs_list = &self.data[start..start + (SHA1_SIZE * usize::from(self.base_graph_count))]; base_graphs_list - .chunks_exact(SHA1_SIZE) + .chunks(SHA1_SIZE) .map(|bytes| borrowed::Id::try_from(bytes).expect("20 bytes SHA1 to be alright")) } From 28f94b4bccdf317c9f4ccb62e0e3f3314f3995c9 Mon Sep 17 00:00:00 2001 From: Conor Davis Date: Sat, 10 Oct 2020 21:08:45 -0500 Subject: [PATCH 6/9] [commitgraph] Replace `T as U` with `U::from(T)` or `t.try_into()`. --- git-commitgraph/src/file/access.rs | 2 +- git-commitgraph/src/file/init.rs | 16 +++++++++++----- git-commitgraph/src/graph/init.rs | 4 ++-- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/git-commitgraph/src/file/access.rs b/git-commitgraph/src/file/access.rs index e5142f9a2d1..9feca7aa5da 100644 --- a/git-commitgraph/src/file/access.rs +++ b/git-commitgraph/src/file/access.rs @@ -59,7 +59,7 @@ impl File { // copied from git-odb/src/pack/index/access.rs pub fn lookup(&self, id: borrowed::Id<'_>) -> Option { - let first_byte = id.first_byte() as usize; + let first_byte = usize::from(id.first_byte()); let mut upper_bound = self.fan[first_byte]; let mut lower_bound = if first_byte != 0 { self.fan[first_byte - 1] } else { 0 }; diff --git a/git-commitgraph/src/file/init.rs b/git-commitgraph/src/file/init.rs index 7a62870ff27..6fadb070ca0 100644 --- a/git-commitgraph/src/file/init.rs +++ b/git-commitgraph/src/file/init.rs @@ -115,7 +115,7 @@ impl TryFrom<&Path> for File { let base_graph_count = data[ofs]; ofs += 1; - let chunk_lookup_end = ofs + ((chunk_count as usize + 1) * CHUNK_LOOKUP_SIZE); + let chunk_lookup_end = ofs + ((usize::from(chunk_count) + 1) * CHUNK_LOOKUP_SIZE); if chunk_lookup_end > data_size { return Err(Error::Corrupt(format!( "Commit-graph file is too small to hold {} chunks", @@ -179,8 +179,10 @@ impl TryFrom<&Path> for File { msg: format!("chunk size {} is not a multiple of {}", chunk_size, SHA1_SIZE), }); } - let chunk_base_graph_count = (chunk_size / SHA1_SIZE) as u32; - if chunk_base_graph_count != base_graph_count as u32 { + let chunk_base_graph_count: u32 = (chunk_size / SHA1_SIZE) + .try_into() + .expect("base graph count to fit in 32-bits"); + if chunk_base_graph_count != u32::from(base_graph_count) { return Err(Error::BaseGraphMismatch { from_chunk: chunk_base_graph_count, from_header: base_graph_count, @@ -202,7 +204,9 @@ impl TryFrom<&Path> for File { }); } commit_data_offset = Some(chunk_offset); - commit_data_count = (chunk_size / COMMIT_DATA_ENTRY_SIZE) as u32; + commit_data_count = (chunk_size / COMMIT_DATA_ENTRY_SIZE) + .try_into() + .expect("number of commits in CDAT chunk to fit in 32 bits"); } EXTENDED_EDGES_LIST_CHUNK_ID => { if extra_edges_list_range.is_some() { @@ -241,7 +245,9 @@ impl TryFrom<&Path> for File { }); } oid_lookup_offset = Some(chunk_offset); - oid_lookup_count = (chunk_size / OID_LOOKUP_ENTRY_SIZE) as u32; + oid_lookup_count = (chunk_size / OID_LOOKUP_ENTRY_SIZE) + .try_into() + .expect("number of commits in OIDL chunk to fit in 32 bits"); // TODO(ST): Figure out how to handle this. Don't know what to do with the commented code. // git allows extra garbage in the extra edges list chunk? // if oid_lookup_count > 0 { diff --git a/git-commitgraph/src/graph/init.rs b/git-commitgraph/src/graph/init.rs index 569ecb8ba2f..d2cb64d23b4 100644 --- a/git-commitgraph/src/graph/init.rs +++ b/git-commitgraph/src/graph/init.rs @@ -82,8 +82,8 @@ impl Graph { } pub fn new(files: Vec) -> Result { - let num_commits: u64 = files.iter().map(|f| f.num_commits() as u64).sum(); - if num_commits > MAX_COMMITS as u64 { + let num_commits: u64 = files.iter().map(|f| u64::from(f.num_commits())).sum(); + if num_commits > u64::from(MAX_COMMITS) { return Err(Error::TooManyCommits(num_commits)); } From 701f33c06b80deaabe7625b01d36e2a1b1af3a78 Mon Sep 17 00:00:00 2001 From: Conor Davis Date: Sat, 10 Oct 2020 21:25:44 -0500 Subject: [PATCH 7/9] [commitgraph] Loosen lifetime restrictions on return values. --- git-commitgraph/src/file/commit.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/git-commitgraph/src/file/commit.rs b/git-commitgraph/src/file/commit.rs index 4bdd347a9a9..e0ae2f69430 100644 --- a/git-commitgraph/src/file/commit.rs +++ b/git-commitgraph/src/file/commit.rs @@ -77,7 +77,12 @@ impl<'a> Commit<'a> { } } - pub fn id(&self) -> borrowed::Id<'_> { + // Allow the return value to outlive this Commit object, as it only needs to be bound by the + // lifetime of the parent file. + pub fn id<'b>(&'b self) -> borrowed::Id<'a> + where + 'a: 'b, + { self.file.id_at(self.pos) } @@ -85,7 +90,12 @@ impl<'a> Commit<'a> { self.iter_parents().next().transpose() } - pub fn root_tree_id(&self) -> borrowed::Id<'_> { + // Allow the return value to outlive this Commit object, as it only needs to be bound by the + // lifetime of the parent file. + pub fn root_tree_id<'b>(&'b self) -> borrowed::Id<'a> + where + 'a: 'b, + { self.root_tree_id } } From 2571113fea516737acedac08d66632ead499b474 Mon Sep 17 00:00:00 2001 From: Conor Davis Date: Sat, 10 Oct 2020 20:01:29 -0500 Subject: [PATCH 8/9] [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 ``` --- Cargo.lock | 2 + git-commitgraph/Cargo.toml | 5 +- git-commitgraph/src/file/access.rs | 4 + git-commitgraph/src/file/commit.rs | 4 + git-commitgraph/src/file/mod.rs | 2 + git-commitgraph/src/file/verify.rs | 167 ++++++++++++++++++ git-commitgraph/src/graph/access.rs | 12 +- git-commitgraph/src/graph/mod.rs | 1 + git-commitgraph/src/graph/verify.rs | 158 +++++++++++++++++ git-commitgraph/src/lib.rs | 3 + gitoxide-core/Cargo.toml | 2 +- gitoxide-core/src/commitgraph/verify.rs | 56 ++++-- src/plumbing/lean/main.rs | 9 +- src/plumbing/lean/options.rs | 4 + src/plumbing/pretty/main.rs | 12 +- src/plumbing/pretty/options.rs | 3 + .../statistics-json-success | 8 + .../commit-graph-verify/statistics-success | 6 + tests/stateless-journey.sh | 22 +++ 19 files changed, 459 insertions(+), 21 deletions(-) create mode 100644 git-commitgraph/src/file/verify.rs create mode 100644 git-commitgraph/src/graph/verify.rs create mode 100644 tests/snapshots/plumbing/commit-graph-verify/statistics-json-success create mode 100644 tests/snapshots/plumbing/commit-graph-verify/statistics-success diff --git a/Cargo.lock b/Cargo.lock index ceda5e6b692..a3b955889b8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -698,7 +698,9 @@ dependencies = [ "bstr", "byteorder", "filebuffer", + "git-features", "git-object", + "serde", "tempfile", "thiserror", ] diff --git a/git-commitgraph/Cargo.toml b/git-commitgraph/Cargo.toml index 5bd0c76c82e..f72e3124f71 100644 --- a/git-commitgraph/Cargo.toml +++ b/git-commitgraph/Cargo.toml @@ -12,14 +12,17 @@ include = ["src/**/*"] [lib] doctest = false -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html +[features] +serde1 = ["serde", "git-object/serde1"] [dependencies] +git-features = { version = "^0.6.0", path = "../git-features" } git-object = { version = "^0.4.0", path = "../git-object" } bstr = { version = "0.2.13", default-features = false, features = ["std"] } byteorder = "1.2.3" filebuffer = "0.4.0" +serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] } thiserror = "1.0.20" [dev-dependencies] diff --git a/git-commitgraph/src/file/access.rs b/git-commitgraph/src/file/access.rs index 9feca7aa5da..b20f245f12c 100644 --- a/git-commitgraph/src/file/access.rs +++ b/git-commitgraph/src/file/access.rs @@ -8,6 +8,10 @@ use std::{ /// Access impl File { + pub fn base_graph_count(&self) -> u8 { + self.base_graph_count + } + /// Returns the commit data for the commit located at the given lexigraphical position. /// /// `pos` must range from 0 to self.num_commits(). diff --git a/git-commitgraph/src/file/commit.rs b/git-commitgraph/src/file/commit.rs index e0ae2f69430..0e0b5e28f1a 100644 --- a/git-commitgraph/src/file/commit.rs +++ b/git-commitgraph/src/file/commit.rs @@ -90,6 +90,10 @@ impl<'a> Commit<'a> { self.iter_parents().next().transpose() } + pub fn position(&self) -> file::Position { + self.pos + } + // Allow the return value to outlive this Commit object, as it only needs to be bound by the // lifetime of the parent file. pub fn root_tree_id<'b>(&'b self) -> borrowed::Id<'a> diff --git a/git-commitgraph/src/file/mod.rs b/git-commitgraph/src/file/mod.rs index 844e62349e5..afa4b866555 100644 --- a/git-commitgraph/src/file/mod.rs +++ b/git-commitgraph/src/file/mod.rs @@ -2,6 +2,8 @@ mod access; pub mod commit; mod init; +pub mod verify; + pub use init::Error; pub use commit::Commit; diff --git a/git-commitgraph/src/file/verify.rs b/git-commitgraph/src/file/verify.rs new file mode 100644 index 00000000000..427edcd0863 --- /dev/null +++ b/git-commitgraph/src/file/verify.rs @@ -0,0 +1,167 @@ +use crate::{ + file::{self, File}, + GENERATION_NUMBER_INFINITY, GENERATION_NUMBER_MAX, +}; +use bstr::ByteSlice; +use git_object::{borrowed, owned, SHA1_SIZE}; +use std::cmp::{max, min}; +use std::collections::HashMap; +use std::convert::TryFrom; +use std::path::Path; + +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error(transparent)] + Commit(#[from] file::commit::Error), + #[error("commit at file position {pos} has invalid ID {id}")] + CommitId { id: owned::Id, pos: file::Position }, + #[error("commit at file position {pos} with ID {id} is out of order relative to its predecessor with ID {predecessor_id}")] + CommitsOutOfOrder { + id: owned::Id, + pos: file::Position, + predecessor_id: owned::Id, + }, + #[error("commit-graph filename should be {0}")] + Filename(String), + #[error("commit {id} has invalid generation {generation}")] + Generation { generation: u32, id: owned::Id }, + #[error("checksum mismatch: expected {expected}, got {actual}")] + Mismatch { expected: owned::Id, actual: owned::Id }, + #[error("commit {id} has invalid root tree ID {root_tree_id}")] + RootTreeId { id: owned::Id, root_tree_id: owned::Id }, +} + +// This is a separate type to let `traverse`'s caller use the same error type for its result and its +// processor error type while also letting that error type contain file::verify::Error values. +// Is there a better way? Should the caller's error type just use boxes to avoid recursive type +// errors? +#[derive(thiserror::Error, Debug)] +pub enum EitherError { + #[error(transparent)] + Internal(#[from] E1), + // Why can't I use #[from] here? Boo! + #[error("{0}")] + Processor(#[source] E2), +} + +#[derive(Clone, Debug, Eq, PartialEq)] +#[cfg_attr(feature = "serde1", derive(serde::Deserialize, serde::Serialize))] +pub struct Outcome { + pub max_generation: u32, + pub max_parents: u32, + pub min_generation: u32, + pub num_commits: u32, + pub parent_counts: HashMap, +} + +impl File { + pub fn checksum(&self) -> borrowed::Id<'_> { + borrowed::Id::try_from(&self.data[self.data.len() - SHA1_SIZE..]).expect("file to be large enough for a hash") + } + + pub fn traverse<'a, E, Processor>(&'a self, mut processor: Processor) -> Result> + where + E: std::error::Error + 'static, + Processor: FnMut(&file::Commit<'a>) -> Result<(), E>, + { + self.verify_checksum()?; + verify_split_chain_filename_hash(&self.path, self.checksum())?; + + // This probably belongs in borrowed::Id itself? + let null_id = borrowed::Id::from(&[0u8; SHA1_SIZE]); + + let mut stats = Outcome { + max_generation: 0, + max_parents: 0, + min_generation: GENERATION_NUMBER_INFINITY, + num_commits: self.num_commits(), + parent_counts: HashMap::new(), + }; + + // TODO: Verify self.fan values as we go. + let mut prev_id: borrowed::Id<'a> = null_id; + for commit in self.iter_commits() { + if commit.id() <= prev_id { + if commit.id() == null_id { + return Err(Error::CommitId { + pos: commit.position(), + id: commit.id().into(), + } + .into()); + } + return Err(Error::CommitsOutOfOrder { + pos: commit.position(), + id: commit.id().into(), + predecessor_id: prev_id.into(), + } + .into()); + } + if commit.root_tree_id() == null_id { + return Err(Error::RootTreeId { + id: commit.id().into(), + root_tree_id: commit.root_tree_id().into(), + } + .into()); + } + if commit.generation() > GENERATION_NUMBER_MAX { + return Err(Error::Generation { + generation: commit.generation(), + id: commit.id().into(), + } + .into()); + } + + processor(&commit).map_err(EitherError::Processor)?; + + stats.max_generation = max(stats.max_generation, commit.generation()); + stats.min_generation = min(stats.min_generation, commit.generation()); + let parent_count = commit + .iter_parents() + .try_fold(0u32, |acc, pos| pos.map(|_| acc + 1)) + .map_err(Error::Commit)?; + *stats.parent_counts.entry(parent_count).or_insert(0) += 1; + prev_id = commit.id(); + } + + if stats.min_generation == GENERATION_NUMBER_INFINITY { + stats.min_generation = 0; + } + + Ok(stats) + } + + pub fn verify_checksum(&self) -> Result { + // TODO: Use/copy git_odb::hash::bytes_of_file. + let data_len_without_trailer = self.data.len() - SHA1_SIZE; + let mut hasher = git_features::hash::Sha1::default(); + hasher.update(&self.data[..data_len_without_trailer]); + let actual = owned::Id::new_sha1(hasher.digest()); + + let expected = self.checksum(); + if actual.to_borrowed() == expected { + Ok(actual) + } else { + Err(Error::Mismatch { + actual, + expected: expected.into(), + }) + } + } +} + +/// If the given path's filename matches "graph-{hash}.graph", check that `hash` matches the +/// expected hash. +fn verify_split_chain_filename_hash(path: impl AsRef, expected: borrowed::Id<'_>) -> Result<(), Error> { + let path = path.as_ref(); + path.file_name() + .and_then(|filename| filename.to_str()) + .and_then(|filename| filename.strip_suffix(".graph")) + .and_then(|stem| stem.strip_prefix("graph-")) + .map_or(Ok(()), |hex| match owned::Id::from_40_bytes_in_hex(hex.as_bytes()) { + Ok(actual) if actual.to_borrowed() == expected => Ok(()), + _ => Err(Error::Filename(format!( + "graph-{}.graph", + expected.to_sha1_hex().as_bstr() + ))), + }) +} diff --git a/git-commitgraph/src/graph/access.rs b/git-commitgraph/src/graph/access.rs index 4a3a7616c2f..7590499b532 100644 --- a/git-commitgraph/src/graph/access.rs +++ b/git-commitgraph/src/graph/access.rs @@ -42,7 +42,7 @@ impl Graph { /// Access fundamentals impl Graph { - fn lookup_by_id(&self, id: borrowed::Id<'_>) -> Option> { + pub(crate) fn lookup_by_id(&self, id: borrowed::Id<'_>) -> Option> { let mut current_file_start = 0; for file in &self.files { if let Some(lex_pos) = file.lookup(id) { @@ -57,14 +57,15 @@ impl Graph { None } - fn lookup_by_pos(&self, pos: graph::Position) -> LookupByPositionResult<'_> { + pub(crate) fn lookup_by_pos(&self, pos: graph::Position) -> LookupByPositionResult<'_> { let mut remaining = pos.0; - for file in &self.files { + for (file_index, file) in self.files.iter().enumerate() { match remaining.checked_sub(file.num_commits()) { Some(v) => remaining = v, None => { return LookupByPositionResult { file, + file_index, pos: file::Position(remaining), } } @@ -75,14 +76,15 @@ impl Graph { } #[derive(Clone)] -struct LookupByIdResult<'a> { +pub(crate) struct LookupByIdResult<'a> { pub file: &'a File, pub graph_pos: graph::Position, pub file_pos: file::Position, } #[derive(Clone)] -struct LookupByPositionResult<'a> { +pub(crate) struct LookupByPositionResult<'a> { pub file: &'a File, + pub file_index: usize, pub pos: file::Position, } diff --git a/git-commitgraph/src/graph/mod.rs b/git-commitgraph/src/graph/mod.rs index e113d7317fa..c022b52db42 100644 --- a/git-commitgraph/src/graph/mod.rs +++ b/git-commitgraph/src/graph/mod.rs @@ -1,6 +1,7 @@ //! Operations on a complete commit graph. mod access; mod init; +pub mod verify; use crate::file::File; use std::fmt; diff --git a/git-commitgraph/src/graph/verify.rs b/git-commitgraph/src/graph/verify.rs new file mode 100644 index 00000000000..1334ed43a82 --- /dev/null +++ b/git-commitgraph/src/graph/verify.rs @@ -0,0 +1,158 @@ +use crate::{ + file::{self, commit}, + graph, Graph, GENERATION_NUMBER_MAX, +}; +use git_object::owned; +use std::cmp::{max, min}; +use std::collections::BTreeMap; +use std::convert::TryInto; +use std::path::PathBuf; + +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error("'{}' should have {expected} base graphs, but claims {actual} base graphs", .path.display())] + BaseGraphCount { actual: u8, expected: u8, path: PathBuf }, + #[error("'{}' base graph at index {index} should have ID {expected} but is {actual}", .path.display())] + BaseGraphId { + actual: owned::Id, + expected: owned::Id, + index: u8, + path: PathBuf, + }, + #[error(transparent)] + Commit(#[from] commit::Error), + #[error("{}: {err}", .path.display())] + File { err: file::verify::Error, path: PathBuf }, + #[error("Commit {id}'s generation should be {expected} but is {actual}")] + Generation { actual: u32, expected: u32, id: owned::Id }, + #[error( + "Commit {id} has parent position {parent_pos} that is out of range (should be in range 0-{max_valid_pos})" + )] + ParentOutOfRange { + id: owned::Id, + max_valid_pos: graph::Position, + parent_pos: graph::Position, + }, + #[error("{0}")] + Processor(#[source] E), + #[error("Commit-graph should be composed of at most 256 files but actually contains {0} files")] + TooManyFiles(usize), +} + +#[derive(Clone, Debug, Eq, PartialEq)] +#[cfg_attr(feature = "serde1", derive(serde::Deserialize, serde::Serialize))] +pub struct Outcome { + pub longest_path_length: Option, + pub num_commits: u32, + pub parent_counts: BTreeMap, +} + +impl Graph { + pub fn verify_integrity( + &self, + mut processor: impl FnMut(&file::Commit<'_>) -> Result<(), E>, + ) -> Result> + where + E: std::error::Error + 'static, + { + if self.files.len() > 256 { + // A file in a split chain can only have up to 255 base files. + return Err(Error::TooManyFiles(self.files.len())); + } + + let mut stats = Outcome { + longest_path_length: None, + num_commits: 0, + parent_counts: BTreeMap::new(), + }; + let mut max_generation = 0u32; + + // TODO: Detect duplicate commit IDs across different files. Not sure how to do this without + // a separate loop, e.g. self.iter_sorted_ids(). + + let mut file_start_pos = graph::Position(0); + for (file_index, file) in self.files.iter().enumerate() { + if usize::from(file.base_graph_count()) != file_index { + return Err(Error::BaseGraphCount { + actual: file.base_graph_count(), + expected: file_index + .try_into() + .expect("files.len() check to protect against this"), + path: file.path().to_owned(), + }); + } + + for (base_graph_index, (expected, actual)) in self.files[..file_index] + .iter() + .map(|base_file| base_file.checksum()) + .zip(file.iter_base_graph_ids()) + .enumerate() + { + if actual != expected { + return Err(Error::BaseGraphId { + actual: actual.into(), + expected: expected.into(), + index: base_graph_index + .try_into() + .expect("files.len() check to protect against this"), + path: file.path().to_owned(), + }); + } + } + + let next_file_start_pos = graph::Position(file_start_pos.0 + file.num_commits()); + let file_stats = file + .traverse(|commit| { + let mut max_parent_generation = 0u32; + for parent_pos in commit.iter_parents() { + let parent_pos = parent_pos.map_err(Error::Commit)?; + if parent_pos >= next_file_start_pos { + return Err(Error::ParentOutOfRange { + parent_pos, + id: commit.id().into(), + max_valid_pos: graph::Position(next_file_start_pos.0 - 1), + }); + } + let parent = self.commit_at(parent_pos); + max_parent_generation = max(max_parent_generation, parent.generation()); + } + + // If the max parent generation is GENERATION_NUMBER_MAX, then this commit's + // generation should be GENERATION_NUMBER_MAX too. + let expected_generation = min(max_parent_generation + 1, GENERATION_NUMBER_MAX); + if commit.generation() != expected_generation { + return Err(Error::Generation { + actual: commit.generation(), + expected: expected_generation, + id: commit.id().into(), + }); + } + + processor(commit).map_err(Error::Processor)?; + + Ok(()) + }) + .map_err(|e| match e { + file::verify::EitherError::Internal(err) => Error::File { + err, + path: file.path().to_owned(), + }, + file::verify::EitherError::Processor(e) => e, + })?; + + max_generation = max(max_generation, file_stats.max_generation); + stats.num_commits += file_stats.num_commits; + for (key, value) in file_stats.parent_counts.into_iter() { + *stats.parent_counts.entry(key).or_insert(0) += value; + } + file_start_pos = next_file_start_pos; + } + + stats.longest_path_length = if max_generation < GENERATION_NUMBER_MAX { + Some(max_generation.saturating_sub(1)) + } else { + None + }; + Ok(stats) + } +} diff --git a/git-commitgraph/src/lib.rs b/git-commitgraph/src/lib.rs index 9295094e53e..72ea21330a7 100644 --- a/git-commitgraph/src/lib.rs +++ b/git-commitgraph/src/lib.rs @@ -6,5 +6,8 @@ pub mod graph; pub use graph::Graph; +pub const GENERATION_NUMBER_INFINITY: u32 = 0xffff_ffff; +pub const GENERATION_NUMBER_MAX: u32 = 0x3fff_ffff; + /// The maximum number of commits that can be stored in a commit graph. pub const MAX_COMMITS: u32 = (1 << 30) + (1 << 29) + (1 << 28) - 1; diff --git a/gitoxide-core/Cargo.toml b/gitoxide-core/Cargo.toml index 850b4ccff05..b708d256aae 100644 --- a/gitoxide-core/Cargo.toml +++ b/gitoxide-core/Cargo.toml @@ -12,7 +12,7 @@ doctest = false test = false [features] -serde1 = ["git-object/serde1", "git-odb/serde1", "git-protocol/serde1", "serde_json", "serde"] +serde1 = ["git-commitgraph/serde1", "git-object/serde1", "git-odb/serde1", "git-protocol/serde1", "serde_json", "serde"] [package.metadata.docs.rs] all-features = true diff --git a/gitoxide-core/src/commitgraph/verify.rs b/gitoxide-core/src/commitgraph/verify.rs index 8bb2df8ecd7..77af8f3fbf3 100644 --- a/gitoxide-core/src/commitgraph/verify.rs +++ b/gitoxide-core/src/commitgraph/verify.rs @@ -1,20 +1,23 @@ +use crate::OutputFormat; use anyhow::{Context as AnyhowContext, Result}; -use git_commitgraph::Graph; +use git_commitgraph::{graph::verify::Outcome, Graph}; use std::{io, path::Path}; /// A general purpose context for many operations provided here pub struct Context { + /// A stream to which to output errors + pub err: W2, /// A stream to which to output operation results pub out: W1, - /// A stream to which to errors - pub err: W2, + pub output_statistics: Option, } impl Default for Context, Vec> { fn default() -> Self { Context { - out: Vec::new(), err: Vec::new(), + out: Vec::new(), + output_statistics: None, } } } @@ -22,16 +25,49 @@ impl Default for Context, Vec> { pub fn graph_or_file( path: impl AsRef, Context { - out: mut _out, err: mut _err, + mut out, + output_statistics, }: Context, -) -> Result<()> +) -> Result where W1: io::Write, W2: io::Write, { - // TODO: Handle `path` being objects/info, objects/info/commit-graph, - // or objects/info/commit-graphs/graph-xyz.graph. - let _g = Graph::at(path).with_context(|| "Could not open commit graph")?; - Err(anyhow::Error::msg("not implemented")) + let g = Graph::at(path).with_context(|| "Could not open commit graph")?; + + fn dummy_processor(_commit: &git_commitgraph::file::Commit<'_>) -> std::result::Result<(), std::fmt::Error> { + Ok(()) + } + let stats = g + .verify_integrity(dummy_processor) + .with_context(|| "Verification failure")?; + + match output_statistics { + Some(OutputFormat::Human) => drop(print_statistics(&mut out, &stats)), + #[cfg(feature = "serde1")] + Some(OutputFormat::Json) => serde_json::to_writer_pretty(out, &stats)?, + _ => {} + } + + Ok(stats) +} + +fn print_statistics(out: &mut impl io::Write, stats: &Outcome) -> io::Result<()> { + writeln!(out, "number of commits with the given number of parents")?; + let mut parent_counts: Vec<_> = stats.parent_counts.iter().map(|(a, b)| (*a, *b)).collect(); + parent_counts.sort_by_key(|e| e.0); + for (parent_count, commit_count) in parent_counts.into_iter() { + writeln!(out, "\t{:>2}: {}", parent_count, commit_count)?; + } + writeln!(out, "\t->: {}", stats.num_commits)?; + + write!(out, "\nlongest path length between two commits: ")?; + if let Some(n) = stats.longest_path_length { + writeln!(out, "{}", n)?; + } else { + writeln!(out, "unknown")?; + } + + Ok(()) } diff --git a/src/plumbing/lean/main.rs b/src/plumbing/lean/main.rs index 13c894243e1..57254c38db4 100644 --- a/src/plumbing/lean/main.rs +++ b/src/plumbing/lean/main.rs @@ -140,14 +140,19 @@ pub fn main() -> Result<()> { ) .map(|_| ()) } - SubCommands::CommitGraphVerify(CommitGraphVerify { path }) => { + SubCommands::CommitGraphVerify(CommitGraphVerify { path, statistics }) => { use self::core::commitgraph::verify; verify::graph_or_file( path, verify::Context { - out: stdout(), err: stderr(), + out: stdout(), + output_statistics: if statistics { + Some(core::OutputFormat::Human) + } else { + None + }, }, ) .map(|_| ()) diff --git a/src/plumbing/lean/options.rs b/src/plumbing/lean/options.rs index c8bfb93ead0..8f60f8fdae9 100644 --- a/src/plumbing/lean/options.rs +++ b/src/plumbing/lean/options.rs @@ -196,4 +196,8 @@ pub struct PackVerify { pub struct CommitGraphVerify { #[argh(positional)] pub path: PathBuf, + + /// output statistical information about the pack + #[argh(switch, short = 's')] + pub statistics: bool, } diff --git a/src/plumbing/pretty/main.rs b/src/plumbing/pretty/main.rs index 1d8bd009353..e19a50bcc9a 100644 --- a/src/plumbing/pretty/main.rs +++ b/src/plumbing/pretty/main.rs @@ -256,14 +256,22 @@ pub fn main() -> Result<()> { }, ) .map(|_| ()), - Subcommands::CommitGraphVerify { path } => prepare_and_run( + Subcommands::CommitGraphVerify { path, statistics } => prepare_and_run( "commit-graph-verify", verbose, progress, progress_keep_open, None, move |_progress, out, err| { - core::commitgraph::verify::graph_or_file(path, core::commitgraph::verify::Context { out, err }) + let output_statistics = if statistics { Some(format) } else { None }; + core::commitgraph::verify::graph_or_file( + path, + core::commitgraph::verify::Context { + err, + out, + output_statistics, + }, + ) }, ) .map(|_| ()), diff --git a/src/plumbing/pretty/options.rs b/src/plumbing/pretty/options.rs index 91c4ecf16ad..e632b62b8fd 100644 --- a/src/plumbing/pretty/options.rs +++ b/src/plumbing/pretty/options.rs @@ -193,5 +193,8 @@ pub enum Subcommands { /// The 'objects/info/' dir, 'objects/info/commit-graphs' dir, or 'objects/info/commit-graph' file to validate. #[clap(parse(from_os_str))] path: PathBuf, + /// output statistical information about the pack + #[clap(long, short = 's')] + statistics: bool, }, } diff --git a/tests/snapshots/plumbing/commit-graph-verify/statistics-json-success b/tests/snapshots/plumbing/commit-graph-verify/statistics-json-success new file mode 100644 index 00000000000..301cf39a105 --- /dev/null +++ b/tests/snapshots/plumbing/commit-graph-verify/statistics-json-success @@ -0,0 +1,8 @@ +{ + "longest_path_length": 2, + "num_commits": 3, + "parent_counts": { + "0": 1, + "1": 2 + } +} \ No newline at end of file diff --git a/tests/snapshots/plumbing/commit-graph-verify/statistics-success b/tests/snapshots/plumbing/commit-graph-verify/statistics-success new file mode 100644 index 00000000000..b0bf9808c4a --- /dev/null +++ b/tests/snapshots/plumbing/commit-graph-verify/statistics-success @@ -0,0 +1,6 @@ +number of commits with the given number of parents + 0: 1 + 1: 2 + ->: 3 + +longest path length between two commits: 2 \ No newline at end of file diff --git a/tests/stateless-journey.sh b/tests/stateless-journey.sh index dddec419ea6..843d086739b 100755 --- a/tests/stateless-journey.sh +++ b/tests/stateless-journey.sh @@ -476,3 +476,25 @@ snapshot="$snapshot/plumbing" ) ) ) +(when "running 'commit-graph-verify'" + snapshot="$snapshot/commit-graph-verify" + (small-repo-in-sandbox + (with "a valid and complete commit-graph file" + git commit-graph write --reachable + (with "statistics" + it "generates the correct output" && { + WITH_SNAPSHOT="$snapshot/statistics-success" \ + expect_run $SUCCESSFULLY "$exe_plumbing" commit-graph-verify -s .git/objects/info + } + ) + if test "$kind" = "max"; then + (with "statistics --format json" + it "generates the correct output" && { + WITH_SNAPSHOT="$snapshot/statistics-json-success" \ + expect_run $SUCCESSFULLY "$exe_plumbing" --format json commit-graph-verify -s .git/objects/info + } + ) + fi + ) + ) +) From fa22cab259338dc140dd660f4f4b9bbc9d6cc3d0 Mon Sep 17 00:00:00 2001 From: Conor Davis Date: Sun, 11 Oct 2020 15:31:10 -0500 Subject: [PATCH 9/9] [commitgraph] Clean up `{file,graph}::verify::Error` types. Get rid of `file::verify::EitherError` in favor of having `graph::verify::Error` manually copy file errors into the graph error object. So: 1. `graph::verify::Error`'s `File` variant uses a dummy type parameter for its nested `file::verify::Error` value. This dummy type parameter is essentially a never type, as `graph::verify::Error::File(file::verify::Error::Processor(...))` is not valid. 2. `Graph::verify_integrity` calls `File::traverse` with its normal error type (`graph::verify::Error`) also serving as the processor's error type. 3. `Graph::traverse` propagates processor errors to its caller via `Error::Processor(err)`. 4. `Graph::verify_integrity` manually transcribes `file::verify::Error` variants into `file::verify::Error` variants before embedding the file error into `graph::verify::Error::File` variants. --- git-commitgraph/src/file/verify.rs | 54 +++++++++-------------------- git-commitgraph/src/graph/verify.rs | 42 ++++++++++++++++++---- git-commitgraph/src/lib.rs | 12 +++++++ 3 files changed, 64 insertions(+), 44 deletions(-) diff --git a/git-commitgraph/src/file/verify.rs b/git-commitgraph/src/file/verify.rs index 427edcd0863..d8878007ebf 100644 --- a/git-commitgraph/src/file/verify.rs +++ b/git-commitgraph/src/file/verify.rs @@ -10,7 +10,7 @@ use std::convert::TryFrom; use std::path::Path; #[derive(thiserror::Error, Debug)] -pub enum Error { +pub enum Error { #[error(transparent)] Commit(#[from] file::commit::Error), #[error("commit at file position {pos} has invalid ID {id}")] @@ -26,24 +26,13 @@ pub enum Error { #[error("commit {id} has invalid generation {generation}")] Generation { generation: u32, id: owned::Id }, #[error("checksum mismatch: expected {expected}, got {actual}")] - Mismatch { expected: owned::Id, actual: owned::Id }, + Mismatch { actual: owned::Id, expected: owned::Id }, + #[error("{0}")] + Processor(#[source] E), #[error("commit {id} has invalid root tree ID {root_tree_id}")] RootTreeId { id: owned::Id, root_tree_id: owned::Id }, } -// This is a separate type to let `traverse`'s caller use the same error type for its result and its -// processor error type while also letting that error type contain file::verify::Error values. -// Is there a better way? Should the caller's error type just use boxes to avoid recursive type -// errors? -#[derive(thiserror::Error, Debug)] -pub enum EitherError { - #[error(transparent)] - Internal(#[from] E1), - // Why can't I use #[from] here? Boo! - #[error("{0}")] - Processor(#[source] E2), -} - #[derive(Clone, Debug, Eq, PartialEq)] #[cfg_attr(feature = "serde1", derive(serde::Deserialize, serde::Serialize))] pub struct Outcome { @@ -59,13 +48,14 @@ impl File { borrowed::Id::try_from(&self.data[self.data.len() - SHA1_SIZE..]).expect("file to be large enough for a hash") } - pub fn traverse<'a, E, Processor>(&'a self, mut processor: Processor) -> Result> + pub fn traverse<'a, E, Processor>(&'a self, mut processor: Processor) -> Result> where E: std::error::Error + 'static, Processor: FnMut(&file::Commit<'a>) -> Result<(), E>, { - self.verify_checksum()?; - verify_split_chain_filename_hash(&self.path, self.checksum())?; + self.verify_checksum() + .map_err(|(actual, expected)| Error::Mismatch { actual, expected })?; + verify_split_chain_filename_hash(&self.path, self.checksum()).map_err(Error::Filename)?; // This probably belongs in borrowed::Id itself? let null_id = borrowed::Id::from(&[0u8; SHA1_SIZE]); @@ -86,32 +76,28 @@ impl File { return Err(Error::CommitId { pos: commit.position(), id: commit.id().into(), - } - .into()); + }); } return Err(Error::CommitsOutOfOrder { pos: commit.position(), id: commit.id().into(), predecessor_id: prev_id.into(), - } - .into()); + }); } if commit.root_tree_id() == null_id { return Err(Error::RootTreeId { id: commit.id().into(), root_tree_id: commit.root_tree_id().into(), - } - .into()); + }); } if commit.generation() > GENERATION_NUMBER_MAX { return Err(Error::Generation { generation: commit.generation(), id: commit.id().into(), - } - .into()); + }); } - processor(&commit).map_err(EitherError::Processor)?; + processor(&commit).map_err(Error::Processor)?; stats.max_generation = max(stats.max_generation, commit.generation()); stats.min_generation = min(stats.min_generation, commit.generation()); @@ -130,7 +116,7 @@ impl File { Ok(stats) } - pub fn verify_checksum(&self) -> Result { + pub fn verify_checksum(&self) -> Result { // TODO: Use/copy git_odb::hash::bytes_of_file. let data_len_without_trailer = self.data.len() - SHA1_SIZE; let mut hasher = git_features::hash::Sha1::default(); @@ -141,17 +127,14 @@ impl File { if actual.to_borrowed() == expected { Ok(actual) } else { - Err(Error::Mismatch { - actual, - expected: expected.into(), - }) + Err((actual, expected.into())) } } } /// If the given path's filename matches "graph-{hash}.graph", check that `hash` matches the /// expected hash. -fn verify_split_chain_filename_hash(path: impl AsRef, expected: borrowed::Id<'_>) -> Result<(), Error> { +fn verify_split_chain_filename_hash(path: impl AsRef, expected: borrowed::Id<'_>) -> Result<(), String> { let path = path.as_ref(); path.file_name() .and_then(|filename| filename.to_str()) @@ -159,9 +142,6 @@ fn verify_split_chain_filename_hash(path: impl AsRef, expected: borrowed:: .and_then(|stem| stem.strip_prefix("graph-")) .map_or(Ok(()), |hex| match owned::Id::from_40_bytes_in_hex(hex.as_bytes()) { Ok(actual) if actual.to_borrowed() == expected => Ok(()), - _ => Err(Error::Filename(format!( - "graph-{}.graph", - expected.to_sha1_hex().as_bstr() - ))), + _ => Err(format!("graph-{}.graph", expected.to_sha1_hex().as_bstr())), }) } diff --git a/git-commitgraph/src/graph/verify.rs b/git-commitgraph/src/graph/verify.rs index 1334ed43a82..87054b71e7a 100644 --- a/git-commitgraph/src/graph/verify.rs +++ b/git-commitgraph/src/graph/verify.rs @@ -1,6 +1,6 @@ use crate::{ file::{self, commit}, - graph, Graph, GENERATION_NUMBER_MAX, + graph, Graph, ImpossibleVariantError, GENERATION_NUMBER_MAX, }; use git_object::owned; use std::cmp::{max, min}; @@ -22,7 +22,15 @@ pub enum Error { #[error(transparent)] Commit(#[from] commit::Error), #[error("{}: {err}", .path.display())] - File { err: file::verify::Error, path: PathBuf }, + File { + // Use zero-size error type. We will never return + // `graph::verify::Error::File(file::verify::Error::Processor(...))`, because we are the + // file's processor, and we convert`file::verify::Error::Processor` + // variants into direct `graph::verify::Error` values. + // TODO: Use never type when it becomes available. + err: file::verify::Error, + path: PathBuf, + }, #[error("Commit {id}'s generation should be {expected} but is {actual}")] Generation { actual: u32, expected: u32, id: owned::Id }, #[error( @@ -132,12 +140,32 @@ impl Graph { Ok(()) }) - .map_err(|e| match e { - file::verify::EitherError::Internal(err) => Error::File { - err, - path: file.path().to_owned(), + .map_err(|err| Error::File { + err: match err { + file::verify::Error::Processor(e) => return e, + file::verify::Error::RootTreeId { id, root_tree_id } => { + file::verify::Error::RootTreeId { id, root_tree_id } + } + file::verify::Error::Mismatch { actual, expected } => { + file::verify::Error::Mismatch { actual, expected } + } + file::verify::Error::Generation { generation, id } => { + file::verify::Error::Generation { generation, id } + } + file::verify::Error::Filename(expected) => file::verify::Error::Filename(expected), + file::verify::Error::Commit(err) => file::verify::Error::Commit(err), + file::verify::Error::CommitId { id, pos } => file::verify::Error::CommitId { id, pos }, + file::verify::Error::CommitsOutOfOrder { + id, + pos, + predecessor_id, + } => file::verify::Error::CommitsOutOfOrder { + id, + pos, + predecessor_id, + }, }, - file::verify::EitherError::Processor(e) => e, + path: file.path().to_owned(), })?; max_generation = max(max_generation, file_stats.max_generation); diff --git a/git-commitgraph/src/lib.rs b/git-commitgraph/src/lib.rs index 72ea21330a7..9b43b441129 100644 --- a/git-commitgraph/src/lib.rs +++ b/git-commitgraph/src/lib.rs @@ -11,3 +11,15 @@ pub const GENERATION_NUMBER_MAX: u32 = 0x3fff_ffff; /// The maximum number of commits that can be stored in a commit graph. pub const MAX_COMMITS: u32 = (1 << 30) + (1 << 29) + (1 << 28) - 1; + +// TODO: pub type ImpossibleVariantError = !; +#[derive(Debug)] +pub struct ImpossibleVariantError; + +impl std::error::Error for ImpossibleVariantError {} + +impl std::fmt::Display for ImpossibleVariantError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("this enum was constructed with an invalid variant") + } +}