git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v5 6/8] trailer: allow non-trailers in trailer block

From
JTJonathan Tan <jonathantanmy@google.com>
Date
Oct 21, 2016, 17:55 UTC
Message-ID
<b7ff1a633688f3523cb641e9c4faf80094d6a435.1477072247.git.jonathantanmy@google.com>
In-Reply-To
<cover.1477072247.git.jonathantanmy@google.com>

Currently, interpret-trailers requires all lines of a trailer block to be trailers (or comments) - if not it would not identify that block as a trailer block, and thus create its own trailer block, inserting a blank line. For example:

  echo -e "\nSigned-off-by: x\nnot trailer" |
  git interpret-trailers --trailer "c: d"
would result in:
  Signed-off-by: x
  not trailer
  c: d

Relax the definition of a trailer block to require that the trailers (i) are all trailers, or (ii) contain at least one Git-generated trailer and consists of at least 25% trailers.

  Signed-off-by: x
  not trailer
  c: d

(i) is the existing functionality. (ii) allows arbitrary lines to be included in trailer blocks, like those in [1], and still allow interpret-trailers to be used.

[1] https://kernel.googlesource.com/pub/scm/linux/kernel/git/stable/linux-stable/+/e7d316a02f683864a12389f8808570e37fb90aa3

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 Documentation/git-interpret-trailers.txt |   5 +-
 t/t7513-interpret-trailers.sh            | 115 +++++++++++++++++++++++++++++++
 trailer.c                                |  89 ++++++++++++++++++++----
 3 files changed, 194 insertions(+), 15 deletions(-)
diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
index 93d1db6..cf4c5ea 100644
--- a/Documentation/git-interpret-trailers.txt
+++ b/Documentation/git-interpret-trailers.txt
@@ -48,8 +48,9 @@ with only spaces at the end of the commit message part, one blank line
 will be added before the new trailer.
 
 Existing trailers are extracted from the input message by looking for
-a group of one or more lines that contain a colon (by default), where
-the group is preceded by one or more empty (or whitespace-only) lines.
+a group of one or more lines that (i) are all trailers, or (ii) contains at
+least one Git-generated trailer and consists of at least 25% trailers.
+The group must be preceded by one or more empty (or whitespace-only) lines.
 The group must either be at the end of the message or be the last
 non-whitespace lines before a line that starts with '---'. Such three
 minus signs start the patch part of the message.
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index aee785c..003e90f 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -126,6 +126,121 @@ test_expect_success 'with multiline title in the message' '
 	test_cmp expected actual
 '
 
+test_expect_success 'with non-trailer lines mixed with Signed-off-by' '
+	cat >patch <<-\EOF &&
+
+		this is not a trailer
+		this is not a trailer
+		Signed-off-by: a <a@example.com>
+		this is not a trailer
+	EOF
+	cat >expected <<-\EOF &&
+
+		this is not a trailer
+		this is not a trailer
+		Signed-off-by: a <a@example.com>
+		this is not a trailer
+		token: value
+	EOF
+	git interpret-trailers --trailer "token: value" patch >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with non-trailer lines mixed with cherry picked from' '
+	cat >patch <<-\EOF &&
+
+		this is not a trailer
+		this is not a trailer
+		(cherry picked from commit x)
+		this is not a trailer
+	EOF
+	cat >expected <<-\EOF &&
+
+		this is not a trailer
+		this is not a trailer
+		(cherry picked from commit x)
+		this is not a trailer
+		token: value
+	EOF
+	git interpret-trailers --trailer "token: value" patch >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with non-trailer lines mixed with a configured trailer' '
+	cat >patch <<-\EOF &&
+
+		this is not a trailer
+		this is not a trailer
+		My-trailer: x
+		this is not a trailer
+	EOF
+	cat >expected <<-\EOF &&
+
+		this is not a trailer
+		this is not a trailer
+		My-trailer: x
+		this is not a trailer
+		token: value
+	EOF
+	test_config trailer.my.key "My-trailer: " &&
+	git interpret-trailers --trailer "token: value" patch >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with non-trailer lines mixed with a non-configured trailer' '
+	cat >patch <<-\EOF &&
+
+		this is not a trailer
+		this is not a trailer
+		I-am-not-configured: x
+		this is not a trailer
+	EOF
+	cat >expected <<-\EOF &&
+
+		this is not a trailer
+		this is not a trailer
+		I-am-not-configured: x
+		this is not a trailer
+
+		token: value
+	EOF
+	test_config trailer.my.key "My-trailer: " &&
+	git interpret-trailers --trailer "token: value" patch >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with all non-configured trailers' '
+	cat >patch <<-\EOF &&
+
+		I-am-not-configured: x
+		I-am-also-not-configured: x
+	EOF
+	cat >expected <<-\EOF &&
+
+		I-am-not-configured: x
+		I-am-also-not-configured: x
+		token: value
+	EOF
+	test_config trailer.my.key "My-trailer: " &&
+	git interpret-trailers --trailer "token: value" patch >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'with non-trailer lines only' '
+	cat >patch <<-\EOF &&
+
+		this is not a trailer
+	EOF
+	cat >expected <<-\EOF &&
+
+		this is not a trailer
+
+		token: value
+	EOF
+	git interpret-trailers --trailer "token: value" patch >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'with config setup' '
 	git config trailer.ack.key "Acked-by: " &&
 	cat >expected <<-\EOF &&
