{"thread":{"id":"38620","subject":"[BUG] git mangles up commit messages on rebase","startedAt":"2015-02-21T17:48:26Z","lastAt":"2015-03-06T18:59:19Z","messageCount":14,"participants":["Christoph Anton Mitterer","Michael J Gruber","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"256470","messageId":"1424540906.15539.22.camel@scientia.net","threadId":"38620","inReplyTo":null,"subject":"[BUG] git mangles up commit messages on rebase","fromName":"Christoph Anton Mitterer","fromEmail":"calestyo@scientia.net","sentAt":"2015-02-21T17:48:26Z","receivedAt":"2015-02-21T17:48:26Z","isPatch":false,"sender":{"key":"calestyo@scientia.net","avatar":"https://gravatar.com/avatar/74cd78048081efac1fcd21e681751ed55968f949a50e33adbe1a41c572a605cd?d=mp&s=160"},"body":"Hey.\n\nWhen I do a simple interactive rebase, e.g. something like this:\nedit 78f3ba8 editing or just rewriting this commit\npick b621076 foo\npick e06c28e this one had a \"verbatim\" commit message\npick c0a447f bar\n\nand one of the commit messages from the children I edit/rewrite had a\ncommit message that was edited with --cleanup=verbatim (e.g. double\nnewlines, etc.).\n\nThen these get lost once I --continue and it appears that the messages\nare recreated but with the default of --cleanup=default .\n\nIMO that's quite annoying, cause when one intentionally chose e.g.\n-cleanup=verbatim and made commit messages with that, then this is\nprobably what one wanted and it should be dumped just because of\nchanging another commit.\n\nCould that possibly be solved? :)\n\nCheers,\nChris.\n"},{"id":"256503","messageId":"f58ae048d7fd468cfdd7f7d369b3b4fc0a564641.1424697676.git.git@drmicha.warpmail.net","threadId":"38620","inReplyTo":"1424540906.15539.22.camel@scientia.net","subject":"[PATCH] sequencer: preserve commit messages","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-02-23T13:23:41Z","receivedAt":"2015-02-23T13:23:41Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"sequencer calls \"commit\" with default options, which implies\n\"--cleanup=default\" unless the user specified something else in their\nconfig. This leads to cherry-picked commits getting a cleaned up commit\nmessage, which is usually not an intended side-effect.\n\nMake the sequencer use \"--cleanup=verbatim\" so that it preserves commit\nmessages independent of the defaults and user config for \"commit\".\n\nReported-by: Christoph Anton Mitterer <calestyo@scientia.net>\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\n\nNotes:\n    All tests run fine with this changed behavior. I don't know\n    whether this may have any side-effects on other (untested)\n    uses of the sequencer.\n\n sequencer.c              |  1 +\n t/t3511-cherry-pick-x.sh | 28 ++++++++++++++++++++++++++++\n 2 files changed, 29 insertions(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 77a1266..35fe9d9 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -377,6 +377,7 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,\n \targv_array_init(&array);\n \targv_array_push(&array, \"commit\");\n \targv_array_push(&array, \"-n\");\n+\targv_array_push(&array, \"--cleanup=verbatim\");\n \n \tif (opts->gpg_sign)\n \t\targv_array_pushf(&array, \"-S%s\", opts->gpg_sign);\ndiff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh\nindex f977279..b7dff09 100755\n--- a/t/t3511-cherry-pick-x.sh\n+++ b/t/t3511-cherry-pick-x.sh\n@@ -36,6 +36,20 @@ 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_unclean=\"$mesg_one_line\n+\n+\n+leading empty lines\n+\n+\n+consecutive empty lines\n+\n+# hash tag comment\n+\n+trailing empty lines\n+\n+\n+\"\n \n test_expect_success setup '\n \tgit config advice.detachedhead false &&\n@@ -53,6 +67,10 @@ test_expect_success setup '\n \ttest_commit \"$mesg_with_footer_sob\" foo b mesg-with-footer-sob &&\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_config commit.cleanup verbatim &&\n+\ttest_commit \"$mesg_unclean\" foo b mesg-unclean &&\n+\ttest_unconfig commit.cleanup &&\n \tpristine_detach initial &&\n \ttest_commit conflicting unrelated\n '\n@@ -216,4 +234,14 @@ test_expect_success 'cherry-pick -x -s treats \"(cherry picked from...\" line as p\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'cherry-pick preserves commit message' '\n+\tpristine_detach initial &&\n+\tprintf \"$mesg_unclean\" >expect &&\n+\tgit log -1 --pretty=format:%B mesg-unclean >actual &&\n+\ttest_cmp expect actual &&\n+\tgit cherry-pick mesg-unclean &&\n+\tgit log -1 --pretty=format:%B >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.3.0.296.g32c87e1\n"},{"id":"256531","messageId":"xmqqsidwtq4i.fsf@gitster.dls.corp.google.com","threadId":"38620","inReplyTo":"f58ae048d7fd468cfdd7f7d369b3b4fc0a564641.1424697676.git.git@drmicha.warpmail.net","subject":"Re: [PATCH] sequencer: preserve commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-23T18:54:53Z","receivedAt":"2015-02-23T18:54:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@drmicha.warpmail.net> writes:\n\n> sequencer calls \"commit\" with default options, which implies\n> \"--cleanup=default\" unless the user specified something else in their\n> config. This leads to cherry-picked commits getting a cleaned up commit\n> message, which is usually not an intended side-effect.\n>\n> Make the sequencer use \"--cleanup=verbatim\" so that it preserves commit\n> messages independent of the defaults and user config for \"commit\".\n\nHmm, wouldn't it introduce a grave regression for users who\nexplicitly ask to clean crufty messages up (by setting their own\ncommit.cleanup configuration) if you unconditionally force\n\"--cleanup=verbatim\" here?\n\n>\n> Reported-by: Christoph Anton Mitterer <calestyo@scientia.net>\n> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n> ---\n>\n> Notes:\n>     All tests run fine with this changed behavior. I don't know\n>     whether this may have any side-effects on other (untested)\n>     uses of the sequencer.\n>\n>  sequencer.c              |  1 +\n>  t/t3511-cherry-pick-x.sh | 28 ++++++++++++++++++++++++++++\n>  2 files changed, 29 insertions(+)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index 77a1266..35fe9d9 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -377,6 +377,7 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,\n>  \targv_array_init(&array);\n>  \targv_array_push(&array, \"commit\");\n>  \targv_array_push(&array, \"-n\");\n> +\targv_array_push(&array, \"--cleanup=verbatim\");\n\n\n\n>  \n>  \tif (opts->gpg_sign)\n>  \t\targv_array_pushf(&array, \"-S%s\", opts->gpg_sign);\n> diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh\n> index f977279..b7dff09 100755\n> --- a/t/t3511-cherry-pick-x.sh\n> +++ b/t/t3511-cherry-pick-x.sh\n> @@ -36,6 +36,20 @@ 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_unclean=\"$mesg_one_line\n> +\n> +\n> +leading empty lines\n> +\n> +\n> +consecutive empty lines\n> +\n> +# hash tag comment\n> +\n> +trailing empty lines\n> +\n> +\n> +\"\n>  \n>  test_expect_success setup '\n>  \tgit config advice.detachedhead false &&\n> @@ -53,6 +67,10 @@ test_expect_success setup '\n>  \ttest_commit \"$mesg_with_footer_sob\" foo b mesg-with-footer-sob &&\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_config commit.cleanup verbatim &&\n> +\ttest_commit \"$mesg_unclean\" foo b mesg-unclean &&\n> +\ttest_unconfig commit.cleanup &&\n>  \tpristine_detach initial &&\n>  \ttest_commit conflicting unrelated\n>  '\n> @@ -216,4 +234,14 @@ test_expect_success 'cherry-pick -x -s treats \"(cherry picked from...\" line as p\n>  \ttest_cmp expect actual\n>  '\n>  \n> +test_expect_success 'cherry-pick preserves commit message' '\n> +\tpristine_detach initial &&\n> +\tprintf \"$mesg_unclean\" >expect &&\n> +\tgit log -1 --pretty=format:%B mesg-unclean >actual &&\n> +\ttest_cmp expect actual &&\n> +\tgit cherry-pick mesg-unclean &&\n> +\tgit log -1 --pretty=format:%B >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n>  test_done\n"},{"id":"256577","messageId":"54EC98BD.7060100@drmicha.warpmail.net","threadId":"38620","inReplyTo":"xmqqsidwtq4i.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] sequencer: preserve commit messages","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-02-24T15:29:01Z","receivedAt":"2015-02-24T15:29:01Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 23.02.2015 19:54:\n> Michael J Gruber <git@drmicha.warpmail.net> writes:\n> \n>> sequencer calls \"commit\" with default options, which implies\n>> \"--cleanup=default\" unless the user specified something else in their\n>> config. This leads to cherry-picked commits getting a cleaned up commit\n>> message, which is usually not an intended side-effect.\n>>\n>> Make the sequencer use \"--cleanup=verbatim\" so that it preserves commit\n>> messages independent of the defaults and user config for \"commit\".\n> \n> Hmm, wouldn't it introduce a grave regression for users who\n> explicitly ask to clean crufty messages up (by setting their own\n> commit.cleanup configuration) if you unconditionally force\n> \"--cleanup=verbatim\" here?\n> \n\nThat's what I meant by possible side-effects below.\n\nThere are no side-effects which our tests would catch.\n\nI don't know sequencer.c well enough to know whether run_git_commit()\nwould be run with a user-edited commit message at all. My (possibly\nwrong) understanding is that it is called only when a cherry-pick\nsucceeds without any conflicts, so that it is called with a commit\nmessage from a pre-existing commit, i.e. a message after cleanup which\nis to be preserved as is.\n\nIn case of a conflict, resolution is left to be done by the user. But I\nguess I'm overlooking --edit and --continue here.\n\nBut git cherry-pick without conflict should no re-cleanup the commit\nmessage either, should it?\n\n>> Reported-by: Christoph Anton Mitterer <calestyo@scientia.net>\n>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n>> ---\n>>\n>> Notes:\n>>     All tests run fine with this changed behavior. I don't know\n>>     whether this may have any side-effects on other (untested)\n>>     uses of the sequencer.\n>>\n>>  sequencer.c              |  1 +\n>>  t/t3511-cherry-pick-x.sh | 28 ++++++++++++++++++++++++++++\n>>  2 files changed, 29 insertions(+)\n>>\n>> diff --git a/sequencer.c b/sequencer.c\n>> index 77a1266..35fe9d9 100644\n>> --- a/sequencer.c\n>> +++ b/sequencer.c\n>> @@ -377,6 +377,7 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,\n>>  \targv_array_init(&array);\n>>  \targv_array_push(&array, \"commit\");\n>>  \targv_array_push(&array, \"-n\");\n>> +\targv_array_push(&array, \"--cleanup=verbatim\");\n> \n> \n> \n>>  \n>>  \tif (opts->gpg_sign)\n>>  \t\targv_array_pushf(&array, \"-S%s\", opts->gpg_sign);\n>> diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh\n>> index f977279..b7dff09 100755\n>> --- a/t/t3511-cherry-pick-x.sh\n>> +++ b/t/t3511-cherry-pick-x.sh\n>> @@ -36,6 +36,20 @@ 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_unclean=\"$mesg_one_line\n>> +\n>> +\n>> +leading empty lines\n>> +\n>> +\n>> +consecutive empty lines\n>> +\n>> +# hash tag comment\n>> +\n>> +trailing empty lines\n>> +\n>> +\n>> +\"\n>>  \n>>  test_expect_success setup '\n>>  \tgit config advice.detachedhead false &&\n>> @@ -53,6 +67,10 @@ test_expect_success setup '\n>>  \ttest_commit \"$mesg_with_footer_sob\" foo b mesg-with-footer-sob &&\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_config commit.cleanup verbatim &&\n>> +\ttest_commit \"$mesg_unclean\" foo b mesg-unclean &&\n>> +\ttest_unconfig commit.cleanup &&\n>>  \tpristine_detach initial &&\n>>  \ttest_commit conflicting unrelated\n>>  '\n>> @@ -216,4 +234,14 @@ test_expect_success 'cherry-pick -x -s treats \"(cherry picked from...\" line as p\n>>  \ttest_cmp expect actual\n>>  '\n>>  \n>> +test_expect_success 'cherry-pick preserves commit message' '\n>> +\tpristine_detach initial &&\n>> +\tprintf \"$mesg_unclean\" >expect &&\n>> +\tgit log -1 --pretty=format:%B mesg-unclean >actual &&\n>> +\ttest_cmp expect actual &&\n>> +\tgit cherry-pick mesg-unclean &&\n>> +\tgit log -1 --pretty=format:%B >actual &&\n>> +\ttest_cmp expect actual\n>> +'\n>> +\n>>  test_done\n"},{"id":"256583","messageId":"xmqq8ufnrwm1.fsf@gitster.dls.corp.google.com","threadId":"38620","inReplyTo":"54EC98BD.7060100@drmicha.warpmail.net","subject":"Re: [PATCH] sequencer: preserve commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-24T18:29:58Z","receivedAt":"2015-02-24T18:29:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@drmicha.warpmail.net> writes:\n\n>> Hmm, wouldn't it introduce a grave regression for users who\n>> explicitly ask to clean crufty messages up (by setting their own\n>> commit.cleanup configuration) if you unconditionally force\n>> \"--cleanup=verbatim\" here?\n>> \n>\n> That's what I meant by possible side-effects below.\n> ...\n> But git cherry-pick without conflict should no re-cleanup the commit\n> message either, should it?\n\nHmm, but if it does not, wouldn't that countermand the wish of the\nuser who explicitly asked to clean crufty messages up by setting\ntheir own commit.cleanup configuration?\n"},{"id":"256622","messageId":"54ED9AF7.6080908@drmicha.warpmail.net","threadId":"38620","inReplyTo":"xmqq8ufnrwm1.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] sequencer: preserve commit messages","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-02-25T09:50:47Z","receivedAt":"2015-02-25T09:50:47Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 24.02.2015 19:29:\n> Michael J Gruber <git@drmicha.warpmail.net> writes:\n> \n>>> Hmm, wouldn't it introduce a grave regression for users who\n>>> explicitly ask to clean crufty messages up (by setting their own\n>>> commit.cleanup configuration) if you unconditionally force\n>>> \"--cleanup=verbatim\" here?\n>>>\n>>\n>> That's what I meant by possible side-effects below.\n>> ...\n>> But git cherry-pick without conflict should no re-cleanup the commit\n>> message either, should it?\n> \n> Hmm, but if it does not, wouldn't that countermand the wish of the\n> user who explicitly asked to clean crufty messages up by setting\n> their own commit.cleanup configuration?\n> \n\nNote that \"verbatim\" is not the default - we cleanup commits even\nwithout being asked to. And this makes sense for \"git commit\", of course.\n\nI myself certainly expected \"git cherry-pick\" to transfer a commit as\nverbatim as possible. \"git rebase\" preserves the commit message (at\nleast more than cherry-pick). What's the difference between them?\nTechnically the difference between commit-tree and commit, sure, but for\nthe user?\n\nMichael\n"},{"id":"256638","messageId":"1424871878.4688.3.camel@scientia.net","threadId":"38620","inReplyTo":"xmqq8ufnrwm1.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] sequencer: preserve commit messages","fromName":"Christoph Anton Mitterer","fromEmail":"calestyo@scientia.net","sentAt":"2015-02-25T13:44:38Z","receivedAt":"2015-02-25T13:44:38Z","isPatch":true,"sender":{"key":"calestyo@scientia.net","avatar":"https://gravatar.com/avatar/74cd78048081efac1fcd21e681751ed55968f949a50e33adbe1a41c572a605cd?d=mp&s=160"},"body":"On Tue, 2015-02-24 at 10:29 -0800, Junio C Hamano wrote: \n> Hmm, but if it does not, wouldn't that countermand the wish of the\n> user who explicitly asked to clean crufty messages up by setting\n> their own commit.cleanup configuration?\n\nIMHO it's just wrong behaviour if the commit messages of people who\nintentionally chose \"verbatim\" to get multiple newline, etc. are mangled\nup, just to allow such people, that also intentionally chose some\nnon-default cleanup mode, but changed their mind later, allow easy clean\nup.\n\nCheers,\nChris.\n"},{"id":"256645","messageId":"xmqqoaohonpt.fsf@gitster.dls.corp.google.com","threadId":"38620","inReplyTo":"54ED9AF7.6080908@drmicha.warpmail.net","subject":"Re: [PATCH] sequencer: preserve commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-25T18:22:38Z","receivedAt":"2015-02-25T18:22:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@drmicha.warpmail.net> writes:\n\n> Junio C Hamano venit, vidit, dixit 24.02.2015 19:29:\n>> Michael J Gruber <git@drmicha.warpmail.net> writes:\n>> \n>>>> Hmm, wouldn't it introduce a grave regression for users who\n>>>> explicitly ask to clean crufty messages up (by setting their own\n>>>> commit.cleanup configuration) if you unconditionally force\n>>>> \"--cleanup=verbatim\" here?\n>>>>\n>>>\n>>> That's what I meant by possible side-effects below.\n>>> ...\n>>> But git cherry-pick without conflict should no re-cleanup the commit\n>>> message either, should it?\n>> \n>> Hmm, but if it does not, wouldn't that countermand the wish of the\n>> user who explicitly asked to clean crufty messages up by setting\n>> their own commit.cleanup configuration?\n>\n> Note that \"verbatim\" is not the default - we cleanup commits even\n> without being asked to. And this makes sense for \"git commit\", of course.\n\nI am fine with the result of the updated code if the user does not\nhave anything in the config and uses the \"default\".  Not touching in\n\"cherry-pick\" would be more desirable than cleaning.  We are in\nagreement for that obvious case.\n\nBut your response is sidestepping my question, isn't it?\n\nWhat does your change do to the user who wants that the clean-up to\nalways happen and expresses that desire by setting\ncommit.cleanup=strip in the config?  Doesn't the internal invocation\nof \"commit --cleanup=verbatim\" that is unconditional override it?\n"},{"id":"256678","messageId":"54EEFDF7.8090306@drmicha.warpmail.net","threadId":"38620","inReplyTo":"xmqqoaohonpt.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] sequencer: preserve commit messages","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-02-26T11:05:27Z","receivedAt":"2015-02-26T11:05:27Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 25.02.2015 19:22:\n> Michael J Gruber <git@drmicha.warpmail.net> writes:\n> \n>> Junio C Hamano venit, vidit, dixit 24.02.2015 19:29:\n>>> Michael J Gruber <git@drmicha.warpmail.net> writes:\n>>>\n>>>>> Hmm, wouldn't it introduce a grave regression for users who\n>>>>> explicitly ask to clean crufty messages up (by setting their own\n>>>>> commit.cleanup configuration) if you unconditionally force\n>>>>> \"--cleanup=verbatim\" here?\n>>>>>\n>>>>\n>>>> That's what I meant by possible side-effects below.\n>>>> ...\n>>>> But git cherry-pick without conflict should no re-cleanup the commit\n>>>> message either, should it?\n>>>\n>>> Hmm, but if it does not, wouldn't that countermand the wish of the\n>>> user who explicitly asked to clean crufty messages up by setting\n>>> their own commit.cleanup configuration?\n>>\n>> Note that \"verbatim\" is not the default - we cleanup commits even\n>> without being asked to. And this makes sense for \"git commit\", of course.\n> \n> I am fine with the result of the updated code if the user does not\n> have anything in the config and uses the \"default\".  Not touching in\n> \"cherry-pick\" would be more desirable than cleaning.  We are in\n> agreement for that obvious case.\n\nI didn't know we were. It's clear now.\n\n> But your response is sidestepping my question, isn't it?\n\nI simply misunderstood it.\n\n> What does your change do to the user who wants that the clean-up to\n> always happen and expresses that desire by setting\n> commit.cleanup=strip in the config?  Doesn't the internal invocation\n> of \"commit --cleanup=verbatim\" that is unconditional override it?\n> \n\nYes, it obviously overrides it.\n\nI have to re-check which cleanups rebase does. I hope none.\n\nBut I would think that to clean up a commit message according to the\ncurrent config settings, a user should have to \"commit --amend\" or\n\"rebase -i with reword\" explicitly.\n\nI still think of rebase and cherry-picks as means to transplant a commit\nas unchanged as possible.\n\nNow, if there are conflicts and the user has to resolve them, they will\nuse \"git commit\" themselves with their current config in effect. That is\nto be effected, and the user can use \"git commit --cleanup=...\" however\nthey want.\n\nThat leaves the case of \"git cherry-pick --edit\". I could easily catch\nthat and not overrride config in this case. But the user cannot\ninfluence that other than by using \"git -c commit.cleanup=...\ncherry-pick --edit\".\n\nHmm. With \"--edit\", current config being in effect should be expected,\nright? So how about:\n\nIn case of no conflict: force cleanup=verbatim unless --edit is used?\n\nMichael\n"},{"id":"256698","messageId":"xmqq61aomp1d.fsf@gitster.dls.corp.google.com","threadId":"38620","inReplyTo":"54EEFDF7.8090306@drmicha.warpmail.net","subject":"Re: [PATCH] sequencer: preserve commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-26T19:49:18Z","receivedAt":"2015-02-26T19:49:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@drmicha.warpmail.net> writes:\n\n> Hmm. With \"--edit\", current config being in effect should be expected,\n> right? So how about:\n>\n> In case of no conflict: force cleanup=verbatim unless --edit is used?\n\nPerhaps something like that.\n\nStepping back a bit and imagine a world where the sole purpose of\ncherry-pick were to recreate the original commit as faithfully as\npossible.  The commit log message would not be cleaned up in such\na case by default, and the users need cherrypick.cleanup setting\nif they do not like that default.\n\nThe implementation of cherry-pick that does not spawn the editor\nin that world would look like this:\n\n    - read the cleanup mode from cherrypick.cleanup config; if there\n      is none, read the cleanup mode from commit.cleanup config; if\n      neither is defined, then use 'verbatim' as the default;\n\n    - invoke \"commit --cleanup=\" + that mode from the command line\n      to force the mode chosen by the above.\n\nThanks to the falling back to commit.cleanup, the above logic would\nbe usable even before we invent cherrypick.cleanup configuration,\ni.e. in today's world.  If there is no commit.cleanup defined by the\nuser, the above logic would still use 'verbatim' as the default for\n'cherry-pick', while using the 'default' for 'commit'.\n\nWhen cherry-pick invokes the editor, then the first part would be\ndifferent.  So my conclusion would be something like:\n\n    #if IN_THE_FUTURE\n        if (config_exists(cherrypick.cleanup))\n            mode = config_value(cherrypick.cleanup);\n        else\n    #endif\n        if (config_exists(commit.cleanup))\n            mode = config_value(commit.cleanup);\n        else\n            mode = editing ? 'verbatim' : 'default';\n\n        invoke \"commit --cleanup=\" + mode;\n\nperhaps?\n"},{"id":"256745","messageId":"54F08DE8.3080907@drmicha.warpmail.net","threadId":"38620","inReplyTo":"xmqq61aomp1d.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] sequencer: preserve commit messages","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-02-27T15:31:52Z","receivedAt":"2015-02-27T15:31:52Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 26.02.2015 20:49:\n> Michael J Gruber <git@drmicha.warpmail.net> writes:\n> \n>> Hmm. With \"--edit\", current config being in effect should be expected,\n>> right? So how about:\n>>\n>> In case of no conflict: force cleanup=verbatim unless --edit is used?\n> \n> Perhaps something like that.\n> \n> Stepping back a bit and imagine a world where the sole purpose of\n> cherry-pick were to recreate the original commit as faithfully as\n> possible.  The commit log message would not be cleaned up in such\n> a case by default, and the users need cherrypick.cleanup setting\n> if they do not like that default.\n> \n> The implementation of cherry-pick that does not spawn the editor\n> in that world would look like this:\n> \n>     - read the cleanup mode from cherrypick.cleanup config; if there\n>       is none, read the cleanup mode from commit.cleanup config; if\n>       neither is defined, then use 'verbatim' as the default;\n> \n>     - invoke \"commit --cleanup=\" + that mode from the command line\n>       to force the mode chosen by the above.\n> \n> Thanks to the falling back to commit.cleanup, the above logic would\n> be usable even before we invent cherrypick.cleanup configuration,\n> i.e. in today's world.  If there is no commit.cleanup defined by the\n> user, the above logic would still use 'verbatim' as the default for\n> 'cherry-pick', while using the 'default' for 'commit'.\n> \n> When cherry-pick invokes the editor, then the first part would be\n> different.  So my conclusion would be something like:\n> \n>     #if IN_THE_FUTURE\n>         if (config_exists(cherrypick.cleanup))\n>             mode = config_value(cherrypick.cleanup);\n>         else\n>     #endif\n>         if (config_exists(commit.cleanup))\n>             mode = config_value(commit.cleanup);\n>         else\n>             mode = editing ? 'verbatim' : 'default';\n> \n>         invoke \"commit --cleanup=\" + mode;\n> \n> perhaps?\n> \n\nWithout any config being set the result is certainly what I'm after.\n\nWhat I'm still wondering about is the case without --edit but with\ncommit.cleanup: It seems to me that \"git commit\" being involved in a\nconflict-less cherry-pick is solely an implemention detail (and it could\nbe done differently). Applying commit.* in this situation is a total\nsurpise to the normal user, isn't it? I mean, again, what's the\ndifference to rebase from a user perspective?\n\nMichael\n"},{"id":"256750","messageId":"xmqqd24vjjf0.fsf@gitster.dls.corp.google.com","threadId":"38620","inReplyTo":"54F08DE8.3080907@drmicha.warpmail.net","subject":"Re: [PATCH] sequencer: preserve commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-27T18:31:15Z","receivedAt":"2015-02-27T18:31:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@drmicha.warpmail.net> writes:\n\n> Without any config being set the result is certainly what I'm after.\n>\n> What I'm still wondering about is the case without --edit but with\n> commit.cleanup: It seems to me that \"git commit\" being involved in a\n> conflict-less cherry-pick is solely an implemention detail (and it could\n> be done differently). Applying commit.* in this situation is a total\n> surpise to the normal user, isn't it? I mean, again, what's the\n> difference to rebase from a user perspective?\n\nOK, a revised logic with the above input from you may look like\nthis:\n\n     #if IN_THE_FUTURE\n         if (config_exists(cherrypick.cleanup))\n             mode = config_value(cherrypick.cleanup);\n         else\n     #endif\n         if (editing && config_exists(commit.cleanup))\n             mode = config_value(commit.cleanup);\n         else\n             mode = 'verbatim';\n \n         invoke \"commit --cleanup=\" + mode;\n\nThis is a change in behavoiur (I just checked with v1.6.0 codebase\nand we seem to run a clean-up without \"--edit\"); what is our plan to\nhelp those who have been relying on the auto clean-up behaviour?\n\nAlso a tangent.\n\nI recently run \"cherry-pick -s\" on two commits, and I am not sure if\n\"with --edit, and only with -edit, do the usual clean-up\" is a\nsensible thing to do, or \"-s\" or any other option should trigger the\nusual clean-up if it implies that the user understands and asks the\nlog message to be different from the original (I am leaning towards\nthe latter).\n"},{"id":"257161","messageId":"3e771e1bf45d972c9192441b7dfe6374f5a72bb5.1425649130.git.git@drmicha.warpmail.net","threadId":"38620","inReplyTo":"xmqqd24vjjf0.fsf@gitster.dls.corp.google.com","subject":"[PATCHv2] sequencer: preserve commit messages","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-03-06T13:55:32Z","receivedAt":"2015-03-06T13:55:32Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"sequencer calls \"commit\" with default options, which implies\n\"--cleanup=default\" unless the user specified something else in their\nconfig. This leads to cherry-picked commits getting a cleaned up commit\nmessage, which is usually not an intended side-effect.\n\nMake the sequencer use \"--cleanup=verbatim\" so that it preserves commit\nmessages independent of the default, unless the user has set config for \"commit\"\nor the message is amended with -s or -x.\n\nReported-by: Christoph Anton Mitterer <calestyo@scientia.net>\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\n sequencer.c              |  5 +++++\n t/t3511-cherry-pick-x.sh | 28 ++++++++++++++++++++++++++++\n 2 files changed, 33 insertions(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 77a1266..8cf575c 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -373,6 +373,7 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,\n {\n \tstruct argv_array array;\n \tint rc;\n+\tconst char *value;\n \n \targv_array_init(&array);\n \targv_array_push(&array, \"commit\");\n@@ -385,6 +386,10 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,\n \tif (!opts->edit) {\n \t\targv_array_push(&array, \"-F\");\n \t\targv_array_push(&array, defmsg);\n+\t\tif (!opts->signoff &&\n+\t\t    !opts->record_origin &&\n+\t\t    git_config_get_value(\"commit.cleanup\", &value))\n+\t\t\targv_array_push(&array, \"--cleanup=verbatim\");\n \t}\n \n \tif (allow_empty)\ndiff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh\nindex f977279..b7dff09 100755\n--- a/t/t3511-cherry-pick-x.sh\n+++ b/t/t3511-cherry-pick-x.sh\n@@ -36,6 +36,20 @@ 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_unclean=\"$mesg_one_line\n+\n+\n+leading empty lines\n+\n+\n+consecutive empty lines\n+\n+# hash tag comment\n+\n+trailing empty lines\n+\n+\n+\"\n \n test_expect_success setup '\n \tgit config advice.detachedhead false &&\n@@ -53,6 +67,10 @@ test_expect_success setup '\n \ttest_commit \"$mesg_with_footer_sob\" foo b mesg-with-footer-sob &&\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_config commit.cleanup verbatim &&\n+\ttest_commit \"$mesg_unclean\" foo b mesg-unclean &&\n+\ttest_unconfig commit.cleanup &&\n \tpristine_detach initial &&\n \ttest_commit conflicting unrelated\n '\n@@ -216,4 +234,14 @@ test_expect_success 'cherry-pick -x -s treats \"(cherry picked from...\" line as p\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'cherry-pick preserves commit message' '\n+\tpristine_detach initial &&\n+\tprintf \"$mesg_unclean\" >expect &&\n+\tgit log -1 --pretty=format:%B mesg-unclean >actual &&\n+\ttest_cmp expect actual &&\n+\tgit cherry-pick mesg-unclean &&\n+\tgit log -1 --pretty=format:%B >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.3.1.303.g5174db1\n"},{"id":"257186","messageId":"xmqqegp2ymt4.fsf@gitster.dls.corp.google.com","threadId":"38620","inReplyTo":"3e771e1bf45d972c9192441b7dfe6374f5a72bb5.1425649130.git.git@drmicha.warpmail.net","subject":"Re: [PATCHv2] sequencer: preserve commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-06T18:59:19Z","receivedAt":"2015-03-06T18:59:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@drmicha.warpmail.net> writes:\n\n> sequencer calls \"commit\" with default options, which implies\n> \"--cleanup=default\" unless the user specified something else in their\n> config. This leads to cherry-picked commits getting a cleaned up commit\n> message, which is usually not an intended side-effect.\n>\n> Make the sequencer use \"--cleanup=verbatim\" so that it preserves commit\n> messages independent of the default, unless the user has set config for \"commit\"\n> or the message is amended with -s or -x.\n>\n> Reported-by: Christoph Anton Mitterer <calestyo@scientia.net>\n> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n> ---\n\nLooks very sensible.\n\nThank you very much for your efforts to tie the loose ends on many\ntopics that were discussed already without leading to their\ncompletion.\n\nWill replace and queue.\n\n>  sequencer.c              |  5 +++++\n>  t/t3511-cherry-pick-x.sh | 28 ++++++++++++++++++++++++++++\n>  2 files changed, 33 insertions(+)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index 77a1266..8cf575c 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -373,6 +373,7 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,\n>  {\n>  \tstruct argv_array array;\n>  \tint rc;\n> +\tconst char *value;\n>  \n>  \targv_array_init(&array);\n>  \targv_array_push(&array, \"commit\");\n> @@ -385,6 +386,10 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,\n>  \tif (!opts->edit) {\n>  \t\targv_array_push(&array, \"-F\");\n>  \t\targv_array_push(&array, defmsg);\n> +\t\tif (!opts->signoff &&\n> +\t\t    !opts->record_origin &&\n> +\t\t    git_config_get_value(\"commit.cleanup\", &value))\n> +\t\t\targv_array_push(&array, \"--cleanup=verbatim\");\n>  \t}\n>  \n>  \tif (allow_empty)\n> diff --git a/t/t3511-cherry-pick-x.sh b/t/t3511-cherry-pick-x.sh\n> index f977279..b7dff09 100755\n> --- a/t/t3511-cherry-pick-x.sh\n> +++ b/t/t3511-cherry-pick-x.sh\n> @@ -36,6 +36,20 @@ 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_unclean=\"$mesg_one_line\n> +\n> +\n> +leading empty lines\n> +\n> +\n> +consecutive empty lines\n> +\n> +# hash tag comment\n> +\n> +trailing empty lines\n> +\n> +\n> +\"\n>  \n>  test_expect_success setup '\n>  \tgit config advice.detachedhead false &&\n> @@ -53,6 +67,10 @@ test_expect_success setup '\n>  \ttest_commit \"$mesg_with_footer_sob\" foo b mesg-with-footer-sob &&\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_config commit.cleanup verbatim &&\n> +\ttest_commit \"$mesg_unclean\" foo b mesg-unclean &&\n> +\ttest_unconfig commit.cleanup &&\n>  \tpristine_detach initial &&\n>  \ttest_commit conflicting unrelated\n>  '\n> @@ -216,4 +234,14 @@ test_expect_success 'cherry-pick -x -s treats \"(cherry picked from...\" line as p\n>  \ttest_cmp expect actual\n>  '\n>  \n> +test_expect_success 'cherry-pick preserves commit message' '\n> +\tpristine_detach initial &&\n> +\tprintf \"$mesg_unclean\" >expect &&\n> +\tgit log -1 --pretty=format:%B mesg-unclean >actual &&\n> +\ttest_cmp expect actual &&\n> +\tgit cherry-pick mesg-unclean &&\n> +\tgit log -1 --pretty=format:%B >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n>  test_done\n"}]}