From 178e8143b4f79290b3ffe40fe2ebf769fccc1ab7 Mon Sep 17 00:00:00 2001 From: Adam Dinwoodie Date: Wed, 6 Apr 2016 18:15:03 +0100 Subject: [PATCH 1/2] commit: --amend -m '' silently fails to wipe message `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. Signed-off-by: Adam Dinwoodie Signed-off-by: Junio C Hamano --- t/t7501-commit.sh | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh index 63e04277f9..a7e9322d2f 100755 --- a/t/t7501-commit.sh +++ b/t/t7501-commit.sh @@ -200,6 +200,26 @@ test_expect_success '--amend --edit of empty message' ' test_cmp expect msg ' +test_expect_failure '--amend to set message to empty' ' + echo batá >file && + git add file && + git commit -m "unamended" && + git commit --amend --allow-empty-message -m "" && + git diff-tree -s --format=%s HEAD >msg && + echo "" >expect && + test_cmp expect msg +' + +test_expect_failure '--amend to set empty message needs --allow-empty-message' ' + echo conga >file && + git add file && + git commit -m "unamended" && + test_must_fail git commit --amend -m "" && + git diff-tree -s --format=%s HEAD >msg && + echo "unamended" >expect && + test_cmp expect msg +' + test_expect_success '-m --edit' ' echo amended >expect && git commit --allow-empty -m buffer && From 27014cbc04e369624f6c1754b72ef4eddb911166 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 7 Apr 2016 12:56:26 -0700 Subject: [PATCH 2/2] commit: do not ignore an empty message given by -m '' When f9568530 (builtin-commit: resurrect behavior for multiple -m options, 2007-11-11) converted a "char *message" to "struct strbuf message" to hold the messages given with the "-m" option, it incorrectly changed the checks "did we get a message with the -m option?" to "is message.len 0?". Later, we noticed one breakage from this change and corrected it with 25206778 (commit: don't start editor if empty message is given with -m, 2013-05-25). However, "we got a message with -m, even though an empty one, so we shouldn't be launching an editor" was not the only breakage. * "git commit --amend -m '' --allow-empty", even though it looks strange, is a valid request to amend the commit to have no message at all. Due to the misdetection of the presence of -m on the command line, we ended up keeping the log messsage from the original commit. * "git commit -m "$msg" -F file" should be rejected whether $msg is an empty string or not, but due to the same bug, was not rejected when $msg is empty. * "git -c template=file -m "$msg"" should ignore the template even when $msg is empty, but it didn't and instead used the contents from the template file. Correct these by checking have_option_m, which the earlier 25206778 introduced to fix the same bug. Reported-by: Adam Dinwoodie Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/commit.c | 6 +++--- t/t7501-commit.sh | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index c2ebea4ed3..9a203bde74 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -694,7 +694,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, "-")) { @@ -1162,9 +1162,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; diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh index a7e9322d2f..900f7de05a 100755 --- a/t/t7501-commit.sh +++ b/t/t7501-commit.sh @@ -200,8 +200,8 @@ test_expect_success '--amend --edit of empty message' ' test_cmp expect msg ' -test_expect_failure '--amend to set message to empty' ' - echo batá >file && +test_expect_success '--amend to set message to empty' ' + echo bata >file && git add file && git commit -m "unamended" && git commit --amend --allow-empty-message -m "" && @@ -210,7 +210,7 @@ test_expect_failure '--amend to set message to empty' ' test_cmp expect msg ' -test_expect_failure '--amend to set empty message needs --allow-empty-message' ' +test_expect_success '--amend to set empty message needs --allow-empty-message' ' echo conga >file && git add file && git commit -m "unamended" &&