am: fix signoff when other trailers are present
authorPhillip Wood <phillip.wood@dunelm.org.uk>
Tue, 8 Aug 2017 10:25:33 +0000 (11:25 +0100)
committerJunio C Hamano <gitster@pobox.com>
Tue, 8 Aug 2017 19:27:23 +0000 (12:27 -0700)
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>
builtin/am.c
t/t4150-am.sh

index 593c1b5..ba9b57a 100644 (file)
@@ -1188,34 +1188,10 @@ static void NORETURN die_user_resolve(const struct am_state *state)
  */
 static void am_append_signoff(struct am_state *state)
 {
  */
 static void am_append_signoff(struct am_state *state)
 {
-       char *cp;
-       struct strbuf mine = STRBUF_INIT;
        struct strbuf sb = STRBUF_INIT;
 
        strbuf_attach(&sb, state->msg, state->msg_len, state->msg_len);
        struct strbuf sb = STRBUF_INIT;
 
        strbuf_attach(&sb, state->msg, state->msg_len, state->msg_len);
-
-       /* our sign-off */
-       strbuf_addf(&mine, "\n%s%s\n",
-                   sign_off_header,
-                   fmt_name(getenv("GIT_COMMITTER_NAME"),
-                            getenv("GIT_COMMITTER_EMAIL")));
-
-       /* Does sb end with it already? */
-       if (mine.len < sb.len &&
-           !strcmp(mine.buf, sb.buf + sb.len - mine.len))
-               goto exit; /* no need to duplicate */
-
-       /* Does it have any Signed-off-by: in the text */
-       for (cp = sb.buf;
-            cp && *cp && (cp = strstr(cp, sign_off_header)) != NULL;
-            cp = strchr(cp, '\n')) {
-               if (sb.buf == cp || cp[-1] == '\n')
-                       break;
-       }
-
-       strbuf_addstr(&sb, mine.buf + !!cp);
-exit:
-       strbuf_release(&mine);
+       append_signoff(&sb, 0, 0);
        state->msg = strbuf_detach(&sb, &state->msg_len);
 }
 
        state->msg = strbuf_detach(&sb, &state->msg_len);
 }
 
index 44807e2..73b67b4 100755 (executable)
@@ -40,6 +40,8 @@ test_expect_success 'setup: messages' '
        dolore eu feugiat nulla facilisis at vero eros et accumsan et iusto odio
        dignissim qui blandit praesent luptatum zzril delenit augue duis dolore te
        feugait nulla facilisi.
        dolore eu feugiat nulla facilisis at vero eros et accumsan et iusto odio
        dignissim qui blandit praesent luptatum zzril delenit augue duis dolore te
        feugait nulla facilisi.
+
+       Reported-by: A N Other <a.n.other@example.com>
        EOF
 
        cat >failmail <<-\EOF &&
        EOF
 
        cat >failmail <<-\EOF &&
@@ -93,7 +95,7 @@ test_expect_success setup '
        echo world >>file &&
        git add file &&
        test_tick &&
        echo world >>file &&
        git add file &&
        test_tick &&
-       git commit -s -F msg &&
+       git commit -F msg &&
        git tag second &&
 
        git format-patch --stdout first >patch1 &&
        git tag second &&
 
        git format-patch --stdout first >patch1 &&
@@ -124,8 +126,6 @@ test_expect_success setup '
                echo "Date: $GIT_AUTHOR_DATE" &&
                echo &&
                sed -e "1,2d" msg &&
                echo "Date: $GIT_AUTHOR_DATE" &&
                echo &&
                sed -e "1,2d" msg &&
-               echo &&
-               echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" &&
                echo "---" &&
                git diff-tree --no-commit-id --stat -p second
        } >patch1-stgit.eml &&
                echo "---" &&
                git diff-tree --no-commit-id --stat -p second
        } >patch1-stgit.eml &&
@@ -144,8 +144,6 @@ test_expect_success setup '
                echo "# Parent  $_z40" &&
                cat msg &&
                echo &&
                echo "# Parent  $_z40" &&
                cat msg &&
                echo &&
-               echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" &&
-               echo &&
                git diff-tree --no-commit-id -p second
        } >patch1-hg.eml &&
 
                git diff-tree --no-commit-id -p second
        } >patch1-hg.eml &&
 
@@ -470,13 +468,15 @@ test_expect_success 'am --signoff adds Signed-off-by: line' '
        git reset --hard &&
        git checkout -b master2 first &&
        git am --signoff <patch2 &&
        git reset --hard &&
        git checkout -b master2 first &&
        git am --signoff <patch2 &&