diff --git a/trailer.c b/trailer.c
index aff858b..199f86a 100644
--- a/trailer.c
+++ b/trailer.c
@@ -27,6 +27,10 @@ static struct conf_info default_conf_info;
 
 struct trailer_item {
 	struct list_head list;
+	/*
+	 * If this is not a trailer line, the line is stored in value
+	 * (excluding the terminating newline) and token is NULL.
+	 */
 	char *token;
 	char *value;
 };
@@ -44,6 +48,12 @@ static char *separators = ":";
 
 #define TRAILER_ARG_STRING "$ARG"
 
+static const char *git_generated_prefixes[] = {
+	"Signed-off-by: ",
+	"(cherry picked from commit ",
+	NULL
+};
+
 /* Iterate over the elements of the list. */
 #define list_for_each_dir(pos, head, is_reverse) \
 	for (pos = is_reverse ? (head)->prev : (head)->next; \
@@ -70,9 +80,14 @@ static size_t token_len_without_separator(const char *token, size_t len)
 
 static int same_token(struct trailer_item *a, struct arg_item *b)
 {
-	size_t a_len = token_len_without_separator(a->token, strlen(a->token));
-	size_t b_len = token_len_without_separator(b->token, strlen(b->token));
-	size_t min_len = (a_len > b_len) ? b_len : a_len;
+	size_t a_len, b_len, min_len;
+
+	if (!a->token)
+		return 0;
+
+	a_len = token_len_without_separator(a->token, strlen(a->token));
+	b_len = token_len_without_separator(b->token, strlen(b->token));
+	min_len = (a_len > b_len) ? b_len : a_len;
 
 	return !strncasecmp(a->token, b->token, min_len);
 }
@@ -130,7 +145,14 @@ static char last_non_space_char(const char *s)
 
 static void print_tok_val(FILE *outfile, const char *tok, const char *val)
 {
-	char c = last_non_space_char(tok);
+	char c;
+
+	if (!tok) {
+		fprintf(outfile, "%s\n", val);
+		return;
+	}
+
+	c = last_non_space_char(tok);
 	if (!c)
 		return;
 	if (strchr(separators, c))
@@ -709,6 +731,7 @@ static int find_patch_start(struct strbuf **lines, int count)
 static int find_trailer_start(struct strbuf **lines, int count)
 {
 	int start, end_of_title, only_spaces = 1;
+	int recognized_prefix = 0, trailer_lines = 0, non_trailer_lines = 0;
 
 	/* The first paragraph is the title and cannot be trailers */
 	for (start = 0; start < count; start++) {
@@ -720,26 +743,60 @@ static int find_trailer_start(struct strbuf **lines, int count)
 	end_of_title = start;
 
 	/*
-	 * Get the start of the trailers by looking starting from the end
-	 * for a line with only spaces before lines with one separator.
+	 * Get the start of the trailers by looking starting from the end for a
+	 * blank line before a set of non-blank lines that (i) are all
+	 * trailers, or (ii) contains at least one Git-generated trailer and
+	 * consists of at least 25% trailers.
 	 */
 	for (start = count - 1; start >= end_of_title; start--) {
+		const char **p;
+		int separator_pos;
+
 		if (lines[start]->buf[0] == comment_line_char)
 			continue;
 		if (contains_only_spaces(lines[start]->buf)) {
 			if (only_spaces)
 				continue;
-			return start + 1;
+			if (recognized_prefix &&
+			    trailer_lines * 3 >= non_trailer_lines)
+				return start + 1;
+			if (trailer_lines && !non_trailer_lines)
+				return start + 1;
+			return count;
 		}
-		if (strcspn(lines[start]->buf, separators) < lines[start]->len) {
-			if (only_spaces)
-				only_spaces = 0;
-			continue;
+		only_spaces = 0;
+
+		for (p = git_generated_prefixes; *p; p++) {
+			if (starts_with(lines[start]->buf, *p)) {
+				trailer_lines++;
+				recognized_prefix = 1;
+				goto continue_outer_loop;
+			}
 		}
-		return count;
+
+		separator_pos = find_separator(lines[start]->buf);
+		if (separator_pos >= 1) {
+			struct list_head *pos;
+
+			trailer_lines++;
+			if (recognized_prefix)
+				continue;
+			list_for_each(pos, &conf_head) {
+				struct arg_item *item;
+				item = list_entry(pos, struct arg_item, list);
+				if (token_matches_item(lines[start]->buf, item,
+						       separator_pos)) {
+					recognized_prefix = 1;
+					break;
+				}
+			}
+		} else
+			non_trailer_lines++;
+continue_outer_loop:
+		;
 	}
 
-	return only_spaces ? count : 0;
+	return count;
 }
 
 /* Get the index of the end of the trailers */
@@ -810,6 +867,12 @@ static int process_input_file(FILE *outfile,
 			add_trailer_item(head,
 					 strbuf_detach(&tok, NULL),
 					 strbuf_detach(&val, NULL));
+		} else {
+			strbuf_addbuf(&val, lines[i]);
+			strbuf_strip_suffix(&val, "\n");
+			add_trailer_item(head,
+					 NULL,
+					 strbuf_detach(&val, NULL));
 		}
 	}
 

-- 
2.8.0.rc3.226.g39d4020
Previous: Jonathan TanNext: Jonathan Tan
Message 43 of 48 in “allow non-trailers and multiple-line trailers”
  1. 0/5 allow non-trailers and multiple-line trailersJonathan Tan, Oct 12, 2016
  2. 1/5 trailer: use singly-linked list, not doublyJonathan Tan, Oct 12, 2016
  3. Junio C HamanoOct 12, 2016
  4. Christian CouderOct 12, 2016
  5. Jeff KingOct 12, 2016
  6. 5/5 trailer: support values folded to multiple linesJonathan Tan, Oct 12, 2016
  7. Junio C HamanoOct 12, 2016
  8. 2/5 trailer: streamline trailer item create and addJonathan Tan, Oct 12, 2016
  9. 4/5 trailer: allow non-trailers in trailer blockJonathan Tan, Oct 12, 2016
  10. 3/5 trailer: make args have their own structJonathan Tan, Oct 12, 2016
  11. 0/6 allow non-trailers and multiple-line trailersJonathan Tan, Oct 12, 2016
  12. 1/6 trailer: improve const correctnessJonathan Tan, Oct 12, 2016
  13. 2/6 trailer: use list.h for doubly-linked listJonathan Tan, Oct 12, 2016
  14. Junio C HamanoOct 14, 2016
  15. Jakub NarębskiOct 14, 2016
  16. 3/6 trailer: streamline trailer item create and addJonathan Tan, Oct 12, 2016
  17. 4/6 trailer: make args have their own structJonathan Tan, Oct 12, 2016
  18. 5/6 trailer: allow non-trailers in trailer blockJonathan Tan, Oct 12, 2016
  19. 6/6 trailer: support values folded to multiple linesJonathan Tan, Oct 12, 2016
  20. 0/6 allow non-trailers and multiple-line trailersJonathan Tan, Oct 14, 2016
  21. 1/6 trailer: improve const correctnessJonathan Tan, Oct 14, 2016
  22. Stefan BellerOct 17, 2016
  23. 2/6 trailer: use list.h for doubly-linked listJonathan Tan, Oct 14, 2016
  24. 3/6 trailer: streamline trailer item create and addJonathan Tan, Oct 14, 2016
  25. Stefan BellerOct 17, 2016
  26. 5/6 trailer: allow non-trailers in trailer blockJonathan Tan, Oct 14, 2016
  27. Stefan BellerOct 18, 2016
  28. Junio C HamanoOct 18, 2016
  29. Jonathan TanOct 18, 2016
  30. Junio C HamanoOct 18, 2016
  31. Jonathan TanOct 19, 2016
  32. Junio C HamanoOct 19, 2016
  33. 6/6 trailer: support values folded to multiple linesJonathan Tan, Oct 14, 2016
  34. Stefan BellerOct 18, 2016
  35. 4/6 trailer: make args have their own structJonathan Tan, Oct 14, 2016
  36. Stefan BellerOct 17, 2016
  37. Junio C HamanoOct 18, 2016
  38. 0/8 allow non-trailers and multiple-line trailersJonathan Tan, Oct 21, 2016
  39. 1/8 trailer: improve const correctnessJonathan Tan, Oct 21, 2016
  40. 5/8 trailer: clarify failure modes in parse_trailerJonathan Tan, Oct 21, 2016
  41. 4/8 trailer: make args have their own structJonathan Tan, Oct 21, 2016
  42. 7/8 trailer: forbid leading whitespace in trailersJonathan Tan, Oct 21, 2016
  43. 6/8 trailer: allow non-trailers in trailer blockJonathan Tan, Oct 21, 2016
  44. 8/8 trailer: support values folded to multiple linesJonathan Tan, Oct 21, 2016
  45. 3/8 trailer: streamline trailer item create and addJonathan Tan, Oct 21, 2016
  46. 2/8 trailer: use list.h for doubly-linked listJonathan Tan, Oct 21, 2016
  47. Junio C HamanoOct 21, 2016
  48. Stefan BellerOct 22, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.