diff --git a/examples/log.rs b/examples/log.rs index 293d358529e..651bb9b4e57 100644 --- a/examples/log.rs +++ b/examples/log.rs @@ -142,15 +142,16 @@ fn run(args: Args) -> anyhow::Result<()> { let info = info?; let commit = info.object()?; let commit_ref = commit.decode()?; + let author = commit_ref.author()?; Ok(LogEntryInfo { commit_id: commit.id().to_hex().to_string(), parents: info.parent_ids().map(|id| id.shorten_or_id().to_string()).collect(), author: { let mut buf = Vec::new(); - commit_ref.author.actor().write_to(&mut buf)?; + author.actor().write_to(&mut buf)?; buf.into() }, - time: commit_ref.author.time()?.format_or_unix(format::DEFAULT), + time: author.time()?.format_or_unix(format::DEFAULT), message: commit_ref.message.to_owned(), }) }), diff --git a/gix-merge/src/commit/virtual_merge_base.rs b/gix-merge/src/commit/virtual_merge_base.rs index d452a1c0fcd..53e4bde81fc 100644 --- a/gix-merge/src/commit/virtual_merge_base.rs +++ b/gix-merge/src/commit/virtual_merge_base.rs @@ -19,6 +19,8 @@ pub enum Error { MergeTree(#[from] crate::tree::Error), #[error("Failed to write tree for merged merge-base or virtual commit")] WriteObject(gix_object::write::Error), + #[error("Failed to decode a commit needed to build a virtual merge-base")] + DecodeCommit(#[from] gix_object::decode::Error), #[error( "Conflicts occurred when trying to resolve multiple merge-bases by merging them. This is most certainly a bug." )] @@ -28,6 +30,8 @@ pub enum Error { } pub(super) mod function { + use std::convert::TryFrom; + use gix_object::FindExt; use super::Error; @@ -128,7 +132,8 @@ pub(super) mod function { tree_id: gix_hash::ObjectId, ) -> Result { let mut buf = Vec::new(); - let mut commit: gix_object::Commit = objects.find_commit(&parent_a, &mut buf)?.into(); + let commit_ref = objects.find_commit(&parent_a, &mut buf)?; + let mut commit = gix_object::Commit::try_from(commit_ref)?; commit.parents = vec![parent_a, parent_b].into(); commit.tree = tree_id; objects.write(&commit).map_err(Error::WriteObject) diff --git a/gix-object/src/commit/decode.rs b/gix-object/src/commit/decode.rs index a3efcc0596d..8fc14a9b49a 100644 --- a/gix-object/src/commit/decode.rs +++ b/gix-object/src/commit/decode.rs @@ -40,10 +40,20 @@ pub fn commit<'a, E: ParserError<&'a [u8]> + AddContext<&'a [u8], StrContext>>( .context(StrContext::Expected( "zero or more 'parent <40 lowercase hex char>'".into(), )), - (|i: &mut _| parse::header_field(i, b"author", parse::signature)) - .context(StrContext::Expected("author ".into())), - (|i: &mut _| parse::header_field(i, b"committer", parse::signature)) - .context(StrContext::Expected("committer ".into())), + (|i: &mut _| { + parse::header_field(i, b"author", parse::signature_with_raw).map(|(signature, raw)| { + let _ = signature; + raw + }) + }) + .context(StrContext::Expected("author ".into())), + (|i: &mut _| { + parse::header_field(i, b"committer", parse::signature_with_raw).map(|(signature, raw)| { + let _ = signature; + raw + }) + }) + .context(StrContext::Expected("committer ".into())), opt(|i: &mut _| parse::header_field(i, b"encoding", take_till(0.., NL))) .context(StrContext::Expected("encoding ".into())), repeat( diff --git a/gix-object/src/commit/mod.rs b/gix-object/src/commit/mod.rs index aeac4454733..f493e2abd4c 100644 --- a/gix-object/src/commit/mod.rs +++ b/gix-object/src/commit/mod.rs @@ -74,6 +74,11 @@ impl<'a> CommitRef<'a> { /// Access impl<'a> CommitRef<'a> { + fn parse_signature(raw: &'a BStr) -> Result, crate::decode::Error> { + gix_actor::SignatureRef::from_bytes::(raw.as_ref()) + .map_err(|err| crate::decode::Error::with_err(err, raw.as_ref())) + } + /// Return the `tree` fields hash digest. pub fn tree(&self) -> gix_hash::ObjectId { gix_hash::ObjectId::from_hex(self.tree).expect("prior validation of tree hash during parsing") @@ -94,15 +99,15 @@ impl<'a> CommitRef<'a> { /// Return the author, with whitespace trimmed. /// /// This is different from the `author` field which may contain whitespace. - pub fn author(&self) -> gix_actor::SignatureRef<'a> { - self.author.trim() + pub fn author(&self) -> Result, crate::decode::Error> { + Self::parse_signature(self.author).map(|signature| signature.trim()) } /// Return the committer, with whitespace trimmed. /// /// This is different from the `committer` field which may contain whitespace. - pub fn committer(&self) -> gix_actor::SignatureRef<'a> { - self.committer.trim() + pub fn committer(&self) -> Result, crate::decode::Error> { + Self::parse_signature(self.committer).map(|signature| signature.trim()) } /// Returns a partially parsed message from which more information can be derived. @@ -111,21 +116,21 @@ impl<'a> CommitRef<'a> { } /// Returns the time at which this commit was created, or a default time if it could not be parsed. - pub fn time(&self) -> gix_date::Time { - self.committer.time.parse().unwrap_or_default() + pub fn time(&self) -> Result { + Self::parse_signature(self.committer).map(|signature| signature.time().unwrap_or_default()) } } /// Conversion impl CommitRef<'_> { /// Copy all fields of this instance into a fully owned commit, consuming this instance. - pub fn into_owned(self) -> Commit { - self.into() + pub fn into_owned(self) -> Result { + Commit::try_from(self) } /// Copy all fields of this instance into a fully owned commit, internally cloning this instance. - pub fn to_owned(self) -> Commit { - self.clone().into() + pub fn to_owned(self) -> Result { + Commit::try_from(self.clone()) } } diff --git a/gix-object/src/commit/write.rs b/gix-object/src/commit/write.rs index fc9cf025b18..856d72840ec 100644 --- a/gix-object/src/commit/write.rs +++ b/gix-object/src/commit/write.rs @@ -58,8 +58,8 @@ impl crate::WriteTo for CommitRef<'_> { for parent in self.parents() { encode::trusted_header_id(b"parent", &parent, &mut out)?; } - encode::trusted_header_signature(b"author", &self.author, &mut out)?; - encode::trusted_header_signature(b"committer", &self.committer, &mut out)?; + encode::trusted_header_field(b"author", self.author.as_ref(), &mut out)?; + encode::trusted_header_field(b"committer", self.committer.as_ref(), &mut out)?; if let Some(encoding) = self.encoding.as_ref() { encode::header_field(b"encoding", encoding, &mut out)?; } @@ -78,8 +78,8 @@ impl crate::WriteTo for CommitRef<'_> { let hash_in_hex = self.tree().kind().len_in_hex(); (b"tree".len() + 1 /* space */ + hash_in_hex + 1 /* nl */ + self.parents.iter().count() * (b"parent".len() + 1 /* space */ + hash_in_hex + 1 /* nl */) - + b"author".len() + 1 /* space */ + self.author.size() + 1 /* nl */ - + b"committer".len() + 1 /* space */ + self.committer.size() + 1 /* nl */ + + b"author".len() + 1 /* space */ + self.author.len() + 1 /* nl */ + + b"committer".len() + 1 /* space */ + self.committer.len() + 1 /* nl */ + self .encoding .as_ref() diff --git a/gix-object/src/lib.rs b/gix-object/src/lib.rs index 5bc9d6f066f..410ff3ca306 100644 --- a/gix-object/src/lib.rs +++ b/gix-object/src/lib.rs @@ -88,17 +88,16 @@ pub struct CommitRef<'a> { pub tree: &'a BStr, /// HEX hash of each parent commit. Empty for first commit in repository. pub parents: SmallVec<[&'a BStr; 1]>, - /// Who wrote this commit. Name and email might contain whitespace and are not trimmed to ensure round-tripping. + /// The raw author header value as encountered during parsing. /// - /// Use the [`author()`](CommitRef::author()) method to received a trimmed version of it. - pub author: gix_actor::SignatureRef<'a>, - /// Who committed this commit. Name and email might contain whitespace and are not trimmed to ensure round-tripping. - /// - /// Use the [`committer()`](CommitRef::committer()) method to received a trimmed version of it. + /// Use the [`author()`](CommitRef::author()) method to obtain a parsed version of it. + #[cfg_attr(feature = "serde", serde(borrow))] + pub author: &'a BStr, + /// The raw committer header value as encountered during parsing. /// - /// This may be different from the `author` in case the author couldn't write to the repository themselves and - /// is commonly encountered with contributed commits. - pub committer: gix_actor::SignatureRef<'a>, + /// Use the [`committer()`](CommitRef::committer()) method to obtain a parsed version of it. + #[cfg_attr(feature = "serde", serde(borrow))] + pub committer: &'a BStr, /// The name of the message encoding, otherwise [UTF-8 should be assumed](https://github.com/git/git/blob/e67fbf927dfdf13d0b21dc6ea15dc3c7ef448ea0/commit.c#L1493:L1493). pub encoding: Option<&'a BStr>, /// The commit message documenting the change. @@ -150,8 +149,11 @@ pub struct TagRef<'a> { pub target_kind: Kind, /// The name of the tag, e.g. "v1.0". pub name: &'a BStr, - /// The author of the tag. - pub tagger: Option>, + /// The raw tagger header value as encountered during parsing. + /// + /// Use the [`tagger()`](TagRef::tagger()) method to obtain a parsed version of it. + #[cfg_attr(feature = "serde", serde(borrow))] + pub tagger: Option<&'a BStr>, /// The message describing this release. pub message: &'a BStr, /// A cryptographic signature over the entire content of the serialized tag object thus far. diff --git a/gix-object/src/object/convert.rs b/gix-object/src/object/convert.rs index 73c77dfc191..d0b1cdd576d 100644 --- a/gix-object/src/object/convert.rs +++ b/gix-object/src/object/convert.rs @@ -1,38 +1,55 @@ +use std::convert::TryFrom; + use crate::{tree, Blob, BlobRef, Commit, CommitRef, Object, ObjectRef, Tag, TagRef, Tree, TreeRef}; -impl From> for Tag { - fn from(other: TagRef<'_>) -> Tag { +impl TryFrom> for Tag { + type Error = crate::decode::Error; + + fn try_from(other: TagRef<'_>) -> Result { let TagRef { target, name, target_kind, message, - tagger: signature, + tagger, pgp_signature, } = other; - Tag { + let tagger = tagger + .map(|raw| { + gix_actor::SignatureRef::from_bytes::(raw.as_ref()) + .map_err(|err| crate::decode::Error::with_err(err, raw.as_ref())) + }) + .transpose()? + .map(Into::into); + Ok(Tag { target: gix_hash::ObjectId::from_hex(target).expect("prior parser validation"), name: name.to_owned(), target_kind, message: message.to_owned(), - tagger: signature.map(Into::into), + tagger, pgp_signature: pgp_signature.map(ToOwned::to_owned), - } + }) } } -impl From> for Commit { - fn from(other: CommitRef<'_>) -> Commit { +impl TryFrom> for Commit { + type Error = crate::decode::Error; + + fn try_from(other: CommitRef<'_>) -> Result { let CommitRef { tree, parents, - author, - committer, + author: author_raw, + committer: committer_raw, encoding, message, extra_headers, } = other; - Commit { + let author = gix_actor::SignatureRef::from_bytes::(author_raw.as_ref()) + .map_err(|err| crate::decode::Error::with_err(err, author_raw.as_ref()))?; + let committer = gix_actor::SignatureRef::from_bytes::(committer_raw.as_ref()) + .map_err(|err| crate::decode::Error::with_err(err, committer_raw.as_ref()))?; + Ok(Commit { tree: gix_hash::ObjectId::from_hex(tree).expect("prior parser validation"), parents: parents .iter() @@ -46,7 +63,7 @@ impl From> for Commit { .into_iter() .map(|(k, v)| (k.into(), v.into_owned())) .collect(), - } + }) } } @@ -89,14 +106,16 @@ impl<'a> From<&'a tree::Entry> for tree::EntryRef<'a> { } } -impl From> for Object { - fn from(v: ObjectRef<'_>) -> Self { - match v { +impl TryFrom> for Object { + type Error = crate::decode::Error; + + fn try_from(v: ObjectRef<'_>) -> Result { + Ok(match v { ObjectRef::Tree(v) => Object::Tree(v.into()), ObjectRef::Blob(v) => Object::Blob(v.into()), - ObjectRef::Commit(v) => Object::Commit(v.into()), - ObjectRef::Tag(v) => Object::Tag(v.into()), - } + ObjectRef::Commit(v) => Object::Commit(Commit::try_from(v)?), + ObjectRef::Tag(v) => Object::Tag(Tag::try_from(v)?), + }) } } diff --git a/gix-object/src/object/mod.rs b/gix-object/src/object/mod.rs index 853d62bb521..bcee9b78d48 100644 --- a/gix-object/src/object/mod.rs +++ b/gix-object/src/object/mod.rs @@ -216,15 +216,15 @@ impl<'a> ObjectRef<'a> { /// Convert the immutable object into a mutable version, consuming the source in the process. /// /// Note that this is an expensive operation. - pub fn into_owned(self) -> Object { - self.into() + pub fn into_owned(self) -> Result { + Object::try_from(self) } /// Convert this immutable object into its mutable counterpart. /// /// Note that this is an expensive operation. - pub fn to_owned(&self) -> Object { - self.clone().into() + pub fn to_owned(&self) -> Result { + Object::try_from(self.clone()) } } diff --git a/gix-object/src/parse.rs b/gix-object/src/parse.rs index bbe63abd1df..10a69a417a4 100644 --- a/gix-object/src/parse.rs +++ b/gix-object/src/parse.rs @@ -71,3 +71,13 @@ pub(crate) fn signature<'a, E: ParserError<&'a [u8]> + AddContext<&'a [u8], StrC ) -> ModalResult, E> { gix_actor::signature::decode(i) } + +pub(crate) fn signature_with_raw<'a, E: ParserError<&'a [u8]> + AddContext<&'a [u8], StrContext>>( + i: &mut &'a [u8], +) -> ModalResult<(gix_actor::SignatureRef<'a>, &'a BStr), E> { + let original = *i; + gix_actor::signature::decode(i).map(|signature| { + let consumed = original.len() - i.len(); + (signature, original[..consumed].as_bstr()) + }) +} diff --git a/gix-object/src/tag/decode.rs b/gix-object/src/tag/decode.rs index c30015b84a0..4bb482a0e59 100644 --- a/gix-object/src/tag/decode.rs +++ b/gix-object/src/tag/decode.rs @@ -19,20 +19,23 @@ pub fn git_tag<'a, E: ParserError<&'a [u8]> + AddContext<&'a [u8], StrContext>>( .context(StrContext::Expected("type ".into())), (|i: &mut _| parse::header_field(i, b"tag", take_while(1.., |b| b != NL[0]))) .context(StrContext::Expected("tag ".into())), - opt(|i: &mut _| parse::header_field(i, b"tagger", parse::signature)) - .context(StrContext::Expected("tagger ".into())), + opt(|i: &mut _| { + parse::header_field(i, b"tagger", parse::signature_with_raw).map(|(signature, raw)| { + let _ = signature; + raw + }) + }) + .context(StrContext::Expected("tagger ".into())), terminated(message, eof), ) - .map( - |(target, kind, tag_version, signature, (message, pgp_signature))| TagRef { - target, - name: tag_version.as_bstr(), - target_kind: kind, - message, - tagger: signature, - pgp_signature, - }, - ) + .map(|(target, kind, tag_version, tagger, (message, pgp_signature))| TagRef { + target, + name: tag_version.as_bstr(), + target_kind: kind, + message, + tagger, + pgp_signature, + }) .parse_next(i) } diff --git a/gix-object/src/tag/mod.rs b/gix-object/src/tag/mod.rs index a9976dccebc..64ab05c2272 100644 --- a/gix-object/src/tag/mod.rs +++ b/gix-object/src/tag/mod.rs @@ -24,8 +24,19 @@ impl<'a> TagRef<'a> { gix_hash::ObjectId::from_hex(self.target).expect("prior validation") } + /// Return the tagger, if present. + pub fn tagger(&self) -> Result>, crate::decode::Error> { + self.tagger + .map(|raw| { + gix_actor::SignatureRef::from_bytes::(raw.as_ref()) + .map_err(|err| crate::decode::Error::with_err(err, raw.as_ref())) + .map(|signature| signature.trim()) + }) + .transpose() + } + /// Copy all data into a fully-owned instance. - pub fn into_owned(self) -> crate::Tag { - self.into() + pub fn into_owned(self) -> Result { + crate::Tag::try_from(self) } } diff --git a/gix-object/src/tag/write.rs b/gix-object/src/tag/write.rs index 48df16b8e5a..17c7e75a333 100644 --- a/gix-object/src/tag/write.rs +++ b/gix-object/src/tag/write.rs @@ -64,8 +64,8 @@ impl crate::WriteTo for TagRef<'_> { encode::trusted_header_field(b"object", self.target, &mut out)?; encode::trusted_header_field(b"type", self.target_kind.as_bytes(), &mut out)?; encode::header_field(b"tag", validated_name(self.name)?, &mut out)?; - if let Some(tagger) = &self.tagger { - encode::trusted_header_signature(b"tagger", tagger, &mut out)?; + if let Some(tagger) = self.tagger { + encode::trusted_header_field(b"tagger", tagger.as_ref(), &mut out)?; } if !self.message.iter().all(|b| *b == b'\n') { @@ -89,8 +89,7 @@ impl crate::WriteTo for TagRef<'_> { + b"tag".len() + 1 /* space */ + self.name.len() + 1 /* nl */ + self .tagger - .as_ref() - .map_or(0, |t| b"tagger".len() + 1 /* space */ + t.size() + 1 /* nl */) + .map_or(0, |raw| b"tagger".len() + 1 /* space */ + raw.len() + 1 /* nl */) + if self.message.iter().all(|b| *b == b'\n') { 0 } else { 1 /* nl */ } + self.message.len() + self.pgp_signature.as_ref().map_or(0, |m| 1 /* nl */ + m.len())) as u64 } diff --git a/gix-object/tests/object/commit/from_bytes.rs b/gix-object/tests/object/commit/from_bytes.rs index b89213394da..9bb401d6838 100644 --- a/gix-object/tests/object/commit/from_bytes.rs +++ b/gix-object/tests/object/commit/from_bytes.rs @@ -9,19 +9,14 @@ use crate::{ #[test] fn invalid_timestsamp() { - let actor = gix_actor::SignatureRef { - name: b"Name".as_bstr(), - email: b"name@example.com".as_bstr(), - time: "1312735823 +051800", - }; assert_eq!( CommitRef::from_bytes(&fixture_name("commit", "invalid-timestamp.txt")) .expect("auto-correct invalid timestamp by discarding it (time is still valid UTC)"), CommitRef { tree: b"7989dfb2ec2f41914611a22fb30bbc2b3849df9a".as_bstr(), parents: [b"8845ae683e2688bc619baade49510c17e978518f".as_bstr()].into(), - author: actor, - committer: actor, + author: b"Name 1312735823 +051800".as_bstr(), + committer: b"Name 1312735823 +051800".as_bstr(), encoding: None, message: b"edit changelog to mention about x_sendfile_header default change".as_bstr(), extra_headers: vec![] @@ -37,19 +32,22 @@ fn invalid_email_of_committer() { email: b"gh > 1282910542 +0200".as_bstr(), + committer: b"Gregor Hartmann> 1282910542 +0200".as_bstr(), encoding: None, message: b"build breakers".as_bstr(), extra_headers: vec![] } ); + assert_eq!(commit.author().unwrap(), actor); + assert_eq!(commit.committer().unwrap(), actor); } #[test] @@ -59,8 +57,8 @@ fn unsigned() -> crate::Result { CommitRef { tree: b"1b2dfb4ac5e42080b682fc676e9738c94ce6d54d".as_bstr(), parents: SmallVec::default(), - author: signature("1592437401 +0800"), - committer: signature("1592437401 +0800"), + author: b"Sebastian Thiel 1592437401 +0800".as_bstr(), + committer: b"Sebastian Thiel 1592437401 +0800".as_bstr(), encoding: None, message: b"without sig".as_bstr(), extra_headers: vec![] @@ -76,8 +74,8 @@ fn whitespace() -> crate::Result { CommitRef { tree: b"9bed6275068a0575243ba8409253e61af81ab2ff".as_bstr(), parents: SmallVec::from(vec![b"26b4df046d1776c123ac69d918f5aec247b58cc6".as_bstr()]), - author: signature("1592448450 +0800"), - committer: signature("1592448450 +0800"), + author: b"Sebastian Thiel 1592448450 +0800".as_bstr(), + committer: b"Sebastian Thiel 1592448450 +0800".as_bstr(), encoding: None, message: b" nl".as_bstr(), // this one had a \n trailing it, but git seems to trim that extra_headers: vec![] @@ -93,8 +91,8 @@ fn signed_singleline() -> crate::Result { CommitRef { tree: b"00fc39317701176e326974ce44f5bd545a32ec0b".as_bstr(), parents: SmallVec::from(vec![b"09d8d3a12e161a7f6afb522dbe8900a9c09bce06".as_bstr()]), - author: signature("1592391367 +0800"), - committer: signature("1592391367 +0800"), + author: b"Sebastian Thiel 1592391367 +0800".as_bstr(), + committer: b"Sebastian Thiel 1592391367 +0800".as_bstr(), encoding: None, message: b"update tasks\n".as_bstr(), extra_headers: vec![(b"gpgsig".as_bstr(), b"magic:signature".as_bstr().into())] @@ -106,14 +104,14 @@ fn signed_singleline() -> crate::Result { #[test] fn mergetag() -> crate::Result { let fixture = fixture_name("commit", "mergetag.txt"); - let commit = CommitRef { + let expected = CommitRef { tree: b"1c61918031bf2c7fab9e17dde3c52a6a9884fcb5".as_bstr(), parents: SmallVec::from(vec![ b"44ebe016df3aad96e3be8f95ec52397728dd7701".as_bstr(), b"8d485da0ddee79d0e6713405694253d401e41b93".as_bstr(), ]), - author: linus_signature("1591996221 -0700"), - committer: linus_signature("1591996221 -0700"), + author: b"Linus Torvalds 1591996221 -0700".as_bstr(), + committer: b"Linus Torvalds 1591996221 -0700".as_bstr(), encoding: None, message: LONG_MESSAGE.as_bytes().as_bstr(), extra_headers: vec![( @@ -121,9 +119,12 @@ fn mergetag() -> crate::Result { std::borrow::Cow::Owned(MERGE_TAG.as_bytes().into()), )], }; - assert_eq!(CommitRef::from_bytes(&fixture)?, commit); + let commit = CommitRef::from_bytes(&fixture)?; + assert_eq!(commit, expected); assert_eq!(commit.extra_headers().find_all("mergetag").count(), 1); assert_eq!(commit.extra_headers().mergetags().count(), 1); + assert_eq!(commit.author().unwrap(), linus_signature("1591996221 -0700")); + assert_eq!(commit.committer().unwrap(), linus_signature("1591996221 -0700")); Ok(()) } @@ -134,8 +135,8 @@ fn signed() -> crate::Result { CommitRef { tree: b"00fc39317701176e326974ce44f5bd545a32ec0b".as_bstr(), parents: SmallVec::from(vec![b"09d8d3a12e161a7f6afb522dbe8900a9c09bce06".as_bstr()]), - author: signature("1592391367 +0800"), - committer: signature("1592391367 +0800"), + author: b"Sebastian Thiel 1592391367 +0800".as_bstr(), + committer: b"Sebastian Thiel 1592391367 +0800".as_bstr(), encoding: None, message: b"update tasks\n".as_bstr(), extra_headers: vec![(b"gpgsig".as_bstr(), b"-----BEGIN PGP SIGNATURE-----\n\niQEzBAABCAAdFiEEdjYp/sh4j8NRKLX27gKdHl60AwAFAl7p9tgACgkQ7gKdHl60\nAwBpegf+KQciv9AOIN7+yPmowecGxBnSfpKWTDzFxnyGR8dq63SpWT8WEKG5mf3a\nG6iUqpsDWaMHlzihaMKRvgRpZxFRbjnNPFBj6F4RRqfE+5R7k6DRSLUV5PqnsdSH\nuccfIDWi1imhsm7AaP5trwl1t+83U2JhHqPcPVFLMODYwWeO6NLR/JCzGSTQRa8t\nRgaVMKI19O/fge5OT5Ua8D47VKEhsJX0LfmkP5RfZQ8JJvNd40TupqKRdlv0sAzP\nya7NXkSHXCavHNR6kA+KpWxn900UoGK8/IDlwU6MeOkpPVawb3NFMqnc7KJDaC2p\nSMzpuEG8LTrCx2YSpHNLqHyzvQ1CZA==\n=5ITV\n-----END PGP SIGNATURE-----\n".as_bstr().into())] @@ -151,8 +152,8 @@ fn signed_with_encoding() -> crate::Result { CommitRef { tree: b"1973afa74d87b2bb73fa884aaaa8752aec43ea88".as_bstr(), parents: SmallVec::from(vec![b"79c51cc86923e2b8ca0ee5c4eb75e48027133f9a".as_bstr()]), - author: signature("1592448995 +0800"), - committer: signature("1592449083 +0800"), + author: b"Sebastian Thiel 1592448995 +0800".as_bstr(), + committer: b"Sebastian Thiel 1592449083 +0800".as_bstr(), encoding: Some(b"ISO-8859-1".as_bstr()), message: b"encoding & sig".as_bstr(), extra_headers: vec![(b"gpgsig".as_bstr(), SIGNATURE.as_bstr().into())] @@ -168,8 +169,8 @@ fn with_encoding() -> crate::Result { CommitRef { tree: b"4a1c03029e7407c0afe9fc0320b3258e188b115e".as_bstr(), parents: SmallVec::from(vec![b"7ca98aad461a5c302cb4c9e3acaaa6053cc67a62".as_bstr()]), - author: signature("1592438199 +0800"), - committer: signature("1592438199 +0800"), + author: b"Sebastian Thiel 1592438199 +0800".as_bstr(), + committer: b"Sebastian Thiel 1592438199 +0800".as_bstr(), encoding: Some("ISO-8859-1".into()), message: b"commit with encoding".as_bstr(), extra_headers: vec![] @@ -180,18 +181,13 @@ fn with_encoding() -> crate::Result { #[test] fn pre_epoch() -> crate::Result { - let signature = || SignatureRef { - name: "Législateur".into(), - email: "".into(), - time: "-5263834140 +0009", - }; assert_eq!( CommitRef::from_bytes(&fixture_name("commit", "pre-epoch.txt"))?, CommitRef { tree: b"71cdd4015386b764b178005cad4c88966bc9d61a".as_bstr(), parents: SmallVec::default(), - author: signature(), - committer: signature(), + author: "Législateur <> -5263834140 +0009".as_bytes().as_bstr(), + committer: "Législateur <> -5263834140 +0009".as_bytes().as_bstr(), encoding: None, message: "Version consolidée au 14 mars 1803\n".into(), extra_headers: vec![] @@ -202,18 +198,13 @@ fn pre_epoch() -> crate::Result { #[test] fn double_dash_special_time_offset() -> crate::Result { - let signature = || SignatureRef { - name: "name".into(), - email: "name@example.com".into(), - time: "1288373970 --700", - }; assert_eq!( CommitRef::from_bytes(&fixture_name("commit", "double-dash-date-offset.txt"))?, CommitRef { tree: b"0a851d7a2a66084ab10516c406a405d147e974ad".as_bstr(), parents: SmallVec::from(vec![b"31350f4f0f459485eff2131517e3450cf251f6fa".as_bstr()]), - author: signature(), - committer: signature(), + author: "name 1288373970 --700".as_bytes().as_bstr(), + committer: "name 1288373970 --700".as_bytes().as_bstr(), encoding: None, message: "msg\n".into(), extra_headers: vec![] @@ -236,8 +227,8 @@ fn with_trailer() -> crate::Result { CommitRef { tree: b"25a19c29c5e36884c1ad85d8faf23f1246b7961b".as_bstr(), parents: SmallVec::from(vec![b"699ae71105dddfcbb9711ed3a92df09e91a04e90".as_bstr()]), - author: kim, - committer: kim, + author: "Kim Altintop 1631514803 +0200".as_bytes().as_bstr(), + committer: "Kim Altintop 1631514803 +0200".as_bytes().as_bstr(), encoding: None, message: b"test: use gitoxide for link-git-protocol tests @@ -253,6 +244,8 @@ Signed-off-by: Kim Altintop " extra_headers: vec![(b"gpgsig".as_bstr(), b"-----BEGIN PGP SIGNATURE-----\n\niHUEABYIAB0WIQSuZwcGWSQItmusNgR5URpSUCnwXQUCYT7xpAAKCRB5URpSUCnw\nXWB3AP9q323HlxnI8MyqszNOeYDwa7Y3yEZaUM2y/IRjz+z4YQEAq0yr1Syt3mrK\nOSFCqL2vDm3uStP+vF31f6FnzayhNg0=\n=Mhpp\n-----END PGP SIGNATURE-----\n".as_bstr().into())] } ); + assert_eq!(commit.author().unwrap(), kim); + assert_eq!(commit.committer().unwrap(), kim); let message = commit.message(); assert_eq!(message.title, "test: use gitoxide for link-git-protocol tests"); assert_eq!( @@ -316,8 +309,8 @@ fn merge() -> crate::Result { b"6a6054db4ce3c1e4e6a37f8c4d7acb63a4d6ad71".as_bstr(), b"c91d592913d47ac4e4a76daf16fd649b276e211e".as_bstr() ]), - author: signature("1592454703 +0800"), - committer: signature("1592454738 +0800"), + author: b"Sebastian Thiel 1592454703 +0800".as_bstr(), + committer: b"Sebastian Thiel 1592454738 +0800".as_bstr(), encoding: Some("ISO-8859-1".into()), message: b"Merge branch 'branch'".as_bstr(), extra_headers: vec![] @@ -366,3 +359,12 @@ fn bogus_multi_gpgsig_header() -> crate::Result { ); Ok(()) } + +#[test] +fn author_method_returns_trimmed_signature() -> crate::Result { + let backing = fixture_name("commit", "unsigned.txt"); + let commit = CommitRef::from_bytes(&backing)?; + assert_eq!(commit.author().unwrap(), signature("1592437401 +0800")); + assert_eq!(commit.committer().unwrap(), signature("1592437401 +0800")); + Ok(()) +} diff --git a/gix-object/tests/object/commit/message.rs b/gix-object/tests/object/commit/message.rs index 732c1b929a6..a3063375cd8 100644 --- a/gix-object/tests/object/commit/message.rs +++ b/gix-object/tests/object/commit/message.rs @@ -241,8 +241,8 @@ mod summary { CommitRef { tree: "tree".into(), parents: Default::default(), - author: actor, - committer: actor, + author: "name 0 0000".as_bytes().as_bstr(), + committer: "name 0 0000".as_bytes().as_bstr(), encoding: None, message: input.as_bstr(), extra_headers: vec![] @@ -251,6 +251,17 @@ mod summary { summary, "both versions create the same result" ); + let commit = CommitRef { + tree: "tree".into(), + parents: Default::default(), + author: "name 0 0000".as_bytes().as_bstr(), + committer: "name 0 0000".as_bytes().as_bstr(), + encoding: None, + message: input.as_bstr(), + extra_headers: vec![], + }; + assert_eq!(commit.author().unwrap(), actor); + assert_eq!(commit.committer().unwrap(), actor); summary } diff --git a/gix-object/tests/object/encode/mod.rs b/gix-object/tests/object/encode/mod.rs index 154a2585c12..ff362459520 100644 --- a/gix-object/tests/object/encode/mod.rs +++ b/gix-object/tests/object/encode/mod.rs @@ -26,7 +26,7 @@ macro_rules! round_trip { item.write_to(&mut output)?; assert_eq!(output.as_bstr(), input.as_bstr(), "borrowed: {input_name}"); - let item: $owned = item.into(); + let item: $owned = <$owned>::try_from(item)?; output.clear(); item.write_to(&mut output)?; assert_eq!(output.as_bstr(), input.as_bstr()); @@ -37,7 +37,7 @@ macro_rules! round_trip { item.write_to(&mut output)?; assert_eq!(output.as_bstr(), input.as_bstr(), "object-ref"); - let item: Object = item.into(); + let item: Object = Object::try_from(item)?; output.clear(); item.write_to(&mut output)?; assert_eq!(output.as_bstr(), input.as_bstr(), "owned"); @@ -55,14 +55,15 @@ macro_rules! round_trip { assert_eq!(item2, item, "object-ref loose: {input_name} {:?}\n{:?}", output.as_bstr(), input.as_bstr()); } - let item: $owned = item.into(); + let item: $owned = <$owned>::try_from(item)?; // serialise an owned to a tagged loose object output.clear(); let w = &mut output; w.write_all(&item.loose_header())?; item.write_to(w)?; let parsed = ObjectRef::from_loose(&output)?; - let item2: $owned = <$borrowed>::try_from(parsed).or(Err(super::Error::TryFromError))?.into(); + let parsed_borrowed = <$borrowed>::try_from(parsed).or(Err(super::Error::TryFromError))?; + let item2: $owned = <$owned>::try_from(parsed_borrowed).or(Err(super::Error::TryFromError))?; assert_eq!(item2, item, "object-ref loose owned: {input_name} {:?}\n{:?}", output.as_bstr(), input.as_bstr()); } Ok(()) diff --git a/gix-object/tests/object/tag/mod.rs b/gix-object/tests/object/tag/mod.rs index 4d37fbe64f9..4c2f505b9fd 100644 --- a/gix-object/tests/object/tag/mod.rs +++ b/gix-object/tests/object/tag/mod.rs @@ -4,7 +4,6 @@ use crate::fixture_name; mod method { use bstr::ByteSlice; - use gix_date::parse::TimeBuf; use gix_object::TagRef; use pretty_assertions::assert_eq; @@ -24,12 +23,12 @@ mod method { tagger, message, pgp_signature, - } = tag.into_owned(); + } = tag.into_owned()?; assert_eq!(target.to_string(), tag.target); assert_eq!(target_kind, tag.target_kind); assert_eq!(name, tag.name); - let mut buf = TimeBuf::default(); - assert_eq!(tagger.as_ref().map(|s| s.to_ref(&mut buf)), tag.tagger); + let parsed = tag.tagger()?.map(Into::into); + assert_eq!(tagger, parsed); assert_eq!(message, tag.message); assert_eq!(pgp_signature.as_ref().map(|s| s.as_bstr()), tag.pgp_signature); Ok(()) @@ -154,7 +153,6 @@ fn invalid() { } mod from_bytes { - use gix_actor::SignatureRef; use gix_object::{bstr::ByteSlice, Kind, TagRef, WriteTo}; use crate::{fixture_name, signature, tag::tag_fixture}; @@ -176,7 +174,7 @@ mod from_bytes { name: b"empty".as_bstr(), target_kind: Kind::Commit, message: b"\n".as_bstr(), - tagger: Some(signature("1592381636 +0800")), + tagger: Some(b"Sebastian Thiel 1592381636 +0800".as_bstr()), pgp_signature: None } ); @@ -195,7 +193,7 @@ mod from_bytes { name: b"empty".as_bstr(), target_kind: Kind::Commit, message: b"".as_bstr(), - tagger: Some(signature("1592381636 +0800")), + tagger: Some(b"Sebastian Thiel 1592381636 +0800".as_bstr()), pgp_signature: None } ); @@ -212,7 +210,7 @@ mod from_bytes { name: b"baz".as_bstr(), target_kind: Kind::Commit, message: b"hello\n\nworld".as_bstr(), - tagger: Some(signature("1592311808 +0800")), + tagger: Some(b"Sebastian Thiel 1592311808 +0800".as_bstr()), pgp_signature: None } ); @@ -260,7 +258,7 @@ KLMHist5yj0sw1E4hDTyQa0= name: b"whitespace".as_bstr(), target_kind: Kind::Commit, message: b" \ttab\nnewline\n\nlast-with-trailer\n".as_bstr(), - tagger: Some(signature("1592382888 +0800")), + tagger: Some(b"Sebastian Thiel 1592382888 +0800".as_bstr()), pgp_signature: None } ); @@ -276,16 +274,20 @@ KLMHist5yj0sw1E4hDTyQa0= name: b"ChangeLog".as_bstr(), target_kind: Kind::Commit, message: b"".as_bstr(), - tagger: Some(SignatureRef { - name: b"shemminger".as_bstr(), - email: b"shemminger".as_bstr(), - time: "", - }), + tagger: Some(b"shemminger ".as_bstr()), pgp_signature: None } ); Ok(()) } + + #[test] + fn tagger_method_returns_signature() -> crate::Result { + let fixture = fixture_name("tag", "empty.txt"); + let tag = TagRef::from_bytes(&fixture)?; + assert_eq!(tag.tagger()?, Some(signature("1592381636 +0800"))); + Ok(()) + } } fn tag_fixture() -> TagRef<'static> { @@ -314,10 +316,6 @@ cjHJZXWmV4CcRfmLsXzU8s2cR9A0DBvOxhPD1TlKC2JhBFXigjuL9U4Rbq9tdegB -----END PGP SIGNATURE-----" .as_bstr(), ), - tagger: Some(gix_actor::SignatureRef { - name: b"Sebastian Thiel".as_bstr(), - email: b"byronimo@gmail.com".as_bstr(), - time: "1528473343 +0230", - }), + tagger: Some(b"Sebastian Thiel 1528473343 +0230".as_bstr()), } } diff --git a/gix-odb/tests/odb/store/loose.rs b/gix-odb/tests/odb/store/loose.rs index c43699ab0e9..456a3b30ad8 100644 --- a/gix-odb/tests/odb/store/loose.rs +++ b/gix-odb/tests/odb/store/loose.rs @@ -1,7 +1,6 @@ use std::sync::atomic::AtomicBool; use gix_features::progress; -use gix_object::bstr::ByteSlice; use gix_odb::loose::Store; use gix_testtools::fixture_path_standalone; use pretty_assertions::assert_eq; @@ -229,7 +228,7 @@ mod find { use crate::{ hex_to_id, - store::loose::{ldb, locate_oid, signature}, + store::loose::{ldb, locate_oid}, }; fn find<'a>(hex: &str, buf: &'a mut Vec) -> gix_object::Data<'a> { @@ -284,7 +283,7 @@ cjHJZXWmV4CcRfmLsXzU8s2cR9A0DBvOxhPD1TlKC2JhBFXigjuL9U4Rbq9tdegB " .as_bstr(), ), - tagger: Some(signature("1528473343 +0200")), + tagger: Some(b"Sebastian Thiel 1528473343 +0200".as_bstr()), }; assert_eq!(o.decode()?.as_tag().expect("tag"), &expected); Ok(()) @@ -299,8 +298,8 @@ cjHJZXWmV4CcRfmLsXzU8s2cR9A0DBvOxhPD1TlKC2JhBFXigjuL9U4Rbq9tdegB let expected = CommitRef { tree: b"6ba2a0ded519f737fd5b8d5ccfb141125ef3176f".as_bstr(), parents: vec![].into(), - author: signature("1528473303 +0200"), - committer: signature("1528473303 +0200"), + author: b"Sebastian Thiel 1528473303 +0200".as_bstr(), + committer: b"Sebastian Thiel 1528473303 +0200".as_bstr(), encoding: None, message: b"initial commit\n".as_bstr(), extra_headers: vec![(b"gpgsig".as_bstr(), b"-----BEGIN PGP SIGNATURE-----\nComment: GPGTools - https://gpgtools.org\n\niQIzBAABCgAdFiEEw7xSvXbiwjusbsBqZl+Z+p2ZlmwFAlsaptwACgkQZl+Z+p2Z\nlmxXSQ//fj6t7aWoEKeMdFigfj6OXWPUyrRbS0N9kpJeOfA0BIOea/6Jbn8J5qh1\nYRfrySOzHPXR5Y+w4GwLiVas66qyhAbk4yeqZM0JxBjHDyPyRGhjUd3y7WjEa6bj\nP0ACAIkYZQ/Q/LDE3eubmhAwEobBH3nZbwE+/zDIG0i265bD5C0iDumVOiKkSelw\ncr6FZVw1HH+GcabFkeLRZLNGmPqGdbeBwYERqb0U1aRCzV1xLYteoKwyWcYaH8E3\n97z1rwhUO/L7o8WUEJtP3CLB0zuocslMxskf6bCeubBnRNJ0YrRmxGarxCP3vn4D\n3a/MwECnl6mnUU9t+OnfvrzLDN73rlq8iasUq6hGe7Sje7waX6b2UGpxHqwykmXg\nVimD6Ah7svJanHryfJn38DvJW/wOMqmAnSUAp+Y8W9EIe0xVntCmtMyoKuqBoY7T\nJlZ1kHJte6ELIM5JOY9Gx7D0ZCSKZJQqyjoqtl36dsomT0I78/+7QS1DP4S6XB7d\nc3BYH0JkW81p7AAFbE543ttN0Z4wKXErMFqUKnPZUIEuybtlNYV+krRdfDBWQysT\n3MBebjguVQ60oGs06PzeYBosKGQrHggAcwduLFuqXhLTJqN4UQ18RkE0vbtG3YA0\n+XtZQM13vURdfwFI5qitAGgw4EzPVrkWWzApzLCrRPEMbvP+b9A=\n=2qqN\n-----END PGP SIGNATURE-----\n".as_bstr().into())] @@ -427,11 +426,3 @@ cjHJZXWmV4CcRfmLsXzU8s2cR9A0DBvOxhPD1TlKC2JhBFXigjuL9U4Rbq9tdegB } } } - -fn signature(time: &str) -> gix_actor::SignatureRef<'_> { - gix_actor::SignatureRef { - name: b"Sebastian Thiel".as_bstr(), - email: b"byronimo@gmail.com".as_bstr(), - time, - } -} diff --git a/gix/src/commit.rs b/gix/src/commit.rs index e8dd662bfb3..1de192437a1 100644 --- a/gix/src/commit.rs +++ b/gix/src/commit.rs @@ -126,7 +126,12 @@ pub mod describe { let (prio, tag_time) = match target_id { Some(target_id) if peeled_id != *target_id => { let tag = repo.find_object(target_id).ok()?.try_into_tag().ok()?; - (1, tag.tagger().ok()??.seconds()) + let tag_time = tag + .tagger() + .ok() + .and_then(|signature| signature.map(|signature| signature.seconds())) + .unwrap_or(0); + (1, tag_time) } _ => (0, 0), }; @@ -159,7 +164,11 @@ pub mod describe { // TODO: we assume direct refs for tags, which is the common case, but it doesn't have to be // so rather follow symrefs till the first object and then peel tags after the first object was found. let tag = r.try_id()?.object().ok()?.try_into_tag().ok()?; - let tag_time = tag.tagger().ok().and_then(|s| s.map(|s| s.seconds())).unwrap_or(0); + let tag_time = tag + .tagger() + .ok() + .and_then(|signature| signature.map(|signature| signature.seconds())) + .unwrap_or(0); let commit_id = tag.target_id().ok()?.object().ok()?.try_into_commit().ok()?.id; Some((commit_id, tag_time, Cow::::from(r.name().shorten().to_owned()))) }) diff --git a/gix/src/object/commit.rs b/gix/src/object/commit.rs index 93354143eb6..a872f02f972 100644 --- a/gix/src/object/commit.rs +++ b/gix/src/object/commit.rs @@ -74,7 +74,7 @@ impl<'repo> Commit<'repo> { /// Decode the commit and obtain the time at which the commit was created. /// - /// For the time at which it was authored, refer to `.decode()?.author.time`. + /// For the time at which it was authored, refer to `.decode()?.author()?.time()`. pub fn time(&self) -> Result { Ok(self.committer()?.time()?) } diff --git a/gix/src/revision/spec/parse/error.rs b/gix/src/revision/spec/parse/error.rs index 7e043386110..8ddda72d7a7 100644 --- a/gix/src/revision/spec/parse/error.rs +++ b/gix/src/revision/spec/parse/error.rs @@ -96,8 +96,12 @@ impl Error { gix_object::Kind::Commit => { use bstr::ByteSlice; let commit = obj.to_commit_ref(); + let date = match commit.committer() { + Ok(signature) => signature.time.trim().to_owned(), + Err(_) => String::from_utf8_lossy(commit.committer.as_ref()).into_owned(), + }; CandidateInfo::Commit { - date: commit.committer().time.trim().into(), + date, title: commit.message().title.trim().into(), } } diff --git a/gix/tests/gix/repository/object.rs b/gix/tests/gix/repository/object.rs index cc914de7763..befd8bc2118 100644 --- a/gix/tests/gix/repository/object.rs +++ b/gix/tests/gix/repository/object.rs @@ -630,7 +630,7 @@ mod tag { assert_eq!(current_head_id, tag.target(), "the tag points to the commit"); assert_eq!(tag.target_kind, gix_object::Kind::Commit); assert_eq!( - tag.tagger.as_ref().expect("tagger").actor(), + tag.tagger()?.expect("tagger").actor(), repo.committer().expect("present")?.actor() ); assert_eq!(tag.message, message); @@ -845,8 +845,8 @@ fn new_commit_as() -> crate::Result { let commit = commit.decode()?; let mut buf = TimeBuf::default(); - assert_eq!(commit.committer, committer.to_ref(&mut buf)); - assert_eq!(commit.author, author.to_ref(&mut buf)); + assert_eq!(commit.committer()?, committer.to_ref(&mut buf)); + assert_eq!(commit.author()?, author.to_ref(&mut buf)); assert_eq!(commit.message, "message"); assert_eq!(commit.tree(), empty_tree.id); assert_eq!(commit.parents.len(), 0);