Skip to content

Commit 967dfd4

Browse files
jonathantanmygitster
authored andcommitted
sequencer: use trailer's trailer layout
Make sequencer use trailer.c's trailer layout definition, as opposed to parsing the footer by itself. This makes "commit -s", "cherry-pick -x", and "format-patch --signoff" consistent with trailer, allowing non-trailer lines and multiple-line trailers in trailer blocks under certain conditions, and therefore suppressing the extra newline in those cases. Consistency with trailer extends to respecting trailer configs. Tests have been included to show that. Signed-off-by: Jonathan Tan <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent e8c352c commit 967dfd4

File tree

4 files changed

+95
-69
lines changed

4 files changed

+95
-69
lines changed

sequencer.c

Lines changed: 14 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "refs.h"
1717
#include "argv-array.h"
1818
#include "quote.h"
19+
#include "trailer.h"
1920

2021
#define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
2122

@@ -56,30 +57,6 @@ static const char *get_todo_path(const struct replay_opts *opts)
5657
return git_path_todo_file();
5758
}
5859

59-
static int is_rfc2822_line(const char *buf, int len)
60-
{
61-
int i;
62-
63-
for (i = 0; i < len; i++) {
64-
int ch = buf[i];
65-
if (ch == ':')
66-
return 1;
67-
if (!isalnum(ch) && ch != '-')
68-
break;
69-
}
70-
71-
return 0;
72-
}
73-
74-
static int is_cherry_picked_from_line(const char *buf, int len)
75-
{
76-
/*
77-
* We only care that it looks roughly like (cherry picked from ...)
78-
*/
79-
return len > strlen(cherry_picked_prefix) + 1 &&
80-
starts_with(buf, cherry_picked_prefix) && buf[len - 1] == ')';
81-
}
82-
8360
/*
8461
* Returns 0 for non-conforming footer
8562
* Returns 1 for conforming footer
@@ -89,49 +66,25 @@ static int is_cherry_picked_from_line(const char *buf, int len)
8966
static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,
9067
int ignore_footer)
9168
{
92-
char prev;
93-
int i, k;
94-
int len = sb->len - ignore_footer;
95-
const char *buf = sb->buf;
96-
int found_sob = 0;
97-
98-
/* footer must end with newline */
99-
if (!len || buf[len - 1] != '\n')
100-
return 0;
69+
struct trailer_info info;
70+
int i;
71+
int found_sob = 0, found_sob_last = 0;
10172

102-
prev = '\0';
103-
for (i = len - 1; i > 0; i--) {
104-
char ch = buf[i];
105-
if (prev == '\n' && ch == '\n') /* paragraph break */
106-
break;
107-
prev = ch;
108-
}
73+
trailer_info_get(&info, sb->buf);
10974

110-
/* require at least one blank line */
111-
if (prev != '\n' || buf[i] != '\n')
75+
if (info.trailer_start == info.trailer_end)
11276
return 0;
11377

114-
/* advance to start of last paragraph */
115-
while (i < len - 1 && buf[i] == '\n')
116-
i++;
117-
118-
for (; i < len; i = k) {
119-
int found_rfc2822;
120-
121-
for (k = i; k < len && buf[k] != '\n'; k++)
122-
; /* do nothing */
123-
k++;
78+
for (i = 0; i < info.trailer_nr; i++)
79+
if (sob && !strncmp(info.trailers[i], sob->buf, sob->len)) {
80+
found_sob = 1;
81+
if (i == info.trailer_nr - 1)
82+
found_sob_last = 1;
83+
}
12484

125-
found_rfc2822 = is_rfc2822_line(buf + i, k - i - 1);
126-
if (found_rfc2822 && sob &&
127-
!strncmp(buf + i, sob->buf, sob->len))
128-
found_sob = k;
85+
trailer_info_release(&info);
12986

130-
if (!(found_rfc2822 ||
131-
is_cherry_picked_from_line(buf + i, k - i - 1)))
132-
return 0;
133-
}
134-
if (found_sob == i)
87+
if (found_sob_last)
13588
return 3;
13689
if (found_sob)
13790
return 2;

t/t3511-cherry-pick-x.sh

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,8 @@ Signed-off-by: B.U. Thor <[email protected]>"
2525

2626
mesg_broken_footer="$mesg_no_footer
2727
28-
The signed-off-by string should begin with the words Signed-off-by followed
29-
by a colon and space, and then the signers name and email address. e.g.
30-
Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
28+
This is not recognized as a footer because Myfooter is not a recognized token.
29+
Myfooter: A.U. Thor <[email protected]>"
3130

