[PATCH] commit: abort before commit-msg if empty message

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



When a user runs "git commit" without specifying a message, an editor
appears with advice:

    Please enter the commit message for your changes. Lines starting
    with '#' will be ignored, and an empty message aborts the commit.

However, if the user supplies an empty message and has a commit-msg hook
which updates the message to be non-empty, the commit proceeds to occur,
despite what the advice states.

Teach commit to also check the emptiness of the commit message before it
invokes the commit-msg hook if an editor is used and if no_verify is not
set (that is, commit-msg is not suppressed). This makes the advice true.

(The implementation in this commit reads the commit message twice even
if there is no commit-msg hook. I think that this is fine, since this is
for interactive use - an alternative would be to plumb information about
the absence of the hook from run_hook_ve() upwards, which seems more
complicated.)

Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
---
This was noticed with the commit-msg hook that comes with Gerrit, which
basically just calls git interpret-trailers to add a Change-Id trailer.
---
 builtin/commit.c           | 43 ++++++++++++++++++++++++++++----------
 t/t7504-commit-msg-hook.sh | 11 ++++++++++
 2 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index c021b119bb..3681a59af8 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -652,6 +652,21 @@ static void adjust_comment_line_char(const struct strbuf *sb)
 	comment_line_char = *p;
 }
 
+static void read_and_clean_commit_message(struct strbuf *sb)
+{
+	if (strbuf_read_file(sb, git_path_commit_editmsg(), 0) < 0) {
+		int saved_errno = errno;
+		rollback_index_files();
+		die(_("could not read commit message: %s"), strerror(saved_errno));
+	}
+
+	if (verbose || /* Truncate the message just before the diff, if any. */
+	    cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)
+		strbuf_setlen(sb, wt_status_locate_end(sb->buf, sb->len));
+	if (cleanup_mode != COMMIT_MSG_CLEANUP_NONE)
+		strbuf_stripspace(sb, cleanup_mode == COMMIT_MSG_CLEANUP_ALL);
+}
+
 static int prepare_to_commit(const char *index_file, const char *prefix,
 			     struct commit *current_head,
 			     struct wt_status *s,
@@ -970,6 +985,22 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		argv_array_clear(&env);
 	}
 
+	if (use_editor && !no_verify) {
+		/*
+		 * Abort the commit if the user supplied an empty commit
+		 * message in the editor. (Because the commit-msg hook is to be
+		 * run, we need to check this now, since that hook may change
+		 * the commit message.)
+		 */
+		read_and_clean_commit_message(&sb);
+		if (message_is_empty(&sb, cleanup_mode) && !allow_empty_message) {
+			rollback_index_files();
+			fprintf(stderr, _("Aborting commit due to empty commit message.\n"));
+			exit(1);
+		}
+		strbuf_release(&sb);
+	}
+
 	if (!no_verify &&
 	    run_commit_hook(use_editor, index_file, "commit-msg", git_path_commit_editmsg(), NULL)) {
 		return 0;
@@ -1608,17 +1639,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 	/* Finally, get the commit message */
 	strbuf_reset(&sb);
-	if (strbuf_read_file(&sb, git_path_commit_editmsg(), 0) < 0) {
-		int saved_errno = errno;
-		rollback_index_files();
-		die(_("could not read commit message: %s"), strerror(saved_errno));
-	}
-
-	if (verbose || /* Truncate the message just before the diff, if any. */
-	    cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)
-		strbuf_setlen(&sb, wt_status_locate_end(sb.buf, sb.len));
-	if (cleanup_mode != COMMIT_MSG_CLEANUP_NONE)
-		strbuf_stripspace(&sb, cleanup_mode == COMMIT_MSG_CLEANUP_ALL);
+	read_and_clean_commit_message(&sb);
 
 	if (message_is_empty(&sb, cleanup_mode) && !allow_empty_message) {
 		rollback_index_files();
diff --git a/t/t7504-commit-msg-hook.sh b/t/t7504-commit-msg-hook.sh
index 31b9c6a2c1..b44d6fc43e 100755
--- a/t/t7504-commit-msg-hook.sh
+++ b/t/t7504-commit-msg-hook.sh
@@ -122,6 +122,17 @@ test_expect_success 'with failing hook (editor)' '
 
 '
 
+test_expect_success 'hook is not run if commit message was empty' '
+	echo "yet more another" >>file &&
+	git add file &&
+	echo >FAKE_MSG &&
+	test_must_fail env GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit 2>err &&
+
+	# Verify that git stopped because it saw an empty message, not because
+	# the hook exited with non-zero error code
+	test_i18ngrep "Aborting commit due to empty commit message" err
+'
+
 test_expect_success '--no-verify with failing hook' '
 
 	echo "stuff" >> file &&
-- 
2.19.0.271.gfe8321ec05.dirty




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux