Skip to content

Commit 735285b

Browse files
phillipwoodgitster
phillipwood
authored and
gitster
committed
am: fix signoff when other trailers are present
If there was no 'Signed-off-by:' trailer but another trailer such as 'Reported-by:' then 'git am --signoff' would add a blank line between the existing trailers and the added 'Signed-off-by:' line. e.g. Rebase accepts '--rerere-autoupdate' as an option but only honors it if '-m' is also given. Fix it for a non-interactive rebase by passing on the option to 'git am' and 'git cherry-pick'. Reported-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Fix by using the code provided for this purpose in sequencer.c. Change the tests so that they check the formatting of the 'Signed-off-by:' lines rather than just grepping for them. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent cf8899d commit 735285b

File tree

2 files changed

+64
-45
lines changed

2 files changed

+64
-45
lines changed

‎builtin/am.c‎

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1188,34 +1188,10 @@ static void NORETURN die_user_resolve(const struct am_state *state)
11881188
*/
11891189
static void am_append_signoff(struct am_state *state)
11901190
{
1191-
char *cp;
1192-
struct strbuf mine = STRBUF_INIT;
11931191
struct strbuf sb = STRBUF_INIT;
11941192

11951193
strbuf_attach(&sb, state->msg, state->msg_len, state->msg_len);
1196-
1197-
/* our sign-off */
1198-
strbuf_addf(&mine, "\n%s%s\n",
1199-
sign_off_header,
1200-
fmt_name(getenv("GIT_COMMITTER_NAME"),
1201-
getenv("GIT_COMMITTER_EMAIL")));
1202-
1203-
/* Does sb end with it already? */
1204-
if (mine.len < sb.len &&
1205-
!strcmp(mine.buf, sb.buf + sb.len - mine.len))
1206-
goto exit; /* no need to duplicate */
1207-
1208-
/* Does it have any Signed-off-by: in the text */
1209-
for (cp = sb.buf;
1210-
cp && *cp && (cp = strstr(cp, sign_off_header)) != NULL;
1211-
cp = strchr(cp, '\n')) {
1212-
if (sb.buf == cp || cp[-1] == '\n')
1213-
break;
1214-
}
1215-
1216-
strbuf_addstr(&sb, mine.buf + !!cp);
1217-
exit:
1218-
strbuf_release(&mine);
1194+
append_signoff(&sb, 0, 0);
12191195
state->msg = strbuf_detach(&sb, &state->msg_len);
12201196
}
12211197

‎t/t4150-am.sh‎

Lines changed: 63 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ test_expect_success 'setup: messages' '
4040
dolore eu feugiat nulla facilisis at vero eros et accumsan et iusto odio
4141
dignissim qui blandit praesent luptatum zzril delenit augue duis dolore te
4242
feugait nulla facilisi.
43+
44+
Reported-by: A N Other <a.n.other@example.com>
4345
EOF
4446
4547
cat >failmail <<-\EOF &&
@@ -93,7 +95,7 @@ test_expect_success setup '
9395
echo world >>file &&
9496
git add file &&
9597
test_tick &&
96-
git commit -s -F msg &&
98+
git commit -F msg &&
9799
git tag second &&
98100
99101
git format-patch --stdout first >patch1 &&
@@ -124,8 +126,6 @@ test_expect_success setup '
124126
echo "Date: $GIT_AUTHOR_DATE" &&
125127
echo &&
126128
sed -e "1,2d" msg &&
127-
echo &&
128-
echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" &&
129129
echo "---" &&
130130
git diff-tree --no-commit-id --stat -p second
131131
} >patch1-stgit.eml &&
@@ -144,8 +144,6 @@ test_expect_success setup '
144144
echo "# Parent $_z40" &&
145145
cat msg &&
146146
echo &&
147-
echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" &&
148-
echo &&
149147
git diff-tree --no-commit-id -p second
150148
} >patch1-hg.eml &&
151149
@@ -470,13 +468,15 @@ test_expect_success 'am --signoff adds Signed-off-by: line' '
470468
git reset --hard &&
471469
git checkout -b master2 first &&
472470
git am --signoff <patch2 &&
473-
printf "%s\n" "$signoff" >expected &&
474-
echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" >>expected &&
475-
git cat-file commit HEAD^ | grep "Signed-off-by:" >actual &&
476-
test_cmp expected actual &&
477-
echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" >expected &&
478-
git cat-file commit HEAD | grep "Signed-off-by:" >actual &&
479-
test_cmp expected actual
471+
{
472+
printf "third\n\nSigned-off-by: %s <%s>\n\n" \
473+
"$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL" &&
474+
cat msg &&
475+
printf "Signed-off-by: %s <%s>\n\n" \
476+
"$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL"
477+
} >expected-log &&
478+
git log --pretty=%B -2 HEAD >actual &&
479+
test_cmp expected-log actual
480480
'
481481

