git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Adam Dinwoodie <adam@dinwoodie.org>
Date
Apr 7, 2016, 09:50 UTC
Message-ID
<20160407095006.GB2345@dinwoodie.org>
In-Reply-To
<20160407044219.GA29710@sigill.intra.peff.net>
On Thu, Apr 07, 2016 at 12:42:19AM -0400, Jeff King wrote:
Show 32 quoted lines
> On Wed, Apr 06, 2016 at 06:15:03PM +0100, Adam Dinwoodie wrote:
> > `git commit --amend -m ''` seems to be an unambiguous request to blank a
> > commit message, but it actually leaves the commit message as-is.  That's
> > the case regardless of whether `--allow-empty-message` is specified, and
> > doesn't so much as drop a non-zero return code.
> > 
> > Add failing tests to show this behaviour.
> 
> Hmm. Is it just that we check "message.len", which cannot tell the
> difference between "-m was not given" and "-m was given the empty
> string"?
> 
> IOW, would this fix it?
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 109742e..3cdc44e 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, "-")) {
> 
> It passes the test suite and your two new tests, but I didn't look too
> deeply to see if there are other fallouts. I don't think anybody should
> be using it to counteract a previous "-m" or anything like that; we have
> "--no-message" for that.

That makes sense as a fix to me, but I'm not going to claim to be even slightly familiar with the code here.

> PS Is there a previous thread? I see a couple people cc'd, including me,
>    but I don't remember a previous discussion. Did I just forget it?

No previous thread: I noticed the odd behaviour, and figured I'd report it. The best way to report a problem seemed to be providing a test showing the behaviour, and the SubmittingPatches doc said I should CC folk who looked they're involved in the area...

Adam
Previous: Jeff KingNext: Jeff King
Message 8 of 9 in “commit: --amend -m '' silently fails to wipe message”
  1. commit: --amend -m '' silently fails to wipe messageAdam Dinwoodie, Apr 6, 2016
  2. Jeff KingApr 7, 2016
  3. Jeff KingApr 7, 2016
  4. Junio C HamanoApr 7, 2016
  5. Jeff KingApr 7, 2016
  6. Junio C HamanoApr 7, 2016
  7. Jeff KingApr 7, 2016
  8. Adam DinwoodieApr 7, 2016
  9. Jeff KingApr 7, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.