Re: [PATCH] commit: --amend -m '' silently fails to wipe message

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 7, 2016, 18:12 UTC
Message-ID: <xmqqshyxz3ss.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160407044837.GA28859@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I guessed that this might have come from the conversion of "message"
> form a pointer (which could be NULL) into a strbuf. And indeed, it looks
> like f956853 (builtin-commit: resurrect behavior for multiple -m
> options, 2007-11-11) did that.

Yikes.  That is a quite ancient breakage.

The funny thing is that we did address the same breakage in 25206778 (commit: don't start editor if empty message is given with -m, 2013-05-25), but didn't notice that there are other breakages of the same nature.

I think all message.len check can and should be have_option_m instead.

 - The one in the first hunk is a fix for the issue that "-m ''" is
   ignored and we read from use_message etc., which is the original
   issue in this thread.

 - The second one is a fix for your "git commit -m '' -F f" example
   that does not error out.

 - The last one is a fix for "git -c commit.template=f commit -m ''"
   that still uses the template file 'f'.

 builtin/commit.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 98e1527..391126e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -695,7 +695,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		}
 	}
 
-	if (message.len) {
+	if (have_option_m) {
 		strbuf_addbuf(&sb, &message);
 		hook_arg1 = "message";
 	} else if (logfile && !strcmp(logfile, "-")) {
@@ -1172,9 +1172,9 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		f++;
 	if (f > 1)
 		die(_("Only one of -c/-C/-F/--fixup can be used."));
-	if (message.len && f > 0)
+	if (have_option_m && f > 0)
 		die((_("Option -m cannot be combined with -c/-C/-F/--fixup.")));
-	if (f || message.len)
+	if (f || have_option_m)
 		template_file = NULL;
 	if (edit_message)
 		use_message = edit_message;