{"thread":{"id":"31160","subject":"Cherry-picking commits with empty messages","startedAt":"2012-08-01T11:16:59Z","lastAt":"2012-08-06T11:11:22Z","messageCount":11,"participants":["Chris Webb","Junio C Hamano","Angus Hammond","Neil Horman"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"196289","messageId":"20120801111658.GA21272@arachsys.com","threadId":"31160","inReplyTo":null,"subject":"Cherry-picking commits with empty messages","fromName":"Chris Webb","fromEmail":"chris@arachsys.com","sentAt":"2012-08-01T11:16:59Z","receivedAt":"2012-08-01T11:16:59Z","isPatch":false,"sender":{"key":"chris@arachsys.com","avatar":"https://avatars.githubusercontent.com/u/299056?v=4"},"body":"Whilst doing some extra sanity checking of my git-rebase--interactive.sh\npatch yesterday, I came across a behaviour which has been present for some\ntime, but seems surprising. You can reproduce with\n\n $ git init -q foo && cd foo\n $ touch one && git add one && git commit -q -m one\n $ touch two && git add two && git commit -q -m two\n $ touch three && git add three && git commit -q -m '' --allow-empty-message\n $ touch four && git add four && git commit -q -m '' --allow-empty-message\n $ git rebase -i HEAD~3 # and swap the two commits with empty messages\n Aborting commit due to empty commit message.\n Could not apply 59a8fde... \n\nThis happens on my ancient laptop which is apparently running 1.7.8.3, as well\nas current master, so is unconnected to recent changes.\n\nThe reason is that git cherry-pick won't pick a commit with an empty commit\nmessage, even when that message is unmodified from the original:\n\n $ git rebase --abort\n $ git checkout -q HEAD~2\n $ git cherry-pick 59a8fde\n Aborting commit due to empty commit message.\n\nI can see that this check could make sense when the message has been\nmodified, but it seems strange when it hasn't, and isn't ideal behaviour\nwhen called from rebase -i. (We otherwise make sure we call git commit with\n--allow-empty-message to avoid problems with reordering or editing empty\ncommits.)\n\nI could just remove the check in the 'message unmodified' case with\nsomething like\n\ndiff --git a/sequencer.c b/sequencer.c\nindex bf078f2..cf8bc05 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -306,6 +306,7 @@ 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\targv_array_push(&array, \"--allow-empty-message\");\n \t}\n \n \tif (allow_empty)\n\nbut perhaps there are other users of the sequencer for whom this check is\ndesirable? If so, would an --allow-empty-message to git cherry-pick be a\nbetter plan, which git rebase -i can use where appropriate?\n\nBest wishes,\n\nChris.\n"},{"id":"196294","messageId":"7vd33afqjh.fsf@alter.siamese.dyndns.org","threadId":"31160","inReplyTo":"20120801111658.GA21272@arachsys.com","subject":"Re: Cherry-picking commits with empty messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-01T17:52:34Z","receivedAt":"2012-08-01T17:52:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Chris Webb writes:\n\n[summary: this, when 59a8fde does not have any commit log message,\nrefuses to commit]\n\n> $ git cherry-pick 59a8fde\n> Aborting commit due to empty commit message.\n\n> I can see that this check could make sense when the message has been\n> modified, but it seems strange when it hasn't, and isn't ideal behaviour\n> when called from rebase -i. (We otherwise make sure we call git commit with\n> --allow-empty-message to avoid problems with reordering or editing empty\n> commits.)\n> \n> I could just remove the check in the 'message unmodified' case with\n> something like\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index bf078f2..cf8bc05 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -306,6 +306,7 @@ 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\targv_array_push(&array, \"--allow-empty-message\");\n> \t}\n> \n> \tif (allow_empty)\n>\n> but perhaps there are other users of the sequencer for whom this check is\n> desirable? If so, would an --allow-empty-message to git cherry-pick be a\n> better plan, which git rebase -i can use where appropriate?\n\nA few random thoughts.\n\n - Any Porcelain commands that implement the sequencing workflow, if\n they know what message to use when they internally run \"commit\"\n without allowing the user to edit the message, share the same\n issue.\n\n - We generally try to encourage users to describe commits, and\n commits with empty log messages are strongly frowned upon.\n\n In that sense, one could argue that cherry-pick did the right\n thing when it gave control back to you upon seeing an empty\n message. The user is given a chance to fix the commit by running\n \"git commit\" at that point to give it a descriptive message.\n\n - These Porcelain programs, however, work from existing commits,\n and the reason why \"git commit\" invoked by them may be stopped\n due to empty log message is because the original commits had\n empty log message to begin with. The user must have done so on\n purpose (e.g. by using \"commit --allow-empty-message\").\n\n In that sense, it is likely that the user will simply choose to\n run \"git commit --allow-empty-message\", even if given a chance by\n \"cherry-pick\" to correct the empty log message. This is a\n counter-point to the \"give the user a chance to fix\" above.\n We _might_ not be adding much value to the system by giving the\n control back to the user.\n\n - We had a similar discussion on what should happen when one step\n in \"cherry-pick\" results in the same tree as the commit the\n 'pick' builds on (i.e. an empty change). The situation is a bit\n different from yours, because unlike the log message, an empty\n change can result by either (1) the original was an empty change,\n or (2) the change picked was already present in the updated base.\n We added \"--keep-redundant-commits\" and \"--allow-empty\" options\n to underlying \"cherry-pick\" to support this distinction.\n\n We may want to follow suit by triggering your change above only\n when \"cherry-pick --allow-empty-message\" was given. This is\n siding with the \"give the user a chance to fix\" viewpoint to\n choose the default, and giving the users a way to overriding it.\n\n - Regarding the choice of default between \"--allow-empty-message\"\n vs \"--no-allow-empty-message\", one could argue that the best\n choice of the default depends on the Porcelain command.\n\n - A non-range cherry-pick (e.g. \"cherry-pick A B C\") is a strong\n hint from the user that the user wants to replay the specific\n commits that are named on the command line. This fact may\n favor \"the user must have done so on purpose\" viewpoint over\n \"give the user a chance to fix\" viewpoint; defaulting to\n \"--allow-empty-message\" (and \"--allow-empty\", and perhaps\n \"--keep-redundant-commits\") might be more convenient for a\n non-range cherry-pick.\n\n - A range cherry-pick (e.g. \"cherry-pick A..B\") and \"rebase -i\",\n on the other hand, are primarily used to rebuild (and reorder\n in the case of \"rebase -i\") the history to clean it up, which\n may favor \"give the user a chance to fix\", i.e. defaulting not\n to enable \"--allow-empty\"-anything might be more convenient for\n a sequencing operation over a range in general.\n\n But from the bigger UI consistency point of view, it would be\n chaotic to change the default of some options for a single\n command depending on the nature of the operand, so I would\n recommend against going this route, and pick one view between\n \"give the user a chance to fix\" or \"the user must have done so on\n purpose\" and apply it consistently.\n\nMy recommendation, backed by the above line of thought, is to add\nsupport for the \"--allow-empty-message\" option to both \"rebase [-i]\"\nand \"cherry-pick\", defaulting to false.\n"},{"id":"196296","messageId":"CAOBOgRZ9Ouan2htT9m3qBrUvae3nT1az3A61kiRMSJNyFv1MdQ@mail.gmail.com","threadId":"31160","inReplyTo":"7vd33afqjh.fsf@alter.siamese.dyndns.org","subject":"Re: Cherry-picking commits with empty messages","fromName":"Angus Hammond","fromEmail":"angusgh@gmail.com","sentAt":"2012-08-01T18:15:16Z","receivedAt":"2012-08-01T18:15:16Z","isPatch":false,"sender":{"key":"angusgh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1839397?v=4"},"body":"> But from the bigger UI consistency point of view, it would be\n> chaotic to change the default of some options for a single\n> command depending on the nature of the operand, so I would\n> recommend against going this route, and pick one view between\n> \"give the user a chance to fix\" or \"the user must have done so on\n> purpose\" and apply it consistently.\n>\n> My recommendation, backed by the above line of thought, is to add\n> support for the \"--allow-empty-message\" option to both \"rebase [-i]\"\n> and \"cherry-pick\", defaulting to false.\n\nThough I completely agree regarding having a consistent UI that\ndoesn't change it's behaviour based on the operand, I'd argue that\n--allow-empty-message should default to true on cherry-pick for a\ncouple or reasons. Firstly, in the case that git perpetuates an empty\ncommit message that the user does not want, it is only damaging a\nrepository in a way that it is already damaged, clearly this still\nisn't ideal, but it's certainly not as bad as damaging a repository\nthat's pristine. Arguably it's the user's responsibility to ensure\nthey don't TELL git to perpetuate their own bad commit.\n\nSecondly, I'd don't like the idea of a command that 99.9% of the time\nwill run completely independently, but then every so often will become\ninteractive. This is probably a rare enough scenario that script\nwriters would reasonably assume that cherry-pick (without the\n--allow-empty-message flag) is not an interactive command and write\ntheir scripts accordingly. A user who made use of empty commit\nmessages would find any such scripts crashing on them or producing\nstrange results. Even if this is the fringe case, it seems to be a\nsubstantially worse fringe case than that where we make a commit that\nhas no message at the user's instruction.\n\nThanks\nAngus\n"},{"id":"196314","messageId":"7vehnqdz9t.fsf@alter.siamese.dyndns.org","threadId":"31160","inReplyTo":"CAOBOgRZ9Ouan2htT9m3qBrUvae3nT1az3A61kiRMSJNyFv1MdQ@mail.gmail.com","subject":"Re: Cherry-picking commits with empty messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-01T22:26:54Z","receivedAt":"2012-08-01T22:26:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Angus Hammond writes:\n\n>> But from the bigger UI consistency point of view, it would be\n>> chaotic to change the default of some options for a single\n>> command depending on the nature of the operand, so I would\n>> recommend against going this route, and pick one view between\n>> \"give the user a chance to fix\" or \"the user must have done so on\n>> purpose\" and apply it consistently.\n>>\n>> My recommendation, backed by the above line of thought, is to add\n>> support for the \"--allow-empty-message\" option to both \"rebase [-i]\"\n>> and \"cherry-pick\", defaulting to false.\n>\n> Though I completely agree regarding having a consistent UI that\n> doesn't change it's behaviour based on the operand, I'd argue that\n> --allow-empty-message should default to true on cherry-pick for a\n> couple or reasons.\n\nI've read your entire response three times, and I am having a hard\ntime deciding if you are against my suggestion, or you misread my\nsuggestion.\n\n> Firstly, in the case that git perpetuates an empty\n> commit message that the user does not want, it is only damaging a\n> repository in a way that it is already damaged, clearly this still\n> isn't ideal, but it's certainly not as bad as damaging a repository\n> that's pristine. Arguably it's the user's responsibility to ensure\n> they don't TELL git to perpetuate their own bad commit.\n\nI guess by \"perpetuates\" you meant there was already a commit with\nan empty message, by \"the user does not want\" you consider such a\ncommit is a bad thing, and by \"to ensure they don't TELL git\", you\nmeant it is the user's responsibility not to give an extra option to\ncause Git to replay a bad (= having an empty message) commit and\nleave it in the resulting history.\n\nIt sounds to me that you are advocating for \"git cherry-pick\"\nwithout any flags to stop and do not commit when given a commit with\nan empty message.\n\nAnd that is what I thought I was suggesting. Give users a support\nto say \"git cherry-pick --allow-empty\", but do not by default enable\nit. Perhaps I sounded as if I was suggesting the opposite?\n\n> Secondly, I'd don't like the idea of a command that 99.9% of the time\n> will run completely independently, but then every so often will become\n> interactive.\n\nAs \"cherry-pick\" is expected to stop and give control back whenever\nthere is conflicts, this does not apply. Any script that uses\ncherry-pick to replay an existing commit has to be prepared to see\nit stop and give control back to the script already, or the script\nis unusable. Note that the script would not be buggy even if the\nonly thing it does when it sees cherry-pick stop and give control to\nit is to abort and give control back to the user.\n"},{"id":"196325","messageId":"20120802085554.GI19416@arachsys.com","threadId":"31160","inReplyTo":"7vd33afqjh.fsf@alter.siamese.dyndns.org","subject":"Re: Cherry-picking commits with empty messages","fromName":"Chris Webb","fromEmail":"chris@arachsys.com","sentAt":"2012-08-02T08:55:59Z","receivedAt":"2012-08-02T08:55:59Z","isPatch":false,"sender":{"key":"chris@arachsys.com","avatar":"https://avatars.githubusercontent.com/u/299056?v=4"},"body":"Junio C Hamano writes:\n\n> My recommendation, backed by the above line of thought, is to add\n> support for the \"--allow-empty-message\" option to both \"rebase [-i]\"\n> and \"cherry-pick\", defaulting to false.\n\nThanks for the very detailed analysis and advice Junio. I like your\nsuggested --allow-empty-message option for cherry-pick because it's\nconsistent with the same option in standard commit, and doesn't change the\nbehaviour for existing users who might rely on cherry-pick catching blank\nmessages.\n\nWith rebase -i, the fix might be slightly more involved than just passing\nthrough --allow-empty-message (if given) to cherry-pick, especially given\nthat sometimes we git cherry-pick -n && git commit --allow-empty-message,\nand at other times we do standard git cherry-pick which refuses to pick a\ncommit without a message.\n\nGiven a history with empty commits, as a general principle it feels like it\nshould be possible to edit or reword those commits to make them non-empty\nwithout giving --allow-empty-message, but that to generate new history\ncontaining empty messages, --allow-empty-message should be required, whether\nto commit [--amend] during rebase, or to the rebase -i command itself.\n\nCheers,\n\nChris.\n"},{"id":"196326","messageId":"CAOBOgRbH_ddjW+DJiVSYt5bADEgqf_E+L9offvnF8SdH+GR50g@mail.gmail.com","threadId":"31160","inReplyTo":"7vehnqdz9t.fsf@alter.siamese.dyndns.org","subject":"Re: Cherry-picking commits with empty messages","fromName":"Angus Hammond","fromEmail":"angusgh@gmail.com","sentAt":"2012-08-02T10:10:35Z","receivedAt":"2012-08-02T10:10:35Z","isPatch":false,"sender":{"key":"angusgh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1839397?v=4"},"body":"On 1 August 2012 23:26, Junio C Hamano wrote:\n> I've read your entire response three times, and I am having a hard\n> time deciding if you are against my suggestion, or you misread my\n> suggestion.\n\nMy apologies, I can see how my message wasn't as clear as it could have been.\n\n> I guess by \"perpetuates\" you meant there was already a commit with\n> an empty message, by \"the user does not want\" you consider such a\n> commit is a bad thing, and by \"to ensure they don't TELL git\", you\n> meant it is the user's responsibility not to give an extra option to\n> cause Git to replay a bad (= having an empty message) commit and\n> leave it in the resulting history.\n\nI was just trying to say that cherry-pick will only ever create a\nblank commit message where one already exists in the repository, so\ngit shouldn't worry about copying that blank commit message,\nespecially since the user has explicitly told git (by running\ncherry-pick) that they want those commits copied.\n\n> It sounds to me that you are advocating for \"git cherry-pick\"\n> without any flags to stop and do not commit when given a commit with\n> an empty message.\n>\n> And that is what I thought I was suggesting. Give users a support\n> to say \"git cherry-pick --allow-empty\", but do not by default enable\n> it. Perhaps I sounded as if I was suggesting the opposite?\n\nI meant the opposite, that without any flags it should just copy the\nblank commit silently.\n\n>> Secondly, I'd don't like the idea of a command that 99.9% of the time\n>> will run completely independently, but then every so often will become\n>> interactive.\n>\n> As \"cherry-pick\" is expected to stop and give control back whenever\n> there is conflicts, this does not apply. Any script that uses\n> cherry-pick to replay an existing commit has to be prepared to see\n> it stop and give control back to the script already, or the script\n> is unusable. Note that the script would not be buggy even if the\n> only thing it does when it sees cherry-pick stop and give control to\n> it is to abort and give control back to the user.\n\nFair enough, I don't make heavy use of cherry-pick, so that didn't occur to me.\n\nSince you've pointed out that the scripting argument is invalid, I'm\nnow inclined to support what you originally proposed (by default\nrefuse to create an empty commit message, offer a flag to override\nthat default), but just wanted to clear up my original point since it\nwasn't written very clearly.\n\nThanks\nAngus\n"},{"id":"196328","messageId":"86938f900c630d983852a250090f2aa6112fcc3c.1343903931.git.chris@arachsys.com","threadId":"31160","inReplyTo":"20120802085554.GI19416@arachsys.com","subject":"[PATCH] cherry-pick: add --allow-empty-message option","fromName":"Chris Webb","fromEmail":"chris@arachsys.com","sentAt":"2012-08-02T10:38:51Z","receivedAt":"2012-08-02T10:38:51Z","isPatch":true,"sender":{"key":"chris@arachsys.com","avatar":"https://avatars.githubusercontent.com/u/299056?v=4"},"body":"Scripts such as git rebase -i cannot currently cherry-pick commits which\nhave an empty commit message, as git cherry-pick calls git commit\nwithout the --allow-empty-message option.\n\nAdd an --allow-empty-message option to git cherry-pick which is passed\nthrough to git commit, so this behaviour can be overridden.\n\nSigned-off-by: Chris Webb \n---\n Documentation/git-cherry-pick.txt | 5 +++++\n builtin/revert.c | 2 ++\n sequencer.c | 3 +++\n sequencer.h | 1 +\n t/t3505-cherry-pick-empty.sh | 5 +++++\n 5 files changed, 16 insertions(+)\n\ndiff --git a/Documentation/git-cherry-pick.txt b/Documentation/git-cherry-pick.txt\nindex 0e170a5..c205d23 100644\n--- a/Documentation/git-cherry-pick.txt\n+++ b/Documentation/git-cherry-pick.txt\n@@ -118,6 +118,11 @@ effect to your index in a row.\n \tprevious commit are dropped. To force the inclusion of those commits\n \tuse `--keep-redundant-commits`.\n \n+--allow-empty-message::\n+\tBy default, cherry-picking a commit with an empty message will fail.\n+\tThis option overrides that behaviour, allowing commits with empty\n+\tmessages to be cherry picked.\n+\n --keep-redundant-commits::\n \tIf a commit being cherry picked duplicates a commit already in the\n \tcurrent history, it will become empty. By default these\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 82d1bf8..5652f23 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -117,6 +117,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\tOPT_END(),\n \t\tOPT_END(),\n \t\tOPT_END(),\n+\t\tOPT_END(),\n \t};\n \n \tif (opts->action == REPLAY_PICK) {\n@@ -124,6 +125,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\t\tOPT_BOOLEAN('x', NULL, &opts->record_origin, \"append commit name\"),\n \t\t\tOPT_BOOLEAN(0, \"ff\", &opts->allow_ff, \"allow fast-forward\"),\n \t\t\tOPT_BOOLEAN(0, \"allow-empty\", &opts->allow_empty, \"preserve initially empty commits\"),\n+\t\t\tOPT_BOOLEAN(0, \"allow-empty-message\", &opts->allow_empty_message, \"allow commits with empty messages\"),\n \t\t\tOPT_BOOLEAN(0, \"keep-redundant-commits\", &opts->keep_redundant_commits, \"keep redundant, empty commits\"),\n \t\t\tOPT_END(),\n \t\t};\ndiff --git a/sequencer.c b/sequencer.c\nindex bf078f2..1ea5293 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -311,6 +311,9 @@ static int run_git_commit(const char *defmsg, struct replay_opts *opts,\n \tif (allow_empty)\n \t\targv_array_push(&array, \"--allow-empty\");\n \n+\tif (opts->allow_empty_message)\n+\t\targv_array_push(&array, \"--allow-empty-message\");\n+\n \trc = run_command_v_opt(array.argv, RUN_GIT_CMD);\n \targv_array_clear(&array);\n \treturn rc;\ndiff --git a/sequencer.h b/sequencer.h\nindex aa5f17c..d849420 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -30,6 +30,7 @@ struct replay_opts {\n \tint allow_ff;\n \tint allow_rerere_auto;\n \tint allow_empty;\n+\tint allow_empty_message;\n \tint keep_redundant_commits;\n \n \tint mainline;\ndiff --git a/t/t3505-cherry-pick-empty.sh b/t/t3505-cherry-pick-empty.sh\nindex 5a1340c..a0c6e30 100755\n--- a/t/t3505-cherry-pick-empty.sh\n+++ b/t/t3505-cherry-pick-empty.sh\n@@ -53,6 +53,11 @@ test_expect_success 'index lockfile was removed' '\n \n '\n \n+test_expect_success 'cherry-pick a commit with an empty message with --allow-empty-message' '\n+\tgit checkout -f master &&\n+\tgit cherry-pick --allow-empty-message empty-branch\n+'\n+\n test_expect_success 'cherry pick an empty non-ff commit without --allow-empty' '\n \tgit checkout master &&\n \techo fourth >>file2 &&\n"},{"id":"196394","messageId":"20120803002258.GB10407@neilslaptop.think-freely.org","threadId":"31160","inReplyTo":"7vd33afqjh.fsf@alter.siamese.dyndns.org","subject":"Re: Cherry-picking commits with empty messages","fromName":"Neil Horman","fromEmail":"nhorman@tuxdriver.com","sentAt":"2012-08-03T00:22:58Z","receivedAt":"2012-08-03T00:22:58Z","isPatch":false,"sender":{"key":"nhorman@tuxdriver.com","avatar":"https://avatars.githubusercontent.com/u/1032926?v=4"},"body":"On Wed, Aug 01, 2012 at 10:52:34AM -0700, Junio C Hamano wrote:\n> Chris Webb writes:\n> \n> [summary: this, when 59a8fde does not have any commit log message,\n> refuses to commit]\n> \nThanks for CC'ing me on this. I'm on vacation currently, but will look at this\nin detail as soon as I'm back next week\nNeil\n\n> > $ git cherry-pick 59a8fde\n> > Aborting commit due to empty commit message.\n> \n> > I can see that this check could make sense when the message has been\n> > modified, but it seems strange when it hasn't, and isn't ideal behaviour\n> > when called from rebase -i. (We otherwise make sure we call git commit with\n> > --allow-empty-message to avoid problems with reordering or editing empty\n> > commits.)\n> > \n> > I could just remove the check in the 'message unmodified' case with\n> > something like\n> >\n> > diff --git a/sequencer.c b/sequencer.c\n> > index bf078f2..cf8bc05 100644\n> > --- a/sequencer.c\n> > +++ b/sequencer.c\n> > @@ -306,6 +306,7 @@ 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\targv_array_push(&array, \"--allow-empty-message\");\n> > \t}\n> > \n> > \tif (allow_empty)\n> >\n> > but perhaps there are other users of the sequencer for whom this check is\n> > desirable? If so, would an --allow-empty-message to git cherry-pick be a\n> > better plan, which git rebase -i can use where appropriate?\n> \n> A few random thoughts.\n> \n> - Any Porcelain commands that implement the sequencing workflow, if\n> they know what message to use when they internally run \"commit\"\n> without allowing the user to edit the message, share the same\n> issue.\n> \n> - We generally try to encourage users to describe commits, and\n> commits with empty log messages are strongly frowned upon.\n> \n> In that sense, one could argue that cherry-pick did the right\n> thing when it gave control back to you upon seeing an empty\n> message. The user is given a chance to fix the commit by running\n> \"git commit\" at that point to give it a descriptive message.\n> \n> - These Porcelain programs, however, work from existing commits,\n> and the reason why \"git commit\" invoked by them may be stopped\n> due to empty log message is because the original commits had\n> empty log message to begin with. The user must have done so on\n> purpose (e.g. by using \"commit --allow-empty-message\").\n> \n> In that sense, it is likely that the user will simply choose to\n> run \"git commit --allow-empty-message\", even if given a chance by\n> \"cherry-pick\" to correct the empty log message. This is a\n> counter-point to the \"give the user a chance to fix\" above.\n> We _might_ not be adding much value to the system by giving the\n> control back to the user.\n> \n> - We had a similar discussion on what should happen when one step\n> in \"cherry-pick\" results in the same tree as the commit the\n> 'pick' builds on (i.e. an empty change). The situation is a bit\n> different from yours, because unlike the log message, an empty\n> change can result by either (1) the original was an empty change,\n> or (2) the change picked was already present in the updated base.\n> We added \"--keep-redundant-commits\" and \"--allow-empty\" options\n> to underlying \"cherry-pick\" to support this distinction.\n> \n> We may want to follow suit by triggering your change above only\n> when \"cherry-pick --allow-empty-message\" was given. This is\n> siding with the \"give the user a chance to fix\" viewpoint to\n> choose the default, and giving the users a way to overriding it.\n> \n> - Regarding the choice of default between \"--allow-empty-message\"\n> vs \"--no-allow-empty-message\", one could argue that the best\n> choice of the default depends on the Porcelain command.\n> \n> - A non-range cherry-pick (e.g. \"cherry-pick A B C\") is a strong\n> hint from the user that the user wants to replay the specific\n> commits that are named on the command line. This fact may\n> favor \"the user must have done so on purpose\" viewpoint over\n> \"give the user a chance to fix\" viewpoint; defaulting to\n> \"--allow-empty-message\" (and \"--allow-empty\", and perhaps\n> \"--keep-redundant-commits\") might be more convenient for a\n> non-range cherry-pick.\n> \n> - A range cherry-pick (e.g. \"cherry-pick A..B\") and \"rebase -i\",\n> on the other hand, are primarily used to rebuild (and reorder\n> in the case of \"rebase -i\") the history to clean it up, which\n> may favor \"give the user a chance to fix\", i.e. defaulting not\n> to enable \"--allow-empty\"-anything might be more convenient for\n> a sequencing operation over a range in general.\n> \n> But from the bigger UI consistency point of view, it would be\n> chaotic to change the default of some options for a single\n> command depending on the nature of the operand, so I would\n> recommend against going this route, and pick one view between\n> \"give the user a chance to fix\" or \"the user must have done so on\n> purpose\" and apply it consistently.\n> \n> My recommendation, backed by the above line of thought, is to add\n> support for the \"--allow-empty-message\" option to both \"rebase [-i]\"\n> and \"cherry-pick\", defaulting to false.\n> \n"},{"id":"196537","messageId":"20120806105729.GC16873@hmsreliant.think-freely.org","threadId":"31160","inReplyTo":"86938f900c630d983852a250090f2aa6112fcc3c.1343903931.git.chris@arachsys.com","subject":"Re: [PATCH] cherry-pick: add --allow-empty-message option","fromName":"Neil Horman","fromEmail":"nhorman@tuxdriver.com","sentAt":"2012-08-06T10:57:29Z","receivedAt":"2012-08-06T10:57:29Z","isPatch":true,"sender":{"key":"nhorman@tuxdriver.com","avatar":"https://avatars.githubusercontent.com/u/1032926?v=4"},"body":"On Thu, Aug 02, 2012 at 11:38:51AM +0100, Chris Webb wrote:\n> Scripts such as git rebase -i cannot currently cherry-pick commits which\n> have an empty commit message, as git cherry-pick calls git commit\n> without the --allow-empty-message option.\n> \n> Add an --allow-empty-message option to git cherry-pick which is passed\n> through to git commit, so this behaviour can be overridden.\n> \n> Signed-off-by: Chris Webb \nSorry for the late response, but I just pulled back into town.\n\nHaving read over this thread, I think this is definately the way to go. As\ndiscussed having cherry-pick stop and give the user a chance to fix empty\nhistory messages by default, and providing a switch to override that behavior\nmakes sense to me. That said, shouldn't there be extra code here in the rebase\nscripts to automate commit migration in that path as well?\nNeil\n\n> \n"},{"id":"196538","messageId":"20120806110016.GA8587@arachsys.com","threadId":"31160","inReplyTo":"20120806105729.GC16873@hmsreliant.think-freely.org","subject":"Re: [PATCH] cherry-pick: add --allow-empty-message option","fromName":"Chris Webb","fromEmail":"chris@arachsys.com","sentAt":"2012-08-06T11:00:16Z","receivedAt":"2012-08-06T11:00:16Z","isPatch":true,"sender":{"key":"chris@arachsys.com","avatar":"https://avatars.githubusercontent.com/u/299056?v=4"},"body":"Neil Horman writes:\n\n> Having read over this thread, I think this is definately the way to go. As\n> discussed having cherry-pick stop and give the user a chance to fix empty\n> history messages by default, and providing a switch to override that behavior\n> makes sense to me. That said, shouldn't there be extra code here in the rebase\n> scripts to automate commit migration in that path as well?\n\nYes, this patch just adds the support to the low-level git cherry-pick as\nyou say. I'll follow up with a patch to use the new feature in rebase [-i]\nwhen I get some free time, hopefully later this week.\n\nCheers,\n\nChris.\n"},{"id":"196539","messageId":"20120806111122.GD16873@hmsreliant.think-freely.org","threadId":"31160","inReplyTo":"20120806110016.GA8587@arachsys.com","subject":"Re: [PATCH] cherry-pick: add --allow-empty-message option","fromName":"Neil Horman","fromEmail":"nhorman@tuxdriver.com","sentAt":"2012-08-06T11:11:22Z","receivedAt":"2012-08-06T11:11:22Z","isPatch":true,"sender":{"key":"nhorman@tuxdriver.com","avatar":"https://avatars.githubusercontent.com/u/1032926?v=4"},"body":"On Mon, Aug 06, 2012 at 12:00:16PM +0100, Chris Webb wrote:\n> Neil Horman writes:\n> \n> > Having read over this thread, I think this is definately the way to go. As\n> > discussed having cherry-pick stop and give the user a chance to fix empty\n> > history messages by default, and providing a switch to override that behavior\n> > makes sense to me. That said, shouldn't there be extra code here in the rebase\n> > scripts to automate commit migration in that path as well?\n> \n> Yes, this patch just adds the support to the low-level git cherry-pick as\n> you say. I'll follow up with a patch to use the new feature in rebase [-i]\n> when I get some free time, hopefully later this week.\n> \n> Cheers,\n> \n> Chris.\n> \nOk, then\nAcked-by: Neil Horman \n"}]}