{"thread":{"id":"59834","subject":"[PATCH] cherry-pick: use trailer instead of free-text for `-x`","startedAt":"2023-06-03T17:57:02Z","lastAt":"2023-06-04T04:32:13Z","messageCount":3,"participants":["Sean Allred via GitGitGadget","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"478017","messageId":"pull.1519.git.git.1685815011553.gitgitgadget@gmail.com","threadId":"59834","inReplyTo":null,"subject":"[PATCH] cherry-pick: use trailer instead of free-text for `-x`","fromName":"Sean Allred via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-06-03T17:56:51Z","receivedAt":"2023-06-03T17:57:02Z","isPatch":true,"sender":{"key":"allred.sean@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2082195?v=4"},"body":"From: Sean Allred <allred.sean@gmail.com>\n\nWhen recording the origin commit during a cherry-pick, the current label\nused is not understood by git-interpret-trailers. Standardize onto the\n'normal' trailer format that can be reasonably/reliably parsed and used\nby external tooling leveraging git-interpret-trailers.\n\nThis also somewhat improves the readability of resulting commit messages\nin some scenarios where trailers are already in use. Consider the\nexample already present in cd650a4e (2023-02-12, \"recognize '(cherry\npicked from ...' as part of s-o-b footer\"):\n\n>   Signed-off-by: A U Thor <author@example.com>\n>   (cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)\n>   Signed-off-by: C O Mmitter <committer@example.com>\n\nThis will now show as\n\n>   Signed-off-by: A U Thor <author@example.com>\n>   Cherry-Picked-From-Commit: da39a3ee5e6b4b0d3255bfef95601890afd80709\n>   Signed-off-by: C O Mmitter <committer@example.com>\n\nMost tests are adjusted for the new format. A test is added to\ndemonstrate that the old free-text format in existing commit data is\nstill considered part of the trailer block (i.e., the problem fixed by\nthe above commit has not been re-introduced).\n\n---\n    cherry-pick: use trailer instead of free-text for -x\n\nI considered (but did not pursue) a new configuration option for two\nreasons:\n\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1519%2Fvermiculus%2Fsa%2Fcherry-pick-origin-trailer-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1519/vermiculus/sa/cherry-pick-origin-trailer-v1\nPull-Request: https://github.com/git/git/pull/1519\n\n  1. Without regard to historical data, using a 'real' trailer seems\n     inherently better than the current free-text state.\n\n  2. Regarding historical data, adding a user-configurable option\n     doesn't make things simpler for systems maintainers; those systems\n     still have to handle both formats if they have such a need to begin\n     with. As it's still a clear and readable format, end-user\n     developers are unlikely to care to change it back.\n\nThe maintenance and cognitive costs of a new configuration option are\nnot worth the minimal benefit it seems it would bring.\n\nSigned-off-by: Sean Allred <allred.sean@gmail.com>\n---\n sequencer.c                     |  6 ++---\n t/t3510-cherry-pick-sequence.sh | 12 ++++-----\n t/t3511-cherry-pick-x.sh        | 47 +++++++++++++++++++++++----------\n 3 files changed, 42 insertions(+), 23 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex bceb6abcb6c..410f8469379 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -51,7 +51,7 @@\n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n \n static const char sign_off_header[] = \"Signed-off-by: \";\n-static const char cherry_picked_prefix[] = \"(cherry picked from commit \";\n+static const char cherry_picked_header[] = \"Cherry-Picked-From-Commit: \";\n \n GIT_PATH_FUNC(git_path_commit_editmsg, \"COMMIT_EDITMSG\")\n \n@@ -2277,9 +2277,9 @@ static int do_pick_commit(struct repository *r,\n \t\t\tstrbuf_complete_line(&msgbuf);\n \t\t\tif (!has_conforming_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, cherry_picked_header);\n \t\t\tstrbuf_addstr(&msgbuf, oid_to_hex(&commit->object.oid));\n-\t\t\tstrbuf_addstr(&msgbuf, \")\\n\");\n+\t\t\tstrbuf_addstr(&msgbuf, \"\\n\");\n \t\t}\n \t\tif (!is_fixup(command))\n \t\t\tauthor = get_author(msg.message);\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex 3b0fa66c33d..958fa019aed 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -548,10 +548,10 @@ test_expect_success '--continue respects opts' '\n \tgit cat-file commit HEAD~1 >picked_msg &&\n \tgit cat-file commit HEAD~2 >unrelatedpick_msg &&\n \tgit cat-file commit HEAD~3 >initial_msg &&\n-\t! grep \"cherry picked from\" initial_msg &&\n-\tgrep \"cherry picked from\" unrelatedpick_msg &&\n-\tgrep \"cherry picked from\" picked_msg &&\n-\tgrep \"cherry picked from\" anotherpick_msg\n+\t! grep \"Cherry-Picked-From-Commit\" initial_msg &&\n+\tgrep \"Cherry-Picked-From-Commit\" unrelatedpick_msg &&\n+\tgrep \"Cherry-Picked-From-Commit\" picked_msg &&\n+\tgrep \"Cherry-Picked-From-Commit\" anotherpick_msg\n '\n \n test_expect_success '--continue of single-pick respects -x' '\n@@ -562,7 +562,7 @@ test_expect_success '--continue of single-pick respects -x' '\n \tgit cherry-pick --continue &&\n \ttest_path_is_missing .git/sequencer &&\n \tgit cat-file commit HEAD >msg &&\n-\tgrep \"cherry picked from\" msg\n+\tgrep \"Cherry-Picked-From-Commit\" msg\n '\n \n test_expect_success '--continue respects -x in first commit in multi-pick' '\n@@ -574,7 +574,7 @@ test_expect_success '--continue respects -x in first commit in multi-pick' '\n \ttest_path_is_missing .git/sequencer &&\n \tgit cat-file commit HEAD^ >msg &&\n \tpicked=$(git rev-parse --verify picked) &&\n-\tgrep \"cherry picked from.*$picked\" msg\n+\tgrep \"Cherry-Picked-From-Commit: $picked\" msg\n '\n \n test_expect_failure '--signoff is automatically propagated to resolved conflict' '\ndiff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh\nindex dd5d92ef302..809afba48e1 100755\n--- a/t/t3511-cherry-pick-x.sh\n+++ b/t/t3511-cherry-pick-x.sh\n@@ -33,6 +33,10 @@ mesg_with_footer_sob=\"$mesg_with_footer\n Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\"\n \n mesg_with_cherry_footer=\"$mesg_with_footer_sob\n+Cherry-Picked-From-Commit: da39a3ee5e6b4b0d3255bfef95601890afd80709\n+Tested-by: C.U. Thor <cuthor@example.com>\"\n+\n+mesg_with_old_cherry_footer=\"$mesg_with_footer_sob\n (cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)\n Tested-by: C.U. Thor <cuthor@example.com>\"\n \n@@ -68,6 +72,8 @@ test_expect_success setup '\n \tgit reset --hard initial &&\n \ttest_commit \"$mesg_with_cherry_footer\" foo b mesg-with-cherry-footer &&\n \tgit reset --hard initial &&\n+\ttest_commit \"$mesg_with_old_cherry_footer\" foo b mesg-with-old-cherry-footer &&\n+\tgit reset --hard initial &&\n \ttest_config commit.cleanup verbatim &&\n \ttest_commit \"$mesg_unclean\" foo b mesg-unclean &&\n \ttest_unconfig commit.cleanup &&\n@@ -82,7 +88,7 @@ test_expect_success 'cherry-pick -x inserts blank line after one line subject' '\n \tcat <<-EOF >expect &&\n \t\t$mesg_one_line\n \n-\t\t(cherry picked from commit $sha1)\n+\t\tCherry-Picked-From-Commit: $sha1\n \tEOF\n \tgit log -1 --pretty=format:%B >actual &&\n \ttest_cmp expect actual\n@@ -130,7 +136,7 @@ test_expect_success 'cherry-pick -x inserts blank line when conforming footer no\n \tcat <<-EOF >expect &&\n \t\t$mesg_no_footer\n \n-\t\t(cherry picked from commit $sha1)\n+\t\tCherry-Picked-From-Commit: $sha1\n \tEOF\n \tgit log -1 --pretty=format:%B >actual &&\n \ttest_cmp expect actual\n@@ -155,7 +161,7 @@ test_expect_success 'cherry-pick -x -s inserts blank line when conforming footer\n \tcat <<-EOF >expect &&\n \t\t$mesg_no_footer\n \n-\t\t(cherry picked from commit $sha1)\n+\t\tCherry-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@@ -179,7 +185,7 @@ test_expect_success 'cherry-pick -x -s adds sob when last sob doesnt match commi\n \tgit cherry-pick -x -s mesg-with-footer &&\n \tcat <<-EOF >expect &&\n \t\t$mesg_with_footer\n-\t\t(cherry picked from commit $sha1)\n+\t\tCherry-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@@ -202,7 +208,7 @@ test_expect_success 'cherry-pick -x -s adds sob even when trailing sob exists fo\n \tgit cherry-pick -x -s mesg-with-footer-sob &&\n \tcat <<-EOF >expect &&\n \t\t$mesg_with_footer_sob\n-\t\t(cherry picked from commit $sha1)\n+\t\tCherry-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@@ -216,7 +222,7 @@ test_expect_success 'cherry-pick -x handles commits with no NL at end of message\n \tgit cherry-pick -x $sha1 &&\n \tgit log -1 --pretty=format:%B >actual &&\n \n-\tprintf \"\\n(cherry picked from commit %s)\\n\" $sha1 >>msg &&\n+\tprintf \"\\nCherry-Picked-From-Commit: %s\\n\" $sha1 >>msg &&\n \ttest_cmp msg actual\n '\n \n@@ -227,7 +233,7 @@ test_expect_success 'cherry-pick -x handles commits with no footer and no NL at\n \tgit cherry-pick -x $sha1 &&\n \tgit log -1 --pretty=format:%B >actual &&\n \n-\tprintf \"\\n\\n(cherry picked from commit %s)\\n\" $sha1 >>msg &&\n+\tprintf \"\\n\\nCherry-Picked-From-Commit: %s\\n\" $sha1 >>msg &&\n \ttest_cmp msg actual\n '\n \n@@ -253,19 +259,19 @@ test_expect_success 'cherry-pick -s handles commits with no footer and no NL at\n \ttest_cmp msg actual\n '\n \n-test_expect_success 'cherry-pick -x treats \"(cherry picked from...\" line as part of footer' '\n+test_expect_success 'cherry-pick -x treats \"Cherry-Picked-From-Commit\" line as part of footer' '\n \tpristine_detach initial &&\n \tsha1=$(git rev-parse mesg-with-cherry-footer^0) &&\n \tgit cherry-pick -x mesg-with-cherry-footer &&\n \tcat <<-EOF >expect &&\n \t\t$mesg_with_cherry_footer\n-\t\t(cherry picked from commit $sha1)\n+\t\tCherry-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 treats \"(cherry picked from...\" line as part of footer' '\n+test_expect_success 'cherry-pick -s treats \"Cherry-Picked-From-Commit\" line as part of footer' '\n \tpristine_detach initial &&\n \tgit cherry-pick -s mesg-with-cherry-footer &&\n \tcat <<-EOF >expect &&\n@@ -276,13 +282,26 @@ test_expect_success 'cherry-pick -s treats \"(cherry picked from...\" line as part\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'cherry-pick -x -s treats \"(cherry picked from...\" line as part of footer' '\n+test_expect_success 'cherry-pick -x -s treats \"Cherry-Picked-From-Commit\" line as part of footer' '\n \tpristine_detach initial &&\n \tsha1=$(git rev-parse mesg-with-cherry-footer^0) &&\n \tgit cherry-pick -x -s mesg-with-cherry-footer &&\n \tcat <<-EOF >expect &&\n \t\t$mesg_with_cherry_footer\n-\t\t(cherry picked from commit $sha1)\n+\t\tCherry-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 -s still treats \"(cherry picked from commit..\" line as part of footer' '\n+\tpristine_detach initial &&\n+\tsha1=$(git rev-parse mesg-with-old-cherry-footer^0) &&\n+\tgit cherry-pick -x -s mesg-with-old-cherry-footer &&\n+\tcat <<-EOF >expect &&\n+\t\t$mesg_with_old_cherry_footer\n+\t\tCherry-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@@ -303,7 +322,7 @@ test_expect_success 'cherry-pick -x cleans commit message' '\n \tpristine_detach initial &&\n \tgit cherry-pick -x mesg-unclean &&\n \tgit log -1 --pretty=format:%B >actual &&\n-\tprintf \"%s\\n(cherry picked from commit %s)\\n\" \\\n+\tprintf \"%s\\nCherry-Picked-From-Commit: %s\\n\" \\\n \t\t\"$mesg_unclean\" $(git rev-parse mesg-unclean) |\n \t\t\tgit stripspace >expect &&\n \ttest_cmp expect actual\n@@ -313,7 +332,7 @@ test_expect_success 'cherry-pick -x respects commit.cleanup' '\n \tpristine_detach initial &&\n \tgit -c commit.cleanup=strip cherry-pick -x mesg-unclean &&\n \tgit log -1 --pretty=format:%B >actual &&\n-\tprintf \"%s\\n(cherry picked from commit %s)\\n\" \\\n+\tprintf \"%s\\nCherry-Picked-From-Commit: %s\\n\" \\\n \t\t\"$mesg_unclean\" $(git rev-parse mesg-unclean) |\n \t\t\tgit stripspace -s >expect &&\n \ttest_cmp expect actual\n\nbase-commit: fe86abd7511a9a6862d5706c6fa1d9b57a63ba09\n-- \ngitgitgadget\n"},{"id":"478019","messageId":"pull.1519.v2.git.git.1685816463240.gitgitgadget@gmail.com","threadId":"59834","inReplyTo":"pull.1519.git.git.1685815011553.gitgitgadget@gmail.com","subject":"[PATCH v2] cherry-pick: use trailer instead of free-text for `-x`","fromName":"Sean Allred via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-06-03T18:21:03Z","receivedAt":"2023-06-03T18:21:11Z","isPatch":true,"sender":{"key":"allred.sean@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2082195?v=4"},"body":"From: Sean Allred <allred.sean@gmail.com>\n\nWhen recording the origin commit during a cherry-pick, the current label\nused is not understood by git-interpret-trailers. Standardize onto the\n'normal' trailer format that can be reasonably/reliably parsed and used\nby external tooling leveraging git-interpret-trailers.\n\nThe prior language was introduced way back in 2005 (48313592, \"Redo\n'revert' using three-way merge machinery\"), long before\ngit-interpret-trailers was introduced in 2014 (dfd66ddf, \"add\ndocumentation for 'git interpret-trailers'\").\n\nThis also somewhat improves the readability of resulting commit messages\nin some scenarios where trailers are already in use. Consider the\nexample already present in cd650a4e (2023-02-12, \"recognize '(cherry\npicked from ...' as part of s-o-b footer\"):\n\n>   Signed-off-by: A U Thor <author@example.com>\n>   (cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)\n>   Signed-off-by: C O Mmitter <committer@example.com>\n\nThis will now show as\n\n>   Signed-off-by: A U Thor <author@example.com>\n>   Cherry-Picked-From-Commit: da39a3ee5e6b4b0d3255bfef95601890afd80709\n>   Signed-off-by: C O Mmitter <committer@example.com>\n\nMost tests are adjusted for the new format. A test is added to\ndemonstrate that the old free-text format in existing commit data is\nstill considered part of the trailer block (i.e., the problem fixed by\nthe above commit has not been re-introduced).\n\nThe change to trailer.c is not necessary for current tests to pass, but\nappear to be necessary to maintain the stated goal and semantics of the\n`find_trailer_start` with the addition of this new generated header. The\nold format is left, of course, to handle historical commit data.\n\n---\n    cherry-pick: use trailer instead of free-text for -x\n    \n    Sincere apologies for the very quick v2; while I've been sitting on this\n    patch for a while in one form or another, I neglected to update the\n    documentation. This has now been addressed, as well as addressing\n    another reference to the old format in trailer.c. (I've now evaluated\n    results of regexp searches of cherry.picked and picked.from; there is\n    nothing left that seems to be necessary to update.)\n\nI considered (but did not pursue) a new configuration option for two\nreasons:\n\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1519%2Fvermiculus%2Fsa%2Fcherry-pick-origin-trailer-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1519/vermiculus/sa/cherry-pick-origin-trailer-v2\nPull-Request: https://github.com/git/git/pull/1519\n\nRange-diff vs v1:\n\n 1:  14c9e39be69 ! 1:  b163a45b48a cherry-pick: use trailer instead of free-text for `-x`\n     @@ Commit message\n          'normal' trailer format that can be reasonably/reliably parsed and used\n          by external tooling leveraging git-interpret-trailers.\n      \n     +    The prior language was introduced way back in 2005 (48313592, \"Redo\n     +    'revert' using three-way merge machinery\"), long before\n     +    git-interpret-trailers was introduced in 2014 (dfd66ddf, \"add\n     +    documentation for 'git interpret-trailers'\").\n     +\n          This also somewhat improves the readability of resulting commit messages\n          in some scenarios where trailers are already in use. Consider the\n          example already present in cd650a4e (2023-02-12, \"recognize '(cherry\n     @@ Commit message\n          still considered part of the trailer block (i.e., the problem fixed by\n          the above commit has not been re-introduced).\n      \n     +    The change to trailer.c is not necessary for current tests to pass, but\n     +    appear to be necessary to maintain the stated goal and semantics of the\n     +    `find_trailer_start` with the addition of this new generated header. The\n     +    old format is left, of course, to handle historical commit data.\n     +\n          ---\n      \n          I considered (but did not pursue) a new configuration option for two\n     @@ Commit message\n      \n          Signed-off-by: Sean Allred <allred.sean@gmail.com>\n      \n     + ## Documentation/git-cherry-pick.txt ##\n     +@@ Documentation/git-cherry-pick.txt: OPTIONS\n     + \n     + -x::\n     + \tWhen recording the commit, append a line that says\n     +-\t\"(cherry picked from commit ...)\" to the original commit\n     ++\t\"Cherry-Picked-From-Commit:\" to the original commit\n     + \tmessage in order to indicate which commit this change was\n     + \tcherry-picked from.  This is done only for cherry\n     + \tpicks without conflicts.  Do not use this option if\n     +@@ Documentation/git-cherry-pick.txt: OPTIONS\n     + \tvisible branches (e.g. backporting a fix to a\n     + \tmaintenance branch for an older release from a\n     + \tdevelopment branch), adding this information can be\n     +-\tuseful.\n     ++\tuseful. See also linkgit:git-interpret-trailers[1].\n     + \n     + -r::\n     + \tIt used to be that the command defaulted to do `-x`\n     +\n       ## sequencer.c ##\n      @@\n       #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n     @@ t/t3511-cherry-pick-x.sh: test_expect_success 'cherry-pick -x respects commit.cl\n       \t\t\"$mesg_unclean\" $(git rev-parse mesg-unclean) |\n       \t\t\tgit stripspace -s >expect &&\n       \ttest_cmp expect actual\n     +\n     + ## trailer.c ##\n     +@@ trailer.c: static int configured;\n     + static const char *git_generated_prefixes[] = {\n     + \t\"Signed-off-by: \",\n     + \t\"(cherry picked from commit \",\n     ++\t\"Cherry-Picked-From-Commit: \",\n     + \tNULL\n     + };\n     + \n\n\n  1. Without regard to historical data, using a 'real' trailer seems\n     inherently better than the current free-text state.\n\n  2. Regarding historical data, adding a user-configurable option\n     doesn't make things simpler for systems maintainers; those systems\n     still have to handle both formats if they have such a need to begin\n     with. As it's still a clear and readable format, end-user\n     developers are unlikely to care to change it back.\n\nThe maintenance and cognitive costs of a new configuration option are\nnot worth the minimal benefit it seems it would bring.\n\nSigned-off-by: Sean Allred <allred.sean@gmail.com>\n---\n Documentation/git-cherry-pick.txt |  4 +--\n sequencer.c                       |  6 ++--\n t/t3510-cherry-pick-sequence.sh   | 12 ++++----\n t/t3511-cherry-pick-x.sh          | 47 ++++++++++++++++++++++---------\n trailer.c                         |  1 +\n 5 files changed, 45 insertions(+), 25 deletions(-)\n\ndiff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\nindex fdcad3d2006..22217480e45 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -64,7 +64,7 @@ OPTIONS\n \n -x::\n \tWhen recording the commit, append a line that says\n-\t\"(cherry picked from commit ...)\" to the original commit\n+\t\"Cherry-Picked-From-Commit:\" to the original commit\n \tmessage in order to indicate which commit this change was\n \tcherry-picked from.  This is done only for cherry\n \tpicks without conflicts.  Do not use this option if\n@@ -74,7 +74,7 @@ OPTIONS\n \tvisible branches (e.g. backporting a fix to a\n \tmaintenance branch for an older release from a\n \tdevelopment branch), adding this information can be\n-\tuseful.\n+\tuseful. See also linkgit:git-interpret-trailers[1].\n \n -r::\n \tIt used to be that the command defaulted to do `-x`\ndiff --git a/sequencer.c b/sequencer.c\nindex bceb6abcb6c..410f8469379 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -51,7 +51,7 @@\n #define GIT_REFLOG_ACTION \"GIT_REFLOG_ACTION\"\n \n static const char sign_off_header[] = \"Signed-off-by: \";\n-static const char cherry_picked_prefix[] = \"(cherry picked from commit \";\n+static const char cherry_picked_header[] = \"Cherry-Picked-From-Commit: \";\n \n GIT_PATH_FUNC(git_path_commit_editmsg, \"COMMIT_EDITMSG\")\n \n@@ -2277,9 +2277,9 @@ static int do_pick_commit(struct repository *r,\n \t\t\tstrbuf_complete_line(&msgbuf);\n \t\t\tif (!has_conforming_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, cherry_picked_header);\n \t\t\tstrbuf_addstr(&msgbuf, oid_to_hex(&commit->object.oid));\n-\t\t\tstrbuf_addstr(&msgbuf, \")\\n\");\n+\t\t\tstrbuf_addstr(&msgbuf, \"\\n\");\n \t\t}\n \t\tif (!is_fixup(command))\n \t\t\tauthor = get_author(msg.message);\ndiff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh\nindex 3b0fa66c33d..958fa019aed 100755\n--- a/t/t3510-cherry-pick-sequence.sh\n+++ b/t/t3510-cherry-pick-sequence.sh\n@@ -548,10 +548,10 @@ test_expect_success '--continue respects opts' '\n \tgit cat-file commit HEAD~1 >picked_msg &&\n \tgit cat-file commit HEAD~2 >unrelatedpick_msg &&\n \tgit cat-file commit HEAD~3 >initial_msg &&\n-\t! grep \"cherry picked from\" initial_msg &&\n-\tgrep \"cherry picked from\" unrelatedpick_msg &&\n-\tgrep \"cherry picked from\" picked_msg &&\n-\tgrep \"cherry picked from\" anotherpick_msg\n+\t! grep \"Cherry-Picked-From-Commit\" initial_msg &&\n+\tgrep \"Cherry-Picked-From-Commit\" unrelatedpick_msg &&\n+\tgrep \"Cherry-Picked-From-Commit\" picked_msg &&\n+\tgrep \"Cherry-Picked-From-Commit\" anotherpick_msg\n '\n \n test_expect_success '--continue of single-pick respects -x' '\n@@ -562,7 +562,7 @@ test_expect_success '--continue of single-pick respects -x' '\n \tgit cherry-pick --continue &&\n \ttest_path_is_missing .git/sequencer &&\n \tgit cat-file commit HEAD >msg &&\n-\tgrep \"cherry picked from\" msg\n+\tgrep \"Cherry-Picked-From-Commit\" msg\n '\n \n test_expect_success '--continue respects -x in first commit in multi-pick' '\n@@ -574,7 +574,7 @@ test_expect_success '--continue respects -x in first commit in multi-pick' '\n \ttest_path_is_missing .git/sequencer &&\n \tgit cat-file commit HEAD^ >msg &&\n \tpicked=$(git rev-parse --verify picked) &&\n-\tgrep \"cherry picked from.*$picked\" msg\n+\tgrep \"Cherry-Picked-From-Commit: $picked\" msg\n '\n \n test_expect_failure '--signoff is automatically propagated to resolved conflict' '\ndiff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh\nindex dd5d92ef302..809afba48e1 100755\n--- a/t/t3511-cherry-pick-x.sh\n+++ b/t/t3511-cherry-pick-x.sh\n@@ -33,6 +33,10 @@ mesg_with_footer_sob=\"$mesg_with_footer\n Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>\"\n \n mesg_with_cherry_footer=\"$mesg_with_footer_sob\n+Cherry-Picked-From-Commit: da39a3ee5e6b4b0d3255bfef95601890afd80709\n+Tested-by: C.U. Thor <cuthor@example.com>\"\n+\n+mesg_with_old_cherry_footer=\"$mesg_with_footer_sob\n (cherry picked from commit da39a3ee5e6b4b0d3255bfef95601890afd80709)\n Tested-by: C.U. Thor <cuthor@example.com>\"\n \n@@ -68,6 +72,8 @@ test_expect_success setup '\n \tgit reset --hard initial &&\n \ttest_commit \"$mesg_with_cherry_footer\" foo b mesg-with-cherry-footer &&\n \tgit reset --hard initial &&\n+\ttest_commit \"$mesg_with_old_cherry_footer\" foo b mesg-with-old-cherry-footer &&\n+\tgit reset --hard initial &&\n \ttest_config commit.cleanup verbatim &&\n \ttest_commit \"$mesg_unclean\" foo b mesg-unclean &&\n \ttest_unconfig commit.cleanup &&\n@@ -82,7 +88,7 @@ test_expect_success 'cherry-pick -x inserts blank line after one line subject' '\n \tcat <<-EOF >expect &&\n \t\t$mesg_one_line\n \n-\t\t(cherry picked from commit $sha1)\n+\t\tCherry-Picked-From-Commit: $sha1\n \tEOF\n \tgit log -1 --pretty=format:%B >actual &&\n \ttest_cmp expect actual\n@@ -130,7 +136,7 @@ test_expect_success 'cherry-pick -x inserts blank line when conforming footer no\n \tcat <<-EOF >expect &&\n \t\t$mesg_no_footer\n \n-\t\t(cherry picked from commit $sha1)\n+\t\tCherry-Picked-From-Commit: $sha1\n \tEOF\n \tgit log -1 --pretty=format:%B >actual &&\n \ttest_cmp expect actual\n@@ -155,7 +161,7 @@ test_expect_success 'cherry-pick -x -s inserts blank line when conforming footer\n \tcat <<-EOF >expect &&\n \t\t$mesg_no_footer\n \n-\t\t(cherry picked from commit $sha1)\n+\t\tCherry-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@@ -179,7 +185,7 @@ test_expect_success 'cherry-pick -x -s adds sob when last sob doesnt match commi\n \tgit cherry-pick -x -s mesg-with-footer &&\n \tcat <<-EOF >expect &&\n \t\t$mesg_with_footer\n-\t\t(cherry picked from commit $sha1)\n+\t\tCherry-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@@ -202,7 +208,7 @@ test_expect_success 'cherry-pick -x -s adds sob even when trailing sob exists fo\n \tgit cherry-pick -x -s mesg-with-footer-sob &&\n \tcat <<-EOF >expect &&\n \t\t$mesg_with_footer_sob\n-\t\t(cherry picked from commit $sha1)\n+\t\tCherry-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@@ -216,7 +222,7 @@ test_expect_success 'cherry-pick -x handles commits with no NL at end of message\n \tgit cherry-pick -x $sha1 &&\n \tgit log -1 --pretty=format:%B >actual &&\n \n-\tprintf \"\\n(cherry picked from commit %s)\\n\" $sha1 >>msg &&\n+\tprintf \"\\nCherry-Picked-From-Commit: %s\\n\" $sha1 >>msg &&\n \ttest_cmp msg actual\n '\n \n@@ -227,7 +233,7 @@ test_expect_success 'cherry-pick -x handles commits with no footer and no NL at\n \tgit cherry-pick -x $sha1 &&\n \tgit log -1 --pretty=format:%B >actual &&\n \n-\tprintf \"\\n\\n(cherry picked from commit %s)\\n\" $sha1 >>msg &&\n+\tprintf \"\\n\\nCherry-Picked-From-Commit: %s\\n\" $sha1 >>msg &&\n \ttest_cmp msg actual\n '\n \n@@ -253,19 +259,19 @@ test_expect_success 'cherry-pick -s handles commits with no footer and no NL at\n \ttest_cmp msg actual\n '\n \n-test_expect_success 'cherry-pick -x treats \"(cherry picked from...\" line as part of footer' '\n+test_expect_success 'cherry-pick -x treats \"Cherry-Picked-From-Commit\" line as part of footer' '\n \tpristine_detach initial &&\n \tsha1=$(git rev-parse mesg-with-cherry-footer^0) &&\n \tgit cherry-pick -x mesg-with-cherry-footer &&\n \tcat <<-EOF >expect &&\n \t\t$mesg_with_cherry_footer\n-\t\t(cherry picked from commit $sha1)\n+\t\tCherry-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 treats \"(cherry picked from...\" line as part of footer' '\n+test_expect_success 'cherry-pick -s treats \"Cherry-Picked-From-Commit\" line as part of footer' '\n \tpristine_detach initial &&\n \tgit cherry-pick -s mesg-with-cherry-footer &&\n \tcat <<-EOF >expect &&\n@@ -276,13 +282,26 @@ test_expect_success 'cherry-pick -s treats \"(cherry picked from...\" line as part\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'cherry-pick -x -s treats \"(cherry picked from...\" line as part of footer' '\n+test_expect_success 'cherry-pick -x -s treats \"Cherry-Picked-From-Commit\" line as part of footer' '\n \tpristine_detach initial &&\n \tsha1=$(git rev-parse mesg-with-cherry-footer^0) &&\n \tgit cherry-pick -x -s mesg-with-cherry-footer &&\n \tcat <<-EOF >expect &&\n \t\t$mesg_with_cherry_footer\n-\t\t(cherry picked from commit $sha1)\n+\t\tCherry-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 -s still treats \"(cherry picked from commit..\" line as part of footer' '\n+\tpristine_detach initial &&\n+\tsha1=$(git rev-parse mesg-with-old-cherry-footer^0) &&\n+\tgit cherry-pick -x -s mesg-with-old-cherry-footer &&\n+\tcat <<-EOF >expect &&\n+\t\t$mesg_with_old_cherry_footer\n+\t\tCherry-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@@ -303,7 +322,7 @@ test_expect_success 'cherry-pick -x cleans commit message' '\n \tpristine_detach initial &&\n \tgit cherry-pick -x mesg-unclean &&\n \tgit log -1 --pretty=format:%B >actual &&\n-\tprintf \"%s\\n(cherry picked from commit %s)\\n\" \\\n+\tprintf \"%s\\nCherry-Picked-From-Commit: %s\\n\" \\\n \t\t\"$mesg_unclean\" $(git rev-parse mesg-unclean) |\n \t\t\tgit stripspace >expect &&\n \ttest_cmp expect actual\n@@ -313,7 +332,7 @@ test_expect_success 'cherry-pick -x respects commit.cleanup' '\n \tpristine_detach initial &&\n \tgit -c commit.cleanup=strip cherry-pick -x mesg-unclean &&\n \tgit log -1 --pretty=format:%B >actual &&\n-\tprintf \"%s\\n(cherry picked from commit %s)\\n\" \\\n+\tprintf \"%s\\nCherry-Picked-From-Commit: %s\\n\" \\\n \t\t\"$mesg_unclean\" $(git rev-parse mesg-unclean) |\n \t\t\tgit stripspace -s >expect &&\n \ttest_cmp expect actual\ndiff --git a/trailer.c b/trailer.c\nindex a2c3ed6f28c..59f7ef92b29 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -53,6 +53,7 @@ static int configured;\n static const char *git_generated_prefixes[] = {\n \t\"Signed-off-by: \",\n \t\"(cherry picked from commit \",\n+\t\"Cherry-Picked-From-Commit: \",\n \tNULL\n };\n \n\nbase-commit: fe86abd7511a9a6862d5706c6fa1d9b57a63ba09\n-- \ngitgitgadget\n"},{"id":"478020","messageId":"xmqqfs776e62.fsf@gitster.g","threadId":"59834","inReplyTo":"pull.1519.v2.git.git.1685816463240.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] cherry-pick: use trailer instead of free-text for `-x`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-04T04:32:05Z","receivedAt":"2023-06-04T04:32:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Sean Allred via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Sean Allred <allred.sean@gmail.com>\n>\n> When recording the origin commit during a cherry-pick, the current label\n> used is not understood by git-interpret-trailers. Standardize onto the\n> 'normal' trailer format that can be reasonably/reliably parsed and used\n> by external tooling leveraging git-interpret-trailers.\n\nI am somewhat negative on going in this direction, as we originally\nadded these \"cherry-picked-from\" by default, but stopped doing so\nfor a reason [*1*].  I'd be hesitant to see us spend any engineering\nresources on a feature we discourage (not even to deprecate and to\nremove).  It is a different story if we change the previous stance\non the \"cherry-picked-from\" information, though.\n\nI admit I've suggested \"Cherry-picked-from:\" long time ago [*2*], as\nan aside in a discussion, but the discussion was more about treating\nthe line as a very distinct thing that is different from any other\n\"trailer\" lines.  The last time this was brought up, I thought that\nit was deemed unnecessary because interpret-trailer code already\nunderstood by the trailer code [*3*], and we _could_ teach the\ninterpret-trailer code to rewrite it to \"Cherry-picked-from:\".  Any\nrenewed efforts should build on the discussion there, addressing\npoints raised during the discussion, I think.\n\nThanks.\n\n[Footnotes and references]\n\n*1* Unlike \"revert\", \"cherry-pick\" is done from an unrelated and\n    often not even published history, and referring to such a commit\n    that the end-user cannot do \"git show\" does not add much value\n    to the history.\n\n*2* https://lore.kernel.org/git/xmqqtwcycqul.fsf@gitster.mtv.corp.google.com/\n*3* https://lore.kernel.org/git/20181106221118.GA9975@sigill.intra.peff.net/\n"}]}