threads / discuss / 30985

Using git commit --amend on a commit with an empty message

Subject: Using git commit --amend on a commit with an empty message

## tl;dr

5 messages between Jul 9, 2012 and Jul 9, 2012.

replies: 4people: 2as markdown or json

Chris Webb· Jul 9, 2012, 14:24 UTC · lore

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 <chris@arachsys.com>
---
 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· Jul 9, 2012, 17:25 UTC · re: Chris Webb · lore

Re: Using git commit --amend on a commit with an empty message

Chris Webb <chris@arachsys.com> writes:

Show 9 quoted lines

> 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.

Show 30 quoted lines

> -- >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 <chris@arachsys.com>
> ---
>  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· Jul 9, 2012, 18:17 UTC · re: Junio C Hamano · lore

Re: Using git commit --amend on a commit with an empty message

Junio C Hamano <gitster@pobox.com> 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· Jul 9, 2012, 18:53 UTC · re: Chris Webb · lore

[PATCH v2] 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. (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 <chris@arachsys.com>
---
 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· Jul 9, 2012, 19:43 UTC · re: Chris Webb · lore

Re: [PATCH v2] Allow edit of empty message with commit --amend

Chris Webb <chris@arachsys.com> writes:

Show 13 quoted lines

> 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 <chris@arachsys.com>

Thanks.