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
