From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>
Subject: [PATCH] commit: abort before commit-msg if empty message
Date: Fri, 7 Dec 2018 14:48:17 -0800 [thread overview]
Message-ID: <20181207224817.231957-1-jonathantanmy@google.com> (raw)
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@google.com>
---
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
next reply other threads:[~2018-12-07 22:48 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-07 22:48 Jonathan Tan [this message]
2018-12-07 23:07 ` [PATCH] commit: abort before commit-msg if empty message Jonathan Nieder
2018-12-08 5:44 ` Junio C Hamano
2018-12-11 20:14 ` Jonathan Tan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181207224817.231957-1-jonathantanmy@google.com \
--to=jonathantanmy@google.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).