Skip to content

Commit 1462450

Browse files
jonathantanmygitster
jonathantanmy
authored and
gitster
committed
trailer: allow non-trailers in trailer block
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> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent fdbf451 commit 1462450

File tree

3 files changed

+194
-15
lines changed

3 files changed

+194
-15
lines changed

‎Documentation/git-interpret-trailers.txt‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,9 @@ with only spaces at the end of the commit message part, one blank line
4848
will be added before the new trailer.
4949

5050
Existing trailers are extracted from the input message by looking for
51-
a group of one or more lines that contain a colon (by default), where
52-
the group is preceded by one or more empty (or whitespace-only) lines.
51+
a group of one or more lines that (i) are all trailers, or (ii) contains at
52+
least one Git-generated trailer and consists of at least 25% trailers.
53+
The group must be preceded by one or more empty (or whitespace-only) lines.
5354
The group must either be at the end of the message or be the last
5455
non-whitespace lines before a line that starts with '---'. Such three
5556
minus signs start the patch part of the message.

‎t/t7513-interpret-trailers.sh‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,121 @@ test_expect_success 'with multiline title in the message' '
126126
test_cmp expected actual
127127
'
128128

129+
test_expect_success 'with non-trailer lines mixed with Signed-off-by' '
130+
cat >patch <<-\EOF &&
131+
132+
this is not a trailer
133+
this is not a trailer
134+
Signed-off-by: a <a@example.com>
135+
this is not a trailer
136+
EOF
137+
cat >expected <<-\EOF &&
138+
139+
this is not a trailer
140+
this is not a trailer
141+
Signed-off-by: a <a@example.com>
142+
this is not a trailer
143+
token: value
144+
EOF
145+
git interpret-trailers --trailer "token: value" patch >actual &&
146+
test_cmp expected actual
147+
'
148+
149+
test_expect_success 'with non-trailer lines mixed with cherry picked from' '
150+
cat >patch <<-\EOF &&
151+
152+
this is not a trailer
153+
this is not a trailer
154+
(cherry picked from commit x)
155+
this is not a trailer
156+
EOF
157+
cat >expected <<-\EOF &&
158+
159+
this is not a trailer
160+
this is not a trailer
161+
(cherry picked from commit x)
162+
this is not a trailer
163+
token: value
164+
EOF
165+
git interpret-trailers --trailer "token: value" patch >actual &&
166+
test_cmp expected actual
167+
'
168+
169+
test_expect_success 'with non-trailer lines mixed with a configured trailer' '
170+
cat >patch <<-\EOF &&
171+
172+
this is not a trailer
173+
this is not a trailer
174+
My-trailer: x
175+
this is not a trailer
176+
EOF
177+
cat >expected <<-\EOF &&
178+
179+
this is not a trailer
180+
this is not a trailer
181+
My-trailer: x
182+
this is not a trailer
183+
token: value
184+
EOF
185+
test_config trailer.my.key "My-trailer: " &&
186+
git interpret-trailers --trailer "token: value" patch >actual &&
187+
test_cmp expected actual
188+
'
189+
190+
test_expect_success 'with non-trailer lines mixed with a non-configured trailer' '
191+
cat >patch <<-\EOF &&
192+
193+
this is not a trailer
194+
this is not a trailer
195+
I-am-not-configured: x
196+
this is not a trailer
197+
EOF
198+
cat >expected <<-\EOF &&
199+
200+
this is not a trailer
201+
this is not a trailer
202+
I-am-not-configured: x
203+
this is not a trailer
204+
205+
token: value
206+
EOF
207+
test_config trailer.my.key "My-trailer: " &&
208+
git interpret-trailers --trailer "token: value" patch >actual &&
209+
test_cmp expected actual
210+
'
211+
212+
test_expect_success 'with all non-configured trailers' '
213+
cat >patch <<-\EOF &&
214+
215+
I-am-not-configured: x
216+
I-am-also-not-configured: x
217+
EOF
218+
cat >expected <<-\EOF &&
219+
220+
I-am-not-configured: x
221+
I-am-also-not-configured: x
222+
token: value
223+
EOF
224+
test_config trailer.my.key "My-trailer: " &&
225+
git interpret-trailers --trailer "token: value" patch >actual &&
226+
test_cmp expected actual
227+
'
228+
229+
test_expect_success 'with non-trailer lines only' '
230+
cat >patch <<-\EOF &&
231+
232+
this is not a trailer
233+
EOF
234+
cat >expected <<-\EOF &&
235+
236+
this is not a trailer
237+
238+
token: value
239+
EOF
240+
git interpret-trailers --trailer "token: value" patch >actual &&
241+
test_cmp expected actual
242+
'
243+
129244
test_expect_success 'with config setup' '
130245
git config trailer.ack.key "Acked-by: " &&
131246
cat >expected <<-\EOF &&

‎trailer.c‎

Lines changed: 76 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ static struct conf_info default_conf_info;
2727

