{"thread":{"id":"30985","subject":"Using git commit --amend on a commit with an empty message","startedAt":"2012-07-09T14:24:38Z","lastAt":"2012-07-09T19:43:54Z","messageCount":5,"participants":["Chris Webb","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"194811","messageId":"20120709142437.GQ13885@arachsys.com","threadId":"30985","inReplyTo":null,"subject":"Using git commit --amend on a commit with an empty message","fromName":"Chris Webb","fromEmail":"chris@arachsys.com","sentAt":"2012-07-09T14:24:38Z","receivedAt":"2012-07-09T14:24:38Z","isPatch":false,"sender":{"key":"chris@arachsys.com","avatar":"https://avatars.githubusercontent.com/u/299056?v=4"},"body":"Github gists can be cloned as normal git repositories, but the commits made\nthrough the web interface appear with an empty commit message. Running\ngit commit --amend against them exposes a slightly odd behaviour of git,\nwhich I can also demonstrate as follows:\n\n $ git init foo && cd foo\n $ touch one && git add one\n $ git commit -m '' --allow-empty-message\n [master (root-commit) 535cb36] \n 0 files changed\n create mode 100644 one\n\nWhen I try to correct this commit message in an editor, it refuses to\nproceed, objecting to the existing empty commit message:\n\n $ git commit --amend\n fatal: commit has empty message\n\nShouldn't this drop me into the editor and fail only if the resulting\nmessage on exit is empty? (For comparison, git commit --amend -m 'oops' will\nwork fine; it's apparently only the edit case which doesn't.)\n\nIn fact, we even fail to start the editor if --allow-empty-message is\nexplicitly provided:\n\n $ git commit --amend --allow-empty-message\n fatal: commit has empty message\n\nAssuming this isn't intentional for some reason I don't understand, I think\nthis is the correct tiny fix? make test succeeds fine both before and after.\n\n-- >8 --\nSubject: [PATCH] Allow edit of empty message with commit --amend\n\nIf git commit --amend is used on a commit with an empty message, it fails\nunless -m is given, whether or not --allow-empty-message is specified.\n\nInstead, allow it to proceed to the editor with an empty commit message.\nUnless --allow-empty-message is in force, it will still abort later if an\nempty message is saved from the editor. (That check was already present\nand necessary to prevent a non-empty commit message being edited to an\nempty one.)\n\nSigned-off-by: Chris Webb \n---\n builtin/commit.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex f43eaaf..6515da2 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -640,7 +640,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\thook_arg1 = \"message\";\n \t} else if (use_message) {\n \t\tbuffer = strstr(use_message_buffer, \"\\n\\n\");\n-\t\tif (!buffer || buffer[2] == '\\0')\n+\t\tif (!use_editor && (!buffer || buffer[2] == '\\0'))\n \t\t\tdie(_(\"commit has empty message\"));\n \t\tstrbuf_add(&sb, buffer + 2, strlen(buffer + 2));\n \t\thook_arg1 = \"commit\";\n-- \n1.7.10\n"},{"id":"194819","messageId":"7v7guclucl.fsf@alter.siamese.dyndns.org","threadId":"30985","inReplyTo":"20120709142437.GQ13885@arachsys.com","subject":"Re: Using git commit --amend on a commit with an empty message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-09T17:25:46Z","receivedAt":"2012-07-09T17:25:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Chris Webb writes:\n\n> In fact, we even fail to start the editor if --allow-empty-message is\n> explicitly provided:\n>\n> $ git commit --allow-empty --allow-empty-message -m ''\n> $ git commit --amend --allow-empty-message\n> fatal: commit has empty message\n>\n> Assuming this isn't intentional for some reason I don't understand, I think\n> this is the correct tiny fix? make test succeeds fine both before and after.\n\nYeah, it is a \"bug\" that exists only because nobody sane uses empty\nmessage commits, let alone tries to amend such commits, hence went\nunnoticed for a long time.\n\nThe patch looks sane; if we want to keep this as a feature or a\nbugfix, we may want to pretect it with a new test, though.\n\nThanks.\n\n> -- >8 --\n> Subject: [PATCH] Allow edit of empty message with commit --amend\n>\n> If git commit --amend is used on a commit with an empty message, it fails\n> unless -m is given, whether or not --allow-empty-message is specified.\n>\n> Instead, allow it to proceed to the editor with an empty commit message.\n> Unless --allow-empty-message is in force, it will still abort later if an\n> empty message is saved from the editor. (That check was already present\n> and necessary to prevent a non-empty commit message being edited to an\n> empty one.)\n>\n> Signed-off-by: Chris Webb \n> ---\n> builtin/commit.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index f43eaaf..6515da2 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -640,7 +640,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n> \t\thook_arg1 = \"message\";\n> \t} else if (use_message) {\n> \t\tbuffer = strstr(use_message_buffer, \"\\n\\n\");\n> -\t\tif (!buffer || buffer[2] == '\\0')\n> +\t\tif (!use_editor && (!buffer || buffer[2] == '\\0'))\n> \t\t\tdie(_(\"commit has empty message\"));\n> \t\tstrbuf_add(&sb, buffer + 2, strlen(buffer + 2));\n> \t\thook_arg1 = \"commit\";\n"},{"id":"194821","messageId":"20120709181754.GE23859@arachsys.com","threadId":"30985","inReplyTo":"7v7guclucl.fsf@alter.siamese.dyndns.org","subject":"Re: Using git commit --amend on a commit with an empty message","fromName":"Chris Webb","fromEmail":"chris@arachsys.com","sentAt":"2012-07-09T18:17:55Z","receivedAt":"2012-07-09T18:17:55Z","isPatch":false,"sender":{"key":"chris@arachsys.com","avatar":"https://avatars.githubusercontent.com/u/299056?v=4"},"body":"Junio C Hamano writes:\n\n> Yeah, it is a \"bug\" that exists only because nobody sane uses empty\n> message commits, let alone tries to amend such commits, hence went\n> unnoticed for a long time.\n\nQuite. I only noticed it because this is the default behaviour of Github\ngists and I wanted to replace the empty commit messages with more meaningful\nones.\n\n> The patch looks sane; if we want to keep this as a feature or a\n> bugfix, we may want to pretect it with a new test, though.\n\nYes, it's hardly something people will test often. Okay, I'll send a version\ntwo with a suitable test.\n\nBest wishes,\n\nChris.\n"},{"id":"194825","messageId":"20120709185326.GF23859@arachsys.com","threadId":"30985","inReplyTo":"20120709181754.GE23859@arachsys.com","subject":"[PATCH v2] Allow edit of empty message with commit --amend","fromName":"Chris Webb","fromEmail":"chris@arachsys.com","sentAt":"2012-07-09T18:53:26Z","receivedAt":"2012-07-09T18:53:26Z","isPatch":true,"sender":{"key":"chris@arachsys.com","avatar":"https://avatars.githubusercontent.com/u/299056?v=4"},"body":"If git commit --amend is used on a commit with an empty message, it fails\nunless -m is given, whether or not --allow-empty-message is specified.\n\nInstead, allow it to proceed to the editor with an empty commit message.\nUnless --allow-empty-message is in force, it will still abort later if an\nempty message is saved from the editor. (This check was already necessary\nto prevent a non-empty commit message being edited to an empty one.)\n\nAdd a test for --amend --edit of an empty commit message which fails\nwithout this fix, as it's a rare case that won't get frequently tested\notherwise.\n\nSigned-off-by: Chris Webb \n---\n builtin/commit.c | 2 +-\n t/t7501-commit.sh | 15 +++++++++++++++\n 2 files changed, 16 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex f43eaaf..6515da2 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -640,7 +640,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\thook_arg1 = \"message\";\n \t} else if (use_message) {\n \t\tbuffer = strstr(use_message_buffer, \"\\n\\n\");\n-\t\tif (!buffer || buffer[2] == '\\0')\n+\t\tif (!use_editor && (!buffer || buffer[2] == '\\0'))\n \t\t\tdie(_(\"commit has empty message\"));\n \t\tstrbuf_add(&sb, buffer + 2, strlen(buffer + 2));\n \t\thook_arg1 = \"commit\";\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex b20ca0e..5ad636b 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -138,6 +138,21 @@ test_expect_success '--amend --edit' '\n \ttest_cmp expect msg\n '\n \n+test_expect_success '--amend --edit of empty message' '\n+\tcat >replace <<-\\EOF &&\n+\t#!/bin/sh\n+\techo \"amended\" >\"$1\"\n+\tEOF\n+\tchmod 755 replace &&\n+\techo amended >expect &&\n+\tgit commit --allow-empty --allow-empty-message -m \"\" &&\n+\techo more bongo >file &&\n+\tgit add file &&\n+\tEDITOR=./replace git commit --edit --amend &&\n+\tgit diff-tree -s --format=%s HEAD >msg &&\n+\ttest_cmp expect msg\n+'\n+\n test_expect_success '-m --edit' '\n \techo amended >expect &&\n \tgit commit --allow-empty -m buffer &&\n-- \n1.7.10\n"},{"id":"194828","messageId":"7vliisk9dx.fsf@alter.siamese.dyndns.org","threadId":"30985","inReplyTo":"20120709185326.GF23859@arachsys.com","subject":"Re: [PATCH v2] Allow edit of empty message with commit --amend","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-09T19:43:54Z","receivedAt":"2012-07-09T19:43:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Chris Webb writes:\n\n> If git commit --amend is used on a commit with an empty message, it fails\n> unless -m is given, whether or not --allow-empty-message is specified.\n>\n> Instead, allow it to proceed to the editor with an empty commit message.\n> Unless --allow-empty-message is in force, it will still abort later if an\n> empty message is saved from the editor. (This check was already necessary\n> to prevent a non-empty commit message being edited to an empty one.)\n>\n> Add a test for --amend --edit of an empty commit message which fails\n> without this fix, as it's a rare case that won't get frequently tested\n> otherwise.\n>\n> Signed-off-by: Chris Webb \n\nThanks.\n"}]}