Skip to content

Commit fba9654

Browse files
gitstergitster
committed
Merge branch 'jk/trailer-fixes'
"git interpret-trailers" and its underlying machinery had a buggy code that attempted to ignore patch text after commit log message, which triggered in various codepaths that will always get the log message alone and never get such an input. * jk/trailer-fixes: append_signoff: use size_t for string offsets sequencer: ignore "---" divider when parsing trailers pretty, ref-filter: format %(trailers) with no_divider option interpret-trailers: allow suppressing "---" divider interpret-trailers: tighten check for "---" patch boundary trailer: pass process_trailer_opts to trailer_info_get() trailer: use size_t for iterating trailer list trailer: use size_t for string offsets
2 parents 30035d1 + 66e83d9 commit fba9654

14 files changed

Lines changed: 175 additions & 39 deletions

‎Documentation/git-interpret-trailers.txt‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,9 @@ least one Git-generated or user-configured trailer and consists of at
5656
least 25% trailers.
5757
The group must be preceded by one or more empty (or whitespace-only) lines.
5858
The group must either be at the end of the message or be the last
59-
non-whitespace lines before a line that starts with '---'. Such three
60-
minus signs start the patch part of the message.
59+
non-whitespace lines before a line that starts with '---' (followed by a
60+
space or the end of the line). Such three minus signs start the patch
61+
part of the message. See also `--no-divider` below.
6162

6263
When reading trailers, there can be whitespaces after the
6364
token, the separator and the value. There can also be whitespaces
@@ -125,6 +126,11 @@ OPTIONS
125126
A convenience alias for `--only-trailers --only-input
126127
--unfold`.
127128

129+
--no-divider::
130+
Do not treat `---` as the end of the commit message. Use this
131+
when you know your input contains just the commit message itself
132+
(and not an email or the output of `git format-patch`).
133+
128134
CONFIGURATION VARIABLES
129135
-----------------------
130136

‎builtin/interpret-trailers.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
104104
OPT_BOOL(0, "unfold", &opts.unfold, N_("join whitespace-continued values")),
105105
{ OPTION_CALLBACK, 0, "parse", &opts, NULL, N_("set parsing options"),
106106
PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_parse },
107+
OPT_BOOL(0, "no-divider", &opts.no_divider, N_("do not treat --- specially")),
107108
OPT_CALLBACK(0, "trailer", &trailers, N_("trailer"),
108109
N_("trailer(s) to add"), option_parse_trailer),
109110
OPT_END()

