# Using git commit --amend on a commit with an empty message 5 messages from 2012-07-09 to 2012-07-09. Participants: Chris Webb, Junio C Hamano. Thread: https://gitlist.dev/t/30985 ## Chris Webb, 2012-07-09 14:24 Subject: Using git commit --amend on a commit with an empty message Message-ID: <20120709142437.GQ13885@arachsys.com> URL: https://gitlist.dev/e/20120709142437.GQ13885%40arachsys.com ``` Github gists can be cloned as normal git repositories, but the commits made through the web interface appear with an empty commit message. Running git commit --amend against them exposes a slightly odd behaviour of git, which I can also demonstrate as follows: $ git init foo && cd foo $ touch one && git add one $ git commit -m '' --allow-empty-message [master (root-commit) 535cb36] 0 files changed create mode 100644 one When I try to correct this commit message in an editor, it refuses to proceed, objecting to the existing empty commit message: $ git commit --amend fatal: commit has empty message Shouldn't this drop me into the editor and fail only if the resulting message on exit is empty? (For comparison, git commit --amend -m 'oops' will work fine; it's apparently only the edit case which doesn't.) In fact, we even fail to start the editor if --allow-empty-message is explicitly provided: $ git commit --amend --allow-empty-message fatal: commit has empty message Assuming this isn't intentional for some reason I don't understand, I think this is the correct tiny fix? make test succeeds fine both before and after. -- >8 -- Subject: [PATCH] Allow edit of empty message with commit --amend If git commit --amend is used on a commit with an empty message, it fails unless -m is given, whether or not --allow-empty-message is specified. Instead, allow it to proceed to the editor with an empty commit message. Unless --allow-empty-message is in force, it will still abort later if an empty message is saved from the editor. (That check was already present and necessary to prevent a non-empty commit message being edited to an empty one.) Signed-off-by: Chris Webb --- builtin/commit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/commit.c b/builtin/commit.c index f43eaaf..6515da2 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -640,7 +640,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, hook_arg1 = "message"; } else if (use_message) { buffer = strstr(use_message_buffer, "\n\n"); - if (!buffer || buffer[2] == '\0') + if (!use_editor && (!buffer || buffer[2] == '\0')) die(_("commit has empty message")); strbuf_add(&sb, buffer + 2, strlen(buffer + 2)); hook_arg1 = "commit"; -- 1.7.10 ``` ## Junio C Hamano, 2012-07-09 17:25 Subject: Re: Using git commit --amend on a commit with an empty message Message-ID: <7v7guclucl.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v7guclucl.fsf%40alter.siamese.dyndns.org In-Reply-To: <20120709142437.GQ13885@arachsys.com> ``` Chris Webb writes: > In fact, we even fail to start the editor if --allow-empty-message is > explicitly provided: > > $ git commit --allow-empty --allow-empty-message -m '' > $ git commit --amend --allow-empty-message > fatal: commit has empty message > > Assuming this isn't intentional for some reason I don't understand, I think > this is the correct tiny fix? make test succeeds fine both before and after. Yeah, it is a "bug" that exists only because nobody sane uses empty message commits, let alone tries to amend such commits, hence went unnoticed for a long time. The patch looks sane; if we want to keep this as a feature or a bugfix, we may want to pretect it with a new test, though. Thanks. > -- >8 -- > Subject: [PATCH] Allow edit of empty message with commit --amend > > If git commit --amend is used on a commit with an empty message, it fails > unless -m is given, whether or not --allow-empty-message is specified. > > Instead, allow it to proceed to the editor with an empty commit message. > Unless --allow-empty-message is in force, it will still abort later if an > empty message is saved from the editor. (That check was already present > and necessary to prevent a non-empty commit message being edited to an > empty one.) > > Signed-off-by: Chris Webb > --- > builtin/commit.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/builtin/commit.c b/builtin/commit.c > index f43eaaf..6515da2 100644 > --- a/builtin/commit.c > +++ b/builtin/commit.c > @@ -640,7 +640,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, > hook_arg1 = "message"; > } else if (use_message) { > buffer = strstr(use_message_buffer, "\n\n"); > - if (!buffer || buffer[2] == '\0') > + if (!use_editor && (!buffer || buffer[2] == '\0')) > die(_("commit has empty message")); > strbuf_add(&sb, buffer + 2, strlen(buffer + 2)); > hook_arg1 = "commit"; ``` ## Chris Webb, 2012-07-09 18:17 Subject: Re: Using git commit --amend on a commit with an empty message Message-ID: <20120709181754.GE23859@arachsys.com> URL: https://gitlist.dev/e/20120709181754.GE23859%40arachsys.com In-Reply-To: <7v7guclucl.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano writes: > Yeah, it is a "bug" that exists only because nobody sane uses empty > message commits, let alone tries to amend such commits, hence went > unnoticed for a long time. Quite. I only noticed it because this is the default behaviour of Github gists and I wanted to replace the empty commit messages with more meaningful ones. > The patch looks sane; if we want to keep this as a feature or a > bugfix, we may want to pretect it with a new test, though. Yes, it's hardly something people will test often. Okay, I'll send a version two with a suitable test. Best wishes, Chris. ``` ## Chris Webb, 2012-07-09 18:53 Subject: [PATCH v2] Allow edit of empty message with commit --amend Message-ID: <20120709185326.GF23859@arachsys.com> URL: https://gitlist.dev/e/20120709185326.GF23859%40arachsys.com In-Reply-To: <20120709181754.GE23859@arachsys.com> ``` If git commit --amend is used on a commit with an empty message, it fails unless -m is given, whether or not --allow-empty-message is specified. Instead, allow it to proceed to the editor with an empty commit message. Unless --allow-empty-message is in force, it will still abort later if an empty message is saved from the editor. (This check was already necessary to prevent a non-empty commit message being edited to an empty one.) Add a test for --amend --edit of an empty commit message which fails without this fix, as it's a rare case that won't get frequently tested otherwise. Signed-off-by: Chris Webb --- builtin/commit.c | 2 +- t/t7501-commit.sh | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/builtin/commit.c b/builtin/commit.c index f43eaaf..6515da2 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -640,7 +640,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, hook_arg1 = "message"; } else if (use_message) { buffer = strstr(use_message_buffer, "\n\n"); - if (!buffer || buffer[2] == '\0') + if (!use_editor && (!buffer || buffer[2] == '\0')) die(_("commit has empty message")); strbuf_add(&sb, buffer + 2, strlen(buffer + 2)); hook_arg1 = "commit"; diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh index b20ca0e..5ad636b 100755 --- a/t/t7501-commit.sh +++ b/t/t7501-commit.sh @@ -138,6 +138,21 @@ test_expect_success '--amend --edit' ' test_cmp expect msg ' +test_expect_success '--amend --edit of empty message' ' + cat >replace <<-\EOF && + #!/bin/sh + echo "amended" >"$1" + EOF + chmod 755 replace && + echo amended >expect && + git commit --allow-empty --allow-empty-message -m "" && + echo more bongo >file && + git add file && + EDITOR=./replace git commit --edit --amend && + git diff-tree -s --format=%s HEAD >msg && + test_cmp expect msg +' + test_expect_success '-m --edit' ' echo amended >expect && git commit --allow-empty -m buffer && -- 1.7.10 ``` ## Junio C Hamano, 2012-07-09 19:43 Subject: Re: [PATCH v2] Allow edit of empty message with commit --amend Message-ID: <7vliisk9dx.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vliisk9dx.fsf%40alter.siamese.dyndns.org In-Reply-To: <20120709185326.GF23859@arachsys.com> ``` Chris Webb writes: > If git commit --amend is used on a commit with an empty message, it fails > unless -m is given, whether or not --allow-empty-message is specified. > > Instead, allow it to proceed to the editor with an empty commit message. > Unless --allow-empty-message is in force, it will still abort later if an > empty message is saved from the editor. (This check was already necessary > to prevent a non-empty commit message being edited to an empty one.) > > Add a test for --amend --edit of an empty commit message which fails > without this fix, as it's a rare case that won't get frequently tested > otherwise. > > Signed-off-by: Chris Webb Thanks. ```