{"thread":{"id":"46525","subject":"[PATCH v1] am: fix signoff when other trailers are present","startedAt":"2017-08-07T10:30:20Z","lastAt":"2017-08-12T15:55:51Z","messageCount":8,"participants":["Phillip Wood","Junio C Hamano","Jonathan Tan"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"325674","messageId":"20170807102929.25151-1-phillip.wood@talktalk.net","threadId":"46525","inReplyTo":null,"subject":"[PATCH v1] am: fix signoff when other trailers are present","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2017-08-07T10:29:29Z","receivedAt":"2017-08-07T10:30:20Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nIf there was no 'Signed-off-by:' trailer but another trailer such as\n'Reported-by:' then 'git am --signoff' would add a blank line between\nthe existing trailers and the added 'Signed-off-by:' line. e.g.\n\n    Rebase accepts '--rerere-autoupdate' as an option but only honors\n    it if '-m' is also given. Fix it for a non-interactive rebase by\n    passing on the option to 'git am' and 'git cherry-pick'.\n\n    Reported-by: Junio C Hamano <gitster@pobox.com>\n\n    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nFix by using the code provided for this purpose in sequencer.c.\nChange the tests so that they check the formatting of the\n'Signed-off-by:' lines rather than just grepping for them.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\nI'm not sure if this should be calling ignore_non_trailer() or not -\ngit commit does but git cherry-pick does not. This follows commit and\ncherry-pick in ignoring the value of trailer.ifExists for the signoff.\nI'm a bit surprised they do that - is it correct?\n\n builtin/am.c  | 26 +------------------\n t/t4150-am.sh | 83 +++++++++++++++++++++++++++++++++++++++++++++--------------\n 2 files changed, 64 insertions(+), 45 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex c973bd96dcb5d630d56e935733bfa4530ccd2872..f88d47c9d956f8e9d044cb1a4afa85c80f0c17cb 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -1181,34 +1181,10 @@ static void NORETURN die_user_resolve(const struct am_state *state)\n  */\n static void am_append_signoff(struct am_state *state)\n {\n-\tchar *cp;\n-\tstruct strbuf mine = STRBUF_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n \n \tstrbuf_attach(&sb, state->msg, state->msg_len, state->msg_len);\n-\n-\t/* our sign-off */\n-\tstrbuf_addf(&mine, \"\\n%s%s\\n\",\n-\t\t    sign_off_header,\n-\t\t    fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n-\t\t\t     getenv(\"GIT_COMMITTER_EMAIL\")));\n-\n-\t/* Does sb end with it already? */\n-\tif (mine.len < sb.len &&\n-\t    !strcmp(mine.buf, sb.buf + sb.len - mine.len))\n-\t\tgoto exit; /* no need to duplicate */\n-\n-\t/* Does it have any Signed-off-by: in the text */\n-\tfor (cp = sb.buf;\n-\t     cp && *cp && (cp = strstr(cp, sign_off_header)) != NULL;\n-\t     cp = strchr(cp, '\\n')) {\n-\t\tif (sb.buf == cp || cp[-1] == '\\n')\n-\t\t\tbreak;\n-\t}\n-\n-\tstrbuf_addstr(&sb, mine.buf + !!cp);\n-exit:\n-\tstrbuf_release(&mine);\n+\tappend_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0);\n \tstate->msg = strbuf_detach(&sb, &state->msg_len);\n }\n \ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex 44807e218d7016f58bd41b89af71104a37f31a8b..73b67b4280b99e0328e201e6b69c3d88b766ea84 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -40,6 +40,8 @@ test_expect_success 'setup: messages' '\n \tdolore eu feugiat nulla facilisis at vero eros et accumsan et iusto odio\n \tdignissim qui blandit praesent luptatum zzril delenit augue duis dolore te\n \tfeugait nulla facilisi.\n+\n+\tReported-by: A N Other <a.n.other@example.com>\n \tEOF\n \n \tcat >failmail <<-\\EOF &&\n@@ -93,7 +95,7 @@ test_expect_success setup '\n \techo world >>file &&\n \tgit add file &&\n \ttest_tick &&\n-\tgit commit -s -F msg &&\n+\tgit commit -F msg &&\n \tgit tag second &&\n \n \tgit format-patch --stdout first >patch1 &&\n@@ -124,8 +126,6 @@ test_expect_success setup '\n \t\techo \"Date: $GIT_AUTHOR_DATE\" &&\n \t\techo &&\n \t\tsed -e \"1,2d\" msg &&\n-\t\techo &&\n-\t\techo \"Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\" &&\n \t\techo \"---\" &&\n \t\tgit diff-tree --no-commit-id --stat -p second\n \t} >patch1-stgit.eml &&\n@@ -144,8 +144,6 @@ test_expect_success setup '\n \t\techo \"# Parent  $_z40\" &&\n \t\tcat msg &&\n \t\techo &&\n-\t\techo \"Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\" &&\n-\t\techo &&\n \t\tgit diff-tree --no-commit-id -p second\n \t} >patch1-hg.eml &&\n \n@@ -470,13 +468,15 @@ test_expect_success 'am --signoff adds Signed-off-by: line' '\n \tgit reset --hard &&\n \tgit checkout -b master2 first &&\n \tgit am --signoff <patch2 &&\n-\tprintf \"%s\\n\" \"$signoff\" >expected &&\n-\techo \"Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\" >>expected &&\n-\tgit cat-file commit HEAD^ | grep \"Signed-off-by:\" >actual &&\n-\ttest_cmp expected actual &&\n-\techo \"Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\" >expected &&\n-\tgit cat-file commit HEAD | grep \"Signed-off-by:\" >actual &&\n-\ttest_cmp expected actual\n+\t{\n+\t\tprintf \"third\\n\\nSigned-off-by: %s <%s>\\n\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" &&\n+\t\tcat msg &&\n+\t\tprintf \"Signed-off-by: %s <%s>\\n\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\"\n+\t} >expected-log &&\n+\tgit log --pretty=%B -2 HEAD >actual &&\n+\ttest_cmp expected-log actual\n '\n \n test_expect_success 'am stays in branch' '\n@@ -486,17 +486,60 @@ test_expect_success 'am stays in branch' '\n '\n \n test_expect_success 'am --signoff does not add Signed-off-by: line if already there' '\n-\tgit format-patch --stdout HEAD^ >patch3 &&\n-\tsed -e \"/^Subject/ s,\\[PATCH,Re: Re: Re: & 1/5 v2] [foo,\" patch3 >patch4 &&\n-\trm -fr .git/rebase-apply &&\n-\tgit reset --hard &&\n-\tgit checkout HEAD^ &&\n-\tgit am --signoff patch4 &&\n-\tgit cat-file commit HEAD >actual &&\n-\ttest $(grep -c \"^Signed-off-by:\" actual) -eq 1\n+\tgit format-patch --stdout first >patch3 &&\n+\tgit reset --hard first &&\n+\tgit am --signoff <patch3 &&\n+\tgit log --pretty=%B -2 HEAD >actual &&\n+\ttest_cmp expected-log actual\n+'\n+\n+test_expect_success 'am --signoff adds Signed-off-by: if another author is preset' '\n+\tNAME=\"A N Other\" &&\n+\tEMAIL=\"a.n.other@example.com\" &&\n+\t{\n+\t\tprintf \"third\\n\\nSigned-off-by: %s <%s>\\nSigned-off-by: %s <%s>\\n\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" \\\n+\t\t\t\"$NAME\" \"$EMAIL\" &&\n+\t\tcat msg &&\n+\t\tprintf \"Signed-off-by: %s <%s>\\nSigned-off-by: %s <%s>\\n\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" \\\n+\t\t\t\"$NAME\" \"$EMAIL\"\n+\t} >expected-log &&\n+\tgit reset --hard first &&\n+\tGIT_COMMITTER_NAME=\"$NAME\" GIT_COMMITTER_EMAIL=\"$EMAIL\" \\\n+\t\tgit am --signoff <patch3 &&\n+\tgit log --pretty=%B -2 HEAD >actual &&\n+\ttest_cmp expected-log actual\n+'\n+\n+test_expect_success 'am --signoff duplicates Signed-off-by: if it is not the last one' '\n+\tNAME=\"A N Other\" &&\n+\tEMAIL=\"a.n.other@example.com\" &&\n+\t{\n+\t\tprintf \"third\\n\\nSigned-off-by: %s <%s>\\n\\\n+Signed-off-by: %s <%s>\\nSigned-off-by: %s <%s>\\n\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" \\\n+\t\t\t\"$NAME\" \"$EMAIL\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" &&\n+\t\tcat msg &&\n+\t\tprintf \"Signed-off-by: %s <%s>\\nSigned-off-by: %s <%s>\\n\\\n+Signed-off-by: %s <%s>\\n\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" \\\n+\t\t\t\"$NAME\" \"$EMAIL\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\"\n+\t} >expected-log &&\n+\tgit format-patch --stdout first >patch3 &&\n+\tgit reset --hard first &&\n+\tgit am --signoff <patch3 &&\n+\tgit log --pretty=%B -2 HEAD >actual &&\n+\ttest_cmp expected-log actual\n '\n \n test_expect_success 'am without --keep removes Re: and [PATCH] stuff' '\n+\tgit format-patch --stdout HEAD^ >tmp &&\n+\tsed -e \"/^Subject/ s,\\[PATCH,Re: Re: Re: & 1/5 v2] [foo,\" tmp >patch4 &&\n+\tgit reset --hard HEAD^ &&\n+\tgit am <patch4 &&\n \tgit rev-parse HEAD >expected &&\n \tgit rev-parse master2 >actual &&\n \ttest_cmp expected actual\n-- \n2.13.3\n\n"},{"id":"325706","messageId":"xmqqfud3qndj.fsf@gitster.mtv.corp.google.com","threadId":"46525","inReplyTo":"20170807102929.25151-1-phillip.wood@talktalk.net","subject":"Re: [PATCH v1] am: fix signoff when other trailers are present","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-07T17:49:28Z","receivedAt":"2017-08-07T17:49:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood@talktalk.net> writes:\n\n> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n>\n> If there was no 'Signed-off-by:' trailer but another trailer such as\n> 'Reported-by:' then 'git am --signoff' would add a blank line between\n> the existing trailers and the added 'Signed-off-by:' line. e.g.\n>\n>     Rebase accepts '--rerere-autoupdate' as an option but only honors\n>     it if '-m' is also given. Fix it for a non-interactive rebase by\n>     passing on the option to 'git am' and 'git cherry-pick'.\n>\n>     Reported-by: Junio C Hamano <gitster@pobox.com>\n>\n>     Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n>\n> Fix by using the code provided for this purpose in sequencer.c.\n> Change the tests so that they check the formatting of the\n> 'Signed-off-by:' lines rather than just grepping for them.\n>\n> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n> ---\n> I'm not sure if this should be calling ignore_non_trailer() or not -\n> git commit does but git cherry-pick does not. This follows commit and\n> cherry-pick in ignoring the value of trailer.ifExists for the signoff.\n> I'm a bit surprised they do that - is it correct?\n\nThese built-in \"sign-off\" machinery long predates the \"trailer\"\nthing, so I am not surprised if they do not behave the same.  I\nvaguely recall having discussions on this earlier this year, but\ndetails escape me.  \n\nAsking Jonathan, who did a series that ends at 44dc738a (\"sequencer:\nadd newline before adding footers\", 2017-04-26), and Christian, who\nis the original contirbutor to the \"trailer\" machinery, for input.\n\n"},{"id":"325710","messageId":"20170807110817.3cf7d8ff@twelve2.svl.corp.google.com","threadId":"46525","inReplyTo":"xmqqfud3qndj.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v1] am: fix signoff when other trailers are present","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-08-07T18:08:17Z","receivedAt":"2017-08-07T18:08:29Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Mon, 07 Aug 2017 10:49:28 -0700\nJunio C Hamano <gitster@pobox.com> wrote:\n\n> Phillip Wood <phillip.wood@talktalk.net> writes:\n> \n> > From: Phillip Wood <phillip.wood@dunelm.org.uk>\n> >\n> > If there was no 'Signed-off-by:' trailer but another trailer such as\n> > 'Reported-by:' then 'git am --signoff' would add a blank line between\n> > the existing trailers and the added 'Signed-off-by:' line. e.g.\n> >\n> >     Rebase accepts '--rerere-autoupdate' as an option but only honors\n> >     it if '-m' is also given. Fix it for a non-interactive rebase by\n> >     passing on the option to 'git am' and 'git cherry-pick'.\n> >\n> >     Reported-by: Junio C Hamano <gitster@pobox.com>\n> >\n> >     Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n> >\n> > Fix by using the code provided for this purpose in sequencer.c.\n> > Change the tests so that they check the formatting of the\n> > 'Signed-off-by:' lines rather than just grepping for them.\n> >\n> > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n> > ---\n> > I'm not sure if this should be calling ignore_non_trailer() or not -\n> > git commit does but git cherry-pick does not. This follows commit and\n> > cherry-pick in ignoring the value of trailer.ifExists for the signoff.\n> > I'm a bit surprised they do that - is it correct?\n> \n> These built-in \"sign-off\" machinery long predates the \"trailer\"\n> thing, so I am not surprised if they do not behave the same.  I\n> vaguely recall having discussions on this earlier this year, but\n> details escape me.  \n> \n> Asking Jonathan, who did a series that ends at 44dc738a (\"sequencer:\n> add newline before adding footers\", 2017-04-26), and Christian, who\n> is the original contirbutor to the \"trailer\" machinery, for input.\n\nRegarding ignore_non_trailer(), I believe that's because \"git commit\"\nwants to tolerate blank lines and comments after the \"real\" commit\nmessage, whereas \"git cherry-pick\" doesn't need to. As far as I can\ntell, this \"git am\" case is similar to \"git cherry-pick\".\n\nRegarding trailer.ifExists, the then existing behavior was to refrain\nfrom writing a new sign-off line only if it would be a duplicate of the\nlast one, regardless of trailer.ifExists (as Junio says, back then, the\nsign-off mechanism and the trailer mechanism were independent). I\npreserved that behavior.\n"},{"id":"325798","messageId":"532d2ea6-0562-e9d0-b1a6-43b3805854c8@talktalk.net","threadId":"46525","inReplyTo":"20170807110817.3cf7d8ff@twelve2.svl.corp.google.com","subject":"Re: [PATCH v1] am: fix signoff when other trailers are present","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2017-08-08T10:19:13Z","receivedAt":"2017-08-08T10:19:24Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 07/08/17 19:08, Jonathan Tan wrote:\n> On Mon, 07 Aug 2017 10:49:28 -0700\n> Junio C Hamano <gitster@pobox.com> wrote:\n> \n>> Phillip Wood <phillip.wood@talktalk.net> writes:\n>>\n>>> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n>>>\n>>> If there was no 'Signed-off-by:' trailer but another trailer such as\n>>> 'Reported-by:' then 'git am --signoff' would add a blank line between\n>>> the existing trailers and the added 'Signed-off-by:' line. e.g.\n>>>\n>>>     Rebase accepts '--rerere-autoupdate' as an option but only honors\n>>>     it if '-m' is also given. Fix it for a non-interactive rebase by\n>>>     passing on the option to 'git am' and 'git cherry-pick'.\n>>>\n>>>     Reported-by: Junio C Hamano <gitster@pobox.com>\n>>>\n>>>     Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n>>>\n>>> Fix by using the code provided for this purpose in sequencer.c.\n>>> Change the tests so that they check the formatting of the\n>>> 'Signed-off-by:' lines rather than just grepping for them.\n>>>\n>>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n>>> ---\n>>> I'm not sure if this should be calling ignore_non_trailer() or not -\n>>> git commit does but git cherry-pick does not. This follows commit and\n>>> cherry-pick in ignoring the value of trailer.ifExists for the signoff.\n>>> I'm a bit surprised they do that - is it correct?\n>>\n>> These built-in \"sign-off\" machinery long predates the \"trailer\"\n>> thing, so I am not surprised if they do not behave the same.  I\n>> vaguely recall having discussions on this earlier this year, but\n>> details escape me.  \n>>\n>> Asking Jonathan, who did a series that ends at 44dc738a (\"sequencer:\n>> add newline before adding footers\", 2017-04-26), and Christian, who\n>> is the original contirbutor to the \"trailer\" machinery, for input.\n> \n> Regarding ignore_non_trailer(), I believe that's because \"git commit\"\n> wants to tolerate blank lines and comments after the \"real\" commit\n> message, whereas \"git cherry-pick\" doesn't need to. As far as I can\n> tell, this \"git am\" case is similar to \"git cherry-pick\".\n> \n> Regarding trailer.ifExists, the then existing behavior was to refrain\n> from writing a new sign-off line only if it would be a duplicate of the\n> last one, regardless of trailer.ifExists (as Junio says, back then, the\n> sign-off mechanism and the trailer mechanism were independent). I\n> preserved that behavior.\n> \nHi Jonathan\n\nThanks for the background. I'll remove the call to ignore_non_trailer()\n"},{"id":"325799","messageId":"866e9789-73f7-697d-a68c-c536b0e9b90d@talktalk.net","threadId":"46525","inReplyTo":"xmqqfud3qndj.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v1] am: fix signoff when other trailers are present","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2017-08-08T10:22:10Z","receivedAt":"2017-08-08T10:22:30Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 07/08/17 18:49, Junio C Hamano wrote:\n> Phillip Wood <phillip.wood@talktalk.net> writes:\n> \n>> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n>>\n>> If there was no 'Signed-off-by:' trailer but another trailer such as\n>> 'Reported-by:' then 'git am --signoff' would add a blank line between\n>> the existing trailers and the added 'Signed-off-by:' line. e.g.\n>>\n>>     Rebase accepts '--rerere-autoupdate' as an option but only honors\n>>     it if '-m' is also given. Fix it for a non-interactive rebase by\n>>     passing on the option to 'git am' and 'git cherry-pick'.\n>>\n>>     Reported-by: Junio C Hamano <gitster@pobox.com>\n>>\n>>     Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n>>\n>> Fix by using the code provided for this purpose in sequencer.c.\n>> Change the tests so that they check the formatting of the\n>> 'Signed-off-by:' lines rather than just grepping for them.\n>>\n>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n>> ---\n>> I'm not sure if this should be calling ignore_non_trailer() or not -\n>> git commit does but git cherry-pick does not. This follows commit and\n>> cherry-pick in ignoring the value of trailer.ifExists for the signoff.\n>> I'm a bit surprised they do that - is it correct?\n> \n> These built-in \"sign-off\" machinery long predates the \"trailer\"\n> thing, so I am not surprised if they do not behave the same.  I\n> vaguely recall having discussions on this earlier this year, but\n> details escape me.  \n\nAh, that explains it. I did a quick search on public-inbox but didn't\nturn up any obvious discussion about --signoff and trailer config. If\nthat's the behavior people are used to then lets leave it as it is\nunless there is a call for it to be changed.\n\n> Asking Jonathan, who did a series that ends at 44dc738a (\"sequencer:\n> add newline before adding footers\", 2017-04-26), and Christian, who\n> is the original contirbutor to the \"trailer\" machinery, for input.\n> \n\n"},{"id":"325800","messageId":"20170808102533.27516-1-phillip.wood@talktalk.net","threadId":"46525","inReplyTo":"20170807102929.25151-1-phillip.wood@talktalk.net","subject":"[PATCH v2] am: fix signoff when other trailers are present","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2017-08-08T10:25:33Z","receivedAt":"2017-08-08T10:26:05Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nIf there was no 'Signed-off-by:' trailer but another trailer such as\n'Reported-by:' then 'git am --signoff' would add a blank line between\nthe existing trailers and the added 'Signed-off-by:' line. e.g.\n\n    Rebase accepts '--rerere-autoupdate' as an option but only honors\n    it if '-m' is also given. Fix it for a non-interactive rebase by\n    passing on the option to 'git am' and 'git cherry-pick'.\n\n    Reported-by: Junio C Hamano <gitster@pobox.com>\n\n    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nFix by using the code provided for this purpose in sequencer.c.\nChange the tests so that they check the formatting of the\n'Signed-off-by:' lines rather than just grepping for them.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\nI've removed the call to ignore_non_trailer(), otherwise this is\nunchanged from v1\n\n builtin/am.c  | 26 +------------------\n t/t4150-am.sh | 83 +++++++++++++++++++++++++++++++++++++++++++++--------------\n 2 files changed, 64 insertions(+), 45 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex c973bd96dcb5d630d56e935733bfa4530ccd2872..3aaef59676452fd2e7c6d4a375dc7c95558744c6 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -1181,34 +1181,10 @@ static void NORETURN die_user_resolve(const struct am_state *state)\n  */\n static void am_append_signoff(struct am_state *state)\n {\n-\tchar *cp;\n-\tstruct strbuf mine = STRBUF_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n \n \tstrbuf_attach(&sb, state->msg, state->msg_len, state->msg_len);\n-\n-\t/* our sign-off */\n-\tstrbuf_addf(&mine, \"\\n%s%s\\n\",\n-\t\t    sign_off_header,\n-\t\t    fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n-\t\t\t     getenv(\"GIT_COMMITTER_EMAIL\")));\n-\n-\t/* Does sb end with it already? */\n-\tif (mine.len < sb.len &&\n-\t    !strcmp(mine.buf, sb.buf + sb.len - mine.len))\n-\t\tgoto exit; /* no need to duplicate */\n-\n-\t/* Does it have any Signed-off-by: in the text */\n-\tfor (cp = sb.buf;\n-\t     cp && *cp && (cp = strstr(cp, sign_off_header)) != NULL;\n-\t     cp = strchr(cp, '\\n')) {\n-\t\tif (sb.buf == cp || cp[-1] == '\\n')\n-\t\t\tbreak;\n-\t}\n-\n-\tstrbuf_addstr(&sb, mine.buf + !!cp);\n-exit:\n-\tstrbuf_release(&mine);\n+\tappend_signoff(&sb, 0, 0);\n \tstate->msg = strbuf_detach(&sb, &state->msg_len);\n }\n \ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex 44807e218d7016f58bd41b89af71104a37f31a8b..73b67b4280b99e0328e201e6b69c3d88b766ea84 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -40,6 +40,8 @@ test_expect_success 'setup: messages' '\n \tdolore eu feugiat nulla facilisis at vero eros et accumsan et iusto odio\n \tdignissim qui blandit praesent luptatum zzril delenit augue duis dolore te\n \tfeugait nulla facilisi.\n+\n+\tReported-by: A N Other <a.n.other@example.com>\n \tEOF\n \n \tcat >failmail <<-\\EOF &&\n@@ -93,7 +95,7 @@ test_expect_success setup '\n \techo world >>file &&\n \tgit add file &&\n \ttest_tick &&\n-\tgit commit -s -F msg &&\n+\tgit commit -F msg &&\n \tgit tag second &&\n \n \tgit format-patch --stdout first >patch1 &&\n@@ -124,8 +126,6 @@ test_expect_success setup '\n \t\techo \"Date: $GIT_AUTHOR_DATE\" &&\n \t\techo &&\n \t\tsed -e \"1,2d\" msg &&\n-\t\techo &&\n-\t\techo \"Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\" &&\n \t\techo \"---\" &&\n \t\tgit diff-tree --no-commit-id --stat -p second\n \t} >patch1-stgit.eml &&\n@@ -144,8 +144,6 @@ test_expect_success setup '\n \t\techo \"# Parent  $_z40\" &&\n \t\tcat msg &&\n \t\techo &&\n-\t\techo \"Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\" &&\n-\t\techo &&\n \t\tgit diff-tree --no-commit-id -p second\n \t} >patch1-hg.eml &&\n \n@@ -470,13 +468,15 @@ test_expect_success 'am --signoff adds Signed-off-by: line' '\n \tgit reset --hard &&\n \tgit checkout -b master2 first &&\n \tgit am --signoff <patch2 &&\n-\tprintf \"%s\\n\" \"$signoff\" >expected &&\n-\techo \"Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\" >>expected &&\n-\tgit cat-file commit HEAD^ | grep \"Signed-off-by:\" >actual &&\n-\ttest_cmp expected actual &&\n-\techo \"Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\" >expected &&\n-\tgit cat-file commit HEAD | grep \"Signed-off-by:\" >actual &&\n-\ttest_cmp expected actual\n+\t{\n+\t\tprintf \"third\\n\\nSigned-off-by: %s <%s>\\n\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" &&\n+\t\tcat msg &&\n+\t\tprintf \"Signed-off-by: %s <%s>\\n\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\"\n+\t} >expected-log &&\n+\tgit log --pretty=%B -2 HEAD >actual &&\n+\ttest_cmp expected-log actual\n '\n \n test_expect_success 'am stays in branch' '\n@@ -486,17 +486,60 @@ test_expect_success 'am stays in branch' '\n '\n \n test_expect_success 'am --signoff does not add Signed-off-by: line if already there' '\n-\tgit format-patch --stdout HEAD^ >patch3 &&\n-\tsed -e \"/^Subject/ s,\\[PATCH,Re: Re: Re: & 1/5 v2] [foo,\" patch3 >patch4 &&\n-\trm -fr .git/rebase-apply &&\n-\tgit reset --hard &&\n-\tgit checkout HEAD^ &&\n-\tgit am --signoff patch4 &&\n-\tgit cat-file commit HEAD >actual &&\n-\ttest $(grep -c \"^Signed-off-by:\" actual) -eq 1\n+\tgit format-patch --stdout first >patch3 &&\n+\tgit reset --hard first &&\n+\tgit am --signoff <patch3 &&\n+\tgit log --pretty=%B -2 HEAD >actual &&\n+\ttest_cmp expected-log actual\n+'\n+\n+test_expect_success 'am --signoff adds Signed-off-by: if another author is preset' '\n+\tNAME=\"A N Other\" &&\n+\tEMAIL=\"a.n.other@example.com\" &&\n+\t{\n+\t\tprintf \"third\\n\\nSigned-off-by: %s <%s>\\nSigned-off-by: %s <%s>\\n\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" \\\n+\t\t\t\"$NAME\" \"$EMAIL\" &&\n+\t\tcat msg &&\n+\t\tprintf \"Signed-off-by: %s <%s>\\nSigned-off-by: %s <%s>\\n\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" \\\n+\t\t\t\"$NAME\" \"$EMAIL\"\n+\t} >expected-log &&\n+\tgit reset --hard first &&\n+\tGIT_COMMITTER_NAME=\"$NAME\" GIT_COMMITTER_EMAIL=\"$EMAIL\" \\\n+\t\tgit am --signoff <patch3 &&\n+\tgit log --pretty=%B -2 HEAD >actual &&\n+\ttest_cmp expected-log actual\n+'\n+\n+test_expect_success 'am --signoff duplicates Signed-off-by: if it is not the last one' '\n+\tNAME=\"A N Other\" &&\n+\tEMAIL=\"a.n.other@example.com\" &&\n+\t{\n+\t\tprintf \"third\\n\\nSigned-off-by: %s <%s>\\n\\\n+Signed-off-by: %s <%s>\\nSigned-off-by: %s <%s>\\n\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" \\\n+\t\t\t\"$NAME\" \"$EMAIL\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" &&\n+\t\tcat msg &&\n+\t\tprintf \"Signed-off-by: %s <%s>\\nSigned-off-by: %s <%s>\\n\\\n+Signed-off-by: %s <%s>\\n\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" \\\n+\t\t\t\"$NAME\" \"$EMAIL\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\"\n+\t} >expected-log &&\n+\tgit format-patch --stdout first >patch3 &&\n+\tgit reset --hard first &&\n+\tgit am --signoff <patch3 &&\n+\tgit log --pretty=%B -2 HEAD >actual &&\n+\ttest_cmp expected-log actual\n '\n \n test_expect_success 'am without --keep removes Re: and [PATCH] stuff' '\n+\tgit format-patch --stdout HEAD^ >tmp &&\n+\tsed -e \"/^Subject/ s,\\[PATCH,Re: Re: Re: & 1/5 v2] [foo,\" tmp >patch4 &&\n+\tgit reset --hard HEAD^ &&\n+\tgit am <patch4 &&\n \tgit rev-parse HEAD >expected &&\n \tgit rev-parse master2 >actual &&\n \ttest_cmp expected actual\n-- \n2.13.3\n\n"},{"id":"325846","messageId":"xmqqlgmtc0m2.fsf@gitster.mtv.corp.google.com","threadId":"46525","inReplyTo":"20170808102533.27516-1-phillip.wood@talktalk.net","subject":"Re: [PATCH v2] am: fix signoff when other trailers are present","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-08T19:37:09Z","receivedAt":"2017-08-08T19:37:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood@talktalk.net> writes:\n\n>  test_expect_success 'am --signoff does not add Signed-off-by: line if already there' '\n> -\tgit format-patch --stdout HEAD^ >patch3 &&\n> -\tsed -e \"/^Subject/ s,\\[PATCH,Re: Re: Re: & 1/5 v2] [foo,\" patch3 >patch4 &&\n> -\trm -fr .git/rebase-apply &&\n> -\tgit reset --hard &&\n> -\tgit checkout HEAD^ &&\n> -\tgit am --signoff patch4 &&\n> -\tgit cat-file commit HEAD >actual &&\n> -\ttest $(grep -c \"^Signed-off-by:\" actual) -eq 1\n> +\tgit format-patch --stdout first >patch3 &&\n> +\tgit reset --hard first &&\n> +\tgit am --signoff <patch3 &&\n> +\tgit log --pretty=%B -2 HEAD >actual &&\n> +\ttest_cmp expected-log actual\n> +'\n> +\n> +test_expect_success 'am --signoff adds Signed-off-by: if another author is preset' '\n\nPresent?\n\nI think the motivation for adding this is to make sure that what the\nprevious test checks will be true only when the one we are about to\nadd is already at the end.  So perhaps the previous test needs to be\nretitled from \"if already there\" to something a bit tighter,\ne.g. \"if already at the end\"?\n\nAlso, strictly speaking, I think this isn't testing if another\nauthor is present---it is specifying what should happen when the\nlast one is not us.\n\n> +\tNAME=\"A N Other\" &&\n> +\tEMAIL=\"a.n.other@example.com\" &&\n> +\t{\n> +\t\tprintf \"third\\n\\nSigned-off-by: %s <%s>\\nSigned-off-by: %s <%s>\\n\\n\" \\\n> +\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" \\\n> +\t\t\t\"$NAME\" \"$EMAIL\" &&\n\nA \"printf\" tip: you can do\n\n\tprintf \"third\\n\\n\"\n\tprintf \"S-o-b: %s <%s>\\n\" A B C D\n\nto get two lines of the latter.  That would clarify what the next\ntest does which wants to add three of them.\n\n> +\t\tcat msg &&\n> +\t\tprintf \"Signed-off-by: %s <%s>\\nSigned-off-by: %s <%s>\\n\\n\" \\\n> +\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" \\\n> +\t\t\t\"$NAME\" \"$EMAIL\"\n> +\t} >expected-log &&\n> +\tgit reset --hard first &&\n> +\tGIT_COMMITTER_NAME=\"$NAME\" GIT_COMMITTER_EMAIL=\"$EMAIL\" \\\n> +\t\tgit am --signoff <patch3 &&\n> +\tgit log --pretty=%B -2 HEAD >actual &&\n> +\ttest_cmp expected-log actual\n> +'\n> +\n> +test_expect_success 'am --signoff duplicates Signed-off-by: if it is not the last one' '\n> +\tNAME=\"A N Other\" &&\n> +\tEMAIL=\"a.n.other@example.com\" &&\n> +\t{\n> +\t\tprintf \"third\\n\\nSigned-off-by: %s <%s>\\n\\\n> +Signed-off-by: %s <%s>\\nSigned-off-by: %s <%s>\\n\\n\" \\\n> +\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" \\\n> +\t\t\t\"$NAME\" \"$EMAIL\" \\\n> +\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" &&\n> +\t\tcat msg &&\n> +\t\tprintf \"Signed-off-by: %s <%s>\\nSigned-off-by: %s <%s>\\n\\\n> +Signed-off-by: %s <%s>\\n\\n\" \\\n> +\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" \\\n> +\t\t\t\"$NAME\" \"$EMAIL\" \\\n> +\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\"\n> +\t} >expected-log &&\n> +\tgit format-patch --stdout first >patch3 &&\n> +\tgit reset --hard first &&\n> +\tgit am --signoff <patch3 &&\n> +\tgit log --pretty=%B -2 HEAD >actual &&\n> +\ttest_cmp expected-log actual\n>  '\n>  \n>  test_expect_success 'am without --keep removes Re: and [PATCH] stuff' '\n> +\tgit format-patch --stdout HEAD^ >tmp &&\n> +\tsed -e \"/^Subject/ s,\\[PATCH,Re: Re: Re: & 1/5 v2] [foo,\" tmp >patch4 &&\n> +\tgit reset --hard HEAD^ &&\n> +\tgit am <patch4 &&\n>  \tgit rev-parse HEAD >expected &&\n>  \tgit rev-parse master2 >actual &&\n>  \ttest_cmp expected actual\n"},{"id":"326223","messageId":"20170812155530.22434-1-phillip.wood@talktalk.net","threadId":"46525","inReplyTo":"20170807102929.25151-1-phillip.wood@talktalk.net","subject":"[PATCH v3] am: fix signoff when other trailers are present","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2017-08-12T15:55:30Z","receivedAt":"2017-08-12T15:55:51Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nIf there was no 'Signed-off-by:' trailer but another trailer such as\n'Reported-by:' then 'git am --signoff' would add a blank line between\nthe existing trailers and the added 'Signed-off-by:' line. e.g.\n\n    Rebase accepts '--rerere-autoupdate' as an option but only honors\n    it if '-m' is also given. Fix it for a non-interactive rebase by\n    passing on the option to 'git am' and 'git cherry-pick'.\n\n    Reported-by: Junio C Hamano <gitster@pobox.com>\n\n    Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nFix by using the code provided for this purpose in sequencer.c.\nChange the tests so that they check the formatting of the\n'Signed-off-by:' lines rather than just grepping for them.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\nI've changed the test names and cleaned up the printf in the tests as\nJunio suggested. Otherwise this is unchanged from the previous version.\n\n builtin/am.c  | 26 +----------------\n t/t4150-am.sh | 89 +++++++++++++++++++++++++++++++++++++++++++++--------------\n 2 files changed, 69 insertions(+), 46 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex c973bd96dcb5d630d56e935733bfa4530ccd2872..3aaef59676452fd2e7c6d4a375dc7c95558744c6 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -1181,34 +1181,10 @@ static void NORETURN die_user_resolve(const struct am_state *state)\n  */\n static void am_append_signoff(struct am_state *state)\n {\n-\tchar *cp;\n-\tstruct strbuf mine = STRBUF_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n \n \tstrbuf_attach(&sb, state->msg, state->msg_len, state->msg_len);\n-\n-\t/* our sign-off */\n-\tstrbuf_addf(&mine, \"\\n%s%s\\n\",\n-\t\t    sign_off_header,\n-\t\t    fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n-\t\t\t     getenv(\"GIT_COMMITTER_EMAIL\")));\n-\n-\t/* Does sb end with it already? */\n-\tif (mine.len < sb.len &&\n-\t    !strcmp(mine.buf, sb.buf + sb.len - mine.len))\n-\t\tgoto exit; /* no need to duplicate */\n-\n-\t/* Does it have any Signed-off-by: in the text */\n-\tfor (cp = sb.buf;\n-\t     cp && *cp && (cp = strstr(cp, sign_off_header)) != NULL;\n-\t     cp = strchr(cp, '\\n')) {\n-\t\tif (sb.buf == cp || cp[-1] == '\\n')\n-\t\t\tbreak;\n-\t}\n-\n-\tstrbuf_addstr(&sb, mine.buf + !!cp);\n-exit:\n-\tstrbuf_release(&mine);\n+\tappend_signoff(&sb, 0, 0);\n \tstate->msg = strbuf_detach(&sb, &state->msg_len);\n }\n \ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex 44807e218d7016f58bd41b89af71104a37f31a8b..ae23adfb7548a4a28aaa52e689094f727f0f4cf2 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -40,6 +40,8 @@ test_expect_success 'setup: messages' '\n \tdolore eu feugiat nulla facilisis at vero eros et accumsan et iusto odio\n \tdignissim qui blandit praesent luptatum zzril delenit augue duis dolore te\n \tfeugait nulla facilisi.\n+\n+\tReported-by: A N Other <a.n.other@example.com>\n \tEOF\n \n \tcat >failmail <<-\\EOF &&\n@@ -93,7 +95,7 @@ test_expect_success setup '\n \techo world >>file &&\n \tgit add file &&\n \ttest_tick &&\n-\tgit commit -s -F msg &&\n+\tgit commit -F msg &&\n \tgit tag second &&\n \n \tgit format-patch --stdout first >patch1 &&\n@@ -124,8 +126,6 @@ test_expect_success setup '\n \t\techo \"Date: $GIT_AUTHOR_DATE\" &&\n \t\techo &&\n \t\tsed -e \"1,2d\" msg &&\n-\t\techo &&\n-\t\techo \"Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\" &&\n \t\techo \"---\" &&\n \t\tgit diff-tree --no-commit-id --stat -p second\n \t} >patch1-stgit.eml &&\n@@ -144,8 +144,6 @@ test_expect_success setup '\n \t\techo \"# Parent  $_z40\" &&\n \t\tcat msg &&\n \t\techo &&\n-\t\techo \"Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\" &&\n-\t\techo &&\n \t\tgit diff-tree --no-commit-id -p second\n \t} >patch1-hg.eml &&\n \n@@ -470,13 +468,15 @@ test_expect_success 'am --signoff adds Signed-off-by: line' '\n \tgit reset --hard &&\n \tgit checkout -b master2 first &&\n \tgit am --signoff <patch2 &&\n-\tprintf \"%s\\n\" \"$signoff\" >expected &&\n-\techo \"Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\" >>expected &&\n-\tgit cat-file commit HEAD^ | grep \"Signed-off-by:\" >actual &&\n-\ttest_cmp expected actual &&\n-\techo \"Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\" >expected &&\n-\tgit cat-file commit HEAD | grep \"Signed-off-by:\" >actual &&\n-\ttest_cmp expected actual\n+\t{\n+\t\tprintf \"third\\n\\nSigned-off-by: %s <%s>\\n\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" &&\n+\t\tcat msg &&\n+\t\tprintf \"Signed-off-by: %s <%s>\\n\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\"\n+\t} >expected-log &&\n+\tgit log --pretty=%B -2 HEAD >actual &&\n+\ttest_cmp expected-log actual\n '\n \n test_expect_success 'am stays in branch' '\n@@ -485,18 +485,65 @@ test_expect_success 'am stays in branch' '\n \ttest_cmp expected actual\n '\n \n-test_expect_success 'am --signoff does not add Signed-off-by: line if already there' '\n-\tgit format-patch --stdout HEAD^ >patch3 &&\n-\tsed -e \"/^Subject/ s,\\[PATCH,Re: Re: Re: & 1/5 v2] [foo,\" patch3 >patch4 &&\n-\trm -fr .git/rebase-apply &&\n-\tgit reset --hard &&\n-\tgit checkout HEAD^ &&\n-\tgit am --signoff patch4 &&\n-\tgit cat-file commit HEAD >actual &&\n-\ttest $(grep -c \"^Signed-off-by:\" actual) -eq 1\n+test_expect_success 'am --signoff does not add Signed-off-by: line if it matches the last trailer' '\n+\tgit format-patch --stdout first >patch3 &&\n+\tgit reset --hard first &&\n+\tgit am --signoff <patch3 &&\n+\tgit log --pretty=%B -2 HEAD >actual &&\n+\ttest_cmp expected-log actual\n+'\n+\n+test_expect_success 'am --signoff adds Signed-off-by: if it does not match the last trailer' '\n+\tNAME=\"A N Other\" &&\n+\tEMAIL=\"a.n.other@example.com\" &&\n+\t{\n+\t\tprintf \"third\\n\\n\" &&\n+\t\tprintf \"Signed-off-by: %s <%s>\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" \\\n+\t\t\t\"$NAME\" \"$EMAIL\" &&\n+\t\tprintf \"\\n\" &&\n+\t\tcat msg &&\n+\t\tprintf \"Signed-off-by: %s <%s>\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" \\\n+\t\t\t\"$NAME\" \"$EMAIL\" &&\n+\t\tprintf \"\\n\"\n+\t} >expected-log &&\n+\tgit reset --hard first &&\n+\tGIT_COMMITTER_NAME=\"$NAME\" GIT_COMMITTER_EMAIL=\"$EMAIL\" \\\n+\t\tgit am --signoff <patch3 &&\n+\tgit log --pretty=%B -2 HEAD >actual &&\n+\ttest_cmp expected-log actual\n+'\n+\n+test_expect_success 'am --signoff duplicates Signed-off-by: if it does not match the last trailer' '\n+\tNAME=\"A N Other\" &&\n+\tEMAIL=\"a.n.other@example.com\" &&\n+\t{\n+\t\tprintf \"third\\n\\n\" &&\n+\t\tprintf \"Signed-off-by: %s <%s>\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" \\\n+\t\t\t\"$NAME\" \"$EMAIL\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" &&\n+\t\tprintf \"\\n\" &&\n+\t\tcat msg &&\n+\t\tprintf \"Signed-off-by: %s <%s>\\n\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\" \\\n+\t\t\t\"$NAME\" \"$EMAIL\" \\\n+\t\t\t\"$GIT_COMMITTER_NAME\" \"$GIT_COMMITTER_EMAIL\"\n+\t\tprintf \"\\n\"\n+\t} >expected-log &&\n+\tgit format-patch --stdout first >patch3 &&\n+\tgit reset --hard first &&\n+\tgit am --signoff <patch3 &&\n+\tgit log --pretty=%B -2 HEAD >actual &&\n+\ttest_cmp expected-log actual\n '\n \n test_expect_success 'am without --keep removes Re: and [PATCH] stuff' '\n+\tgit format-patch --stdout HEAD^ >tmp &&\n+\tsed -e \"/^Subject/ s,\\[PATCH,Re: Re: Re: & 1/5 v2] [foo,\" tmp >patch4 &&\n+\tgit reset --hard HEAD^ &&\n+\tgit am <patch4 &&\n \tgit rev-parse HEAD >expected &&\n \tgit rev-parse master2 >actual &&\n \ttest_cmp expected actual\n-- \n2.13.3\n\n"}]}