diff --git a/gitoxide-core/src/repository/revision/resolve.rs b/gitoxide-core/src/repository/revision/resolve.rs index db00e40e9ca..531c6b5fda2 100644 --- a/gitoxide-core/src/repository/revision/resolve.rs +++ b/gitoxide-core/src/repository/revision/resolve.rs @@ -4,6 +4,12 @@ pub struct Options { pub format: OutputFormat, pub explain: bool, pub cat_file: bool, + pub tree_mode: TreeMode, +} + +pub enum TreeMode { + Raw, + Pretty, } pub(crate) mod function { @@ -13,6 +19,7 @@ pub(crate) mod function { use gix::revision::Spec; use super::Options; + use crate::repository::revision::resolve::TreeMode; use crate::{repository::revision, OutputFormat}; pub fn resolve( @@ -23,6 +30,7 @@ pub(crate) mod function { format, explain, cat_file, + tree_mode, }: Options, ) -> anyhow::Result<()> { repo.object_cache_size_if_unset(1024 * 1024); @@ -36,7 +44,7 @@ pub(crate) mod function { let spec = gix::path::os_str_into_bstr(&spec)?; let spec = repo.rev_parse(spec)?; if cat_file { - return display_object(spec, out); + return display_object(spec, tree_mode, out); } writeln!(out, "{spec}", spec = spec.detach())?; } @@ -63,11 +71,11 @@ pub(crate) mod function { Ok(()) } - fn display_object(spec: Spec<'_>, mut out: impl std::io::Write) -> anyhow::Result<()> { + fn display_object(spec: Spec<'_>, tree_mode: TreeMode, mut out: impl std::io::Write) -> anyhow::Result<()> { let id = spec.single().context("rev-spec must resolve to a single object")?; let object = id.object()?; match object.kind { - gix::object::Kind::Tree => { + gix::object::Kind::Tree if matches!(tree_mode, TreeMode::Pretty) => { for entry in object.into_tree().iter() { writeln!(out, "{}", entry?)?; } diff --git a/gix-object/Cargo.toml b/gix-object/Cargo.toml index fef2f6a5d8b..62ff1504cdb 100644 --- a/gix-object/Cargo.toml +++ b/gix-object/Cargo.toml @@ -25,7 +25,7 @@ serde = ["dep:serde", "bstr/serde", "smallvec/serde", "gix-hash/serde", "gix-act ## details information about the error location will be collected. ## Use it in applications which expect broken or invalid objects or for debugging purposes. Incorrectly formatted objects aren't at all ## common otherwise. -verbose-object-parsing-errors = [] +verbose-object-parsing-errors = ["winnow/std"] [dependencies] gix-features = { version = "^0.36.0", path = "../gix-features", features = ["rustsha1", "progress"] } diff --git a/gix-object/benches/decode_objects.rs b/gix-object/benches/decode_objects.rs index 947d4ffa383..4cd05f3a249 100644 --- a/gix-object/benches/decode_objects.rs +++ b/gix-object/benches/decode_objects.rs @@ -19,10 +19,10 @@ fn parse_tag(c: &mut Criterion) { } fn parse_tree(c: &mut Criterion) { - c.bench_function("TreeRef(sig)", |b| { + c.bench_function("TreeRef()", |b| { b.iter(|| black_box(gix_object::TreeRef::from_bytes(TREE)).unwrap()) }); - c.bench_function("TreeRefIter(sig)", |b| { + c.bench_function("TreeRefIter()", |b| { b.iter(|| black_box(gix_object::TreeRefIter::from_bytes(TREE).count())) }); } diff --git a/gix-object/src/lib.rs b/gix-object/src/lib.rs index 845ded59634..ceb74a2e7cd 100644 --- a/gix-object/src/lib.rs +++ b/gix-object/src/lib.rs @@ -290,6 +290,12 @@ pub mod decode { self.inner.fmt(f) } } + + impl std::error::Error for Error { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + self.inner.cause().map(|v| v as &(dyn std::error::Error + 'static)) + } + } } /// @@ -318,14 +324,15 @@ pub mod decode { } impl std::fmt::Display for Error { - fn fmt(&self, _f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - Ok(()) + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("object parsing failed") } } + + impl std::error::Error for Error {} } pub(crate) use _decode::empty_error; pub use _decode::{Error, ParseError}; - impl std::error::Error for Error {} /// Returned by [`loose_header()`] #[derive(Debug, thiserror::Error)] diff --git a/gix-object/src/tree/ref_iter.rs b/gix-object/src/tree/ref_iter.rs index be1244910ef..c18a0d5f532 100644 --- a/gix-object/src/tree/ref_iter.rs +++ b/gix-object/src/tree/ref_iter.rs @@ -67,11 +67,11 @@ impl<'a> Iterator for TreeRefIter<'a> { Some(Ok(entry)) } None => { + let failing = self.data; self.data = &[]; - let empty = &[] as &[u8]; #[allow(clippy::unit_arg)] Some(Err(crate::decode::Error::with_err( - winnow::error::ErrMode::from_error_kind(&empty, winnow::error::ErrorKind::Verify), + winnow::error::ErrMode::from_error_kind(&failing, winnow::error::ErrorKind::Verify), ))) } } @@ -116,17 +116,9 @@ mod decode { use std::convert::TryFrom; use bstr::ByteSlice; - use winnow::{ - combinator::{eof, repeat, terminated}, - error::ParserError, - prelude::*, - stream::AsChar, - token::{take, take_while}, - }; + use winnow::{error::ParserError, prelude::*}; - use crate::{parse::SPACE, tree, tree::EntryRef, TreeRef}; - - const NULL: &[u8] = b"\0"; + use crate::{tree, tree::EntryRef, TreeRef}; pub fn fast_entry(i: &[u8]) -> Option<(&[u8], EntryRef<'_>)> { let mut mode = 0u32; @@ -157,24 +149,20 @@ mod decode { )) } - pub fn entry<'a, E: ParserError<&'a [u8]>>(i: &mut &'a [u8]) -> PResult, E> { - ( - terminated(take_while(5..=6, AsChar::is_dec_digit), SPACE) - .verify_map(|mode| tree::EntryMode::try_from(mode).ok()), - terminated(take_while(1.., |b| b != NULL[0]), NULL), - take(20u8), // TODO(SHA256): make this compatible with other hash lengths - ) - .map(|(mode, filename, oid): (_, &[u8], _)| EntryRef { - mode, - filename: filename.as_bstr(), - oid: gix_hash::oid::try_from_bytes(oid).expect("we counted exactly 20 bytes"), - }) - .parse_next(i) - } - pub fn tree<'a, E: ParserError<&'a [u8]>>(i: &mut &'a [u8]) -> PResult, E> { - terminated(repeat(0.., entry), eof) - .map(|entries| TreeRef { entries }) - .parse_next(i) + let mut out = Vec::new(); + let mut i = &**i; + while !i.is_empty() { + let Some((rest, entry)) = fast_entry(i) else { + #[allow(clippy::unit_arg)] + return Err(winnow::error::ErrMode::from_error_kind( + &i, + winnow::error::ErrorKind::Verify, + )); + }; + i = rest; + out.push(entry); + } + Ok(TreeRef { entries: out }) } } diff --git a/gix-object/tests/commit/mod.rs b/gix-object/tests/commit/mod.rs index 7a503b48231..84cdd8b2be4 100644 --- a/gix-object/tests/commit/mod.rs +++ b/gix-object/tests/commit/mod.rs @@ -1,3 +1,6 @@ +use crate::fixture_name; +use gix_object::{CommitRef, CommitRefIter}; + const SIGNATURE: & [u8; 487] = b"-----BEGIN PGP SIGNATURE-----\n\niQEzBAABCAAdFiEEdjYp/sh4j8NRKLX27gKdHl60AwAFAl7q2DsACgkQ7gKdHl60\nAwDvewgAkL5UjEztzeVXlzceom0uCrAkCw9wSGLTmYcMKW3JwEaTRgQ4FX+sDuFT\nLZ8DoPu3UHUP0QnKrUwHulTTlKcOAvsczHbVPIKtXCxo6QpUfhsJQwz/J29kiE4L\nsOd+lqKGnn4oati/de2xwqNGi081fO5KILX75z6KfsAe7Qz7R3jxRF4uzHI033O+\nJc2Y827XeaELxW40SmzoLanWgEcdreXf3PstXEWW77CAu0ozXmvYj56vTviVybxx\nG7bc8lwc+SSKVe2VVB+CCfVbs0i541gmghUpZfMhUgaqttcCH8ysrUJDhne1BLG8\nCrOJIWTwAeEDtomV1p76qrMeqr1GFg==\n=qlSN\n-----END PGP SIGNATURE-----"; const LONG_MESSAGE: &str = "Merge tag 'thermal-v5.8-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux @@ -159,6 +162,27 @@ mod method { } } +#[test] +fn invalid() { + let fixture = fixture_name("commit", "unsigned.txt"); + let partial_commit = &fixture[..fixture.len() / 2]; + assert_eq!( + CommitRef::from_bytes(partial_commit).unwrap_err().to_string(), + if cfg!(feature = "verbose-object-parsing-errors") { + "expected ``, ` <> <+|->`, `author `" + } else { + "object parsing failed" + } + ); + assert_eq!( + CommitRefIter::from_bytes(partial_commit) + .take_while(Result::is_ok) + .count(), + 1, + "we can decode some fields before failing" + ); +} + mod from_bytes; mod iter; mod message; diff --git a/gix-object/tests/fixtures/tree/special-1.tree b/gix-object/tests/fixtures/tree/special-1.tree new file mode 100644 index 00000000000..3d6ceeb55d6 Binary files /dev/null and b/gix-object/tests/fixtures/tree/special-1.tree differ diff --git a/gix-object/tests/fixtures/tree/special-2.tree b/gix-object/tests/fixtures/tree/special-2.tree new file mode 100644 index 00000000000..f0760808013 Binary files /dev/null and b/gix-object/tests/fixtures/tree/special-2.tree differ diff --git a/gix-object/tests/fixtures/tree/special-3.tree b/gix-object/tests/fixtures/tree/special-3.tree new file mode 100644 index 00000000000..13cce9a504a Binary files /dev/null and b/gix-object/tests/fixtures/tree/special-3.tree differ diff --git a/gix-object/tests/fixtures/tree/special-4.tree b/gix-object/tests/fixtures/tree/special-4.tree new file mode 100644 index 00000000000..6e20f30ef0d Binary files /dev/null and b/gix-object/tests/fixtures/tree/special-4.tree differ diff --git a/gix-object/tests/tag/mod.rs b/gix-object/tests/tag/mod.rs index 3da603c5f6b..53be4987f43 100644 --- a/gix-object/tests/tag/mod.rs +++ b/gix-object/tests/tag/mod.rs @@ -1,5 +1,6 @@ +use crate::fixture_name; use gix_date::time::Sign; -use gix_object::{bstr::ByteSlice, Kind, TagRef}; +use gix_object::{bstr::ByteSlice, Kind, TagRef, TagRefIter}; mod method { use gix_object::TagRef; @@ -115,6 +116,25 @@ KLMHist5yj0sw1E4hDTyQa0= } } +#[test] +fn invalid() { + let fixture = fixture_name("tag", "whitespace.txt"); + let partial_tag = &fixture[..fixture.len() / 2]; + assert_eq!( + TagRef::from_bytes(partial_tag).unwrap_err().to_string(), + if cfg!(feature = "verbose-object-parsing-errors") { + "" + } else { + "object parsing failed" + } + ); + assert_eq!( + TagRefIter::from_bytes(partial_tag).take_while(Result::is_ok).count(), + 4, + "we can decode some fields before failing" + ); +} + mod from_bytes { use gix_object::{bstr::ByteSlice, Kind, TagRef}; diff --git a/gix-object/tests/tree/mod.rs b/gix-object/tests/tree/mod.rs index 0fdc03e7b4a..27a6352a36a 100644 --- a/gix-object/tests/tree/mod.rs +++ b/gix-object/tests/tree/mod.rs @@ -56,7 +56,7 @@ mod iter { } mod from_bytes { - use gix_object::{bstr::ByteSlice, tree, tree::EntryRef, TreeRef}; + use gix_object::{bstr::ByteSlice, tree, tree::EntryRef, TreeRef, TreeRefIter}; use crate::{fixture_name, hex_to_id}; @@ -108,24 +108,46 @@ mod from_bytes { } #[test] - fn maybe_special() -> crate::Result { + fn invalid() { + let fixture = fixture_name("tree", "definitely-special.tree"); + let partial_tree = &fixture[..fixture.len() / 2]; assert_eq!( - TreeRef::from_bytes(&fixture_name("tree", "maybe-special.tree"))? - .entries - .len(), - 160 + TreeRef::from_bytes(partial_tree).unwrap_err().to_string(), + if cfg!(feature = "verbose-object-parsing-errors") { + "" + } else { + "object parsing failed" + } + ); + assert_eq!( + TreeRefIter::from_bytes(partial_tree).take_while(Result::is_ok).count(), + 9, + "we can decode about half of it before failing" ); - Ok(()) } #[test] - fn definitely_special() -> crate::Result { - assert_eq!( - TreeRef::from_bytes(&fixture_name("tree", "definitely-special.tree"))? - .entries - .len(), - 19 - ); + fn special_trees() -> crate::Result { + for (name, expected_entry_count) in [ + ("maybe-special", 160), + ("definitely-special", 19), + ("special-1", 5), + ("special-2", 18), + ("special-3", 5), + ("special-4", 18), + ] { + let fixture = fixture_name("tree", &format!("{name}.tree")); + assert_eq!( + TreeRef::from_bytes(&fixture)?.entries.len(), + expected_entry_count, + "{name}" + ); + assert_eq!( + TreeRefIter::from_bytes(&fixture).map(Result::unwrap).count(), + expected_entry_count, + "{name}" + ); + } Ok(()) } } diff --git a/src/plumbing/main.rs b/src/plumbing/main.rs index f36d0494b26..9102b3fb064 100644 --- a/src/plumbing/main.rs +++ b/src/plumbing/main.rs @@ -925,6 +925,7 @@ pub fn main() -> Result<()> { specs, explain, cat_file, + tree_mode, } => prepare_and_run( "revision-parse", trace, @@ -941,6 +942,12 @@ pub fn main() -> Result<()> { format, explain, cat_file, + tree_mode: match tree_mode.unwrap_or_default() { + revision::resolve::TreeMode::Raw => core::repository::revision::resolve::TreeMode::Raw, + revision::resolve::TreeMode::Pretty => { + core::repository::revision::resolve::TreeMode::Pretty + } + }, }, ) }, diff --git a/src/plumbing/options/mod.rs b/src/plumbing/options/mod.rs index f7b484aaf6f..e91f90c07fd 100644 --- a/src/plumbing/options/mod.rs +++ b/src/plumbing/options/mod.rs @@ -595,6 +595,16 @@ pub mod commitgraph { } pub mod revision { + pub mod resolve { + #[derive(Default, Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, clap::ValueEnum)] + pub enum TreeMode { + /// Show the raw bytes - only useful for piping into files for use with tooling. + Raw, + /// Display a tree in human-readable form. + #[default] + Pretty, + } + } #[derive(Debug, clap::Subcommand)] #[clap(visible_alias = "rev", visible_alias = "r")] pub enum Subcommands { @@ -625,6 +635,8 @@ pub mod revision { /// Show the first resulting object similar to how `git cat-file` would, but don't show the resolved spec. #[clap(short = 'c', long, conflicts_with = "explain")] cat_file: bool, + #[clap(short = 't', long)] + tree_mode: Option, /// rev-specs like `@`, `@~1` or `HEAD^2`. #[clap(required = true)] specs: Vec,