[PATCH v3] interpret-trailers: handle message without trailing newline

From: Brian Lyles <brianmlyles@gmail.com>
Date: Sep 6, 2024, 14:50 UTC
Message-ID: <20240906145743.2059405-1-brianmlyles@gmail.com>
In-Reply-To: <20240905173445.1677704-1-brianmlyles@gmail.com>

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.

Use `strbuf_complete_line` to ensure that the message ends with a newline character when reading the input.

Signed-off-by: Brian Lyles <brianmlyles@gmail.com>
---

Differences from v2:
- We now use `strbuf_complete_line` when reading the input file instead
  of handling the lack of a newline when constructing the output, which
  drastically simplifies the patch. Thanks to Phillip for this
  suggestion.
- Removed some unnecessary `\` in the new tests.

The range-diff from v2 is not included since the patch is so different that range-diff is not able to provide anything meaningful.

 builtin/interpret-trailers.c  |  1 +
 t/t7513-interpret-trailers.sh | 40 +++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 1d969494cf..e6f22459f1 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -132,6 +132,7 @@ static void read_input_file(struct strbuf *sb, const char *file)
 		if (strbuf_read(sb, fileno(stdin), 0) < 0)
 			die_errno(_("could not read from stdin"));
 	}
+	strbuf_complete_line(sb);
 }

 static void interpret_trailers(const struct process_trailer_options *opts,
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