-       printf "%s\n" "$signoff" >expected &&
-       echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" >>expected &&
-       git cat-file commit HEAD^ | grep "Signed-off-by:" >actual &&
-       test_cmp expected actual &&
-       echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" >expected &&
-       git cat-file commit HEAD | grep "Signed-off-by:" >actual &&
-       test_cmp expected actual
+       {
+               printf "third\n\nSigned-off-by: %s <%s>\n\n" \
+                       "$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL" &&
+               cat msg &&
+               printf "Signed-off-by: %s <%s>\n\n" \
+                       "$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL"
+       } >expected-log &&
+       git log --pretty=%B -2 HEAD >actual &&
+       test_cmp expected-log actual
 '
 
 test_expect_success 'am stays in branch' '
 '
 
 test_expect_success 'am stays in branch' '
@@ -486,17 +486,60 @@ test_expect_success 'am stays in branch' '
 '
 
 test_expect_success 'am --signoff does not add Signed-off-by: line if already there' '
 '
 
 test_expect_success 'am --signoff does not add Signed-off-by: line if already there' '
-       git format-patch --stdout HEAD^ >patch3 &&
-       sed -e "/^Subject/ s,\[PATCH,Re: Re: Re: & 1/5 v2] [foo," patch3 >patch4 &&
-       rm -fr .git/rebase-apply &&
-       git reset --hard &&
-       git checkout HEAD^ &&
-       git am --signoff patch4 &&
-       git cat-file commit HEAD >actual &&
-       test $(grep -c "^Signed-off-by:" actual) -eq 1
+       git format-patch --stdout first >patch3 &&
+       git reset --hard first &&
+       git am --signoff <patch3 &&
+       git log --pretty=%B -2 HEAD >actual &&
+       test_cmp expected-log actual
+'
+
+test_expect_success 'am --signoff adds Signed-off-by: if another author is preset' '
+       NAME="A N Other" &&
+       EMAIL="a.n.other@example.com" &&
+       {
+               printf "third\n\nSigned-off-by: %s <%s>\nSigned-off-by: %s <%s>\n\n" \
+                       "$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL" \
+                       "$NAME" "$EMAIL" &&
+               cat msg &&
+               printf "Signed-off-by: %s <%s>\nSigned-off-by: %s <%s>\n\n" \
+                       "$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL" \
+                       "$NAME" "$EMAIL"
+       } >expected-log &&
+       git reset --hard first &&
+       GIT_COMMITTER_NAME="$NAME" GIT_COMMITTER_EMAIL="$EMAIL" \
+               git am --signoff <patch3 &&
+       git log --pretty=%B -2 HEAD >actual &&
+       test_cmp expected-log actual
+'
+
+test_expect_success 'am --signoff duplicates Signed-off-by: if it is not the last one' '
+       NAME="A N Other" &&
+       EMAIL="a.n.other@example.com" &&
+       {
+               printf "third\n\nSigned-off-by: %s <%s>\n\
+Signed-off-by: %s <%s>\nSigned-off-by: %s <%s>\n\n" \
+                       "$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL" \
+                       "$NAME" "$EMAIL" \
+                       "$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL" &&
+               cat msg &&
+               printf "Signed-off-by: %s <%s>\nSigned-off-by: %s <%s>\n\
+Signed-off-by: %s <%s>\n\n" \
+                       "$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL" \
+                       "$NAME" "$EMAIL" \
+                       "$GIT_COMMITTER_NAME" "$GIT_COMMITTER_EMAIL"
+       } >expected-log &&
+       git format-patch --stdout first >patch3 &&
+       git reset --hard first &&
+       git am --signoff <patch3 &&
+       git log --pretty=%B -2 HEAD >actual &&
+       test_cmp expected-log actual
 '
 
 test_expect_success 'am without --keep removes Re: and [PATCH] stuff' '
 '
 
 test_expect_success 'am without --keep removes Re: and [PATCH] stuff' '
+       git format-patch --stdout HEAD^ >tmp &&
+       sed -e "/^Subject/ s,\[PATCH,Re: Re: Re: & 1/5 v2] [foo," tmp >patch4 &&
+       git reset --hard HEAD^ &&
+       git am <patch4 &&
        git rev-parse HEAD >expected &&
        git rev-parse master2 >actual &&
        test_cmp expected actual
        git rev-parse HEAD >expected &&
        git rev-parse master2 >actual &&
        test_cmp expected actual