3231
mesg_with_footer_sob="$mesg_with_footer
3332
Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
@@ -112,6 +111,17 @@ test_expect_success 'cherry-pick -s inserts blank line after non-conforming foot
112111
test_cmp expect actual
113112
'
114113

114+
test_expect_success 'cherry-pick -s recognizes trailer config' '
115+
pristine_detach initial &&
116+
git -c "trailer.Myfooter.ifexists=add" cherry-pick -s mesg-broken-footer &&
117+
cat <<-EOF >expect &&
118+
$mesg_broken_footer
119+
Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>
120+
EOF
121+
git log -1 --pretty=format:%B >actual &&
122+
test_cmp expect actual
123+
'
124+
115125
test_expect_success 'cherry-pick -x inserts blank line when conforming footer not found' '
116126
pristine_detach initial &&
117127
sha1=$(git rev-parse mesg-no-footer^0) &&

t/t4014-format-patch.sh

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1294,8 +1294,7 @@ EOF
12941294
4:Subject: [PATCH] subject
12951295
8:
12961296
10:Signed-off-by: example happens to be wrapped here.
1297-
11:
1298-
12:Signed-off-by: C O Mitter <[email protected]>
1297+
11:Signed-off-by: C O Mitter <[email protected]>
12991298
EOF
13001299
test_cmp expected actual
13011300
'
@@ -1368,7 +1367,7 @@ EOF
13681367
test_cmp expected actual
13691368
'
13701369

1371-
test_expect_success 'signoff: detect garbage in non-conforming footer' '
1370+
test_expect_success 'signoff: tolerate garbage in conforming footer' '
13721371
append_signoff <<\EOF >actual &&
13731372
subject
13741373
@@ -1383,8 +1382,36 @@ EOF
13831382
8:
13841383
10:
13851384
13:Signed-off-by: C O Mitter <[email protected]>
1386-
14:
1387-
15:Signed-off-by: C O Mitter <[email protected]>
1385+
EOF
1386+
test_cmp expected actual
1387+
'
1388+
1389+
test_expect_success 'signoff: respect trailer config' '
1390+
append_signoff <<\EOF >actual &&
1391+
subject
1392+
1393+
Myfooter: x
1394+
Some Trash
1395+
EOF
1396+
cat >expected <<\EOF &&
1397+
4:Subject: [PATCH] subject
1398+
8:
1399+
11:
1400+
12:Signed-off-by: C O Mitter <[email protected]>
1401+
EOF
1402+
test_cmp expected actual &&
1403+
1404+
test_config trailer.Myfooter.ifexists add &&
1405+
append_signoff <<\EOF >actual &&
1406+
subject
1407+
1408+
Myfooter: x
1409+
Some Trash
1410+
EOF
1411+
cat >expected <<\EOF &&
1412+
4:Subject: [PATCH] subject
1413+
8:
1414+
11:Signed-off-by: C O Mitter <[email protected]>
13881415
EOF
13891416
test_cmp expected actual
13901417
'

t/t7501-commit.sh

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,42 @@ $alt" &&
460460
test_cmp expected actual
461461
'
462462

463+
test_expect_success 'signoff respects trailer config' '
464+
465+
echo 5 >positive &&
466+
git add positive &&
467+
git commit -s -m "subject
468+
469+
non-trailer line
470+
Myfooter: x" &&
471+
git cat-file commit HEAD | sed -e "1,/^\$/d" > actual &&
472+
(
473+
echo subject
474+
echo
475+
echo non-trailer line
476+
echo Myfooter: x
477+
echo
478+
echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
479+
) >expected &&
480+
test_cmp expected actual &&
481+
482+
echo 6 >positive &&
483+
git add positive &&
484+
git -c "trailer.Myfooter.ifexists=add" commit -s -m "subject
485+
486+
non-trailer line
487+
Myfooter: x" &&
488+
git cat-file commit HEAD | sed -e "1,/^\$/d" > actual &&
489+
(
490+
echo subject
491+
echo
492+
echo non-trailer line
493+
echo Myfooter: x
494+
echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
495+
) >expected &&
496+
test_cmp expected actual
497+
'
498+
463499
test_expect_success 'multiple -m' '
464500
465501
>negative &&

0 commit comments

Comments
 (0)