‎commit.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1426,10 +1426,10 @@ const char *find_commit_header(const char *msg, const char *key, size_t *out_len
14261426
* Returns the number of bytes from the tail to ignore, to be fed as
14271427
* the second parameter to append_signoff().
14281428
*/
1429-
int ignore_non_trailer(const char *buf, size_t len)
1429+
size_t ignore_non_trailer(const char *buf, size_t len)
14301430
{
1431-
int boc = 0;
1432-
int bol = 0;
1431+
size_t boc = 0;
1432+
size_t bol = 0;
14331433
int in_old_conflicts_block = 0;
14341434
size_t cutoff = wt_status_locate_end(buf, len);
14351435

‎commit.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ extern const char *find_commit_header(const char *msg, const char *key,
293293
size_t *out_len);
294294

295295
/* Find the end of the log message, the right place for a new trailer. */
296-
extern int ignore_non_trailer(const char *buf, size_t len);
296+
extern size_t ignore_non_trailer(const char *buf, size_t len);
297297

298298
typedef int (*each_mergetag_fn)(struct commit *commit, struct commit_extra_header *extra,
299299
void *cb_data);

‎pretty.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,6 +1304,9 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
13041304

13051305
if (skip_prefix(placeholder, "(trailers", &arg)) {
13061306
struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
1307+
1308+
opts.no_divider = 1;
1309+
13071310
if (*arg == ':') {
13081311
arg++;
13091312
for (;;) {

‎ref-filter.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,8 @@ static int trailers_atom_parser(const struct ref_format *format, struct used_ato
264264
struct string_list params = STRING_LIST_INIT_DUP;
265265
int i;
266266

267+
atom->u.contents.trailer_opts.no_divider = 1;
268+
267269
if (arg) {
268270
string_list_split(&params, arg, ',', -1);
269271
for (i = 0; i < params.nr; i++) {

‎sequencer.c‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -226,13 +226,16 @@ static const char *get_todo_path(const struct replay_opts *opts)
226226
* Returns 3 when sob exists within conforming footer as last entry
227227
*/
228228
static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
229-
int ignore_footer)
229+
size_t ignore_footer)
230230
{
231+
struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
231232
struct trailer_info info;
232-
int i;
233+
size_t i;
233234
int found_sob = 0, found_sob_last = 0;
234235

235-
trailer_info_get(&info, sb->buf);
236+
opts.no_divider = 1;
237+
238+
trailer_info_get(&info, sb->buf, &opts);
236239

237240
if (info.trailer_start == info.trailer_end)
238241
return 0;
@@ -3829,7 +3832,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)
38293832
return res;
38303833
}
38313834

3832-
void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
3835+
void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag)
38333836
{
38343837
unsigned no_dup_sob = flag & APPEND_SIGNOFF_DEDUP;
38353838
struct strbuf sob = STRBUF_INIT;

‎sequencer.h‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,14 @@ int rearrange_squash(void);
9090

9191
extern const char sign_off_header[];
9292

93-
void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);
93+
/*
94+
* Append a signoff to the commit message in "msgbuf". The ignore_footer
95+
* parameter specifies the number of bytes at the end of msgbuf that should
96+
* not be considered at all. I.e., they are not checked for existing trailers,
97+
* and the new signoff will be spliced into the buffer before those bytes.
98+
*/
99+
void append_signoff(struct strbuf *msgbuf, size_t ignore_footer, unsigned flag);
100+
94101
void append_conflicts_hint(struct strbuf *msgbuf);
95102
int message_is_empty(const struct strbuf *sb,
96103
enum commit_msg_cleanup_mode cleanup_mode);

‎t/t4205-log-pretty-formats.sh‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,4 +598,27 @@ test_expect_success ':only and :unfold work together' '
598598
test_cmp expect actual
599599
'
600600

601+
test_expect_success 'trailer parsing not fooled by --- line' '
602+
git commit --allow-empty -F - <<-\EOF &&
603+
this is the subject
604+
605+
This is the body. The message has a "---" line which would confuse a
606+
message+patch parser. But here we know we have only a commit message,
607+
so we get it right.
608+
609+
trailer: wrong
610+
---
611+
This is more body.
612+
613+
trailer: right
614+
EOF
615+
616+
{
617+
echo "trailer: right" &&
618+
echo
619+
} >expect &&
620+
git log --no-walk --format="%(trailers)" >actual &&
621+
test_cmp expect actual
622+
'
623+
601624
test_done

‎t/t6300-for-each-ref.sh‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -715,6 +715,29 @@ test_expect_success 'basic atom: head contents:trailers' '
715715
test_cmp expect actual.clean
716716
'
717717

718+
test_expect_success 'trailer parsing not fooled by --- line' '
719+
git commit --allow-empty -F - <<-\EOF &&
720+
this is the subject
721+
722+
This is the body. The message has a "---" line which would confuse a
723+
message+patch parser. But here we know we have only a commit message,
724+
so we get it right.
725+
726+
trailer: wrong
727+
---
728+
This is more body.
729+
730+
trailer: right
731+
EOF
732+
733+
{
734+
echo "trailer: right" &&
735+
echo
736+
} >expect &&
737+
git for-each-ref --format="%(trailers)" refs/heads/master >actual &&
738+
test_cmp expect actual
739+
'
740+
718741
test_expect_success 'Add symbolic ref for the following tests' '
719742
git symbolic-ref refs/heads/sym refs/heads/master
720743
'

0 commit comments

Comments
 (0)