{"thread":{"id":"31515","subject":"[PATCH] cherry-pick: don't forget -s on failure","startedAt":"2012-09-12T19:57:33Z","lastAt":"2012-09-14T06:52:03Z","messageCount":9,"participants":["Miklos Vajna","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"198875","messageId":"20120912195732.GB4722@suse.cz","threadId":"31515","inReplyTo":null,"subject":"[PATCH] cherry-pick: don't forget -s on failure","fromName":"Miklos Vajna","fromEmail":"vmiklos@suse.cz","sentAt":"2012-09-12T19:57:33Z","receivedAt":"2012-09-12T19:57:33Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"In case 'git cherry-pick -s <commit>' failed, the user had to use 'git\ncommit -s' (i.e. state the -s option again), which is easy to forget\nabout.  Instead, write the signed-off-by line early, so plain 'git\ncommit' will have the same result.\n\nSigned-off-by: Miklos Vajna <vmiklos@suse.cz>\n---\n\nHi,\n\nSee\nhttp://article.gmane.org/gmane.comp.documentfoundation.libreoffice.devel/36103\nfor motivation. :-)\n\nGiven that I needed sign_off_header / ends_rfc2822_footer and some code\nfrom builtin/commit.c in sequencer.c, I moved them there, let me know if\nthere is a place better than sequencer.c for them.\n\nThanks,\n\nMiklos\n\n builtin/commit.c                |   63 ++------------------------------------\n sequencer.c                     |   65 +++++++++++++++++++++++++++++++++++++++\n sequencer.h                     |    4 ++\n t/t3507-cherry-pick-conflict.sh |    6 +++\n 4 files changed, 78 insertions(+), 60 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 778cf16..7d0df9a 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -28,6 +28,7 @@\n #include \"submodule.h\"\n #include \"gpg-interface.h\"\n #include \"column.h\"\n+#include \"sequencer.h\"\n \n static const char * const builtin_commit_usage[] = {\n \tN_(\"git commit [options] [--] <filepattern>...\"),\n@@ -466,8 +467,6 @@ static int is_a_merge(const struct commit *current_head)\n \treturn !!(current_head->parents && current_head->parents->next);\n }\n \n-static const char sign_off_header[] = \"Signed-off-by: \";\n-\n static void export_one(const char *var, const char *s, const char *e, int hack)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -552,47 +551,6 @@ static void determine_author_info(struct strbuf *author_ident)\n \t}\n }\n \n-static int ends_rfc2822_footer(struct strbuf *sb)\n-{\n-\tint ch;\n-\tint hit = 0;\n-\tint i, j, k;\n-\tint len = sb->len;\n-\tint first = 1;\n-\tconst char *buf = sb->buf;\n-\n-\tfor (i = len - 1; i > 0; i--) {\n-\t\tif (hit && buf[i] == '\\n')\n-\t\t\tbreak;\n-\t\thit = (buf[i] == '\\n');\n-\t}\n-\n-\twhile (i < len - 1 && buf[i] == '\\n')\n-\t\ti++;\n-\n-\tfor (; i < len; i = k) {\n-\t\tfor (k = i; k < len && buf[k] != '\\n'; k++)\n-\t\t\t; /* do nothing */\n-\t\tk++;\n-\n-\t\tif ((buf[k] == ' ' || buf[k] == '\\t') && !first)\n-\t\t\tcontinue;\n-\n-\t\tfirst = 0;\n-\n-\t\tfor (j = 0; i + j < len; j++) {\n-\t\t\tch = buf[i + j];\n-\t\t\tif (ch == ':')\n-\t\t\t\tbreak;\n-\t\t\tif (isalnum(ch) ||\n-\t\t\t    (ch == '-'))\n-\t\t\t\tcontinue;\n-\t\t\treturn 0;\n-\t\t}\n-\t}\n-\treturn 1;\n-}\n-\n static char *cut_ident_timestamp_part(char *string)\n {\n \tchar *ket = strrchr(string, '>');\n@@ -716,23 +674,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \tif (clean_message_contents)\n \t\tstripspace(&sb, 0);\n \n-\tif (signoff) {\n-\t\tstruct strbuf sob = STRBUF_INIT;\n-\t\tint i;\n-\n-\t\tstrbuf_addstr(&sob, sign_off_header);\n-\t\tstrbuf_addstr(&sob, fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n-\t\t\t\t\t     getenv(\"GIT_COMMITTER_EMAIL\")));\n-\t\tstrbuf_addch(&sob, '\\n');\n-\t\tfor (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\\n'; i--)\n-\t\t\t; /* do nothing */\n-\t\tif (prefixcmp(sb.buf + i, sob.buf)) {\n-\t\t\tif (!i || !ends_rfc2822_footer(&sb))\n-\t\t\t\tstrbuf_addch(&sb, '\\n');\n-\t\t\tstrbuf_addbuf(&sb, &sob);\n-\t\t}\n-\t\tstrbuf_release(&sob);\n-\t}\n+\tif (signoff)\n+\t\tappend_signoff(&sb);\n \n \tif (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)\n \t\tdie_errno(_(\"could not write commit template\"));\ndiff --git a/sequencer.c b/sequencer.c\nindex f86f116..402dcd6 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -17,6 +17,8 @@\n \n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n \n+const char sign_off_header[] = \"Signed-off-by: \";\n+\n void remove_sequencer_state(void)\n {\n \tstruct strbuf seq_dir = STRBUF_INIT;\n@@ -249,6 +251,9 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n \t\t}\n \t}\n \n+\tif (opts->signoff)\n+\t\tappend_signoff(msgbuf);\n+\n \treturn !clean;\n }\n \n@@ -1011,3 +1016,63 @@ int sequencer_pick_revisions(struct replay_opts *opts)\n \tsave_opts(opts);\n \treturn pick_commits(todo_list, opts);\n }\n+\n+static int ends_rfc2822_footer(struct strbuf *sb)\n+{\n+\tint ch;\n+\tint hit = 0;\n+\tint i, j, k;\n+\tint len = sb->len;\n+\tint first = 1;\n+\tconst char *buf = sb->buf;\n+\n+\tfor (i = len - 1; i > 0; i--) {\n+\t\tif (hit && buf[i] == '\\n')\n+\t\t\tbreak;\n+\t\thit = (buf[i] == '\\n');\n+\t}\n+\n+\twhile (i < len - 1 && buf[i] == '\\n')\n+\t\ti++;\n+\n+\tfor (; i < len; i = k) {\n+\t\tfor (k = i; k < len && buf[k] != '\\n'; k++)\n+\t\t\t; /* do nothing */\n+\t\tk++;\n+\n+\t\tif ((buf[k] == ' ' || buf[k] == '\\t') && !first)\n+\t\t\tcontinue;\n+\n+\t\tfirst = 0;\n+\n+\t\tfor (j = 0; i + j < len; j++) {\n+\t\t\tch = buf[i + j];\n+\t\t\tif (ch == ':')\n+\t\t\t\tbreak;\n+\t\t\tif (isalnum(ch) ||\n+\t\t\t    (ch == '-'))\n+\t\t\t\tcontinue;\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\treturn 1;\n+}\n+\n+void append_signoff(struct strbuf *msgbuf)\n+{\n+\tstruct strbuf sob = STRBUF_INIT;\n+\tint i;\n+\n+\tstrbuf_addstr(&sob, sign_off_header);\n+\tstrbuf_addstr(&sob, fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n+\t\t\t\tgetenv(\"GIT_COMMITTER_EMAIL\")));\n+\tstrbuf_addch(&sob, '\\n');\n+\tfor (i = msgbuf->len - 1; i > 0 && msgbuf->buf[i - 1] != '\\n'; i--)\n+\t\t; /* do nothing */\n+\tif (prefixcmp(msgbuf->buf + i, sob.buf)) {\n+\t\tif (!i || !ends_rfc2822_footer(msgbuf))\n+\t\t\tstrbuf_addch(msgbuf, '\\n');\n+\t\tstrbuf_addbuf(msgbuf, &sob);\n+\t}\n+\tstrbuf_release(&sob);\n+}\ndiff --git a/sequencer.h b/sequencer.h\nindex d849420..440b0c9 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -49,4 +49,8 @@ extern void remove_sequencer_state(void);\n \n int sequencer_pick_revisions(struct replay_opts *opts);\n \n+extern const char sign_off_header[];\n+\n+void append_signoff(struct strbuf *msgbuf);\n+\n #endif\ndiff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\nindex 0c81b3c..eb52f1e 100755\n--- a/t/t3507-cherry-pick-conflict.sh\n+++ b/t/t3507-cherry-pick-conflict.sh\n@@ -340,4 +340,10 @@ test_expect_success 'revert conflict, diff3 -m style' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'failed cherry-pick does not forget -s' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick -s picked &&\n+\ttest_i18ngrep -e \"Signed-off-by\" .git/MERGE_MSG\n+'\n+\n test_done\n-- \n1.7.7\n"},{"id":"198891","messageId":"7vd31qc1p3.fsf@alter.siamese.dyndns.org","threadId":"31515","inReplyTo":"20120912195732.GB4722@suse.cz","subject":"Re: [PATCH] cherry-pick: don't forget -s on failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-12T22:32:08Z","receivedAt":"2012-09-12T22:32:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@suse.cz> writes:\n\n> In case 'git cherry-pick -s <commit>' failed, the user had to use 'git\n> commit -s' (i.e. state the -s option again), which is easy to forget\n> about.  Instead, write the signed-off-by line early, so plain 'git\n> commit' will have the same result.\n>\n> Signed-off-by: Miklos Vajna <vmiklos@suse.cz>\n> ---\n>\n> Hi,\n>\n> See\n> http://article.gmane.org/gmane.comp.documentfoundation.libreoffice.devel/36103\n> for motivation. :-)\n>\n> Given that I needed sign_off_header / ends_rfc2822_footer and some code\n> from builtin/commit.c in sequencer.c, I moved them there, let me know if\n> there is a place better than sequencer.c for them.\n\nI think we had a separate topic around cherry-pick that needs the\nfooter thing accessible from cherry-pick recently ($gmane/204755).\n\nI think the code movement in this patch is a good one.\n\nThanks.\n\n>\n> Thanks,\n>\n> Miklos\n>\n>  builtin/commit.c                |   63 ++------------------------------------\n>  sequencer.c                     |   65 +++++++++++++++++++++++++++++++++++++++\n>  sequencer.h                     |    4 ++\n>  t/t3507-cherry-pick-conflict.sh |    6 +++\n>  4 files changed, 78 insertions(+), 60 deletions(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 778cf16..7d0df9a 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -28,6 +28,7 @@\n>  #include \"submodule.h\"\n>  #include \"gpg-interface.h\"\n>  #include \"column.h\"\n> +#include \"sequencer.h\"\n>  \n>  static const char * const builtin_commit_usage[] = {\n>  \tN_(\"git commit [options] [--] <filepattern>...\"),\n> @@ -466,8 +467,6 @@ static int is_a_merge(const struct commit *current_head)\n>  \treturn !!(current_head->parents && current_head->parents->next);\n>  }\n>  \n> -static const char sign_off_header[] = \"Signed-off-by: \";\n> -\n>  static void export_one(const char *var, const char *s, const char *e, int hack)\n>  {\n>  \tstruct strbuf buf = STRBUF_INIT;\n> @@ -552,47 +551,6 @@ static void determine_author_info(struct strbuf *author_ident)\n>  \t}\n>  }\n>  \n> -static int ends_rfc2822_footer(struct strbuf *sb)\n> -{\n> -\tint ch;\n> -\tint hit = 0;\n> -\tint i, j, k;\n> -\tint len = sb->len;\n> -\tint first = 1;\n> -\tconst char *buf = sb->buf;\n> -\n> -\tfor (i = len - 1; i > 0; i--) {\n> -\t\tif (hit && buf[i] == '\\n')\n> -\t\t\tbreak;\n> -\t\thit = (buf[i] == '\\n');\n> -\t}\n> -\n> -\twhile (i < len - 1 && buf[i] == '\\n')\n> -\t\ti++;\n> -\n> -\tfor (; i < len; i = k) {\n> -\t\tfor (k = i; k < len && buf[k] != '\\n'; k++)\n> -\t\t\t; /* do nothing */\n> -\t\tk++;\n> -\n> -\t\tif ((buf[k] == ' ' || buf[k] == '\\t') && !first)\n> -\t\t\tcontinue;\n> -\n> -\t\tfirst = 0;\n> -\n> -\t\tfor (j = 0; i + j < len; j++) {\n> -\t\t\tch = buf[i + j];\n> -\t\t\tif (ch == ':')\n> -\t\t\t\tbreak;\n> -\t\t\tif (isalnum(ch) ||\n> -\t\t\t    (ch == '-'))\n> -\t\t\t\tcontinue;\n> -\t\t\treturn 0;\n> -\t\t}\n> -\t}\n> -\treturn 1;\n> -}\n> -\n>  static char *cut_ident_timestamp_part(char *string)\n>  {\n>  \tchar *ket = strrchr(string, '>');\n> @@ -716,23 +674,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n>  \tif (clean_message_contents)\n>  \t\tstripspace(&sb, 0);\n>  \n> -\tif (signoff) {\n> -\t\tstruct strbuf sob = STRBUF_INIT;\n> -\t\tint i;\n> -\n> -\t\tstrbuf_addstr(&sob, sign_off_header);\n> -\t\tstrbuf_addstr(&sob, fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n> -\t\t\t\t\t     getenv(\"GIT_COMMITTER_EMAIL\")));\n> -\t\tstrbuf_addch(&sob, '\\n');\n> -\t\tfor (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\\n'; i--)\n> -\t\t\t; /* do nothing */\n> -\t\tif (prefixcmp(sb.buf + i, sob.buf)) {\n> -\t\t\tif (!i || !ends_rfc2822_footer(&sb))\n> -\t\t\t\tstrbuf_addch(&sb, '\\n');\n> -\t\t\tstrbuf_addbuf(&sb, &sob);\n> -\t\t}\n> -\t\tstrbuf_release(&sob);\n> -\t}\n> +\tif (signoff)\n> +\t\tappend_signoff(&sb);\n>  \n>  \tif (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)\n>  \t\tdie_errno(_(\"could not write commit template\"));\n> diff --git a/sequencer.c b/sequencer.c\n> index f86f116..402dcd6 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -17,6 +17,8 @@\n>  \n>  #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n>  \n> +const char sign_off_header[] = \"Signed-off-by: \";\n> +\n>  void remove_sequencer_state(void)\n>  {\n>  \tstruct strbuf seq_dir = STRBUF_INIT;\n> @@ -249,6 +251,9 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n>  \t\t}\n>  \t}\n>  \n> +\tif (opts->signoff)\n> +\t\tappend_signoff(msgbuf);\n> +\n>  \treturn !clean;\n>  }\n>  \n> @@ -1011,3 +1016,63 @@ int sequencer_pick_revisions(struct replay_opts *opts)\n>  \tsave_opts(opts);\n>  \treturn pick_commits(todo_list, opts);\n>  }\n> +\n> +static int ends_rfc2822_footer(struct strbuf *sb)\n> +{\n> +\tint ch;\n> +\tint hit = 0;\n> +\tint i, j, k;\n> +\tint len = sb->len;\n> +\tint first = 1;\n> +\tconst char *buf = sb->buf;\n> +\n> +\tfor (i = len - 1; i > 0; i--) {\n> +\t\tif (hit && buf[i] == '\\n')\n> +\t\t\tbreak;\n> +\t\thit = (buf[i] == '\\n');\n> +\t}\n> +\n> +\twhile (i < len - 1 && buf[i] == '\\n')\n> +\t\ti++;\n> +\n> +\tfor (; i < len; i = k) {\n> +\t\tfor (k = i; k < len && buf[k] != '\\n'; k++)\n> +\t\t\t; /* do nothing */\n> +\t\tk++;\n> +\n> +\t\tif ((buf[k] == ' ' || buf[k] == '\\t') && !first)\n> +\t\t\tcontinue;\n> +\n> +\t\tfirst = 0;\n> +\n> +\t\tfor (j = 0; i + j < len; j++) {\n> +\t\t\tch = buf[i + j];\n> +\t\t\tif (ch == ':')\n> +\t\t\t\tbreak;\n> +\t\t\tif (isalnum(ch) ||\n> +\t\t\t    (ch == '-'))\n> +\t\t\t\tcontinue;\n> +\t\t\treturn 0;\n> +\t\t}\n> +\t}\n> +\treturn 1;\n> +}\n> +\n> +void append_signoff(struct strbuf *msgbuf)\n> +{\n> +\tstruct strbuf sob = STRBUF_INIT;\n> +\tint i;\n> +\n> +\tstrbuf_addstr(&sob, sign_off_header);\n> +\tstrbuf_addstr(&sob, fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n> +\t\t\t\tgetenv(\"GIT_COMMITTER_EMAIL\")));\n> +\tstrbuf_addch(&sob, '\\n');\n> +\tfor (i = msgbuf->len - 1; i > 0 && msgbuf->buf[i - 1] != '\\n'; i--)\n> +\t\t; /* do nothing */\n> +\tif (prefixcmp(msgbuf->buf + i, sob.buf)) {\n> +\t\tif (!i || !ends_rfc2822_footer(msgbuf))\n> +\t\t\tstrbuf_addch(msgbuf, '\\n');\n> +\t\tstrbuf_addbuf(msgbuf, &sob);\n> +\t}\n> +\tstrbuf_release(&sob);\n> +}\n> diff --git a/sequencer.h b/sequencer.h\n> index d849420..440b0c9 100644\n> --- a/sequencer.h\n> +++ b/sequencer.h\n> @@ -49,4 +49,8 @@ extern void remove_sequencer_state(void);\n>  \n>  int sequencer_pick_revisions(struct replay_opts *opts);\n>  \n> +extern const char sign_off_header[];\n> +\n> +void append_signoff(struct strbuf *msgbuf);\n> +\n>  #endif\n> diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\n> index 0c81b3c..eb52f1e 100755\n> --- a/t/t3507-cherry-pick-conflict.sh\n> +++ b/t/t3507-cherry-pick-conflict.sh\n> @@ -340,4 +340,10 @@ test_expect_success 'revert conflict, diff3 -m style' '\n>  \ttest_cmp expected actual\n>  '\n>  \n> +test_expect_success 'failed cherry-pick does not forget -s' '\n> +\tpristine_detach initial &&\n> +\ttest_must_fail git cherry-pick -s picked &&\n> +\ttest_i18ngrep -e \"Signed-off-by\" .git/MERGE_MSG\n> +'\n> +\n>  test_done\n"},{"id":"198894","messageId":"7v8vcec13d.fsf@alter.siamese.dyndns.org","threadId":"31515","inReplyTo":"7vd31qc1p3.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] cherry-pick: don't forget -s on failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-12T22:45:10Z","receivedAt":"2012-09-12T22:45:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I think we had a separate topic around cherry-pick that needs the\n> footer thing accessible from cherry-pick recently ($gmane/204755).\n>\n> I think the code movement in this patch is a good one.\n>\n> Thanks.\n\nHaving said that, the behaviour after this patch is applied is not\nquite right.\n\nA typical .git/MERGE_MSG that is left after \"cherry-pick\" gives the\ncontrol back to you asking for help, with your patch that adds the\nsign-off at the end, would look like this:\n\n\n    cherry-pick: don't forget -s on failure\n\n    In case 'git cherry-pick -s <commit>' failed, the user had to use 'git\n    commit -s' (i.e. state the -s option again), which is easy to forget\n    about.  Instead, write the signed-off-by line early, so plain 'git\n    commit' will have the same result.\n\n    Signed-off-by: Miklos Vajna <vmiklos@suse.cz>\n    Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\n    Conflicts:\n            builtin/commit.c\n\n    Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\n\nNotice two issues?\n\n - The additional S-o-b should come immediately after the existing\n   block of footers.\n\n - And the last entry in the existing footer block is already mine,\n   so there shouldn't have been a new and duplicated one added.\n\n\nI am not sure how reusable the moved function is without\nenhancements for your purpose.  The logic to identify the footer\nneeds to be enhanced so that an \"end\" pointer to point at the byte\nbefore the caller added \"Conflicts: \" can be given, and pretend as\nif it is the end of the buffer, unlike in the fresh commit case\nwhere it can consider the real end of the buffer as such.\n\nOr something like that.\n"},{"id":"198906","messageId":"20120913073324.GA14383@suse.cz","threadId":"31515","inReplyTo":"7v8vcec13d.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] cherry-pick: don't forget -s on failure","fromName":"Miklos Vajna","fromEmail":"vmiklos@suse.cz","sentAt":"2012-09-13T07:33:25Z","receivedAt":"2012-09-13T07:33:25Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"In case 'git cherry-pick -s <commit>' failed, the user had to use 'git\ncommit -s' (i.e. state the -s option again), which is easy to forget\nabout.  Instead, write the signed-off-by line early, so plain 'git\ncommit' will have the same result.\n\nAlso update 'git commit -s', so that in case there is already a relevant\nSigned-off-by line before the Conflicts: line, it won't add one more at\nthe end of the message.\n\nSigned-off-by: Miklos Vajna <vmiklos@suse.cz>\n---\n\nOn Wed, Sep 12, 2012 at 03:45:10PM -0700, Junio C Hamano <gitster@pobox.com> wrote:\n>  - The additional S-o-b should come immediately after the existing\n>    block of footers.\n\nThis was trivial to fix.\n\n>  - And the last entry in the existing footer block is already mine,\n>    so there shouldn't have been a new and duplicated one added.\n> \n> \n> I am not sure how reusable the moved function is without\n> enhancements for your purpose.  The logic to identify the footer\n> needs to be enhanced so that an \"end\" pointer to point at the byte\n> before the caller added \"Conflicts: \" can be given, and pretend as\n> if it is the end of the buffer, unlike in the fresh commit case\n> where it can consider the real end of the buffer as such.\n\nBelow is what I came up with. It simply ignores anything after the \nConclicts: line, when checking for the last Signed-off-by line.\n\nAn other thing: I forgot to run 'make test' for the initial patch, it \nseems t3510-cherry-pick-sequence.sh has 3 tests that basically ensures \nthe opposite of what my patch does. Given that there are already \ntestcases for the new behaviour, can they be just removed? For now, I \njust disabled them.\n\n builtin/commit.c                |   79 +++++++++++---------------------------\n sequencer.c                     |   65 ++++++++++++++++++++++++++++++++\n sequencer.h                     |    4 ++\n t/t3507-cherry-pick-conflict.sh |   14 +++++++\n t/t3510-cherry-pick-sequence.sh |    6 +-\n t/test-lib-functions.sh         |    8 +++-\n 6 files changed, 116 insertions(+), 60 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 778cf16..4d50484 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -28,6 +28,7 @@\n #include \"submodule.h\"\n #include \"gpg-interface.h\"\n #include \"column.h\"\n+#include \"sequencer.h\"\n \n static const char * const builtin_commit_usage[] = {\n \tN_(\"git commit [options] [--] <filepattern>...\"),\n@@ -466,8 +467,6 @@ static int is_a_merge(const struct commit *current_head)\n \treturn !!(current_head->parents && current_head->parents->next);\n }\n \n-static const char sign_off_header[] = \"Signed-off-by: \";\n-\n static void export_one(const char *var, const char *s, const char *e, int hack)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -552,47 +551,6 @@ static void determine_author_info(struct strbuf *author_ident)\n \t}\n }\n \n-static int ends_rfc2822_footer(struct strbuf *sb)\n-{\n-\tint ch;\n-\tint hit = 0;\n-\tint i, j, k;\n-\tint len = sb->len;\n-\tint first = 1;\n-\tconst char *buf = sb->buf;\n-\n-\tfor (i = len - 1; i > 0; i--) {\n-\t\tif (hit && buf[i] == '\\n')\n-\t\t\tbreak;\n-\t\thit = (buf[i] == '\\n');\n-\t}\n-\n-\twhile (i < len - 1 && buf[i] == '\\n')\n-\t\ti++;\n-\n-\tfor (; i < len; i = k) {\n-\t\tfor (k = i; k < len && buf[k] != '\\n'; k++)\n-\t\t\t; /* do nothing */\n-\t\tk++;\n-\n-\t\tif ((buf[k] == ' ' || buf[k] == '\\t') && !first)\n-\t\t\tcontinue;\n-\n-\t\tfirst = 0;\n-\n-\t\tfor (j = 0; i + j < len; j++) {\n-\t\t\tch = buf[i + j];\n-\t\t\tif (ch == ':')\n-\t\t\t\tbreak;\n-\t\t\tif (isalnum(ch) ||\n-\t\t\t    (ch == '-'))\n-\t\t\t\tcontinue;\n-\t\t\treturn 0;\n-\t\t}\n-\t}\n-\treturn 1;\n-}\n-\n static char *cut_ident_timestamp_part(char *string)\n {\n \tchar *ket = strrchr(string, '>');\n@@ -717,21 +675,30 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tstripspace(&sb, 0);\n \n \tif (signoff) {\n-\t\tstruct strbuf sob = STRBUF_INIT;\n-\t\tint i;\n+\t\t/*\n+\t\t * See if we have a Conflicts: block at the end. If yes, count\n+\t\t * its size, so we can ignore it.\n+\t\t */\n+\t\tint ignore_footer = 0;\n+\t\tint i, eol, previous = 0;\n+\t\tconst char *nl;\n \n-\t\tstrbuf_addstr(&sob, sign_off_header);\n-\t\tstrbuf_addstr(&sob, fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n-\t\t\t\t\t     getenv(\"GIT_COMMITTER_EMAIL\")));\n-\t\tstrbuf_addch(&sob, '\\n');\n-\t\tfor (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\\n'; i--)\n-\t\t\t; /* do nothing */\n-\t\tif (prefixcmp(sb.buf + i, sob.buf)) {\n-\t\t\tif (!i || !ends_rfc2822_footer(&sb))\n-\t\t\t\tstrbuf_addch(&sb, '\\n');\n-\t\t\tstrbuf_addbuf(&sb, &sob);\n+\t\tfor (i = 0; i < sb.len; i++) {\n+\t\t\tnl = memchr(sb.buf + i, '\\n', sb.len - i);\n+\t\t\tif (nl)\n+\t\t\t\teol = nl - sb.buf;\n+\t\t\telse\n+\t\t\t\teol = sb.len;\n+\t\t\tif (!prefixcmp(sb.buf + previous, \"\\nConflicts:\\n\")) {\n+\t\t\t\tignore_footer = sb.len - previous;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\twhile (i < eol)\n+\t\t\t\ti++;\n+\t\t\tprevious = eol;\n \t\t}\n-\t\tstrbuf_release(&sob);\n+\n+\t\tappend_signoff(&sb, ignore_footer);\n \t}\n \n \tif (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)\ndiff --git a/sequencer.c b/sequencer.c\nindex f86f116..e691b08 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -17,6 +17,8 @@\n \n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n \n+const char sign_off_header[] = \"Signed-off-by: \";\n+\n void remove_sequencer_state(void)\n {\n \tstruct strbuf seq_dir = STRBUF_INIT;\n@@ -233,6 +235,9 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n \t\tdie(_(\"%s: Unable to write new index file\"), action_name(opts));\n \trollback_lock_file(&index_lock);\n \n+\tif (opts->signoff)\n+\t\tappend_signoff(msgbuf, 0);\n+\n \tif (!clean) {\n \t\tint i;\n \t\tstrbuf_addstr(msgbuf, \"\\nConflicts:\\n\");\n@@ -1011,3 +1016,63 @@ int sequencer_pick_revisions(struct replay_opts *opts)\n \tsave_opts(opts);\n \treturn pick_commits(todo_list, opts);\n }\n+\n+static int ends_rfc2822_footer(struct strbuf *sb)\n+{\n+\tint ch;\n+\tint hit = 0;\n+\tint i, j, k;\n+\tint len = sb->len;\n+\tint first = 1;\n+\tconst char *buf = sb->buf;\n+\n+\tfor (i = len - 1; i > 0; i--) {\n+\t\tif (hit && buf[i] == '\\n')\n+\t\t\tbreak;\n+\t\thit = (buf[i] == '\\n');\n+\t}\n+\n+\twhile (i < len - 1 && buf[i] == '\\n')\n+\t\ti++;\n+\n+\tfor (; i < len; i = k) {\n+\t\tfor (k = i; k < len && buf[k] != '\\n'; k++)\n+\t\t\t; /* do nothing */\n+\t\tk++;\n+\n+\t\tif ((buf[k] == ' ' || buf[k] == '\\t') && !first)\n+\t\t\tcontinue;\n+\n+\t\tfirst = 0;\n+\n+\t\tfor (j = 0; i + j < len; j++) {\n+\t\t\tch = buf[i + j];\n+\t\t\tif (ch == ':')\n+\t\t\t\tbreak;\n+\t\t\tif (isalnum(ch) ||\n+\t\t\t    (ch == '-'))\n+\t\t\t\tcontinue;\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\treturn 1;\n+}\n+\n+void append_signoff(struct strbuf *msgbuf, int ignore_footer)\n+{\n+\tstruct strbuf sob = STRBUF_INIT;\n+\tint i;\n+\n+\tstrbuf_addstr(&sob, sign_off_header);\n+\tstrbuf_addstr(&sob, fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n+\t\t\t\tgetenv(\"GIT_COMMITTER_EMAIL\")));\n+\tstrbuf_addch(&sob, '\\n');\n+\tfor (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\\n'; i--)\n+\t\t; /* do nothing */\n+\tif (prefixcmp(msgbuf->buf + i, sob.buf)) {\n+\t\tif (!i || !ends_rfc2822_footer(msgbuf))\n+\t\t\tstrbuf_addch(msgbuf, '\\n');\n+\t\tstrbuf_addbuf(msgbuf, &sob);\n+\t}\n+\tstrbuf_release(&sob);\n+}\ndiff --git a/sequencer.h b/sequencer.h\nindex d849420..60287b8 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -49,4 +49,8 @@ extern void remove_sequencer_state(void);\n \n int sequencer_pick_revisions(struct replay_opts *opts);\n \n+extern const char sign_off_header[];\n+\n+void append_signoff(struct strbuf *msgbuf, int ignore_footer);\n+\n #endif\ndiff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\nindex 0c81b3c..b67756f 100755\n--- a/t/t3507-cherry-pick-conflict.sh\n+++ b/t/t3507-cherry-pick-conflict.sh\n@@ -30,6 +30,7 @@ test_expect_success setup '\n \ttest_commit initial foo a &&\n \ttest_commit base foo b &&\n \ttest_commit picked foo c &&\n+\ttest_commit --signoff picked-signed foo d &&\n \tgit config advice.detachedhead false\n \n '\n@@ -340,4 +341,17 @@ test_expect_success 'revert conflict, diff3 -m style' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'failed cherry-pick does not forget -s' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick -s picked &&\n+\ttest_i18ngrep -e \"Signed-off-by\" .git/MERGE_MSG\n+'\n+\n+test_expect_success 'commit after failed cherry-pick does not add duplicated -s' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick -s picked-signed &&\n+\tgit commit -a -s &&\n+\ttest $(git show -s |grep -c \"Signed-off-by\") = 1\n+'\n+\n test_done\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex f4e6450..b5fb527 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -410,7 +410,7 @@ test_expect_success '--continue respects -x in first commit in multi-pick' '\n \tgrep \"cherry picked from.*$picked\" msg\n '\n \n-test_expect_success '--signoff is not automatically propagated to resolved conflict' '\n+test_expect_failure '--signoff is automatically propagated to resolved conflict' '\n \tpristine_detach initial &&\n \ttest_expect_code 1 git cherry-pick --signoff base..anotherpick &&\n \techo \"c\" >foo &&\n@@ -428,7 +428,7 @@ test_expect_success '--signoff is not automatically propagated to resolved confl\n \tgrep \"Signed-off-by:\" anotherpick_msg\n '\n \n-test_expect_success '--signoff dropped for implicit commit of resolution, multi-pick case' '\n+test_expect_failure '--signoff dropped for implicit commit of resolution, multi-pick case' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick -s picked anotherpick &&\n \techo c >foo &&\n@@ -441,7 +441,7 @@ test_expect_success '--signoff dropped for implicit commit of resolution, multi-\n \t! grep Signed-off-by: msg\n '\n \n-test_expect_success 'sign-off needs to be reaffirmed after conflict resolution, single-pick case' '\n+test_expect_failure 'sign-off needs to be reaffirmed after conflict resolution, single-pick case' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick -s picked &&\n \techo c >foo &&\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 9bc57d2..bbb7b7d 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -149,6 +149,12 @@ test_commit () {\n \t\tnotick=yes\n \t\tshift\n \tfi &&\n+\tsignoff= &&\n+\tif test \"z$1\" = \"z--signoff\"\n+\tthen\n+\t\tsignoff=\"$1\"\n+\t\tshift\n+\tfi &&\n \tfile=${2:-\"$1.t\"} &&\n \techo \"${3-$1}\" > \"$file\" &&\n \tgit add \"$file\" &&\n@@ -156,7 +162,7 @@ test_commit () {\n \tthen\n \t\ttest_tick\n \tfi &&\n-\tgit commit -m \"$1\" &&\n+\tgit commit $signoff -m \"$1\" &&\n \tgit tag \"$1\"\n }\n \n-- \n1.7.7\n"},{"id":"198937","messageId":"7vd31pam6t.fsf@alter.siamese.dyndns.org","threadId":"31515","inReplyTo":"20120913073324.GA14383@suse.cz","subject":"Re: [PATCH v2] cherry-pick: don't forget -s on failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-13T17:04:42Z","receivedAt":"2012-09-13T17:04:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@suse.cz> writes:\n\n> In case 'git cherry-pick -s <commit>' failed, the user had to use 'git\n> commit -s' (i.e. state the -s option again), which is easy to forget\n> about.  Instead, write the signed-off-by line early, so plain 'git\n> commit' will have the same result.\n>\n> Also update 'git commit -s', so that in case there is already a relevant\n> Signed-off-by line before the Conflicts: line, it won't add one more at\n> the end of the message.\n>\n> Signed-off-by: Miklos Vajna <vmiklos@suse.cz>\n> ---\n>\n> On Wed, Sep 12, 2012 at 03:45:10PM -0700, Junio C Hamano <gitster@pobox.com> wrote:\n>>  - The additional S-o-b should come immediately after the existing\n>>    block of footers.\n>\n> This was trivial to fix.\n\nIndeed.  Just inserting before starting to add \"Oh, there were\nconflicts, and add the info on them\" before doing it at the end is\nall it takes.  Simple and straightforward---I like it.\n\n> --- a/t/test-lib-functions.sh\n> +++ b/t/test-lib-functions.sh\n> @@ -149,6 +149,12 @@ test_commit () {\n>  \t\tnotick=yes\n>  \t\tshift\n>  \tfi &&\n> +\tsignoff= &&\n> +\tif test \"z$1\" = \"z--signoff\"\n> +\tthen\n> +\t\tsignoff=\"$1\"\n> +\t\tshift\n> +\tfi &&\n>  \tfile=${2:-\"$1.t\"} &&\n>  \techo \"${3-$1}\" > \"$file\" &&\n>  \tgit add \"$file\" &&\n\nThis is somewhat iffy.  Shouldn't \"test_commit --signoff --notick\" work?\n\n> @@ -156,7 +162,7 @@ test_commit () {\n>  \tthen\n>  \t\ttest_tick\n>  \tfi &&\n> -\tgit commit -m \"$1\" &&\n> +\tgit commit $signoff -m \"$1\" &&\n>  \tgit tag \"$1\"\n>  }\n\nThanks.\n"},{"id":"198938","messageId":"7v8vcdalby.fsf@alter.siamese.dyndns.org","threadId":"31515","inReplyTo":"20120913073324.GA14383@suse.cz","subject":"Re: [PATCH v2] cherry-pick: don't forget -s on failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-13T17:23:13Z","receivedAt":"2012-09-13T17:23:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@suse.cz> writes:\n\n> +void append_signoff(struct strbuf *msgbuf, int ignore_footer)\n> +{\n> +\tstruct strbuf sob = STRBUF_INIT;\n> +\tint i;\n> +\n> +\tstrbuf_addstr(&sob, sign_off_header);\n> +\tstrbuf_addstr(&sob, fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n> +\t\t\t\tgetenv(\"GIT_COMMITTER_EMAIL\")));\n> +\tstrbuf_addch(&sob, '\\n');\n> +\tfor (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\\n'; i--)\n> +\t\t; /* do nothing */\n> +\tif (prefixcmp(msgbuf->buf + i, sob.buf)) {\n> +\t\tif (!i || !ends_rfc2822_footer(msgbuf))\n> +\t\t\tstrbuf_addch(msgbuf, '\\n');\n> +\t\tstrbuf_addbuf(msgbuf, &sob);\n> +\t}\n> +\tstrbuf_release(&sob);\n> +}\n\nHrm, what is this thing trying to do?  It does start scanning from\nthe end (ignoring the \"Conflicts:\" thing) to see who the last person\nthat signed it off was, but once it decides that it needs to add a\nnew sign-off, it still adds it at the very end anyway.\n"},{"id":"198962","messageId":"20120913202714.GD14383@suse.cz","threadId":"31515","inReplyTo":"7v8vcdalby.fsf@alter.siamese.dyndns.org","subject":"[PATCH v3] cherry-pick: don't forget -s on failure","fromName":"Miklos Vajna","fromEmail":"vmiklos@suse.cz","sentAt":"2012-09-13T20:27:15Z","receivedAt":"2012-09-13T20:27:15Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"In case 'git cherry-pick -s <commit>' failed, the user had to use 'git\ncommit -s' (i.e. state the -s option again), which is easy to forget\nabout.  Instead, write the signed-off-by line early, so plain 'git\ncommit' will have the same result.\n\nAlso update 'git commit -s', so that in case there is already a relevant\nSigned-off-by line before the Conflicts: line, it won't add one more at\nthe end of the message. If there is no such line, then add it before the\nthe Conflicts: line.\n\nSigned-off-by: Miklos Vajna <vmiklos@suse.cz>\n---\n\n> This is somewhat iffy.  Shouldn't \"test_commit --signoff --notick\"\n> work?\n\nIndeed, fixed now.\n\n> Hrm, what is this thing trying to do?  It does start scanning from\n> the end (ignoring the \"Conflicts:\" thing) to see who the last person\n> that signed it off was, but once it decides that it needs to add a\n> new sign-off, it still adds it at the very end anyway.\n\nAh, I did not handle that, as the original git commit -s didn't do it \neither -- and originally I just wanted to touch git cherry-pick. \nImplemented now.\n\n builtin/commit.c                |   79 +++++++++++---------------------------\n sequencer.c                     |   72 +++++++++++++++++++++++++++++++++++\n sequencer.h                     |    4 ++\n t/t3507-cherry-pick-conflict.sh |   32 ++++++++++++++++\n t/t3510-cherry-pick-sequence.sh |    6 +-\n t/test-lib-functions.sh         |   20 +++++++--\n 6 files changed, 149 insertions(+), 64 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 778cf16..4d50484 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -28,6 +28,7 @@\n #include \"submodule.h\"\n #include \"gpg-interface.h\"\n #include \"column.h\"\n+#include \"sequencer.h\"\n \n static const char * const builtin_commit_usage[] = {\n \tN_(\"git commit [options] [--] <filepattern>...\"),\n@@ -466,8 +467,6 @@ static int is_a_merge(const struct commit *current_head)\n \treturn !!(current_head->parents && current_head->parents->next);\n }\n \n-static const char sign_off_header[] = \"Signed-off-by: \";\n-\n static void export_one(const char *var, const char *s, const char *e, int hack)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -552,47 +551,6 @@ static void determine_author_info(struct strbuf *author_ident)\n \t}\n }\n \n-static int ends_rfc2822_footer(struct strbuf *sb)\n-{\n-\tint ch;\n-\tint hit = 0;\n-\tint i, j, k;\n-\tint len = sb->len;\n-\tint first = 1;\n-\tconst char *buf = sb->buf;\n-\n-\tfor (i = len - 1; i > 0; i--) {\n-\t\tif (hit && buf[i] == '\\n')\n-\t\t\tbreak;\n-\t\thit = (buf[i] == '\\n');\n-\t}\n-\n-\twhile (i < len - 1 && buf[i] == '\\n')\n-\t\ti++;\n-\n-\tfor (; i < len; i = k) {\n-\t\tfor (k = i; k < len && buf[k] != '\\n'; k++)\n-\t\t\t; /* do nothing */\n-\t\tk++;\n-\n-\t\tif ((buf[k] == ' ' || buf[k] == '\\t') && !first)\n-\t\t\tcontinue;\n-\n-\t\tfirst = 0;\n-\n-\t\tfor (j = 0; i + j < len; j++) {\n-\t\t\tch = buf[i + j];\n-\t\t\tif (ch == ':')\n-\t\t\t\tbreak;\n-\t\t\tif (isalnum(ch) ||\n-\t\t\t    (ch == '-'))\n-\t\t\t\tcontinue;\n-\t\t\treturn 0;\n-\t\t}\n-\t}\n-\treturn 1;\n-}\n-\n static char *cut_ident_timestamp_part(char *string)\n {\n \tchar *ket = strrchr(string, '>');\n@@ -717,21 +675,30 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tstripspace(&sb, 0);\n \n \tif (signoff) {\n-\t\tstruct strbuf sob = STRBUF_INIT;\n-\t\tint i;\n+\t\t/*\n+\t\t * See if we have a Conflicts: block at the end. If yes, count\n+\t\t * its size, so we can ignore it.\n+\t\t */\n+\t\tint ignore_footer = 0;\n+\t\tint i, eol, previous = 0;\n+\t\tconst char *nl;\n \n-\t\tstrbuf_addstr(&sob, sign_off_header);\n-\t\tstrbuf_addstr(&sob, fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n-\t\t\t\t\t     getenv(\"GIT_COMMITTER_EMAIL\")));\n-\t\tstrbuf_addch(&sob, '\\n');\n-\t\tfor (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\\n'; i--)\n-\t\t\t; /* do nothing */\n-\t\tif (prefixcmp(sb.buf + i, sob.buf)) {\n-\t\t\tif (!i || !ends_rfc2822_footer(&sb))\n-\t\t\t\tstrbuf_addch(&sb, '\\n');\n-\t\t\tstrbuf_addbuf(&sb, &sob);\n+\t\tfor (i = 0; i < sb.len; i++) {\n+\t\t\tnl = memchr(sb.buf + i, '\\n', sb.len - i);\n+\t\t\tif (nl)\n+\t\t\t\teol = nl - sb.buf;\n+\t\t\telse\n+\t\t\t\teol = sb.len;\n+\t\t\tif (!prefixcmp(sb.buf + previous, \"\\nConflicts:\\n\")) {\n+\t\t\t\tignore_footer = sb.len - previous;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\twhile (i < eol)\n+\t\t\t\ti++;\n+\t\t\tprevious = eol;\n \t\t}\n-\t\tstrbuf_release(&sob);\n+\n+\t\tappend_signoff(&sb, ignore_footer);\n \t}\n \n \tif (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)\ndiff --git a/sequencer.c b/sequencer.c\nindex f86f116..4420807 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -17,6 +17,8 @@\n \n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n \n+const char sign_off_header[] = \"Signed-off-by: \";\n+\n void remove_sequencer_state(void)\n {\n \tstruct strbuf seq_dir = STRBUF_INIT;\n@@ -233,6 +235,9 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n \t\tdie(_(\"%s: Unable to write new index file\"), action_name(opts));\n \trollback_lock_file(&index_lock);\n \n+\tif (opts->signoff)\n+\t\tappend_signoff(msgbuf, 0);\n+\n \tif (!clean) {\n \t\tint i;\n \t\tstrbuf_addstr(msgbuf, \"\\nConflicts:\\n\");\n@@ -1011,3 +1016,70 @@ int sequencer_pick_revisions(struct replay_opts *opts)\n \tsave_opts(opts);\n \treturn pick_commits(todo_list, opts);\n }\n+\n+static int ends_rfc2822_footer(struct strbuf *sb)\n+{\n+\tint ch;\n+\tint hit = 0;\n+\tint i, j, k;\n+\tint len = sb->len;\n+\tint first = 1;\n+\tconst char *buf = sb->buf;\n+\n+\tfor (i = len - 1; i > 0; i--) {\n+\t\tif (hit && buf[i] == '\\n')\n+\t\t\tbreak;\n+\t\thit = (buf[i] == '\\n');\n+\t}\n+\n+\twhile (i < len - 1 && buf[i] == '\\n')\n+\t\ti++;\n+\n+\tfor (; i < len; i = k) {\n+\t\tfor (k = i; k < len && buf[k] != '\\n'; k++)\n+\t\t\t; /* do nothing */\n+\t\tk++;\n+\n+\t\tif ((buf[k] == ' ' || buf[k] == '\\t') && !first)\n+\t\t\tcontinue;\n+\n+\t\tfirst = 0;\n+\n+\t\tfor (j = 0; i + j < len; j++) {\n+\t\t\tch = buf[i + j];\n+\t\t\tif (ch == ':')\n+\t\t\t\tbreak;\n+\t\t\tif (isalnum(ch) ||\n+\t\t\t    (ch == '-'))\n+\t\t\t\tcontinue;\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\treturn 1;\n+}\n+\n+void append_signoff(struct strbuf *msgbuf, int ignore_footer)\n+{\n+\tstruct strbuf sob = STRBUF_INIT;\n+\tint i;\n+\n+\tstrbuf_addstr(&sob, sign_off_header);\n+\tstrbuf_addstr(&sob, fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n+\t\t\t\tgetenv(\"GIT_COMMITTER_EMAIL\")));\n+\tstrbuf_addch(&sob, '\\n');\n+\tfor (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\\n'; i--)\n+\t\t; /* do nothing */\n+\tstruct strbuf footer = STRBUF_INIT;\n+\tif (ignore_footer > 0) {\n+\t\tstrbuf_addstr(&footer, msgbuf->buf + msgbuf->len - ignore_footer);\n+\t\tstrbuf_setlen(msgbuf, msgbuf->len - ignore_footer);\n+\t}\n+\tif (prefixcmp(msgbuf->buf + i, sob.buf)) {\n+\t\tif (!i || !ends_rfc2822_footer(msgbuf))\n+\t\t\tstrbuf_addch(msgbuf, '\\n');\n+\t\tstrbuf_addbuf(msgbuf, &sob);\n+\t}\n+\tstrbuf_release(&sob);\n+\tstrbuf_addbuf(msgbuf, &footer);\n+\tstrbuf_release(&footer);\n+}\ndiff --git a/sequencer.h b/sequencer.h\nindex d849420..60287b8 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -49,4 +49,8 @@ extern void remove_sequencer_state(void);\n \n int sequencer_pick_revisions(struct replay_opts *opts);\n \n+extern const char sign_off_header[];\n+\n+void append_signoff(struct strbuf *msgbuf, int ignore_footer);\n+\n #endif\ndiff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\nindex 0c81b3c..c82f721 100755\n--- a/t/t3507-cherry-pick-conflict.sh\n+++ b/t/t3507-cherry-pick-conflict.sh\n@@ -30,6 +30,7 @@ test_expect_success setup '\n \ttest_commit initial foo a &&\n \ttest_commit base foo b &&\n \ttest_commit picked foo c &&\n+\ttest_commit --signoff picked-signed foo d &&\n \tgit config advice.detachedhead false\n \n '\n@@ -340,4 +341,35 @@ test_expect_success 'revert conflict, diff3 -m style' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'failed cherry-pick does not forget -s' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick -s picked &&\n+\ttest_i18ngrep -e \"Signed-off-by\" .git/MERGE_MSG\n+'\n+\n+test_expect_success 'commit after failed cherry-pick does not add duplicated -s' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick -s picked-signed &&\n+\tgit commit -a -s &&\n+\ttest $(git show -s |grep -c \"Signed-off-by\") = 1\n+'\n+\n+test_expect_success 'commit after failed cherry-pick adds -s at the right place' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick picked &&\n+\tgit commit -a -s &&\n+\tpwd &&\n+\tcat <<EOF > expected &&\n+picked\n+\n+Signed-off-by: C O Mitter <committer@example.com>\n+\n+Conflicts:\n+\tfoo\n+EOF\n+\n+\tgit show -s --pretty=format:%B > actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex f4e6450..b5fb527 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -410,7 +410,7 @@ test_expect_success '--continue respects -x in first commit in multi-pick' '\n \tgrep \"cherry picked from.*$picked\" msg\n '\n \n-test_expect_success '--signoff is not automatically propagated to resolved conflict' '\n+test_expect_failure '--signoff is automatically propagated to resolved conflict' '\n \tpristine_detach initial &&\n \ttest_expect_code 1 git cherry-pick --signoff base..anotherpick &&\n \techo \"c\" >foo &&\n@@ -428,7 +428,7 @@ test_expect_success '--signoff is not automatically propagated to resolved confl\n \tgrep \"Signed-off-by:\" anotherpick_msg\n '\n \n-test_expect_success '--signoff dropped for implicit commit of resolution, multi-pick case' '\n+test_expect_failure '--signoff dropped for implicit commit of resolution, multi-pick case' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick -s picked anotherpick &&\n \techo c >foo &&\n@@ -441,7 +441,7 @@ test_expect_success '--signoff dropped for implicit commit of resolution, multi-\n \t! grep Signed-off-by: msg\n '\n \n-test_expect_success 'sign-off needs to be reaffirmed after conflict resolution, single-pick case' '\n+test_expect_failure 'sign-off needs to be reaffirmed after conflict resolution, single-pick case' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick -s picked &&\n \techo c >foo &&\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 9bc57d2..0802da5 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -144,11 +144,21 @@ test_pause () {\n \n test_commit () {\n \tnotick= &&\n-\tif test \"z$1\" = \"z--notick\"\n-\tthen\n-\t\tnotick=yes\n+\tsignoff= &&\n+\twhile test $# != 0; do\n+\t\tcase \"$1\" in\n+\t\t--notick)\n+\t\t\tnotick=yes\n+\t\t\t;;\n+\t\t--signoff)\n+\t\t\tsignoff=\"$1\"\n+\t\t\t;;\n+\t\t*)\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac\n \t\tshift\n-\tfi &&\n+\tdone &&\n \tfile=${2:-\"$1.t\"} &&\n \techo \"${3-$1}\" > \"$file\" &&\n \tgit add \"$file\" &&\n@@ -156,7 +166,7 @@ test_commit () {\n \tthen\n \t\ttest_tick\n \tfi &&\n-\tgit commit -m \"$1\" &&\n+\tgit commit $signoff -m \"$1\" &&\n \tgit tag \"$1\"\n }\n \n-- \n1.7.7\n"},{"id":"198967","messageId":"7v1ui57hit.fsf@alter.siamese.dyndns.org","threadId":"31515","inReplyTo":"20120913202714.GD14383@suse.cz","subject":"Re: [PATCH v3] cherry-pick: don't forget -s on failure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-13T21:13:46Z","receivedAt":"2012-09-13T21:13:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@suse.cz> writes:\n\n> +void append_signoff(struct strbuf *msgbuf, int ignore_footer)\n> +{\n> +\tstruct strbuf sob = STRBUF_INIT;\n> +\tint i;\n> +\n> +\tstrbuf_addstr(&sob, sign_off_header);\n> +\tstrbuf_addstr(&sob, fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n> +\t\t\t\tgetenv(\"GIT_COMMITTER_EMAIL\")));\n> +\tstrbuf_addch(&sob, '\\n');\n> +\tfor (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\\n'; i--)\n> +\t\t; /* do nothing */\n> +\tstruct strbuf footer = STRBUF_INIT;\n> +\tif (ignore_footer > 0) {\n> +\t\tstrbuf_addstr(&footer, msgbuf->buf + msgbuf->len - ignore_footer);\n> +\t\tstrbuf_setlen(msgbuf, msgbuf->len - ignore_footer);\n> +\t}\n\nThat's decl-after-stmt.\n\nI would have expected that you can just do strbuf_splice() to add\nthe &sob into &msgbuf with the original code structure, without a\nsubstantial rewrite of the function like this.  Perhaps I am missing\nsomething?\n"},{"id":"199002","messageId":"20120914065203.GB878@suse.cz","threadId":"31515","inReplyTo":"7v1ui57hit.fsf@alter.siamese.dyndns.org","subject":"[PATCH v4] cherry-pick: don't forget -s on failure","fromName":"Miklos Vajna","fromEmail":"vmiklos@suse.cz","sentAt":"2012-09-14T06:52:03Z","receivedAt":"2012-09-14T06:52:03Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"In case 'git cherry-pick -s <commit>' failed, the user had to use 'git\ncommit -s' (i.e. state the -s option again), which is easy to forget\nabout.  Instead, write the signed-off-by line early, so plain 'git\ncommit' will have the same result.\n\nAlso update 'git commit -s', so that in case there is already a relevant\nSigned-off-by line before the Conflicts: line, it won't add one more at\nthe end of the message. If there is no such line, then add it before the\nthe Conflicts: line.\n\nSigned-off-by: Miklos Vajna <vmiklos@suse.cz>\n---\n\nOn Thu, Sep 13, 2012 at 02:13:46PM -0700, Junio C Hamano <gitster@pobox.com> wrote:\n> That's decl-after-stmt.\n\nSorry, added -Wdeclaration-after-statement to CFLAGS now.\n\n> I would have expected that you can just do strbuf_splice() to add\n> the &sob into &msgbuf with the original code structure, without a\n> substantial rewrite of the function like this.  Perhaps I am missing\n> something?\n\nI forgot about strbuf_splice(). ;-) Here is a version with it -- it's \nindeed shorter, even if ends_rfc2822_footer() now has to be aware of a \npossible footer.\n\n builtin/commit.c                |   79 +++++++++++---------------------------\n sequencer.c                     |   65 ++++++++++++++++++++++++++++++++\n sequencer.h                     |    4 ++\n t/t3507-cherry-pick-conflict.sh |   32 ++++++++++++++++\n t/t3510-cherry-pick-sequence.sh |    6 +-\n t/test-lib-functions.sh         |   20 +++++++--\n 6 files changed, 142 insertions(+), 64 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 778cf16..4d50484 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -28,6 +28,7 @@\n #include \"submodule.h\"\n #include \"gpg-interface.h\"\n #include \"column.h\"\n+#include \"sequencer.h\"\n \n static const char * const builtin_commit_usage[] = {\n \tN_(\"git commit [options] [--] <filepattern>...\"),\n@@ -466,8 +467,6 @@ static int is_a_merge(const struct commit *current_head)\n \treturn !!(current_head->parents && current_head->parents->next);\n }\n \n-static const char sign_off_header[] = \"Signed-off-by: \";\n-\n static void export_one(const char *var, const char *s, const char *e, int hack)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -552,47 +551,6 @@ static void determine_author_info(struct strbuf *author_ident)\n \t}\n }\n \n-static int ends_rfc2822_footer(struct strbuf *sb)\n-{\n-\tint ch;\n-\tint hit = 0;\n-\tint i, j, k;\n-\tint len = sb->len;\n-\tint first = 1;\n-\tconst char *buf = sb->buf;\n-\n-\tfor (i = len - 1; i > 0; i--) {\n-\t\tif (hit && buf[i] == '\\n')\n-\t\t\tbreak;\n-\t\thit = (buf[i] == '\\n');\n-\t}\n-\n-\twhile (i < len - 1 && buf[i] == '\\n')\n-\t\ti++;\n-\n-\tfor (; i < len; i = k) {\n-\t\tfor (k = i; k < len && buf[k] != '\\n'; k++)\n-\t\t\t; /* do nothing */\n-\t\tk++;\n-\n-\t\tif ((buf[k] == ' ' || buf[k] == '\\t') && !first)\n-\t\t\tcontinue;\n-\n-\t\tfirst = 0;\n-\n-\t\tfor (j = 0; i + j < len; j++) {\n-\t\t\tch = buf[i + j];\n-\t\t\tif (ch == ':')\n-\t\t\t\tbreak;\n-\t\t\tif (isalnum(ch) ||\n-\t\t\t    (ch == '-'))\n-\t\t\t\tcontinue;\n-\t\t\treturn 0;\n-\t\t}\n-\t}\n-\treturn 1;\n-}\n-\n static char *cut_ident_timestamp_part(char *string)\n {\n \tchar *ket = strrchr(string, '>');\n@@ -717,21 +675,30 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tstripspace(&sb, 0);\n \n \tif (signoff) {\n-\t\tstruct strbuf sob = STRBUF_INIT;\n-\t\tint i;\n+\t\t/*\n+\t\t * See if we have a Conflicts: block at the end. If yes, count\n+\t\t * its size, so we can ignore it.\n+\t\t */\n+\t\tint ignore_footer = 0;\n+\t\tint i, eol, previous = 0;\n+\t\tconst char *nl;\n \n-\t\tstrbuf_addstr(&sob, sign_off_header);\n-\t\tstrbuf_addstr(&sob, fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n-\t\t\t\t\t     getenv(\"GIT_COMMITTER_EMAIL\")));\n-\t\tstrbuf_addch(&sob, '\\n');\n-\t\tfor (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\\n'; i--)\n-\t\t\t; /* do nothing */\n-\t\tif (prefixcmp(sb.buf + i, sob.buf)) {\n-\t\t\tif (!i || !ends_rfc2822_footer(&sb))\n-\t\t\t\tstrbuf_addch(&sb, '\\n');\n-\t\t\tstrbuf_addbuf(&sb, &sob);\n+\t\tfor (i = 0; i < sb.len; i++) {\n+\t\t\tnl = memchr(sb.buf + i, '\\n', sb.len - i);\n+\t\t\tif (nl)\n+\t\t\t\teol = nl - sb.buf;\n+\t\t\telse\n+\t\t\t\teol = sb.len;\n+\t\t\tif (!prefixcmp(sb.buf + previous, \"\\nConflicts:\\n\")) {\n+\t\t\t\tignore_footer = sb.len - previous;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\twhile (i < eol)\n+\t\t\t\ti++;\n+\t\t\tprevious = eol;\n \t\t}\n-\t\tstrbuf_release(&sob);\n+\n+\t\tappend_signoff(&sb, ignore_footer);\n \t}\n \n \tif (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)\ndiff --git a/sequencer.c b/sequencer.c\nindex f86f116..dbef5ce 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -17,6 +17,8 @@\n \n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n \n+const char sign_off_header[] = \"Signed-off-by: \";\n+\n void remove_sequencer_state(void)\n {\n \tstruct strbuf seq_dir = STRBUF_INIT;\n@@ -233,6 +235,9 @@ static int do_recursive_merge(struct commit *base, struct commit *next,\n \t\tdie(_(\"%s: Unable to write new index file\"), action_name(opts));\n \trollback_lock_file(&index_lock);\n \n+\tif (opts->signoff)\n+\t\tappend_signoff(msgbuf, 0);\n+\n \tif (!clean) {\n \t\tint i;\n \t\tstrbuf_addstr(msgbuf, \"\\nConflicts:\\n\");\n@@ -1011,3 +1016,63 @@ int sequencer_pick_revisions(struct replay_opts *opts)\n \tsave_opts(opts);\n \treturn pick_commits(todo_list, opts);\n }\n+\n+static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)\n+{\n+\tint ch;\n+\tint hit = 0;\n+\tint i, j, k;\n+\tint len = sb->len - ignore_footer;\n+\tint first = 1;\n+\tconst char *buf = sb->buf;\n+\n+\tfor (i = len - 1; i > 0; i--) {\n+\t\tif (hit && buf[i] == '\\n')\n+\t\t\tbreak;\n+\t\thit = (buf[i] == '\\n');\n+\t}\n+\n+\twhile (i < len - 1 && buf[i] == '\\n')\n+\t\ti++;\n+\n+\tfor (; i < len; i = k) {\n+\t\tfor (k = i; k < len && buf[k] != '\\n'; k++)\n+\t\t\t; /* do nothing */\n+\t\tk++;\n+\n+\t\tif ((buf[k] == ' ' || buf[k] == '\\t') && !first)\n+\t\t\tcontinue;\n+\n+\t\tfirst = 0;\n+\n+\t\tfor (j = 0; i + j < len; j++) {\n+\t\t\tch = buf[i + j];\n+\t\t\tif (ch == ':')\n+\t\t\t\tbreak;\n+\t\t\tif (isalnum(ch) ||\n+\t\t\t    (ch == '-'))\n+\t\t\t\tcontinue;\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\treturn 1;\n+}\n+\n+void append_signoff(struct strbuf *msgbuf, int ignore_footer)\n+{\n+\tstruct strbuf sob = STRBUF_INIT;\n+\tint i;\n+\n+\tstrbuf_addstr(&sob, sign_off_header);\n+\tstrbuf_addstr(&sob, fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n+\t\t\t\tgetenv(\"GIT_COMMITTER_EMAIL\")));\n+\tstrbuf_addch(&sob, '\\n');\n+\tfor (i = msgbuf->len - 1 - ignore_footer; i > 0 && msgbuf->buf[i - 1] != '\\n'; i--)\n+\t\t; /* do nothing */\n+\tif (prefixcmp(msgbuf->buf + i, sob.buf)) {\n+\t\tif (!i || !ends_rfc2822_footer(msgbuf, ignore_footer))\n+\t\t\tstrbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, \"\\n\", 1);\n+\t\tstrbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf, sob.len);\n+\t}\n+\tstrbuf_release(&sob);\n+}\ndiff --git a/sequencer.h b/sequencer.h\nindex d849420..60287b8 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -49,4 +49,8 @@ extern void remove_sequencer_state(void);\n \n int sequencer_pick_revisions(struct replay_opts *opts);\n \n+extern const char sign_off_header[];\n+\n+void append_signoff(struct strbuf *msgbuf, int ignore_footer);\n+\n #endif\ndiff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\nindex 0c81b3c..c82f721 100755\n--- a/t/t3507-cherry-pick-conflict.sh\n+++ b/t/t3507-cherry-pick-conflict.sh\n@@ -30,6 +30,7 @@ test_expect_success setup '\n \ttest_commit initial foo a &&\n \ttest_commit base foo b &&\n \ttest_commit picked foo c &&\n+\ttest_commit --signoff picked-signed foo d &&\n \tgit config advice.detachedhead false\n \n '\n@@ -340,4 +341,35 @@ test_expect_success 'revert conflict, diff3 -m style' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'failed cherry-pick does not forget -s' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick -s picked &&\n+\ttest_i18ngrep -e \"Signed-off-by\" .git/MERGE_MSG\n+'\n+\n+test_expect_success 'commit after failed cherry-pick does not add duplicated -s' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick -s picked-signed &&\n+\tgit commit -a -s &&\n+\ttest $(git show -s |grep -c \"Signed-off-by\") = 1\n+'\n+\n+test_expect_success 'commit after failed cherry-pick adds -s at the right place' '\n+\tpristine_detach initial &&\n+\ttest_must_fail git cherry-pick picked &&\n+\tgit commit -a -s &&\n+\tpwd &&\n+\tcat <<EOF > expected &&\n+picked\n+\n+Signed-off-by: C O Mitter <committer@example.com>\n+\n+Conflicts:\n+\tfoo\n+EOF\n+\n+\tgit show -s --pretty=format:%B > actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex f4e6450..b5fb527 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -410,7 +410,7 @@ test_expect_success '--continue respects -x in first commit in multi-pick' '\n \tgrep \"cherry picked from.*$picked\" msg\n '\n \n-test_expect_success '--signoff is not automatically propagated to resolved conflict' '\n+test_expect_failure '--signoff is automatically propagated to resolved conflict' '\n \tpristine_detach initial &&\n \ttest_expect_code 1 git cherry-pick --signoff base..anotherpick &&\n \techo \"c\" >foo &&\n@@ -428,7 +428,7 @@ test_expect_success '--signoff is not automatically propagated to resolved confl\n \tgrep \"Signed-off-by:\" anotherpick_msg\n '\n \n-test_expect_success '--signoff dropped for implicit commit of resolution, multi-pick case' '\n+test_expect_failure '--signoff dropped for implicit commit of resolution, multi-pick case' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick -s picked anotherpick &&\n \techo c >foo &&\n@@ -441,7 +441,7 @@ test_expect_success '--signoff dropped for implicit commit of resolution, multi-\n \t! grep Signed-off-by: msg\n '\n \n-test_expect_success 'sign-off needs to be reaffirmed after conflict resolution, single-pick case' '\n+test_expect_failure 'sign-off needs to be reaffirmed after conflict resolution, single-pick case' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick -s picked &&\n \techo c >foo &&\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 9bc57d2..0802da5 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -144,11 +144,21 @@ test_pause () {\n \n test_commit () {\n \tnotick= &&\n-\tif test \"z$1\" = \"z--notick\"\n-\tthen\n-\t\tnotick=yes\n+\tsignoff= &&\n+\twhile test $# != 0; do\n+\t\tcase \"$1\" in\n+\t\t--notick)\n+\t\t\tnotick=yes\n+\t\t\t;;\n+\t\t--signoff)\n+\t\t\tsignoff=\"$1\"\n+\t\t\t;;\n+\t\t*)\n+\t\t\tbreak\n+\t\t\t;;\n+\t\tesac\n \t\tshift\n-\tfi &&\n+\tdone &&\n \tfile=${2:-\"$1.t\"} &&\n \techo \"${3-$1}\" > \"$file\" &&\n \tgit add \"$file\" &&\n@@ -156,7 +166,7 @@ test_commit () {\n \tthen\n \t\ttest_tick\n \tfi &&\n-\tgit commit -m \"$1\" &&\n+\tgit commit $signoff -m \"$1\" &&\n \tgit tag \"$1\"\n }\n \n-- \n1.7.7\n"}]}