[PATCH v2 2/2] sequencer: run 'prepare-commit-msg' hook

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

 



From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>

Commit 356ee4659b ("sequencer: try to commit without forking 'git
commit'", 2017-11-24) forgot to run the 'prepare-commit-msg' hook when
creating the commit. Fix this by writing the commit message to a
different file and running the hook. Using a different file means that
if the commit is cancelled the original message file is
unchanged. Also move the checks for an empty commit so the order
matches 'git commit'.

Reported-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
Reviewed-by: Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx>
---

Notes:
    Changes since v1:
      - marked run_prepare_commit_msg_hook() as static (Thanks to Ramsey Jones)

 builtin/commit.c                   |  2 --
 sequencer.c                        | 69 +++++++++++++++++++++++++++++++-------
 sequencer.h                        |  1 +
 t/t7505-prepare-commit-msg-hook.sh |  8 ++---
 4 files changed, 61 insertions(+), 19 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 4a64428ca8ed8c3066aec7cfd8ad7b71217af7dd..5dd766af2842dddb80d30cd73b8be8ccb4956eac 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -66,8 +66,6 @@ N_("If you wish to skip this commit, use:\n"
 "Then \"git cherry-pick --continue\" will resume cherry-picking\n"
 "the remaining commits.\n");
 
-static GIT_PATH_FUNC(git_path_commit_editmsg, "COMMIT_EDITMSG")
-
 static const char *use_message_buffer;
 static struct lock_file index_lock; /* real index */
 static struct lock_file false_lock; /* used only for partial commits */
diff --git a/sequencer.c b/sequencer.c
index 63a8ec9a61e7a7bf603ffa494621af79b60e0b76..5bfdc4044233d5f809f9f1fbc55ebe3da477e3f0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -29,6 +29,8 @@
 const char sign_off_header[] = "Signed-off-by: ";
 static const char cherry_picked_prefix[] = "(cherry picked from commit ";
 
+GIT_PATH_FUNC(git_path_commit_editmsg, "COMMIT_EDITMSG")
+
 GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
 
 static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
@@ -891,6 +893,31 @@ void commit_post_rewrite(const struct commit *old_head,
 	run_rewrite_hook(&old_head->object.oid, new_head);
 }
 
+static int run_prepare_commit_msg_hook(struct strbuf *msg, const char *commit)
+{
+	struct argv_array hook_env = ARGV_ARRAY_INIT;
+	int ret;
+	const char *name;
+
+	name = git_path_commit_editmsg();
+	if (write_message(msg->buf, msg->len, name, 0))
+		return -1;
+
+	argv_array_pushf(&hook_env, "GIT_INDEX_FILE=%s", get_index_file());
+	argv_array_push(&hook_env, "GIT_EDITOR=:");
+	if (commit)
+		ret = run_hook_le(hook_env.argv, "prepare-commit-msg", name,
+				  "commit", commit, NULL);
+	else
+		ret = run_hook_le(hook_env.argv, "prepare-commit-msg", name,
+				  "message", NULL);
+	if (ret)
+		ret = error(_("'prepare-commit-msg' hook failed"));
+	argv_array_clear(&hook_env);
+
+	return ret;
+}
+
 static const char implicit_ident_advice_noconfig[] =
 N_("Your name and email address were configured automatically based\n"
 "on your username and hostname. Please check that they are accurate.\n"
@@ -1051,8 +1078,9 @@ static int try_to_commit(struct strbuf *msg, const char *author,
 	struct commit_list *parents = NULL;
 	struct commit_extra_header *extra = NULL;
 	struct strbuf err = STRBUF_INIT;
-	struct strbuf amend_msg = STRBUF_INIT;
+	struct strbuf commit_msg = STRBUF_INIT;
 	char *amend_author = NULL;
+	const char *hook_commit = NULL;
 	enum commit_msg_cleanup_mode cleanup;
 	int res = 0;
 
@@ -1069,8 +1097,9 @@ static int try_to_commit(struct strbuf *msg, const char *author,
 			const char *orig_message = NULL;
 
 			find_commit_subject(message, &orig_message);
-			msg = &amend_msg;
+			msg = &commit_msg;
 			strbuf_addstr(msg, orig_message);
+			hook_commit = "HEAD";
 		}
 		author = amend_author = get_author(message);
 		unuse_commit_buffer(current_head, message);
@@ -1084,16 +1113,6 @@ static int try_to_commit(struct strbuf *msg, const char *author,
 		commit_list_insert(current_head, &parents);
 	}
 
-	cleanup = (flags & CLEANUP_MSG) ? COMMIT_MSG_CLEANUP_ALL :
-					  opts->default_msg_cleanup;
-
-	if (cleanup != COMMIT_MSG_CLEANUP_NONE)
-		strbuf_stripspace(msg, cleanup == COMMIT_MSG_CLEANUP_ALL);
-	if (!opts->allow_empty_message && message_is_empty(msg, cleanup)) {
-		res = 1; /* run 'git commit' to display error message */
-		goto out;
-	}
-
 	if (write_cache_as_tree(tree.hash, 0, NULL)) {
 		res = error(_("git write-tree failed to write a tree"));
 		goto out;
@@ -1106,6 +1125,30 @@ static int try_to_commit(struct strbuf *msg, const char *author,
 		goto out;
 	}
 
+	if (find_hook("prepare-commit-msg")) {
+		res = run_prepare_commit_msg_hook(msg, hook_commit);
+		if (res)
+			goto out;
+		if (strbuf_read_file(&commit_msg, git_path_commit_editmsg(),
+				     2048) < 0) {
+			res = error_errno(_("unable to read commit message "
+					      "from '%s'"),
+					    git_path_commit_editmsg());
+			goto out;
+		}
+		msg = &commit_msg;
+	}
+
+	cleanup = (flags & CLEANUP_MSG) ? COMMIT_MSG_CLEANUP_ALL :
+					  opts->default_msg_cleanup;
+
+	if (cleanup != COMMIT_MSG_CLEANUP_NONE)
+		strbuf_stripspace(msg, cleanup == COMMIT_MSG_CLEANUP_ALL);
+	if (!opts->allow_empty_message && message_is_empty(msg, cleanup)) {
+		res = 1; /* run 'git commit' to display error message */
+		goto out;
+	}
+
 	if (commit_tree_extended(msg->buf, msg->len, tree.hash, parents,
 				 oid->hash, author, opts->gpg_sign, extra)) {
 		res = error(_("failed to write commit object"));
@@ -1124,7 +1167,7 @@ static int try_to_commit(struct strbuf *msg, const char *author,
 out:
 	free_commit_extra_headers(extra);
 	strbuf_release(&err);
-	strbuf_release(&amend_msg);
+	strbuf_release(&commit_msg);
 	free(amend_author);
 
 	return res;
diff --git a/sequencer.h b/sequencer.h
index 24401b07d57b7ca875dea939f465f3e6cf1162a5..e45b178dfc41d723bf186f20674c4515d7c7fa00 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -1,6 +1,7 @@
 #ifndef SEQUENCER_H
 #define SEQUENCER_H
 
+const char *git_path_commit_editmsg(void);
 const char *git_path_seq_dir(void);
 
 #define APPEND_SIGNOFF_DEDUP (1u << 0)
diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
index a3dd3545030c2a29a1ca4502f26b412f92f0375a..512ebdde8acf38db5b3d11558e4f38c6804871c5 100755
--- a/t/t7505-prepare-commit-msg-hook.sh
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -251,10 +251,10 @@ test_rebase () {
 	'
 }
 
-test_rebase failure -i
-test_rebase failure -p
+test_rebase success -i
+test_rebase success -p
 
-test_expect_failure 'with hook (cherry-pick)' '
+test_expect_success 'with hook (cherry-pick)' '
 	test_when_finished "git checkout -f master" &&
 	git checkout -B other b &&
 	git cherry-pick rebase-1 &&
@@ -309,7 +309,7 @@ test_expect_success 'with failing hook (merge)' '
 
 '
 
-test_expect_failure C_LOCALE_OUTPUT 'with failing hook (cherry-pick)' '
+test_expect_success C_LOCALE_OUTPUT 'with failing hook (cherry-pick)' '
 	test_when_finished "git checkout -f master" &&
 	git checkout -B other b &&
 	test_must_fail git cherry-pick rebase-1 2>actual &&
-- 
2.15.1




[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