Skip to content

Commit

Permalink
commit: do not complain of empty messages from -C
Browse files Browse the repository at this point in the history
When we pick another commit's message, we die() immediately
if we find that it's empty and we are not going to run an
editor (i.e., when running "-C" instead of "-c").  However,
this check is redundant and harmful.

It's redundant because we will already notice the empty
message later, after we would have run the editor, and die
there (just as we would for a regular, not "-C" case, where
the user provided an empty message in the editor).

It's harmful for a few reasons:

  1. It does not respect --allow-empty-message. As a result,
     a "git rebase -i" cannot "pick" such a commit. So you
     cannot even go back in time to fix it with a "reword"
     or "edit" instruction.

  2. It does not take into account other ways besides the
     editor to modify the message. For example, "git commit
     -C empty-commit -m foo" could take the author
     information from empty-commit, but add a message to it.
     There's more to do to make that work correctly (and
     right now we explicitly forbid "-C with -m"), but this
     removes one roadblock.

  3. The existing check is not enough to prevent segfaults.
     We try to find the "\n\n" header/body boundary in the
     commit. If it is at the end of the string (i.e., no
     body), _or_ if we cannot find it at all (i.e., a
     truncated commit object), we consider the message
     empty. With "-C", that's OK; we die in either case. But
     with "-c", we continue on, and in the case of a
     truncated commit may end up dereferencing NULL+2.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Apr 28, 2014
1 parent 7bbc4e8 commit 076cbd6
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
5 changes: 2 additions & 3 deletions builtin/commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -650,9 +650,8 @@ 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 (!use_editor && (!buffer || buffer[2] == '\0'))
die(_("commit has empty message"));
strbuf_add(&sb, buffer + 2, strlen(buffer + 2));
if (buffer)
strbuf_add(&sb, buffer + 2, strlen(buffer + 2));
hook_arg1 = "commit";
hook_arg2 = use_message;
} else if (fixup_message) {
Expand Down
11 changes: 10 additions & 1 deletion t/t7500-commit.sh
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,8 @@ test_expect_success 'Commit without message is allowed with --allow-empty-messag
git add foo &&
>empty &&
git commit --allow-empty-message <empty &&
commit_msg_is ""
commit_msg_is "" &&
git tag empty-message-commit
'

test_expect_success 'Commit without message is no-no without --allow-empty-message' '
Expand All @@ -240,6 +241,14 @@ test_expect_success 'Commit a message with --allow-empty-message' '
commit_msg_is "hello there"
'

test_expect_success 'commit -C empty respects --allow-empty-message' '
echo more >>foo &&
git add foo &&
test_must_fail git commit -C empty-message-commit &&
git commit -C empty-message-commit --allow-empty-message &&
commit_msg_is ""
'

commit_for_rebase_autosquash_setup () {
echo "first content line" >>foo &&
git add foo &&
Expand Down

0 comments on commit 076cbd6

Please sign in to comment.