482482
test_expect_success 'am stays in branch' '
@@ -486,17 +486,60 @@ test_expect_success 'am stays in branch' '
486486
'
487487

488488
test_expect_success 'am --signoff does not add Signed-off-by: line if already there' '
489-
git format-patch --stdout HEAD^ >patch3 &&
490-
sed -e "/^Subject/ s,\[PATCH,Re: Re: Re: & 1/5 v2] [foo," patch3 >patch4 &&
491-
rm -fr .git/rebase-apply &&
492-
git reset --hard &&
493-
git checkout HEAD^ &&
494-
git am --signoff patch4 &&
495-
git cat-file commit HEAD >actual &&
496-
test $(grep -c "^Signed-off-by:" actual) -eq 1
489+
git format-patch --stdout first >patch3 &&
490+
git reset --hard first &&
491+
git am --signoff <patch3 &&
492+
git log --pretty=%B -2 HEAD >actual &&
493+
test_cmp expected-log actual
494+
'
495+
496+
test_expect_success 'am --signoff adds Signed-off-by: if another author is preset' '
497+
NAME="A N Other" &&
498+
EMAIL="a.n.other@example.com" &&
499+
{
500+
printf "third\n\nSigned-off-by: %s <%s>\nSigned-off-by: %s <%s>\n\n" \
501+
"$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL" \
502+
"$NAME" "$EMAIL" &&
503+
cat msg &&
504+
printf "Signed-off-by: %s <%s>\nSigned-off-by: %s <%s>\n\n" \
505+
"$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL" \
506+
"$NAME" "$EMAIL"
507+
} >expected-log &&
508+
git reset --hard first &&
509+
GIT_COMMITTER_NAME="$NAME" GIT_COMMITTER_EMAIL="$EMAIL" \
510+
git am --signoff <patch3 &&
511+
git log --pretty=%B -2 HEAD >actual &&
512+
test_cmp expected-log actual
513+
'
514+
515+
test_expect_success 'am --signoff duplicates Signed-off-by: if it is not the last one' '
516+
NAME="A N Other" &&
517+
EMAIL="a.n.other@example.com" &&
518+
{
519+
printf "third\n\nSigned-off-by: %s <%s>\n\
520+
Signed-off-by: %s <%s>\nSigned-off-by: %s <%s>\n\n" \
521+
"$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL" \
522+
"$NAME" "$EMAIL" \
523+
"$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL" &&
524+
cat msg &&
525+
printf "Signed-off-by: %s <%s>\nSigned-off-by: %s <%s>\n\
526+
Signed-off-by: %s <%s>\n\n" \
527+
"$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL" \
528+
"$NAME" "$EMAIL" \
529+
"$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL"
530+
} >expected-log &&
531+
git format-patch --stdout first >patch3 &&
532+
git reset --hard first &&
533+
git am --signoff <patch3 &&
534+
git log --pretty=%B -2 HEAD >actual &&
535+
test_cmp expected-log actual
497536
'
498537

499538
test_expect_success 'am without --keep removes Re: and [PATCH] stuff' '
539+
git format-patch --stdout HEAD^ >tmp &&
540+
sed -e "/^Subject/ s,\[PATCH,Re: Re: Re: & 1/5 v2] [foo," tmp >patch4 &&
541+
git reset --hard HEAD^ &&
542+
git am <patch4 &&
500543
git rev-parse HEAD >expected &&
501544
git rev-parse master2 >actual &&
502545
test_cmp expected actual

0 commit comments

Comments
 (0)