From 3bd33802e072b06d278b93eeae8cfaed19795d6b Mon Sep 17 00:00:00 2001 From: Pierre Chevalier Date: Thu, 17 Nov 2022 08:48:08 -0800 Subject: [PATCH] [git-object] Encode empty tags like git does A tag with an empty commit message is now displayed with an extra newline, consitently with the output of `git cat-file`. The test fixture for `empty.txt` was updated, which means that the `round_trip` test now expects a correct round trip with the new format. Note that the changes to tests in `immutable` reflect the new state of the code: * `TagRefIter` now parses this body with an empty message. * Where the length of the data was divided by 2, the point was to create an inconsistent dataset leading to an error. Unfortunately, with the extra character, it lines up so that the message can be parsed, so I had to change the magic number to keep the previous property. Fixes #603 --- git-object/src/tag/decode.rs | 3 ++- git-object/src/tag/write.rs | 16 ++++------------ git-object/tests/fixtures/tag/empty.txt | 1 + git-object/tests/immutable/tag.rs | 6 +++++- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/git-object/src/tag/decode.rs b/git-object/src/tag/decode.rs index 655e5009af8..ba9460af980 100644 --- a/git-object/src/tag/decode.rs +++ b/git-object/src/tag/decode.rs @@ -53,7 +53,8 @@ pub fn message<'a, E: ParseError<&'a [u8]>>(i: &'a [u8]) -> IResult<&'a [u8], (& let (i, _) = tag(NL)(i)?; fn all_to_end<'a, E: ParseError<&'a [u8]>>(i: &'a [u8]) -> IResult<&'a [u8], (&'a [u8], &'a [u8]), E> { if i.is_empty() { - return Err(nom::Err::Error(E::from_error_kind(i, nom::error::ErrorKind::Eof))); + // Empty message. That's OK. + return Ok((&[], (&[], &[]))); } // an empty signature message signals that there is none - the function signature is needed // to work with 'alt(…)'. PGP signatures are never empty diff --git a/git-object/src/tag/write.rs b/git-object/src/tag/write.rs index e82569a1241..7fdcc138ec2 100644 --- a/git-object/src/tag/write.rs +++ b/git-object/src/tag/write.rs @@ -29,8 +29,8 @@ impl crate::WriteTo for Tag { encode::trusted_header_signature(b"tagger", &tagger.to_ref(), &mut out)?; } + out.write_all(NL)?; if !self.message.is_empty() { - out.write_all(NL)?; out.write_all(&self.message)?; } if let Some(ref message) = self.pgp_signature { @@ -49,11 +49,7 @@ impl crate::WriteTo for Tag { .as_ref() .map(|t| b"tagger".len() + 1 /* space */ + t.size() + 1 /* nl */) .unwrap_or(0) - + if self.message.is_empty() { - 0 - } else { - 1 /* nl */ + self.message.len() - } + + 1 /* nl */ + self.message.len() + self.pgp_signature.as_ref().map(|m| 1 /* nl */ + m.len() ).unwrap_or(0) } @@ -71,8 +67,8 @@ impl<'a> crate::WriteTo for TagRef<'a> { encode::trusted_header_signature(b"tagger", tagger, &mut out)?; } + out.write_all(NL)?; if !self.message.is_empty() { - out.write_all(NL)?; out.write_all(self.message)?; } if let Some(message) = self.pgp_signature { @@ -91,11 +87,7 @@ impl<'a> crate::WriteTo for TagRef<'a> { .as_ref() .map(|t| b"tagger".len() + 1 /* space */ + t.size() + 1 /* nl */) .unwrap_or(0) - + if self.message.is_empty() { - 0 - } else { - 1 /* nl */ + self.message.len() - } + + 1 /* nl */ + self.message.len() + self.pgp_signature.as_ref().map(|m| 1 /* nl */ + m.len()).unwrap_or(0) } diff --git a/git-object/tests/fixtures/tag/empty.txt b/git-object/tests/fixtures/tag/empty.txt index 42dabcc6ed6..018ecbb9cb8 100644 --- a/git-object/tests/fixtures/tag/empty.txt +++ b/git-object/tests/fixtures/tag/empty.txt @@ -2,3 +2,4 @@ object 01dd4e2a978a9f5bd773dae6da7aa4a5ac1cdbbc type commit tag empty tagger Sebastian Thiel 1592381636 +0800 + diff --git a/git-object/tests/immutable/tag.rs b/git-object/tests/immutable/tag.rs index d3457a4ea60..6ddc65ca03a 100644 --- a/git-object/tests/immutable/tag.rs +++ b/git-object/tests/immutable/tag.rs @@ -37,6 +37,10 @@ mod iter { Token::TargetKind(Kind::Commit), Token::Name(b"empty".as_bstr()), Token::Tagger(tagger), + Token::Body { + message: b"".as_bstr(), + pgp_signature: None, + } ] ); assert_eq!(tag_iter.target_id()?, target_id); @@ -103,7 +107,7 @@ KLMHist5yj0sw1E4hDTyQa0= #[test] fn error_handling() -> crate::Result { let data = fixture_bytes("tag", "empty.txt"); - let iter = TagRefIter::from_bytes(&data[..data.len() / 2]); + let iter = TagRefIter::from_bytes(&data[..data.len() / 3]); let tokens = iter.collect::>(); assert!( tokens.last().expect("at least the errored token").is_err(),