On 06/09/2024 17:21, Junio C Hamano wrote:
Brian Lyles <brianmlyles@xxxxxxxxx> writes:
When git-interpret-trailers is used to add a trailer to a message that
does not end in a trailing newline, the new trailer is added on the line
immediately following the message instead of as a trailer block
separated from the message by a blank line.
For example, if a message's text was exactly "The subject" with no
trailing newline present, `git interpret-trailers --trailer
my-trailer=true` will result in the following malformed commit message:
The subject
my-trailer: true
While it is generally expected that a commit message should end with a
newline character, git-interpret-trailers should not be returning an
invalid message in this case.
I am not sure if the above example resulted in "an invalid message",
though ;-) As far as Git is concerned, a commit log can contain any
sequence of bytes.
I assume it means invalid in the sense that the trailers are not
separated from the rest of the message by a blank line, not in the sense
that the resulting commit object is invalid.
Best Wishes
Phillip
But of course, various tools to manipulate the messages (e.g.
"commit --amend" and your editor that gets invoked by it,
"interpret-trailers") may not be prepared to see any arbitrary
bytes. I would have written
While a commit message can contain arbitrary byte sequence, the
fact that the user invoked the interpret-trailers command on it
means that the contents is expected to be a proper text, which
should not end in an incomplete line. Instead of detecting and
erroring out upon seeing such a log message, complete the last
line if it lacks the terminating LF.
or something like that, if I were working on this change.
Use `strbuf_complete_line` to ensure that the message ends with a
newline character when reading the input.
Signed-off-by: Brian Lyles <brianmlyles@xxxxxxxxx>
---
The range-diff from v2 is not included since the patch is so different
that range-diff is not able to provide anything meaningful.
Very sensible.
Will queue. Thanks.
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 3d3e13ccf8..d78cae3e04 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -175,6 +175,46 @@ test_expect_success 'with only a title in the message' '
test_cmp expected actual
'
+test_expect_success 'with a bodiless message that lacks a trailing newline after the subject' '
+ cat >expected <<-\EOF &&
+ area: change
+
+ Reviewed-by: Peff
+ Acked-by: Johan
+ EOF
+ printf "area: change" |
+ git interpret-trailers --trailer "Reviewed-by: Peff" \
+ --trailer "Acked-by: Johan" >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'with a bodied message that lacks a trailing newline after the body' '
+ cat >expected <<-\EOF &&
+ area: change
+
+ details about the change.
+
+ Reviewed-by: Peff
+ Acked-by: Johan
+ EOF
+ printf "area: change\n\ndetails about the change." |
+ git interpret-trailers --trailer "Reviewed-by: Peff" \
+ --trailer "Acked-by: Johan" >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'with a message that lacks a trailing newline after the trailers' '
+ cat >expected <<-\EOF &&
+ area: change
+
+ Reviewed-by: Peff
+ Acked-by: Johan
+ EOF
+ printf "area: change\n\nReviewed-by: Peff" |
+ git interpret-trailers --trailer "Acked-by: Johan" >actual &&
+ test_cmp expected actual
+'
+
test_expect_success 'with multiline title in the message' '
cat >expected <<-\EOF &&
place of
--
2.45.2
