git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] builtin/merge: honor commit-msg hook for merges

From
Stefan Beller <sbeller@google.com>
Date
Sep 5, 2017, 21:01 UTC
Message-ID
<20170905210116.26343-1-sbeller@google.com>

Similar to 65969d43d1 (merge: honor prepare-commit-msg hook, 2011-02-14) merge should also honor the commit-msg hook; the reason is the same as in that commit: When a merge is stopped due to conflicts or --no-commit, the subsequent commit calls the commit-msg hook. However, it is not called after a clean merge. Fix this inconsistency by invoking the hook after clean merges as well.

This change is motivated by Gerrits commit-msg hook to install a change-id trailer into the commit message. Without such a change id, Gerrit refuses to accept (merge) commits by default, such that the inconsistency of (not) running commit-msg hook between commit and merge leads to confusion and might block people from getting their work done.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/merge.c            |  8 ++++++++
 t/t7504-commit-msg-hook.sh | 45 +++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 7df3fe3927..087efd560d 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -73,6 +73,7 @@ static int show_progress = -1;
 static int default_to_upstream = 1;
 static int signoff;
 static const char *sign_commit;
+static int no_verify;
 
 static struct strategy all_strategy[] = {
 	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },
@@ -236,6 +237,7 @@ static struct option builtin_merge_options[] = {
 	  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
 	OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
 	OPT_BOOL(0, "signoff", &signoff, N_("add Signed-off-by:")),
+	OPT_BOOL(0, "no-verify", &no_verify, N_("bypass pre-commit and commit-msg hooks")),
 	OPT_END()
 };
 
@@ -780,6 +782,12 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 		if (launch_editor(git_path_merge_msg(), NULL, NULL))
 			abort_commit(remoteheads, NULL);
 	}
+
+	if (!no_verify && run_commit_hook(0 < option_edit, get_index_file(),
+					  "commit-msg",
+					  git_path_merge_msg(), NULL))
+		abort_commit(remoteheads, NULL);
+
 	read_merge_msg(&msg);
 	strbuf_stripspace(&msg, 0 < option_edit);
 	if (!msg.len)
diff --git a/t/t7504-commit-msg-hook.sh b/t/t7504-commit-msg-hook.sh
index 88d4cda299..1cd54af3cc 100755
--- a/t/t7504-commit-msg-hook.sh
+++ b/t/t7504-commit-msg-hook.sh
@@ -101,6 +101,10 @@ cat > "$HOOK" <<EOF
 exit 1
 EOF
 
+commit_msg_is () {
+	test "$(git log --pretty=format:%s%b -1)" = "$1"
+}
+
 test_expect_success 'with failing hook' '
 
 	echo "another" >> file &&
@@ -135,6 +139,32 @@ test_expect_success '--no-verify with failing hook (editor)' '
 
 '
 
+test_expect_success 'merge fails with failing hook' '
+
+	test_when_finished "git branch -D newbranch" &&
+	test_when_finished "git checkout -f master" &&
+	git checkout --orphan newbranch &&
+	: >file2 &&
+	git add file2 &&
+	git commit --no-verify file2 -m in-side-branch &&
+	test_must_fail git merge --allow-unrelated-histories master &&
+	commit_msg_is "in-side-branch" # HEAD before merge
+
+'
+
+test_expect_success 'merge bypasses failing hook with --no-verify' '
+
+	test_when_finished "git branch -D newbranch" &&
+	test_when_finished "git checkout -f master" &&
+	git checkout --orphan newbranch &&
+	: >file2 &&
+	git add file2 &&
+	git commit --no-verify file2 -m in-side-branch &&
+	git merge --no-verify --allow-unrelated-histories master &&
+	commit_msg_is "Merge branch '\''master'\'' into newbranch"
+'
+
+
 chmod -x "$HOOK"
 test_expect_success POSIXPERM 'with non-executable hook' '
 
@@ -178,10 +208,6 @@ exit 0
 EOF
 chmod +x "$HOOK"
 
-commit_msg_is () {
-	test "$(git log --pretty=format:%s%b -1)" = "$1"
-}
-
 test_expect_success 'hook edits commit message' '
 
 	echo "additional" >> file &&
@@ -217,7 +243,17 @@ test_expect_success "hook doesn't edit commit message (editor)" '
 	echo "more plus" > FAKE_MSG &&
 	GIT_EDITOR="\"\$FAKE_EDITOR\"" git commit --no-verify &&
 	commit_msg_is "more plus"
+'
 
+test_expect_success 'hook called in git-merge picks up commit message' '
+	test_when_finished "git branch -D newbranch" &&
+	test_when_finished "git checkout -f master" &&
+	git checkout --orphan newbranch &&
+	: >file2 &&
+	git add file2 &&
+	git commit --no-verify file2 -m in-side-branch &&
+	git merge --allow-unrelated-histories master &&
+	commit_msg_is "new message"
 '
 
 # set up fake editor to replace `pick` by `reword`
@@ -237,4 +273,5 @@ test_expect_success 'hook is called for reword during `rebase -i`' '
 
 '
 
+
 test_done
-- 
2.14.0.rc0.3.g6c2e499285
Next: Junio C Hamano
Message 1 of 19 in “builtin/merge: honor commit-msg hook for merges”
  1. builtin/merge: honor commit-msg hook for mergesStefan Beller, Sep 5, 2017
  2. Junio C HamanoSep 5, 2017
  3. parse-options: warn developers on negated optionsStefan Beller, Sep 5, 2017
  4. Junio C HamanoSep 6, 2017
  5. Junio C HamanoSep 6, 2017
  6. Stefan BellerSep 6, 2017
  7. Junio C HamanoSep 6, 2017
  8. [PATCHv2] builtin/merge: honor commit-msg hook for mergesStefan Beller, Sep 5, 2017
  9. Junio C HamanoSep 6, 2017
  10. Stefan BellerSep 6, 2017
  11. Junio C HamanoSep 6, 2017
  12. [PATCHv3] builtin/merge: honor commit-msg hook for mergesStefan Beller, Sep 7, 2017
  13. Junio C HamanoSep 8, 2017
  14. Stefan BellerSep 11, 2017
  15. Kaartic SivaraamSep 16, 2017
  16. Stefan BellerSep 20, 2017
  17. Junio C HamanoSep 21, 2017
  18. Documentation/githooks: mention merge in commit-msg hookStefan Beller, Sep 21, 2017
  19. Junio C HamanoSep 22, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.