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>
Jonathan Tan committed
Oct 21, 2016 at 10:55 UTC
146245063e286d5a38b146bd5a38da958bd3957b
3 files changed
+194
-15
Documentation/git-interpret-trailers.txt
+3
-2
@@ -48,8 +48,9 @@ with only spaces at the end of the commit message part, one blank line
48
will be added before the new trailer.
49
50
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.
54
The group must either be at the end of the message or be the last
55
non-whitespace lines before a line that starts with '---'. Such three
56
minus signs start the patch part of the message.
t/t7513-interpret-trailers.sh
+115
@@ -126,6 +126,121 @@ test_expect_success 'with multiline title in the message' '
126
test_cmp expected actual
127
'
128
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
+
244
test_expect_success 'with config setup' '
245
git config trailer.ack.key "Acked-by: " &&
246
cat >expected <<-\EOF &&
trailer.c
+76
-13
@@ -27,6 +27,10 @@ static struct conf_info default_conf_info;
27
28
struct trailer_item {
29
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
+ */
34
char *token;
35
char *value;
36
};
@@ -44,6 +48,12 @@ static char *separators = ":";
48
49
#define TRAILER_ARG_STRING "$ARG"
50
51
+static const char *git_generated_prefixes[] = {
52
+ "Signed-off-by: ",
53
+ "(cherry picked from commit ",
54
+ NULL
55
+};
56
+
57
/* Iterate over the elements of the list. */
58
#define list_for_each_dir(pos, head, is_reverse) \
59
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)
80
81
static int same_token(struct trailer_item *a, struct arg_item *b)
82
{
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;
91
92
return !strncasecmp(a->token, b->token, min_len);
93
}
@@ -130,7 +145,14 @@ static char last_non_space_char(const char *s)
145
146
static void print_tok_val(FILE *outfile, const char *tok, const char *val)
147
{
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);
156
if (!c)
157
return;
158
if (strchr(separators, c))
@@ -709,6 +731,7 @@ static int find_patch_start(struct strbuf **lines, int count)
731
static int find_trailer_start(struct strbuf **lines, int count)
732
{
733
int start, end_of_title, only_spaces = 1;
734
+ int recognized_prefix = 0, trailer_lines = 0, non_trailer_lines = 0;
735
736
/* The first paragraph is the title and cannot be trailers */
737
for (start = 0; start < count; start++) {
@@ -720,26 +743,60 @@ static int find_trailer_start(struct strbuf **lines, int count)
743
end_of_title = start;
744
745
/*
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.
750
*/
751
for (start = count - 1; start >= end_of_title; start--) {
752
+ const char **p;
753
+ int separator_pos;
754
+
755
if (lines[start]->buf[0] == comment_line_char)
756
continue;
757
if (contains_only_spaces(lines[start]->buf)) {
758
if (only_spaces)
759
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;
766
}
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
+ }
775
}
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
+ ;
797
}
798
742
- return only_spaces ? count : 0;
799
+ return count;
800
}
801
802
/* Get the index of the end of the trailers */
@@ -810,6 +867,12 @@ static int process_input_file(FILE *outfile,
867
add_trailer_item(head,
868
strbuf_detach(&tok, NULL),
869
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));
876
}
877
}
878