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>

Phillip Wood committed Aug 8, 2017 at 11:25 UTC 735285b403ee2b09c03d31f2dc8cc95a2d471ead
2 files changed +64 -45
builtin/am.c
+1 -25
@@ -1188,34 +1188,10 @@ static void NORETURN die_user_resolve(const struct am_state *state)
1188 */
1189 static void am_append_signoff(struct am_state *state)
1190 {
1191 - char *cp;
1192 - struct strbuf mine = STRBUF_INIT;
1191 struct strbuf sb = STRBUF_INIT;
1192
1193 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);
1195 state->msg = strbuf_detach(&sb, &state->msg_len);
1196 }
1197
t/t4150-am.sh
+63 -20
@@ -40,6 +40,8 @@ test_expect_success 'setup: messages' '
40 dolore eu feugiat nulla facilisis at vero eros et accumsan et iusto odio
41 dignissim qui blandit praesent luptatum zzril delenit augue duis dolore te
42 feugait nulla facilisi.
43 +
44 + Reported-by: A N Other <a.n.other@example.com>
45 EOF
46
47 cat >failmail <<-\EOF &&
@@ -93,7 +95,7 @@ test_expect_success setup '
95 echo world >>file &&
96 git add file &&
97 test_tick &&
96 - git commit -s -F msg &&
98 + git commit -F msg &&
99 git tag second &&
100
101 git format-patch --stdout first >patch1 &&
@@ -124,8 +126,6 @@ test_expect_success setup '
126 echo "Date: $GIT_AUTHOR_DATE" &&
127 echo &&
128 sed -e "1,2d" msg &&
127 - echo &&
128 - echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" &&
129 echo "---" &&
130 git diff-tree --no-commit-id --stat -p second
131 } >patch1-stgit.eml &&
@@ -144,8 +144,6 @@ test_expect_success setup '
144 echo "# Parent $_z40" &&
145 cat msg &&
146 echo &&
147 - echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" &&
148 - echo &&
147 git diff-tree --no-commit-id -p second
148 } >patch1-hg.eml &&
149
@@ -470,13 +468,15 @@ test_expect_success 'am --signoff adds Signed-off-by: line' '
468 git reset --hard &&
469 git checkout -b master2 first &&
470 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
480 '
481
482 test_expect_success 'am stays in branch' '
@@ -486,17 +486,60 @@ test_expect_success 'am stays in branch' '
486 '
487
488 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
536 '
537
538 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 &&
543 git rev-parse HEAD >expected &&
544 git rev-parse master2 >actual &&
545 test_cmp expected actual