{"thread":{"id":"32118","subject":"[PATCH 1/5] t/test-lib-functions.sh: allow to specify the tag name to test_commit","startedAt":"2012-11-15T01:37:50Z","lastAt":"2012-11-16T23:55:43Z","messageCount":14,"participants":["Brandon Casey","Matt Kraai","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"203251","messageId":"1352943474-15573-1-git-send-email-drafnel@gmail.com","threadId":"32118","inReplyTo":null,"subject":"[PATCH 1/5] t/test-lib-functions.sh: allow to specify the tag name to test_commit","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2012-11-15T01:37:50Z","receivedAt":"2012-11-15T01:37:50Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"The part of test_commit() may not be appropriate for a tag name.\nSo let's allow test_commit to accept a fourth argument to specify the tag\nname.\n\nSigned-off-by: Brandon Casey \n---\n t/test-lib-functions.sh | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 8889ba5..4d6057e 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -135,12 +135,13 @@ test_pause () {\n \tfi\n }\n \n-# Call test_commit with the arguments \" [ []]\"\n+# Call test_commit with the arguments \" [ [ []]]\"\n #\n # This will commit a file with the given contents and the given commit\n-# message. It will also add a tag with as name.\n+# message. It will also add a tag with as name unless is\n+# given.\n #\n-# Both and default to .\n+# Both and default to .\n \n test_commit () {\n \tnotick= &&\n@@ -168,7 +169,7 @@ test_commit () {\n \t\ttest_tick\n \tfi &&\n \tgit commit $signoff -m \"$1\" &&\n-\tgit tag \"$1\"\n+\tgit tag \"${4:-$1}\"\n }\n \n # Call test_merge with the arguments \" \", where \n-- \n1.8.0\n"},{"id":"203252","messageId":"1352943474-15573-2-git-send-email-drafnel@gmail.com","threadId":"32118","inReplyTo":"1352943474-15573-1-git-send-email-drafnel@gmail.com","subject":"[PATCH 2/5] t/t3511: demonstrate breakage in cherry-pick -s","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2012-11-15T01:37:51Z","receivedAt":"2012-11-15T01:37:51Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"The cherry-pick -s functionality is currently broken in two ways.\n\n 1. handling of rfc2822 continuation lines has a bug, and the\n continuation lines are not handled correctly.\n 2. the \"(cherry picked from ...)\" lines are commonly appended to the\n end of the s-o-b footer and should be detected as part of the footer.\n They should not cause a new line to be inserted before the new s-o-b\n is added.\n\n i.e. we should produce this:\n\n Signed-off-by: A.U. Thor \n (cherry picked from )\n Signed-off-by: C O Mmitter \n\n not\n\n Signed-off-by: A.U. Thor \n (cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)\n\n Signed-off-by: C O Mmitter \n\nSigned-off-by: Brandon Casey \n---\n t/t3511-cherry-pick-x.sh | 77 ++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 77 insertions(+)\n create mode 100755 t/t3511-cherry-pick-x.sh\n\ndiff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh\nnew file mode 100755\nindex 0000000..b4e5c65\n--- /dev/null\n+++ b/t/t3511-cherry-pick-x.sh\n@@ -0,0 +1,77 @@\n+#!/bin/sh\n+\n+test_description='Test cherry-pick -x and -s'\n+\n+. ./test-lib.sh\n+\n+pristine_detach () {\n+\tgit cherry-pick --quit &&\n+\tgit checkout -f \"$1^0\" &&\n+\tgit read-tree -u --reset HEAD &&\n+\tgit clean -d -f -f -q -x\n+}\n+\n+non_rfc2822_mesg='base with footer\n+\n+Commit message body is here.'\n+\n+rfc2822_mesg=\"$non_rfc2822_mesg\n+\n+Signed-off-by: A.U. Thor\n+ \n+Signed-off-by: B.U. Thor \"\n+\n+rfc2822_cherry_mesg=\"$rfc2822_mesg\n+(cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)\n+Tested-by: C.U. Thor \"\n+\n+\n+test_expect_success setup '\n+\tgit config advice.detachedhead false &&\n+\techo unrelated >unrelated &&\n+\tgit add unrelated &&\n+\ttest_commit initial foo a &&\n+\ttest_commit \"$non_rfc2822_mesg\" foo b non-rfc2822-base &&\n+\tgit reset --hard initial &&\n+\ttest_commit \"$rfc2822_mesg\" foo b rfc2822-base &&\n+\tgit reset --hard initial &&\n+\ttest_commit \"$rfc2822_cherry_mesg\" foo b rfc2822-cherry-base &&\n+\tpristine_detach initial &&\n+\ttest_commit conflicting unrelated\n+'\n+\n+test_expect_success 'cherry-pick -s inserts blank line after non-rfc2822 footer' '\n+\tpristine_detach initial &&\n+\tgit cherry-pick -s non-rfc2822-base &&\n+\tcat <<-EOF >expect &&\n+\t\t$non_rfc2822_mesg\n+\n+\t\tSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n+\tEOF\n+\tgit log -1 --pretty=format:%B >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_failure 'cherry-pick -s not confused by rfc2822 continuation line' '\n+\tpristine_detach initial &&\n+\tgit cherry-pick -s rfc2822-base &&\n+\tcat <<-EOF >expect &&\n+\t\t$rfc2822_mesg\n+\t\tSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n+\tEOF\n+\tgit log -1 --pretty=format:%B >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_failure 'cherry-pick treats -s \"(cherry picked from...\" line as part of footer' '\n+\tpristine_detach initial &&\n+\tgit cherry-pick -s rfc2822-cherry-base &&\n+\tcat <<-EOF >expect &&\n+\t\t$rfc2822_cherry_mesg\n+\t\tSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n+\tEOF\n+\tgit log -1 --pretty=format:%B >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n1.8.0\n"},{"id":"203256","messageId":"1352943474-15573-3-git-send-email-drafnel@gmail.com","threadId":"32118","inReplyTo":"1352943474-15573-1-git-send-email-drafnel@gmail.com","subject":"[PATCH 3/5] sequencer.c: handle rfc2822 continuation lines correctly","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2012-11-15T01:37:52Z","receivedAt":"2012-11-15T01:37:52Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"ends_rfc2822_footer() was incorrectly checking whether the current line\nwas a continuation of the previous line. It was actually checking the\nnext line instead of the current line. Let's fix this and mark the test\nas expect_success.\n\nSigned-off-by: Brandon Casey \n---\n sequencer.c | 2 +-\n t/t3511-cherry-pick-x.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex be0cb8b..01edec2 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1040,7 +1040,7 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)\n \t\t\t; /* do nothing */\n \t\tk++;\n \n-\t\tif ((buf[k] == ' ' || buf[k] == '\\t') && !first)\n+\t\tif ((buf[i] == ' ' || buf[i] == '\\t') && !first)\n \t\t\tcontinue;\n \n \t\tfirst = 0;\ndiff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh\nindex b4e5c65..b2098e0 100755\n--- a/t/t3511-cherry-pick-x.sh\n+++ b/t/t3511-cherry-pick-x.sh\n@@ -52,7 +52,7 @@ test_expect_success 'cherry-pick -s inserts blank line after non-rfc2822 footer'\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'cherry-pick -s not confused by rfc2822 continuation line' '\n+test_expect_success 'cherry-pick -s not confused by rfc2822 continuation line' '\n \tpristine_detach initial &&\n \tgit cherry-pick -s rfc2822-base &&\n \tcat <<-EOF >expect &&\n-- \n1.8.0\n"},{"id":"203253","messageId":"1352943474-15573-4-git-send-email-drafnel@gmail.com","threadId":"32118","inReplyTo":"1352943474-15573-1-git-send-email-drafnel@gmail.com","subject":"[PATCH 4/5] sequencer.c: recognize \"(cherry picked from ...\" as part of s-o-b footer","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2012-11-15T01:37:53Z","receivedAt":"2012-11-15T01:37:53Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Currently, if the s-o-b footer of a commit message contains a\n\"(cherry picked from ...\" line that was added by a previous cherry-pick -x,\nit is not recognized as a s-o-b footer and will cause a newline to be\ninserted before an additional s-o-b is added.\n\nSo, rework ends_rfc2822_footer to recognize the \"(cherry picked from ...\"\nstring as part of the footer. Plus mark the test in t3511 as fixed.\n\nSigned-off-by: Brandon Casey \n---\n sequencer.c | 44 +++++++++++++++++++++++++++++---------------\n t/t3511-cherry-pick-x.sh | 2 +-\n 2 files changed, 30 insertions(+), 16 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 01edec2..27e684c 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -18,6 +18,7 @@\n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n \n const char sign_off_header[] = \"Signed-off-by: \";\n+const char cherry_picked_prefix[] = \"(cherry picked from commit \";\n \n static void remove_sequencer_state(void)\n {\n@@ -492,7 +493,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \t\t}\n \n \t\tif (opts->record_origin) {\n-\t\t\tstrbuf_addstr(&msgbuf, \"(cherry picked from commit \");\n+\t\t\tstrbuf_addstr(&msgbuf, cherry_picked_prefix);\n \t\t\tstrbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));\n \t\t\tstrbuf_addstr(&msgbuf, \")\\n\");\n \t\t}\n@@ -1017,13 +1018,34 @@ int sequencer_pick_revisions(struct replay_opts *opts)\n \treturn pick_commits(todo_list, opts);\n }\n \n+static int is_rfc2822_line(const char *buf, int len)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < len; i++) {\n+\t\tint ch = buf[i];\n+\t\tif (ch == ':')\n+\t\t\tbreak;\n+\t\tif (isalnum(ch) || (ch == '-'))\n+\t\t\tcontinue;\n+\t\treturn 0;\n+\t}\n+\n+\treturn 1;\n+}\n+\n+static int is_cherry_pick_from_line(const char *buf, int len)\n+{\n+\treturn (strlen(cherry_picked_prefix) + 41) <= len &&\n+\t\t!prefixcmp(buf, cherry_picked_prefix);\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 i, k;\n \tint len = sb->len - ignore_footer;\n-\tint first = 1;\n+\tint last_was_rfc2822 = 0;\n \tconst char *buf = sb->buf;\n \n \tfor (i = len - 1; i > 0; i--) {\n@@ -1040,20 +1062,12 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)\n \t\t\t; /* do nothing */\n \t\tk++;\n \n-\t\tif ((buf[i] == ' ' || buf[i] == '\\t') && !first)\n+\t\tif (last_was_rfc2822 && (buf[i] == ' ' || buf[i] == '\\t'))\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\tif (!((last_was_rfc2822 = is_rfc2822_line(buf+i, k-i)) ||\n+\t\t\tis_cherry_pick_from_line(buf+i, k-i)))\n \t\t\treturn 0;\n-\t\t}\n \t}\n \treturn 1;\n }\ndiff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh\nindex b2098e0..785486e 100755\n--- a/t/t3511-cherry-pick-x.sh\n+++ b/t/t3511-cherry-pick-x.sh\n@@ -63,7 +63,7 @@ test_expect_success 'cherry-pick -s not confused by rfc2822 continuation line' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'cherry-pick treats -s \"(cherry picked from...\" line as part of footer' '\n+test_expect_success 'cherry-pick treats -s \"(cherry picked from...\" line as part of footer' '\n \tpristine_detach initial &&\n \tgit cherry-pick -s rfc2822-cherry-base &&\n \tcat <<-EOF >expect &&\n-- \n1.8.0\n"},{"id":"203254","messageId":"1352943474-15573-5-git-send-email-drafnel@gmail.com","threadId":"32118","inReplyTo":"1352943474-15573-1-git-send-email-drafnel@gmail.com","subject":"[PATCH/RFC 5/5] sequencer.c: always separate \"(cherry picked from\" from commit body","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2012-11-15T01:37:54Z","receivedAt":"2012-11-15T01:37:54Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Start treating the \"(cherry picked from\" line added by cherry-pick -x\nthe same way that the s-o-b lines are treated. Namely, separate them\nfrom the main commit message body with an empty line.\n\nAlso, introduce tests to test this functionality.\n\nSigned-off-by: Brandon Casey \n---\n\n\nThis seems like the right thing to do, but it's more of a change in\npolicy than the others, so I marked it as RFC. Any disagreement here?\n\n-Brandon\n\n\n sequencer.c | 110 ++++++++++++++++++++++++-----------------------\n t/t3511-cherry-pick-x.sh | 77 +++++++++++++++++++++++++++++++++\n 2 files changed, 133 insertions(+), 54 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 27e684c..0da0538 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -20,6 +20,60 @@\n const char sign_off_header[] = \"Signed-off-by: \";\n const char cherry_picked_prefix[] = \"(cherry picked from commit \";\n \n+static int is_rfc2822_line(const char *buf, int len)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < len; i++) {\n+\t\tint ch = buf[i];\n+\t\tif (ch == ':')\n+\t\t\tbreak;\n+\t\tif (isalnum(ch) || (ch == '-'))\n+\t\t\tcontinue;\n+\t\treturn 0;\n+\t}\n+\n+\treturn 1;\n+}\n+\n+static int is_cherry_pick_from_line(const char *buf, int len)\n+{\n+\treturn (strlen(cherry_picked_prefix) + 41) <= len &&\n+\t\t!prefixcmp(buf, cherry_picked_prefix);\n+}\n+\n+static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)\n+{\n+\tint hit = 0;\n+\tint i, k;\n+\tint len = sb->len - ignore_footer;\n+\tint last_was_rfc2822 = 0;\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 (last_was_rfc2822 && (buf[i] == ' ' || buf[i] == '\\t'))\n+\t\t\tcontinue;\n+\n+\t\tif (!((last_was_rfc2822 = is_rfc2822_line(buf+i, k-i)) ||\n+\t\t\tis_cherry_pick_from_line(buf+i, k-i)))\n+\t\t\treturn 0;\n+\t}\n+\treturn 1;\n+}\n+\n static void remove_sequencer_state(void)\n {\n \tstruct strbuf seq_dir = STRBUF_INIT;\n@@ -493,6 +547,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \t\t}\n \n \t\tif (opts->record_origin) {\n+\t\t\tif (!ends_rfc2822_footer(&msgbuf, 0))\n+\t\t\t\tstrbuf_addch(&msgbuf, '\\n');\n \t\t\tstrbuf_addstr(&msgbuf, cherry_picked_prefix);\n \t\t\tstrbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));\n \t\t\tstrbuf_addstr(&msgbuf, \")\\n\");\n@@ -1018,60 +1074,6 @@ int sequencer_pick_revisions(struct replay_opts *opts)\n \treturn pick_commits(todo_list, opts);\n }\n \n-static int is_rfc2822_line(const char *buf, int len)\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < len; i++) {\n-\t\tint ch = buf[i];\n-\t\tif (ch == ':')\n-\t\t\tbreak;\n-\t\tif (isalnum(ch) || (ch == '-'))\n-\t\t\tcontinue;\n-\t\treturn 0;\n-\t}\n-\n-\treturn 1;\n-}\n-\n-static int is_cherry_pick_from_line(const char *buf, int len)\n-{\n-\treturn (strlen(cherry_picked_prefix) + 41) <= len &&\n-\t\t!prefixcmp(buf, cherry_picked_prefix);\n-}\n-\n-static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)\n-{\n-\tint hit = 0;\n-\tint i, k;\n-\tint len = sb->len - ignore_footer;\n-\tint last_was_rfc2822 = 0;\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 (last_was_rfc2822 && (buf[i] == ' ' || buf[i] == '\\t'))\n-\t\t\tcontinue;\n-\n-\t\tif (!((last_was_rfc2822 = is_rfc2822_line(buf+i, k-i)) ||\n-\t\t\tis_cherry_pick_from_line(buf+i, k-i)))\n-\t\t\treturn 0;\n-\t}\n-\treturn 1;\n-}\n-\n void append_signoff(struct strbuf *msgbuf, int ignore_footer)\n {\n \tstruct strbuf sob = STRBUF_INIT;\ndiff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh\nindex 785486e..af7a87c 100755\n--- a/t/t3511-cherry-pick-x.sh\n+++ b/t/t3511-cherry-pick-x.sh\n@@ -40,6 +40,19 @@ test_expect_success setup '\n \ttest_commit conflicting unrelated\n '\n \n+test_expect_success 'cherry-pick -x inserts blank line after non-rfc2822 footer' '\n+\tpristine_detach initial &&\n+\tsha1=`git rev-parse non-rfc2822-base^0` &&\n+\tgit cherry-pick -x non-rfc2822-base &&\n+\tcat <<-EOF >expect &&\n+\t\t$non_rfc2822_mesg\n+\n+\t\t(cherry picked from commit $sha1)\n+\tEOF\n+\tgit log -1 --pretty=format:%B >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'cherry-pick -s inserts blank line after non-rfc2822 footer' '\n \tpristine_detach initial &&\n \tgit cherry-pick -s non-rfc2822-base &&\n@@ -52,6 +65,32 @@ test_expect_success 'cherry-pick -s inserts blank line after non-rfc2822 footer'\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'cherry-pick -x -s inserts blank line after non-rfc2822 footer' '\n+\tpristine_detach initial &&\n+\tsha1=`git rev-parse non-rfc2822-base^0` &&\n+\tgit cherry-pick -x -s non-rfc2822-base &&\n+\tcat <<-EOF >expect &&\n+\t\t$non_rfc2822_mesg\n+\n+\t\t(cherry picked from commit $sha1)\n+\t\tSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n+\tEOF\n+\tgit log -1 --pretty=format:%B >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'cherry-pick -x not confused by rfc2822 continuation line' '\n+\tpristine_detach initial &&\n+\tsha1=`git rev-parse rfc2822-base^0` &&\n+\tgit cherry-pick -x rfc2822-base &&\n+\tcat <<-EOF >expect &&\n+\t\t$rfc2822_mesg\n+\t\t(cherry picked from commit $sha1)\n+\tEOF\n+\tgit log -1 --pretty=format:%B >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'cherry-pick -s not confused by rfc2822 continuation line' '\n \tpristine_detach initial &&\n \tgit cherry-pick -s rfc2822-base &&\n@@ -63,6 +102,31 @@ test_expect_success 'cherry-pick -s not confused by rfc2822 continuation line' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'cherry-pick -x -s not confused by rfc2822 continuation line' '\n+\tpristine_detach initial &&\n+\tsha1=`git rev-parse rfc2822-base^0` &&\n+\tgit cherry-pick -x -s rfc2822-base &&\n+\tcat <<-EOF >expect &&\n+\t\t$rfc2822_mesg\n+\t\t(cherry picked from commit $sha1)\n+\t\tSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n+\tEOF\n+\tgit log -1 --pretty=format:%B >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'cherry-pick treats -x \"(cherry picked from...\" line as part of footer' '\n+\tpristine_detach initial &&\n+\tsha1=`git rev-parse rfc2822-cherry-base^0` &&\n+\tgit cherry-pick -x rfc2822-cherry-base &&\n+\tcat <<-EOF >expect &&\n+\t\t$rfc2822_cherry_mesg\n+\t\t(cherry picked from commit $sha1)\n+\tEOF\n+\tgit log -1 --pretty=format:%B >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'cherry-pick treats -s \"(cherry picked from...\" line as part of footer' '\n \tpristine_detach initial &&\n \tgit cherry-pick -s rfc2822-cherry-base &&\n@@ -74,4 +138,17 @@ test_expect_success 'cherry-pick treats -s \"(cherry picked from...\" line as part\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'cherry-pick treats -x -s \"(cherry picked from...\" line as part of footer' '\n+\tpristine_detach initial &&\n+\tsha1=`git rev-parse rfc2822-cherry-base^0` &&\n+\tgit cherry-pick -x -s rfc2822-cherry-base &&\n+\tcat <<-EOF >expect &&\n+\t\t$rfc2822_cherry_mesg\n+\t\t(cherry picked from commit $sha1)\n+\t\tSigned-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n+\tEOF\n+\tgit log -1 --pretty=format:%B >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n1.8.0\n"},{"id":"203264","messageId":"20121115032005.GA20677@ftbfs.org","threadId":"32118","inReplyTo":"1352943474-15573-1-git-send-email-drafnel@gmail.com","subject":"Re: [PATCH 1/5] t/test-lib-functions.sh: allow to specify the tag name to test_commit","fromName":"Matt Kraai","fromEmail":"kraai@ftbfs.org","sentAt":"2012-11-15T03:20:05Z","receivedAt":"2012-11-15T03:20:05Z","isPatch":true,"sender":{"key":"kraai@ftbfs.org","avatar":null},"body":"On Wed, Nov 14, 2012 at 05:37:50PM -0800, Brandon Casey wrote:\n> -# Both and default to .\n> +# Both and default to .\n\nI think this line would be better as\n\n # , , and all default to .\n\nsince there's now more than two arguments that default to message.\n\n-- \nMatt\n"},{"id":"203266","messageId":"CA+sFfMfq69tawBCrWEmtb-ayxT056Fj-=rO8-ZpGqy5X4yPD6g@mail.gmail.com","threadId":"32118","inReplyTo":"20121115032005.GA20677@ftbfs.org","subject":"Re: [PATCH 1/5] t/test-lib-functions.sh: allow to specify the tag name to test_commit","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2012-11-15T05:43:23Z","receivedAt":"2012-11-15T05:43:23Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Good eye. Thanks.\n\nOn Wed, Nov 14, 2012 at 7:20 PM, Matt Kraai wrote:\n> On Wed, Nov 14, 2012 at 05:37:50PM -0800, Brandon Casey wrote:\n>> -# Both and default to .\n>> +# Both and default to .\n>\n> I think this line would be better as\n>\n> # , , and all default to .\n>\n> since there's now more than two arguments that default to message.\n>\n> --\n> Matt\n"},{"id":"203267","messageId":"1352958570-18724-1-git-send-email-drafnel@gmail.com","threadId":"32118","inReplyTo":"20121115032005.GA20677@ftbfs.org","subject":"[PATCH 1/5 v2] t/test-lib-functions.sh: allow to specify the tag name to test_commit","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2012-11-15T05:49:30Z","receivedAt":"2012-11-15T05:49:30Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"The part of test_commit() may not be appropriate for a tag name.\nSo let's allow test_commit to accept a fourth argument to specify the tag\nname.\n\nSigned-off-by: Brandon Casey \n---\n t/test-lib-functions.sh | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 8889ba5..9e2b8b8 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -135,12 +135,13 @@ test_pause () {\n \tfi\n }\n \n-# Call test_commit with the arguments \" [ []]\"\n+# Call test_commit with the arguments \" [ [ []]]\"\n #\n # This will commit a file with the given contents and the given commit\n-# message. It will also add a tag with as name.\n+# message. It will also add a tag with as name unless is\n+# given.\n #\n-# Both and default to .\n+# , , and all default to .\n \n test_commit () {\n \tnotick= &&\n@@ -168,7 +169,7 @@ test_commit () {\n \t\ttest_tick\n \tfi &&\n \tgit commit $signoff -m \"$1\" &&\n-\tgit tag \"$1\"\n+\tgit tag \"${4:-$1}\"\n }\n \n # Call test_merge with the arguments \" \", where \n-- \n1.8.0\n"},{"id":"203304","messageId":"1353002127-15863-1-git-send-email-drafnel@gmail.com","threadId":"32118","inReplyTo":"1352943474-15573-4-git-send-email-drafnel@gmail.com","subject":"[PATCH v2 4/5] sequencer.c: recognize \"(cherry picked from ...\" as part of s-o-b footer","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2012-11-15T17:55:27Z","receivedAt":"2012-11-15T17:55:27Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Currently, if the s-o-b footer of a commit message contains a\n\"(cherry picked from ...\" line that was added by a previous cherry-pick -x,\nit is not recognized as a s-o-b footer and will cause a newline to be\ninserted before an additional s-o-b is added.\n\nSo, rework ends_rfc2822_footer to recognize the \"(cherry picked from ...\"\nstring as part of the footer. Plus mark the test in t3511 as fixed.\n\nSigned-off-by: Brandon Casey \n---\n\nDeclare cherry_picked_prefix variable as static. This is the only change\nwith respect to v1.\n\n-Brandon\n\n sequencer.c | 44 +++++++++++++++++++++++++++++---------------\n t/t3511-cherry-pick-x.sh | 2 +-\n 2 files changed, 30 insertions(+), 16 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 01edec2..213fa4f 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -18,6 +18,7 @@\n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n \n const char sign_off_header[] = \"Signed-off-by: \";\n+static const char cherry_picked_prefix[] = \"(cherry picked from commit \";\n \n static void remove_sequencer_state(void)\n {\n@@ -492,7 +493,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \t\t}\n \n \t\tif (opts->record_origin) {\n-\t\t\tstrbuf_addstr(&msgbuf, \"(cherry picked from commit \");\n+\t\t\tstrbuf_addstr(&msgbuf, cherry_picked_prefix);\n \t\t\tstrbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));\n \t\t\tstrbuf_addstr(&msgbuf, \")\\n\");\n \t\t}\n@@ -1017,13 +1018,34 @@ int sequencer_pick_revisions(struct replay_opts *opts)\n \treturn pick_commits(todo_list, opts);\n }\n \n+static int is_rfc2822_line(const char *buf, int len)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < len; i++) {\n+\t\tint ch = buf[i];\n+\t\tif (ch == ':')\n+\t\t\tbreak;\n+\t\tif (isalnum(ch) || (ch == '-'))\n+\t\t\tcontinue;\n+\t\treturn 0;\n+\t}\n+\n+\treturn 1;\n+}\n+\n+static int is_cherry_pick_from_line(const char *buf, int len)\n+{\n+\treturn (strlen(cherry_picked_prefix) + 41) <= len &&\n+\t\t!prefixcmp(buf, cherry_picked_prefix);\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 i, k;\n \tint len = sb->len - ignore_footer;\n-\tint first = 1;\n+\tint last_was_rfc2822 = 0;\n \tconst char *buf = sb->buf;\n \n \tfor (i = len - 1; i > 0; i--) {\n@@ -1040,20 +1062,12 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)\n \t\t\t; /* do nothing */\n \t\tk++;\n \n-\t\tif ((buf[i] == ' ' || buf[i] == '\\t') && !first)\n+\t\tif (last_was_rfc2822 && (buf[i] == ' ' || buf[i] == '\\t'))\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\tif (!((last_was_rfc2822 = is_rfc2822_line(buf+i, k-i)) ||\n+\t\t\tis_cherry_pick_from_line(buf+i, k-i)))\n \t\t\treturn 0;\n-\t\t}\n \t}\n \treturn 1;\n }\ndiff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh\nindex b2098e0..785486e 100755\n--- a/t/t3511-cherry-pick-x.sh\n+++ b/t/t3511-cherry-pick-x.sh\n@@ -63,7 +63,7 @@ test_expect_success 'cherry-pick -s not confused by rfc2822 continuation line' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'cherry-pick treats -s \"(cherry picked from...\" line as part of footer' '\n+test_expect_success 'cherry-pick treats -s \"(cherry picked from...\" line as part of footer' '\n \tpristine_detach initial &&\n \tgit cherry-pick -s rfc2822-cherry-base &&\n \tcat <<-EOF >expect &&\n-- \n1.8.0\n"},{"id":"203320","messageId":"1353021875-7552-1-git-send-email-drafnel@gmail.com","threadId":"32118","inReplyTo":"1352943474-15573-5-git-send-email-drafnel@gmail.com","subject":"[PATCH 6/5] sequencer.c: refrain from adding duplicate s-o-b lines","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2012-11-15T23:24:35Z","receivedAt":"2012-11-15T23:24:35Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Detect whether the s-o-b already exists in the commit footer and refrain\nfrom adding a duplicate.\n\nUpdate t3511 to test new behavior.\n\nSigned-off-by: Brandon Casey \n---\n\n\nHi Duy,\n\nHow about this patch on top of my series as a base for your patch to\nunify the code paths that append signoff in format-patch, commit, and\nsequencer?\n\n-Brandon\n\n\n sequencer.c | 28 ++++++++++++++++++----------\n t/t3511-cherry-pick-x.sh | 20 ++++++++++++++++++--\n 2 files changed, 36 insertions(+), 12 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 7ad1163..546dacb 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -42,13 +42,15 @@ static int is_cherry_pick_from_line(const char *buf, int len)\n \t\t!prefixcmp(buf, cherry_picked_prefix);\n }\n \n-static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)\n+static int ends_rfc2822_footer(struct strbuf *sb, struct strbuf *sob,\n+\tint ignore_footer)\n {\n \tint hit = 0;\n \tint i, k;\n \tint len = sb->len - ignore_footer;\n \tint last_was_rfc2822 = 0;\n \tconst char *buf = sb->buf;\n+\tint found_sob = 0;\n \n \tfor (i = len - 1; i > 0; i--) {\n \t\tif (hit && buf[i] == '\\n')\n@@ -66,12 +68,15 @@ static int ends_rfc2822_footer(struct strbuf *sb, int ignore_footer)\n \n \t\tif (last_was_rfc2822 && (buf[i] == ' ' || buf[i] == '\\t'))\n \t\t\tcontinue;\n+\t\tif ((last_was_rfc2822 = is_rfc2822_line(buf+i, k-i)) &&\n+\t\t\tsob && !found_sob &&\n+\t\t\t!strncasecmp(buf+i, sob->buf, sob->len))\n+\t\t\tfound_sob = 1;\n \n-\t\tif (!((last_was_rfc2822 = is_rfc2822_line(buf+i, k-i)) ||\n-\t\t\tis_cherry_pick_from_line(buf+i, k-i)))\n+\t\tif (!(last_was_rfc2822 || is_cherry_pick_from_line(buf+i, k-i)))\n \t\t\treturn 0;\n \t}\n-\treturn 1;\n+\treturn 1 + found_sob;\n }\n \n static void remove_sequencer_state(void)\n@@ -547,7 +552,7 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \t\t}\n \n \t\tif (opts->record_origin) {\n-\t\t\tif (!ends_rfc2822_footer(&msgbuf, 0))\n+\t\t\tif (!ends_rfc2822_footer(&msgbuf, NULL, 0))\n \t\t\t\tstrbuf_addch(&msgbuf, '\\n');\n \t\t\tstrbuf_addstr(&msgbuf, cherry_picked_prefix);\n \t\t\tstrbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));\n@@ -1077,6 +1082,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)\n void append_signoff(struct strbuf *msgbuf, int ignore_footer)\n {\n \tstruct strbuf sob = STRBUF_INIT;\n+\tint has_footer = 0;\n \tint i;\n \n \tstrbuf_addstr(&sob, sign_off_header);\n@@ -1085,10 +1091,12 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer)\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+\tif (!i || !(has_footer =\n+\t\tends_rfc2822_footer(msgbuf, &sob, ignore_footer)))\n+\t\t\tstrbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0,\n+\t\t\t\t\"\\n\", 1);\n+\tif (has_footer != 2)\n+\t\tstrbuf_splice(msgbuf, msgbuf->len - ignore_footer, 0, sob.buf,\n+\t\t\tsob.len);\n \tstrbuf_release(&sob);\n }\ndiff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh\nindex af7a87c..a15b199 100755\n--- a/t/t3511-cherry-pick-x.sh\n+++ b/t/t3511-cherry-pick-x.sh\n@@ -11,9 +11,10 @@ pristine_detach () {\n \tgit clean -d -f -f -q -x\n }\n \n-non_rfc2822_mesg='base with footer\n+non_rfc2822_mesg=\"base with footer\n \n-Commit message body is here.'\n+Commit message body is here.\n+Not an s-o-b Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\"\n \n rfc2822_mesg=\"$non_rfc2822_mesg\n \n@@ -25,6 +26,9 @@ rfc2822_cherry_mesg=\"$rfc2822_mesg\n (cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)\n Tested-by: C.U. Thor \"\n \n+rfc2822_cherry_sob_mesg=\"$rfc2822_cherry_mesg\n+Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\n+Signed-off-by: C.U. Thor \"\n \n test_expect_success setup '\n \tgit config advice.detachedhead false &&\n@@ -36,6 +40,8 @@ test_expect_success setup '\n \ttest_commit \"$rfc2822_mesg\" foo b rfc2822-base &&\n \tgit reset --hard initial &&\n \ttest_commit \"$rfc2822_cherry_mesg\" foo b rfc2822-cherry-base &&\n+\tgit reset --hard initial &&\n+\ttest_commit \"$rfc2822_cherry_sob_mesg\" foo b rfc2822-cherry-sob-base &&\n \tpristine_detach initial &&\n \ttest_commit conflicting unrelated\n '\n@@ -151,4 +157,14 @@ test_expect_success 'cherry-pick treats -x -s \"(cherry picked from...\" line as p\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'cherry-pick -s detects committer s-o-b already exists' '\n+\tpristine_detach initial &&\n+\tgit cherry-pick -s rfc2822-cherry-sob-base &&\n+\tcat <<-EOF >expect &&\n+\t\t$rfc2822_cherry_sob_mesg\n+\tEOF\n+\tgit log -1 --pretty=format:%B >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n1.8.0\n"},{"id":"203324","messageId":"7v8va28g1c.fsf@alter.siamese.dyndns.org","threadId":"32118","inReplyTo":"1352943474-15573-2-git-send-email-drafnel@gmail.com","subject":"Re: [PATCH 2/5] t/t3511: demonstrate breakage in cherry-pick -s","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-16T01:58:39Z","receivedAt":"2012-11-16T01:58:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey writes:\n\n> The cherry-pick -s functionality is currently broken in two ways.\n>\n> 1. handling of rfc2822 continuation lines has a bug, and the\n> continuation lines are not handled correctly.\n\nThis is not limited to you, but people should think twice when\nwriting \"has a bug\" and \"are not handled correctly\" in their log\nmessage. Did you write what the expected and actual behaviours?\n\n> +rfc2822_mesg=\"$non_rfc2822_mesg\n> +\n> +Signed-off-by: A.U. Thor\n> + \n> +Signed-off-by: B.U. Thor \"\n\nThe S-o-b: lines are meant to record people's contact info in human\nreadable forms, and folding the lines like the above makes it a lot\nharder to read. They typically do not have to be folded.\n\nBesides, the footer lines are *not* RFC2822 headers (and are not\nused as such when send-email comes up with Cc: list) in the first\nplace; have we ever said anything about supporting the RFC2822 line\nfolding in the commit footer? If not (and I am reasonably sure we\nnever have), I personally think we should actively *discourage* line\nfolding there.\n\n> i.e. we should produce this:\n>\n> Signed-off-by: A.U. Thor \n> (cherry picked from )\n> Signed-off-by: C O Mmitter \n>\n> not\n>\n> Signed-off-by: A.U. Thor \n> (cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)\n>\n> Signed-off-by: C O Mmitter \n\nI can buy that, but then this makes it very clear that these footer\nlines are not shaped like RFC2822 headers, no?\n\nThanks.\n"},{"id":"203325","messageId":"7v4nkq8fsn.fsf@alter.siamese.dyndns.org","threadId":"32118","inReplyTo":"1353021875-7552-1-git-send-email-drafnel@gmail.com","subject":"Re: [PATCH 6/5] sequencer.c: refrain from adding duplicate s-o-b lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-16T02:03:52Z","receivedAt":"2012-11-16T02:03:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey writes:\n\n> Detect whether the s-o-b already exists in the commit footer and refrain\n> from adding a duplicate.\n\nIf you are trying to forbid\n\n\tgit cherry-pick -s $other\n\nfrom adding s-o-b: A when $other ends with these two existing s-o-b:\n\n\ts-o-b: A\n\ts-o-b: B\n\nthen I think that is a wrong thing to do. \n\nIn such a case, the resulting commit should gain another s-o-b from\nA to record the provenance as a chain of events. A originally wrote\nthe patch, B forwarded it (possibly with his own tweaks), and then A\npicked it up and recorded the result to the history, possibly with a\nfinal tweak or two.\n"},{"id":"203326","messageId":"CA+sFfMfvZg5An5sZ4TvXM5k_op5WwhvKgma3Mq4bX9NG-a6-Cg@mail.gmail.com","threadId":"32118","inReplyTo":"7v8va28g1c.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/5] t/t3511: demonstrate breakage in cherry-pick -s","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2012-11-16T02:40:30Z","receivedAt":"2012-11-16T02:40:30Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On Thu, Nov 15, 2012 at 5:58 PM, Junio C Hamano wrote:\n> Brandon Casey writes:\n>\n>> The cherry-pick -s functionality is currently broken in two ways.\n>>\n>> 1. handling of rfc2822 continuation lines has a bug, and the\n>> continuation lines are not handled correctly.\n>\n> This is not limited to you, but people should think twice when\n> writing \"has a bug\" and \"are not handled correctly\" in their log\n> message. Did you write what the expected and actual behaviours?\n\nYeah, I wasn't clear here. The \"bug\" was that the incorrect index\nvariable was being used, which caused the wrong line to be examined to\nsee if it was an rfc2822 continuation line. I could have mentioned\nthat.\n\n>> +rfc2822_mesg=\"$non_rfc2822_mesg\n>> +\n>> +Signed-off-by: A.U. Thor\n>> + \n>> +Signed-off-by: B.U. Thor \"\n>\n> The S-o-b: lines are meant to record people's contact info in human\n> readable forms, and folding the lines like the above makes it a lot\n> harder to read. They typically do not have to be folded.\n\nWell, I wasn't adding functionality here, I was only fixing what I\nnoticed was broken when I started touching this code.\n\n> Besides, the footer lines are *not* RFC2822 headers (and are not\n> used as such when send-email comes up with Cc: list) in the first\n> place; have we ever said anything about supporting the RFC2822 line\n> folding in the commit footer? If not (and I am reasonably sure we\n> never have), I personally think we should actively *discourage* line\n> folding there.\n\nThat's fine with me. I can't think of a reason that would make it\nnecessary to support line continuation.\n\n>> i.e. we should produce this:\n>>\n>> Signed-off-by: A.U. Thor \n>> (cherry picked from )\n>> Signed-off-by: C O Mmitter \n>>\n>> not\n>>\n>> Signed-off-by: A.U. Thor \n>> (cherry picked from da39a3ee5e6b4b0d3255bfef95601890afd80709)\n>>\n>> Signed-off-by: C O Mmitter \n>\n> I can buy that, but then this makes it very clear that these footer\n> lines are not shaped like RFC2822 headers, no?\n\nThe lines that are _not_ \"(cherry picked from ...)\" lines do follow\nthe format defined by rfc2822 for header lines (mostly). That's\nprobably why the author of the function in sequencer.c that checks for\na s-o-b footer named it \"ends_rfc2822_footer\".\n\nI'll remove the *broken* existing code that was intended to support\ncontinuation lines and submit a new patch.\n\n-Brandon\n"},{"id":"203385","messageId":"CA+sFfMfQz1qWpVZ4GCJdN8rOQNn+zkh9cd5wd7ouZL9xyqZRJQ@mail.gmail.com","threadId":"32118","inReplyTo":"7v4nkq8fsn.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 6/5] sequencer.c: refrain from adding duplicate s-o-b lines","fromName":"Brandon Casey","fromEmail":"drafnel@gmail.com","sentAt":"2012-11-16T23:55:43Z","receivedAt":"2012-11-16T23:55:43Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On Thu, Nov 15, 2012 at 6:03 PM, Junio C Hamano wrote:\n> Brandon Casey writes:\n>\n>> Detect whether the s-o-b already exists in the commit footer and refrain\n>> from adding a duplicate.\n>\n> If you are trying to forbid\n>\n> git cherry-pick -s $other\n>\n> from adding s-o-b: A when $other ends with these two existing s-o-b:\n>\n> s-o-b: A\n> s-o-b: B\n>\n> then I think that is a wrong thing to do.\n>\n> In such a case, the resulting commit should gain another s-o-b from\n> A to record the provenance as a chain of events. A originally wrote\n> the patch, B forwarded it (possibly with his own tweaks), and then A\n> picked it up and recorded the result to the history, possibly with a\n> final tweak or two.\n\nHmm. I've never thought that it was necessary to add an additional\nsob for the patches that I've cherry picked that I had previously\nsigned-off-on. I considered one sign-off to be enough. In your\nexample, A is the committer and the patch set already contains A's\nsign-off. For me that indicates that A still considers the commit to\ncomply with whatever s-o-b implies.\n\nWe also seem to have a few tools to help people avoid adding duplicate\nsob's, like the current behavior of format-patch and the sample\ncommit-msg hook.\n\nI did some quick searching through the kernel commits to try to find\nsome examples that could set a precedence. I didn't find anything\nthat supported either argument. I didn't see any commits that were\ncherry-picked _and_ had an existing sob that was not the last sob. I\ndidn't see any that had duplicate sob lines either. I'm not\nmentioning this to say that lack of a prior use should mean we should\nactively disallow the practice of adding duplicate sob's, I'm just\nproviding it as a data point.\n\nI've always thought that the reason that 'commit -s' and 'cherry-pick\n-s' checked only the last line of the commit message was simply that a\nfull scan of the footer had not been implemented.\n\nWhichever behavior is determined to be the right way for git to do it,\nformat-patch should be brought in-line with the others and be built on\ntop of the code in sequencer.c. So, if git _should_ create duplicate\nsob's then this patch should just be dropped. Duy's unification patch\ncan just be built on top of sequencer.c:append_signoff() without\nbringing over any of the duplicate sob detection from log-tree.c.\n\n-Brandon\n"}]}