2828
struct trailer_item {
2929
struct list_head list;
30+
/*
31+
* If this is not a trailer line, the line is stored in value
32+
* (excluding the terminating newline) and token is NULL.
33+
*/
3034
char *token;
3135
char *value;
3236
};
@@ -44,6 +48,12 @@ static char *separators = ":";
4448

4549
#define TRAILER_ARG_STRING "$ARG"
4650

51+
static const char *git_generated_prefixes[] = {
52+
"Signed-off-by: ",
53+
"(cherry picked from commit ",
54+
NULL
55+
};
56+
4757
/* Iterate over the elements of the list. */
4858
#define list_for_each_dir(pos, head, is_reverse) \
4959
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)
7080

7181
static int same_token(struct trailer_item *a, struct arg_item *b)
7282
{
73-
size_t a_len = token_len_without_separator(a->token, strlen(a->token));
74-
size_t b_len = token_len_without_separator(b->token, strlen(b->token));
75-
size_t min_len = (a_len > b_len) ? b_len : a_len;
83+
size_t a_len, b_len, min_len;
84+
85+
if (!a->token)
86+
return 0;
87+
88+
a_len = token_len_without_separator(a->token, strlen(a->token));
89+
b_len = token_len_without_separator(b->token, strlen(b->token));
90+
min_len = (a_len > b_len) ? b_len : a_len;
7691

7792
return !strncasecmp(a->token, b->token, min_len);
7893
}
@@ -130,7 +145,14 @@ static char last_non_space_char(const char *s)
130145

131146
static void print_tok_val(FILE *outfile, const char *tok, const char *val)
132147
{
133-
char c = last_non_space_char(tok);
148+
char c;
149+
150+
if (!tok) {
151+
fprintf(outfile, "%s\n", val);
152+
return;
153+
}
154+
155+
c = last_non_space_char(tok);
134156
if (!c)
135157
return;
136158
if (strchr(separators, c))
@@ -709,6 +731,7 @@ static int find_patch_start(struct strbuf **lines, int count)
709731
static int find_trailer_start(struct strbuf **lines, int count)
710732
{
711733
int start, end_of_title, only_spaces = 1;
734+
int recognized_prefix = 0, trailer_lines = 0, non_trailer_lines = 0;
712735

713736
/* The first paragraph is the title and cannot be trailers */
714737
for (start = 0; start < count; start++) {
@@ -720,26 +743,60 @@ static int find_trailer_start(struct strbuf **lines, int count)
720743
end_of_title = start;
721744

722745
/*
723-
* Get the start of the trailers by looking starting from the end
724-
* for a line with only spaces before lines with one separator.
746+
* Get the start of the trailers by looking starting from the end for a
747+
* blank line before a set of non-blank lines that (i) are all
748+
* trailers, or (ii) contains at least one Git-generated trailer and
749+
* consists of at least 25% trailers.
725750
*/
726751
for (start = count - 1; start >= end_of_title; start--) {
752+
const char **p;
753+
int separator_pos;
754+
727755
if (lines[start]->buf[0] == comment_line_char)
728756
continue;
729757
if (contains_only_spaces(lines[start]->buf)) {
730758
if (only_spaces)
731759
continue;
732-
return start + 1;
760+
if (recognized_prefix &&
761+
trailer_lines * 3 >= non_trailer_lines)
762+
return start + 1;
763+
if (trailer_lines && !non_trailer_lines)
764+
return start + 1;
765+
return count;
733766
}
734-
if (strcspn(lines[start]->buf, separators) < lines[start]->len) {
735-
if (only_spaces)
736-
only_spaces = 0;
737-
continue;
767+
only_spaces = 0;
768+
769+
for (p = git_generated_prefixes; *p; p++) {
770+
if (starts_with(lines[start]->buf, *p)) {
771+
trailer_lines++;
772+
recognized_prefix = 1;
773+
goto continue_outer_loop;
774+
}
738775
}
739-
return count;
776+
777+
separator_pos = find_separator(lines[start]->buf);
778+
if (separator_pos >= 1) {
779+
struct list_head *pos;
780+
781+
trailer_lines++;
782+
if (recognized_prefix)
783+
continue;
784+
list_for_each(pos, &conf_head) {
785+
struct arg_item *item;
786+
item = list_entry(pos, struct arg_item, list);
787+
if (token_matches_item(lines[start]->buf, item,
788+
separator_pos)) {
789+
recognized_prefix = 1;
790+
break;
791+
}
792+
}
793+
} else
794+
non_trailer_lines++;
795+
continue_outer_loop:
796+
;
740797
}
741798

742-
return only_spaces ? count : 0;
799+
return count;
743800
}
744801

745802
/* Get the index of the end of the trailers */
@@ -810,6 +867,12 @@ static int process_input_file(FILE *outfile,
810867
add_trailer_item(head,
811868
strbuf_detach(&tok, NULL),
812869
strbuf_detach(&val, NULL));
870+
} else {
871+
strbuf_addbuf(&val, lines[i]);
872+
strbuf_strip_suffix(&val, "\n");
873+
add_trailer_item(head,
874+
NULL,
875+
strbuf_detach(&val, NULL));
813876
}
814877
}
815878

0 commit comments

Comments
 (0)