pre-merge hook

Now that f8b863598c ("builtin/merge: honor commit-msg hook for merges", 2017-09-07) has landed, merge is getting closer to behaving like commit, which is important because both are used to complete merges (automatic vs. non-automatic).

This series revives an old suggestion of mine to make merge honor pre-commit hook or a separate pre-merge hook. Since pre-commit does not receive any arguments (which the hook could use tell between commit and merge) and in order to keep existing behaviour (as opposed to the other patch) this series introduces a pre-merge hook, with a sample hook that simply calls the pre-commit hook.

commit and merge have very similar code paths. f8b863598c implemented "--no-verify" for merge differently from the existing implementation in commit. 2/4 changes that and could be first in this series, with the remaining 3 squashed into 2 commits or 1. I've been rebasing and using this series for years now, 2/4 is the new-comer which fixed the breakage from f8b863598c that I encountered because of the no-verify implementation.

Michael J Gruber (4):
git-merge: Honor pre-merge hook
merge: do no-verify like commit
merge: --no-verify to bypass pre-merge hook
t7503: add tests for pre-merge-hook

 Documentation/git-merge.txt       |  2 +-
 Documentation/githooks.txt        |  7 +++++
 Documentation/merge-options.txt   |  4 +++
 builtin/merge.c                   | 17 ++++++++--
 t/t7503-pre-commit-hook.sh        | 66 ++++++++++++++++++++++++++++++++++++++-
 templates/hooks--pre-merge.sample | 13 ++++++++
 6 files changed, 104 insertions(+), 5 deletions(-)
 create mode 100755 templates/hooks--pre-merge.sample

2.14.1.909.g0fa57a0913

From: Michael J Gruber <[email protected]>

git-merge does not honor the pre-commit hook when doing automatic merge commits, and for compatibility reasons this is going to stay.

Introduce a pre-merge hook which is called for an automatic merge commit just like pre-commit is called for a non-automatic merge commit (or any other commit).

---
 Documentation/githooks.txt        |  7 +++++++
 builtin/merge.c                   | 11 +++++++++++
 templates/hooks--pre-merge.sample | 13 +++++++++++++
 3 files changed, 31 insertions(+)
 create mode 100755 templates/hooks--pre-merge.sample

index 1bb4f92d4d..85bedd208c 100644
@@ -99,6 +99,13 @@ All the 'git commit' hooks are invoked with the environment
 variable `GIT_EDITOR=:` if the command will not bring up an editor
 to modify the commit message.
 
+pre-merge
+~~~~~~~~~
+
+This hook is invoked by 'git merge' when doing an automatic merge
+commit; it is equivalent to 'pre-commit' for a non-automatic commit
+for a merge.
+
 prepare-commit-msg
 ~~~~~~~~~~~~~~~~~~
 

index ab5ffe85e8..de254d466b 100644
@@ -769,6 +769,17 @@ static void write_merge_heads(struct commit_list *);
 static void prepare_to_commit(struct commit_list *remoteheads)
 {
 	struct strbuf msg = STRBUF_INIT;
+	const char *index_file = get_index_file();
+
+	if (run_commit_hook(0 < option_edit, index_file, "pre-merge", NULL))
+		abort_commit(remoteheads, NULL);
+	/*
+	 * Re-read the index as pre-merge hook could have updated it,
+	 * and write it out as a tree.  We must do this before we invoke
+	 * the editor and after we invoke run_status above.
+	 */
+	discard_cache();
+	read_cache_from(index_file);
 	strbuf_addbuf(&msg, &merge_msg);
 	strbuf_addch(&msg, '\n');
 	if (squash)

new file mode 100755
index 0000000000..a6313e6d5c
@@ -0,0 +1,13 @@
+#!/bin/sh
+#
+# An example hook script to verify what is about to be committed.
+# Called by "git merge" with no arguments.  The hook should
+# exit with non-zero status after issuing an appropriate message if
+# it wants to stop the commit.
+#
+# To enable this hook, rename this file to "pre-merge".
+
+. git-sh-setup
+test -x "$GIT_DIR/hooks/pre-commit" &&
+        exec "$GIT_DIR/hooks/pre-commit"
+:

2.14.1.909.g0fa57a0913

[...]

f8b863598c ("builtin/merge: honor commit-msg hook for merges", 2017-09-07) introduced the no-verify to merge for bypassing the commit-msg hook, though in a different way from the implementation in commit.c.

Change the implementation in merge.c to be the same as in merge.c so that both do the same in the same way.

---
 builtin/merge.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

index de254d466b..7ba094ee87 100644
@@ -73,7 +73,7 @@ static int show_progress = -1;
 static int default_to_upstream = 1;
 static int signoff;
 static const char *sign_commit;
-static int verify_msg = 1;
+static int no_verify;
 
 static struct strategy all_strategy[] = {
 	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },

@@ -237,7 +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, "verify", &verify_msg, N_("verify commit-msg hook")),
+	OPT_BOOL(0, "no-verify", &no_verify, N_("bypass commit-msg hook")),
 	OPT_END()
 };
 

@@ -798,7 +798,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 			abort_commit(remoteheads, NULL);
 	}
 
-	if (verify_msg && run_commit_hook(0 < option_edit, get_index_file(),
+	if (!no_verify && run_commit_hook(0 < option_edit, get_index_file(),
 					  "commit-msg",
 					  git_path_merge_msg(), NULL))
 		abort_commit(remoteheads, NULL);

2.14.1.909.g0fa57a0913

Thanks for spotting.
Junio spotted it as well; I assumed I had reverted it in the last sent out (accepted) patch.

If we want to further the commit message:
This changes the output of 'git merge --help' in a way that it suggests that the hook is on by default.

Thanks,
Stefan

From: Michael J Gruber <[email protected]>

Analogous to commit, introduce a '--no-verify' option which bypasses the pre-merge hook. The shorthand '-n' is taken by the (non-existing) '--no-stat' already.

---
 Documentation/git-merge.txt     | 2 +-
 Documentation/githooks.txt      | 2 +-
 Documentation/merge-options.txt | 4 ++++
 builtin/merge.c                 | 4 ++--
 4 files changed, 8 insertions(+), 4 deletions(-)

index 4df6431c34..5f0e1768e1 100644
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
-	[-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
+	[--no-verify] [-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
 	[--[no-]allow-unrelated-histories]
 	[--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
 'git merge' --abort

index 85bedd208c..969441d7a2 100644
@@ -104,7 +104,7 @@ pre-merge
 
 This hook is invoked by 'git merge' when doing an automatic merge
 commit; it is equivalent to 'pre-commit' for a non-automatic commit
-for a merge.
+for a merge, and can be bypassed with the `--no-verify` option. 
 
 prepare-commit-msg
 ~~~~~~~~~~~~~~~~~~

index 4e32304301..75019d5919 100644
@@ -74,6 +74,10 @@ merge.
 With --no-squash perform the merge and commit the result. This
 option can be used to override --squash.
 
+--no-verify::
+	This option bypasses the pre-merge and commit-msg hooks.
+	See also linkgit:githooks[5].
+
 -s <strategy>::
 --strategy=<strategy>::
 	Use the given merge strategy; can be supplied more than

index 7ba094ee87..c63510c199 100644
@@ -237,7 +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 commit-msg hook")),
+	OPT_BOOL(0, "no-verify", &no_verify, N_("bypass pre-merge and commit-msg hooks")),
 	OPT_END()
 };
 

@@ -771,7 +771,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 	struct strbuf msg = STRBUF_INIT;
 	const char *index_file = get_index_file();
 
-	if (run_commit_hook(0 < option_edit, index_file, "pre-merge", NULL))
+	if (!no_verify && run_commit_hook(0 < option_edit, index_file, "pre-merge", NULL))
 		abort_commit(remoteheads, NULL);
 	/*
 	 * Re-read the index as pre-merge hook could have updated it,

2.14.1.909.g0fa57a0913

It appears that some of the pieces in this patch, namely, D/git-merge.txt and D/merge-options.txt, belong to 2/4, where we are fixing up an earlier addition of --[no-]verify option to the command, to be updated to only add mention of pre-merge hook in this step?

The end result looks good regardless, I would think, though.

Junio C Hamano venit, vidit, dixit 24.09.2017 01:48:

Oh, sorry, rebaser error. I should also rewrite all commits to my current e-mail.

Pending restructuring and attending to the other comments...

From: Michael J Gruber <[email protected]>

Add tests which make sure that the pre-merge-hook is called when present, allows/disallows merge commits depending on its return value and is suppressed by "--no-verify".

---
 t/t7503-pre-commit-hook.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 65 insertions(+), 1 deletion(-)

index 984889b39d..36ae87f7ef 100755
@@ -1,9 +1,22 @@
 #!/bin/sh
 
-test_description='pre-commit hook'
+test_description='pre-commit and pre-merge hooks'
 
 . ./test-lib.sh
 
+test_expect_success 'root commit' '
+
+	echo "root" > file &&
+	git add file &&
+	git commit -m "zeroth" &&
+	git checkout -b side &&
+	echo "foo" > foo &&
+	git add foo &&
+	git commit -m "make it non-ff" &&
+	git checkout master
+
+'
+
 test_expect_success 'with no hook' '
 
 	echo "foo" > file &&

@@ -12,6 +25,14 @@ test_expect_success 'with no hook' '
 
 '
 
+test_expect_success 'with no hook (merge)' '
+
+	git checkout side &&
+	git merge -m "merge master" master &&
+	git checkout master
+
+'
+
 test_expect_success '--no-verify with no hook' '
 
 	echo "bar" > file &&

@@ -20,15 +41,25 @@ test_expect_success '--no-verify with no hook' '
 
 '
 
+test_expect_success '--no-verify with no hook (merge)' '
+
+	git checkout side &&
+	git merge --no-verify -m "merge master" master &&
+	git checkout master
+
+'
+
 # now install hook that always succeeds
 HOOKDIR="$(git rev-parse --git-dir)/hooks"
 HOOK="$HOOKDIR/pre-commit"
+MERGEHOOK="$HOOKDIR/pre-merge"
 mkdir -p "$HOOKDIR"
 cat > "$HOOK" <<EOF
 #!/bin/sh
 exit 0
 EOF
 chmod +x "$HOOK"
+cp -p "$HOOK" "$MERGEHOOK"
 
 test_expect_success 'with succeeding hook' '
 

@@ -38,6 +69,14 @@ test_expect_success 'with succeeding hook' '
 
 '
 
+test_expect_success 'with succeeding hook (merge)' '
+
+	git checkout side &&
+	git merge -m "merge master" master &&
+	git checkout master
+
+'
+
 test_expect_success '--no-verify with succeeding hook' '
 
 	echo "even more" >> file &&

@@ -46,11 +85,20 @@ test_expect_success '--no-verify with succeeding hook' '
 
 '
 
+test_expect_success '--no-verify with succeeding hook (merge)' '
+
+	git checkout side &&
+	git merge --no-verify -m "merge master" master &&
+	git checkout master
+
+'
+
 # now a hook that fails
 cat > "$HOOK" <<EOF
 #!/bin/sh
 exit 1
 EOF
+cp -p "$HOOK" "$MERGEHOOK"
 
 test_expect_success 'with failing hook' '
 

@@ -68,6 +116,22 @@ test_expect_success '--no-verify with failing hook' '
 
 '
 
+test_expect_success 'with failing hook (merge)' '
+
+	git checkout side &&
+	test_must_fail git merge -m "merge master" master &&
+	git checkout master
+
+'
+
+test_expect_success '--no-verify with failing hook (merge)' '
+
+	git checkout side &&
+	git merge --no-verify -m "merge master" master &&
+	git checkout master
+
+'
+
 chmod -x "$HOOK"
 test_expect_success POSIXPERM 'with non-executable hook' '
 

2.14.1.909.g0fa57a0913

The return value of the hook ought to not matter, so we'd rather want to test for 'no side effect by hook on --no-verify' ?

Just a gentle ping to the thread to see if it is still alive.

I think people agree that this is a good thing to do, but it seems that the execution leaves a few loose ends, judging from the review comments that have yet to be answered.

Thanks.

I agree. IIRC the series was ok in design and goal, just minor comments for coding.

Thanks,
Stefan

This seems to have become an abandoned loose end, so I'll drop the topic from my tree for now; revival of the discussion is _not_ unwelcome (aka "dropping without prejudice").

I would like to revive discussion on this series; I have addressed most of the review comments on the original series sent by Michael, with the following exceptions:

* Martin's objection on 1/4 that the sample hook would always exit successfully is (I believe) incorrect. To test this, I ran "/bin/true && exec test 0 == 1" in a /bin/sh subshell, and it correctly had a non-zero exit status.

* I'm not sure that I understand Stefan's objections on 4/4 regarding the hook's exit status. I am also not familiar with "the same (unsolved) issues [as] commit-msg".

I have noted my changes in each commit message in "[js: ...]" sections.

Michael's original series description is below:

Now that f8b863598c ("builtin/merge: honor commit-msg hook for merges", 2017-09-07) has landed, merge is getting closer to behaving like commit, which is important because both are used to complete merges (automatic vs. non-automatic).

This series revives an old suggestion of mine to make merge honor pre-commit hook or a separate pre-merge hook. Since pre-commit does not receive any arguments (which the hook could use tell between commit and merge) and in order to keep existing behaviour (as opposed to the other patch) this series introduces a pre-merge hook, with a sample hook that simply calls the pre-commit hook.

commit and merge have very similar code paths. f8b863598c implemented "--no-verify" for merge differently from the existing implementation in commit. 2/4 changes that and could be first in this series, with the remaining 3 squashed into 2 commits or 1. I've been rebasing and using this series for years now, 2/4 is the new-comer which fixed the breakage from f8b863598c that I encountered because of the no-verify implementation.

Michael J Gruber (4):
git-merge: Honor pre-merge hook
merge: do no-verify like commit
merge: --no-verify to bypass pre-merge hook
t7503: add tests for pre-merge-hook

 Documentation/git-merge.txt                   |  2 +-
 Documentation/githooks.txt                    |  7 ++
 Documentation/merge-options.txt               |  4 ++
 builtin/merge.c                               | 18 ++++-
 ...> t7503-pre-commit-and-pre-merge-hooks.sh} | 66 ++++++++++++++++++-
 templates/hooks--pre-merge.sample             | 13 ++++
 6 files changed, 105 insertions(+), 5 deletions(-)
 rename t/{t7503-pre-commit-hook.sh => t7503-pre-commit-and-pre-merge-hooks.sh} (67%)
 create mode 100755 templates/hooks--pre-merge.sample

2.22.0.657.g960e92d24f-goog

(I'm resending this cover letter with fixed Message-Id so that threading @ public-inbox works properly. Sorry for the noise.)

I would like to revive discussion on this series; I have addressed most of the review comments on the original series sent by Michael, with the following exceptions:

* Martin's objection on 1/4 that the sample hook would always exit successfully is (I believe) incorrect. To test this, I ran "/bin/true && exec test 0 == 1" in a /bin/sh subshell, and it correctly had a non-zero exit status.

* I'm not sure that I understand Stefan's objections on 4/4 regarding the hook's exit status. I am also not familiar with "the same (unsolved) issues [as] commit-msg".

I have noted my changes in each commit message in "[js: ...]" sections.

Michael's original series description is below:

Now that f8b863598c ("builtin/merge: honor commit-msg hook for merges", 2017-09-07) has landed, merge is getting closer to behaving like commit, which is important because both are used to complete merges (automatic vs. non-automatic).

This series revives an old suggestion of mine to make merge honor pre-commit hook or a separate pre-merge hook. Since pre-commit does not receive any arguments (which the hook could use tell between commit and merge) and in order to keep existing behaviour (as opposed to the other patch) this series introduces a pre-merge hook, with a sample hook that simply calls the pre-commit hook.

commit and merge have very similar code paths. f8b863598c implemented "--no-verify" for merge differently from the existing implementation in commit. 2/4 changes that and could be first in this series, with the remaining 3 squashed into 2 commits or 1. I've been rebasing and using this series for years now, 2/4 is the new-comer which fixed the breakage from f8b863598c that I encountered because of the no-verify implementation.

Michael J Gruber (4):
git-merge: Honor pre-merge hook
merge: do no-verify like commit
merge: --no-verify to bypass pre-merge hook
t7503: add tests for pre-merge-hook

 Documentation/git-merge.txt                   |  2 +-
 Documentation/githooks.txt                    |  7 ++
 Documentation/merge-options.txt               |  4 ++
 builtin/merge.c                               | 18 ++++-
 ...> t7503-pre-commit-and-pre-merge-hooks.sh} | 66 ++++++++++++++++++-
 templates/hooks--pre-merge.sample             | 13 ++++
 6 files changed, 105 insertions(+), 5 deletions(-)
 rename t/{t7503-pre-commit-hook.sh => t7503-pre-commit-and-pre-merge-hooks.sh} (67%)
 create mode 100755 templates/hooks--pre-merge.sample

2.22.0.657.g960e92d24f-goog

From: Michael J Gruber <[email protected]>

git-merge does not honor the pre-commit hook when doing automatic merge commits, and for compatibility reasons this is going to stay.

Introduce a pre-merge hook which is called for an automatic merge commit just like pre-commit is called for a non-automatic merge commit (or any other commit).

[js: addressed review comments:
* clarified that hook should write messages to stderr
* only discard the index if the hook is actually present
]

---
 Documentation/githooks.txt        |  7 +++++++
 builtin/merge.c                   | 12 ++++++++++++
 templates/hooks--pre-merge.sample | 13 +++++++++++++
 3 files changed, 32 insertions(+)
 create mode 100755 templates/hooks--pre-merge.sample

index 786e778ab8..dcc6606d44 100644
@@ -103,6 +103,13 @@ The default 'pre-commit' hook, when enabled--and with the
 `hooks.allownonascii` config option unset or set to false--prevents
 the use of non-ASCII filenames.
 
+pre-merge
+~~~~~~~~~
+
+This hook is invoked by 'git merge' when doing an automatic merge
+commit; it is equivalent to 'pre-commit' for a non-automatic commit
+for a merge.
+
 prepare-commit-msg
 ~~~~~~~~~~~~~~~~~~
 

index 6e99aead46..5cd7752191 100644
@@ -816,6 +816,18 @@ static void write_merge_heads(struct commit_list *);
 static void prepare_to_commit(struct commit_list *remoteheads)
 {
 	struct strbuf msg = STRBUF_INIT;
+	const char *index_file = get_index_file();
+
+	if (run_commit_hook(0 < option_edit, index_file, "pre-merge", NULL))
+		abort_commit(remoteheads, NULL);
+	/*
+	 * Re-read the index as pre-merge hook could have updated it,
+	 * and write it out as a tree.  We must do this before we invoke
+	 * the editor and after we invoke run_status above.
+	 */
+	if (find_hook("pre-merge"))
+		discard_cache();
+	read_cache_from(index_file);
 	strbuf_addbuf(&msg, &merge_msg);
 	if (squash)
 		BUG("the control must not reach here under --squash");

new file mode 100755
index 0000000000..f459b3db44
@@ -0,0 +1,13 @@
+#!/bin/sh
+#
+# An example hook script to verify what is about to be committed.
+# Called by "git merge" with no arguments.  The hook should
+# exit with non-zero status after issuing an appropriate message to
+# stderr if it wants to stop the merge commit.
+#
+# To enable this hook, rename this file to "pre-merge".
+
+. git-sh-setup
+test -x "$GIT_DIR/hooks/pre-commit" &&
+        exec "$GIT_DIR/hooks/pre-commit"
+:

2.22.0.657.g960e92d24f-goog

From: Michael J Gruber <[email protected]>

f8b863598c ("builtin/merge: honor commit-msg hook for merges", 2017-09-07) introduced the no-verify flag to merge for bypassing the commit-msg hook, though in a different way from the implementation in commit.c.

Change the implementation in merge.c to be the same as in commit.c so that both do the same in the same way. This also changes the output of "git merge --help" to be more clear that the hook return code is respected by default.

[js: reworded commit message, and moved documentation changes from patch
3/4 to this commit.]

---
 Documentation/git-merge.txt     | 2 +-
 Documentation/merge-options.txt | 4 ++++
 builtin/merge.c                 | 6 +++---
 3 files changed, 8 insertions(+), 4 deletions(-)

index c01cfa6595..f71ed1d0f9 100644
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
-	[-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
+	[--no-verify] [-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
 	[--[no-]allow-unrelated-histories]
 	[--[no-]rerere-autoupdate] [-m <msg>] [-F <file>] [<commit>...]
 'git merge' --abort

index 79a00d2a4a..d6a9f4b96f 100644
@@ -105,6 +105,10 @@ option can be used to override --squash.
 +
 With --squash, --commit is not allowed, and will fail.
 
+--no-verify::
+	This option bypasses the pre-merge and commit-msg hooks.
+	See also linkgit:githooks[5].
+
 -s <strategy>::
 --strategy=<strategy>::
 	Use the given merge strategy; can be supplied more than

index 5cd7752191..5bbef203f3 100644
@@ -81,7 +81,7 @@ static int show_progress = -1;
 static int default_to_upstream = 1;
 static int signoff;
 static const char *sign_commit;
-static int verify_msg = 1;
+static int no_verify;
 
 static struct strategy all_strategy[] = {
 	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },

@@ -287,7 +287,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, "verify", &verify_msg, N_("verify commit-msg hook")),
+	OPT_BOOL(0, "no-verify", &no_verify, N_("bypass commit-msg hook")),
 	OPT_END()
 };
 

@@ -854,7 +854,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 			abort_commit(remoteheads, NULL);
 	}
 
-	if (verify_msg && run_commit_hook(0 < option_edit, get_index_file(),
+	if (!no_verify && run_commit_hook(0 < option_edit, get_index_file(),
 					  "commit-msg",
 					  git_path_merge_msg(the_repository), NULL))
 		abort_commit(remoteheads, NULL);

2.22.0.657.g960e92d24f-goog

From: Michael J Gruber <[email protected]>

Analogous to commit, introduce a '--no-verify' option which bypasses the pre-merge hook. The shorthand '-n' is taken by the (non-existing) '--no-stat' already.

[js: cleaned up trailing whitespace, moved some documentation changes
from this commit to 2/4.]

---
 Documentation/githooks.txt | 2 +-
 builtin/merge.c            | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

index dcc6606d44..6a93478dcf 100644
@@ -108,7 +108,7 @@ pre-merge
 
 This hook is invoked by 'git merge' when doing an automatic merge
 commit; it is equivalent to 'pre-commit' for a non-automatic commit
-for a merge.
+for a merge, and can be bypassed with the `--no-verify` option.
 
 prepare-commit-msg
 ~~~~~~~~~~~~~~~~~~

index 5bbef203f3..5ed3472c84 100644
@@ -287,7 +287,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 commit-msg hook")),
+	OPT_BOOL(0, "no-verify", &no_verify, N_("bypass pre-merge and commit-msg hooks")),
 	OPT_END()
 };
 

@@ -818,7 +818,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 	struct strbuf msg = STRBUF_INIT;
 	const char *index_file = get_index_file();
 
-	if (run_commit_hook(0 < option_edit, index_file, "pre-merge", NULL))
+	if (!no_verify && run_commit_hook(0 < option_edit, index_file, "pre-merge", NULL))
 		abort_commit(remoteheads, NULL);
 	/*
 	 * Re-read the index as pre-merge hook could have updated it,

2.22.0.657.g960e92d24f-goog

I don't understand this "(non-existing)". I realize this message is two years old, maybe lots older still, and I haven't dug. But at least *now*, "-n" seems to behave like "--no-stat", suppressing "--stat", from my simple testing.

Martin

Fixed in V3.

From: Michael J Gruber <[email protected]>

Add tests which make sure that the pre-merge-hook is called when present, allows/disallows merge commits depending on its return value and is suppressed by "--no-verify".

[js: renamed test as suggested in review comments]

---
 ...> t7503-pre-commit-and-pre-merge-hooks.sh} | 66 ++++++++++++++++++-
 1 file changed, 65 insertions(+), 1 deletion(-)
 rename t/{t7503-pre-commit-hook.sh => t7503-pre-commit-and-pre-merge-hooks.sh} (67%)

similarity index 67%
rename from t/t7503-pre-commit-hook.sh
rename to t/t7503-pre-commit-and-pre-merge-hooks.sh
index 984889b39d..36ae87f7ef 100755
@@ -1,9 +1,22 @@
 #!/bin/sh
 
-test_description='pre-commit hook'
+test_description='pre-commit and pre-merge hooks'
 
 . ./test-lib.sh
 
+test_expect_success 'root commit' '
+
+	echo "root" > file &&
+	git add file &&
+	git commit -m "zeroth" &&
+	git checkout -b side &&
+	echo "foo" > foo &&
+	git add foo &&
+	git commit -m "make it non-ff" &&
+	git checkout master
+
+'
+
 test_expect_success 'with no hook' '
 
 	echo "foo" > file &&

@@ -12,6 +25,14 @@ test_expect_success 'with no hook' '
 
 '
 
+test_expect_success 'with no hook (merge)' '
+
+	git checkout side &&
+	git merge -m "merge master" master &&
+	git checkout master
+
+'
+
 test_expect_success '--no-verify with no hook' '
 
 	echo "bar" > file &&

@@ -20,15 +41,25 @@ test_expect_success '--no-verify with no hook' '
 
 '
 
+test_expect_success '--no-verify with no hook (merge)' '
+
+	git checkout side &&
+	git merge --no-verify -m "merge master" master &&
+	git checkout master
+
+'
+
 # now install hook that always succeeds
 HOOKDIR="$(git rev-parse --git-dir)/hooks"
 HOOK="$HOOKDIR/pre-commit"
+MERGEHOOK="$HOOKDIR/pre-merge"
 mkdir -p "$HOOKDIR"
 cat > "$HOOK" <<EOF
 #!/bin/sh
 exit 0
 EOF
 chmod +x "$HOOK"
+cp -p "$HOOK" "$MERGEHOOK"
 
 test_expect_success 'with succeeding hook' '
 

@@ -38,6 +69,14 @@ test_expect_success 'with succeeding hook' '
 
 '
 
+test_expect_success 'with succeeding hook (merge)' '
+
+	git checkout side &&
+	git merge -m "merge master" master &&
+	git checkout master
+
+'
+
 test_expect_success '--no-verify with succeeding hook' '
 
 	echo "even more" >> file &&

@@ -46,11 +85,20 @@ test_expect_success '--no-verify with succeeding hook' '
 
 '
 
+test_expect_success '--no-verify with succeeding hook (merge)' '
+
+	git checkout side &&
+	git merge --no-verify -m "merge master" master &&
+	git checkout master
+
+'
+
 # now a hook that fails
 cat > "$HOOK" <<EOF
 #!/bin/sh
 exit 1
 EOF
+cp -p "$HOOK" "$MERGEHOOK"
 
 test_expect_success 'with failing hook' '
 

@@ -68,6 +116,22 @@ test_expect_success '--no-verify with failing hook' '
 
 '
 
+test_expect_success 'with failing hook (merge)' '
+
+	git checkout side &&
+	test_must_fail git merge -m "merge master" master &&
+	git checkout master
+
+'
+
+test_expect_success '--no-verify with failing hook (merge)' '
+
+	git checkout side &&
+	git merge --no-verify -m "merge master" master &&
+	git checkout master
+
+'
+
 chmod -x "$HOOK"
 test_expect_success POSIXPERM 'with non-executable hook' '
 

2.22.0.657.g960e92d24f-goog

Will squash these as you said in V3. Will also think about whether another test approach would make more sense here.

Thinking a bit more about this, this test uses two identical hooks, runs some commands and verifies that "the" hook was run (or not, with --no-verify). If the implementation started calling the wrong hook (pre-commit / pre-merge) or both hooks, we wouldn't notice.

Please forgive my braindump below, I'm on the run so I'm just throwing this out there:

Perhaps (first do some modernizing of this script, to protect various setup steps, use "write_script", etc, then) make the existing hook a tiny bit pre-commit-specific, e.g., by doing something like "echo pre-commit >>executed-hooks", then at select places check "test_cmp executed-hooks pre-commit" (against "echo pre-commit >pre-commit"), "test_path_is_missing executed-hooks", and so on, coupled with some "test_when_finished 'rm -f executed_hooks'". Then the tests added for this series would use a very similar hook, appending and checking for "pre-merge[-commit]", That should make us fairly certain that we're running precisely the wanted hook, I think.

Martin

That sounds like a reasonable approach, thank you for the suggestions. I will work on this for V3.

I retract my comment on this. It was incorrect, indeed.

Nit: s/H/h/

Nit (maybe just me): this could be patch 1/N, before getting started with the real focus of this series.

Martin

Fixed in V3.

Agreed, I found the ordering strange as well, but wanted to keep it the same as V1 to start with. Fixed in V3.

Argh, I wanted to comment on this in the cover letter, but forgot and just left a bunch of nits.

I wonder if we might ever regret naming this "pre-merge" and not, e.g., "pre-merge-commit". There's the pre-rebase hook which runs before git-rebase even starts actually rebasing, so I could well imagine a "pre-merge" hook which would get called before merging starts.

I'm also pondering whether there should be an "automatic" in there, but "pre-automatic-merge-commit" looks a bit awkward. Anyway, I'm not even sure an "automatic merge commit" is a well-defined thing. I can figure out what it pretty certainly means, but it's not crystal clear. There might be a need for some more discussion in the added docs for what this new hook is for and how it compares to pre-commit. Right now, the proposed docs suggests they're equivalent in a way, but I think that's a bit confusing -- they are certainly not synonyms, and they'd never both be called, for example. They can be used for the same purpose in different scenarios, sure.

I tried coming up with some proposed docs for githooks.txt, but didn't feel I achieved much of value. :-/

Martin

"pre-merge-commit" sounds reasonable to me. I'll wait for a bit before sending out V3 in case anyone else wants to chime in here though.

This series adds a new pre-merge-commit hook, similar in usage to pre-commit. It also improves hook testing in t7503, by verifying that the correct hooks are run or bypassed as expected.

The original series was done by Michael J Gruber <[email protected]>. I have addressed the outstanding review comments, and noted my changes in the commit messages in "[js: ...]" blocks.

Changes since V2:
* Renamed the hook from "pre-merge" to "pre-merge-commit".
* Added a new patch (1/4) to improve t7503 by verifying that the expected hooks are (or are not) run.
* Squashed test changes (from V2's patch 4/4) into patch 3/4. Modified the tests to follow the example set in patch 1/4.
* Reworded commit messages to match with the current state of certain flags, which changed in between V1 and V2 of this series.

Josh Steadmon (1):
t7503: verify proper hook execution

Michael J Gruber (3):
merge: do no-verify like commit
git-merge: honor pre-merge-commit hook
merge: --no-verify to bypass pre-merge-commit hook

 Documentation/git-merge.txt                   |   2 +-
 Documentation/githooks.txt                    |   7 +
 Documentation/merge-options.txt               |   4 +
 builtin/merge.c                               |  18 +-
 ...3-pre-commit-and-pre-merge-commit-hooks.sh | 237 ++++++++++++++++++
 t/t7503-pre-commit-hook.sh                    | 139 ----------
 templates/hooks--pre-merge-commit.sample      |  13 +
 7 files changed, 277 insertions(+), 143 deletions(-)
 create mode 100755 t/t7503-pre-commit-and-pre-merge-commit-hooks.sh
 delete mode 100755 t/t7503-pre-commit-hook.sh
 create mode 100755 templates/hooks--pre-merge-commit.sample

Range-diff against v2:

1:  e8b3fd8a5b < -:  ---------- git-merge: Honor pre-merge hook
-:  ---------- > 1:  f0476b2b1e t7503: verify proper hook execution
2:  36406a85be ! 2:  89ddbf410f merge: do no-verify like commit

@@ Commit message
"git merge --help" to be more clear that the hook return code is respected by default.

- [js: reworded commit message, and moved documentation changes from patch
- 3/4 to this commit.]
+ [js: * reworded commit message
+ * squashed documentation changes from original series' patch 3/4
+ ]

@@ Documentation/git-merge.txt: SYNOPSIS
+ [--no-verify] [-s <strategy>] [-X <strategy-option>] [-S[<keyid>]] [--[no-]allow-unrelated-histories] [--[no-]rerere-autoupdate] [-m <msg>] [-F <file>] [<commit>...]
- 'git merge' --abort
+ 'git merge' (--continue | --abort | --quit)

## Documentation/merge-options.txt ##
@@ Documentation/merge-options.txt: option can be used to override --squash.

3:  2440ad35e4 < -:  ---------- merge: --no-verify to bypass pre-merge hook
4:  69dc3696e7 < -:  ---------- t7503: add tests for pre-merge-hook
-:  ---------- > 3:  61b989ff16 git-merge: honor pre-merge-commit hook
-:  ---------- > 4:  45828c56fc merge: --no-verify to bypass pre-merge-commit hook

2.22.0.770.g0f2c4a37fd-goog

t7503 did not verify that the expected hooks actually ran during testing. Fix that by making the hook scripts write their $0 into a file so that we can compare actual execution vs. expected execution.

While we're at it, do some test style cleanups, such as using write_script() and doing setup inside a test_expect_success block.

---
 t/t7503-pre-commit-hook.sh | 162 +++++++++++++++++++++----------------
 1 file changed, 94 insertions(+), 68 deletions(-)

index 984889b39d..500bdd97c2 100755
@@ -4,124 +4,149 @@ test_description='pre-commit hook'
 
 . ./test-lib.sh
 
-test_expect_success 'with no hook' '
+HOOKDIR="$(git rev-parse --git-dir)/hooks"
+PRECOMMIT="$HOOKDIR/pre-commit"
+
+# Prepare sample scripts that write their $0 to actual_hooks
+test_expect_success 'sample script setup' '
+	mkdir -p "$HOOKDIR" &&
+	write_script "$HOOKDIR/success.sample" <<-\EOF &&
+	echo $0 >>actual_hooks
+	exit 0
+	EOF
+	write_script "$HOOKDIR/fail.sample" <<-\EOF &&
+	echo $0 >>actual_hooks
+	exit 1
+	EOF
+	write_script "$HOOKDIR/require-prefix.sample" <<-\EOF &&
+	echo $0 >>actual_hooks
+	test $GIT_PREFIX = "success/"
+	EOF
+	write_script "$HOOKDIR/check-author.sample" <<-\EOF
+	echo $0 >>actual_hooks
+	test "$GIT_AUTHOR_NAME" = "New Author" &&
+	test "$GIT_AUTHOR_EMAIL" = "[email protected]"
+	EOF
+'
 
-	echo "foo" > file &&
+test_expect_success 'with no hook' '
+	test_when_finished "rm -f expected_hooks actual_hooks" &&
+	touch expected_hooks actual_hooks &&
+	echo "foo" >file &&
 	git add file &&
-	git commit -m "first"
-
+	git commit -m "first" &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success '--no-verify with no hook' '
-
-	echo "bar" > file &&
+	test_when_finished "rm -f expected_hooks actual_hooks" &&
+	touch expected_hooks actual_hooks &&
+	echo "bar" >file &&
 	git add file &&
-	git commit --no-verify -m "bar"
-
+	git commit --no-verify -m "bar" &&
+	test_cmp expected_hooks actual_hooks
 '
 
-# now install hook that always succeeds
-HOOKDIR="$(git rev-parse --git-dir)/hooks"
-HOOK="$HOOKDIR/pre-commit"
-mkdir -p "$HOOKDIR"
-cat > "$HOOK" <<EOF
-#!/bin/sh
-exit 0
-EOF
-chmod +x "$HOOK"
-
 test_expect_success 'with succeeding hook' '
-
-	echo "more" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
+	ln -s "success.sample" "$PRECOMMIT" &&
+	touch actual_hooks &&
+	echo "$PRECOMMIT" >expected_hooks &&
+	echo "more" >>file &&
 	git add file &&
-	git commit -m "more"
-
+	git commit -m "more" &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success '--no-verify with succeeding hook' '
-
-	echo "even more" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
+	ln -s "success.sample" "$PRECOMMIT" &&
+	touch expected_hooks actual_hooks &&
+	echo "even more" >>file &&
 	git add file &&
-	git commit --no-verify -m "even more"
-
+	git commit --no-verify -m "even more" &&
+	test_cmp expected_hooks actual_hooks
 '
 
-# now a hook that fails
-cat > "$HOOK" <<EOF
-#!/bin/sh
-exit 1
-EOF
-
 test_expect_success 'with failing hook' '
-
-	echo "another" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
+	ln -s "fail.sample" "$PRECOMMIT" &&
+	touch actual_hooks &&
+	echo "$PRECOMMIT" >expected_hooks &&
+	echo "another" >>file &&
 	git add file &&
-	test_must_fail git commit -m "another"
-
+	test_must_fail git commit -m "another" &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success '--no-verify with failing hook' '
-
-	echo "stuff" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
+	ln -s "fail.sample" "$PRECOMMIT" &&
+	touch expected_hooks actual_hooks &&
+	echo "stuff" >>file &&
 	git add file &&
-	git commit --no-verify -m "stuff"
-
+	git commit --no-verify -m "stuff" &&
+	test_cmp expected_hooks actual_hooks
 '
 
-chmod -x "$HOOK"
 test_expect_success POSIXPERM 'with non-executable hook' '
-
-	echo "content" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks; chmod +x \"$HOOKDIR/fail.sample\"" &&
+	ln -s "fail.sample" "$PRECOMMIT" &&
+	chmod -x "$HOOKDIR/fail.sample" &&
+	touch expected_hooks actual_hooks &&
+	echo "content" >>file &&
 	git add file &&
-	git commit -m "content"
-
+	git commit -m "content" &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success POSIXPERM '--no-verify with non-executable hook' '
-
-	echo "more content" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks; chmod +x \"$HOOKDIR/fail.sample\"" &&
+	ln -s "fail.sample" "$PRECOMMIT" &&
+	chmod -x "$HOOKDIR/fail.sample" &&
+	touch expected_hooks actual_hooks &&
+	echo "more content" >>file &&
 	git add file &&
-	git commit --no-verify -m "more content"
-
+	git commit --no-verify -m "more content" &&
+	test_cmp expected_hooks actual_hooks
 '
-chmod +x "$HOOK"
-
-# a hook that checks $GIT_PREFIX and succeeds inside the
-# success/ subdirectory only
-cat > "$HOOK" <<EOF
-#!/bin/sh
-test \$GIT_PREFIX = success/
-EOF
 
 test_expect_success 'with hook requiring GIT_PREFIX' '
-
-	echo "more content" >> file &&
+	test_when_finished "rm -rf \"$PRECOMMIT\" expected_hooks actual_hooks success" &&
+	ln -s "require-prefix.sample" "$PRECOMMIT" &&
+	echo "$PRECOMMIT" >expected_hooks &&
+	echo "more content" >>file &&
 	git add file &&
 	mkdir success &&
 	(
 		cd success &&
 		git commit -m "hook requires GIT_PREFIX = success/"
 	) &&
-	rmdir success
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success 'with failing hook requiring GIT_PREFIX' '
-
-	echo "more content" >> file &&
+	test_when_finished "rm -rf \"$PRECOMMIT\" expected_hooks actual_hooks fail" &&
+	ln -s "require-prefix.sample" "$PRECOMMIT" &&
+	echo "$PRECOMMIT" >expected_hooks &&
+	echo "more content" >>file &&
 	git add file &&
 	mkdir fail &&
 	(
 		cd fail &&
 		test_must_fail git commit -m "hook must fail"
 	) &&
-	rmdir fail &&
-	git checkout -- file
+	git checkout -- file &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success 'check the author in hook' '
-	write_script "$HOOK" <<-\EOF &&
-	test "$GIT_AUTHOR_NAME" = "New Author" &&
-	test "$GIT_AUTHOR_EMAIL" = "[email protected]"
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
+	ln -s "check-author.sample" "$PRECOMMIT" &&
+	cat >expected_hooks <<-EOF &&
+	$PRECOMMIT
+	$PRECOMMIT
+	$PRECOMMIT
 	EOF
 	test_must_fail git commit --allow-empty -m "by a.u.thor" &&
 	(

@@ -133,7 +158,8 @@ test_expect_success 'check the author in hook' '
 	) &&
 	git commit --author="New Author <[email protected]>" \
 		--allow-empty -m "by new.author via command line" &&
-	git show -s
+	git show -s &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_done

2.22.0.770.g0f2c4a37fd-goog

It could be argued that this test is perfectly fine as long as there is just one hook to choose from, and as long as we're fine with failing tests being a bit non-obvious to debug. Since we'll soon have two hooks to choose from in this test, we really want to make sure that we're executing the right one, not just "at least one". (No need to reroll, this is just me thinking out loud.)

Martin

f8b863598c ("builtin/merge: honor commit-msg hook for merges", 2017-09-07) introduced the no-verify flag to merge for bypassing the commit-msg hook, though in a different way from the implementation in commit.c.

Change the implementation in merge.c to be the same as in commit.c so that both do the same in the same way. This also changes the output of "git merge --help" to be more clear that the hook return code is respected by default.

[js: * reworded commit message
* squashed documentation changes from original series' patch 3/4
]

---
 Documentation/git-merge.txt     | 2 +-
 Documentation/merge-options.txt | 4 ++++
 builtin/merge.c                 | 6 +++---
 3 files changed, 8 insertions(+), 4 deletions(-)

index 01fd52dc70..092529c619 100644
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
-	[-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
+	[--no-verify] [-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
 	[--[no-]allow-unrelated-histories]
 	[--[no-]rerere-autoupdate] [-m <msg>] [-F <file>] [<commit>...]
 'git merge' (--continue | --abort | --quit)

index 79a00d2a4a..d6a9f4b96f 100644
@@ -105,6 +105,10 @@ option can be used to override --squash.
 +
 With --squash, --commit is not allowed, and will fail.
 
+--no-verify::
+	This option bypasses the pre-merge and commit-msg hooks.
+	See also linkgit:githooks[5].
+
 -s <strategy>::
 --strategy=<strategy>::
 	Use the given merge strategy; can be supplied more than

index e2ccbc44e2..4425a7a12e 100644
@@ -81,7 +81,7 @@ static int show_progress = -1;
 static int default_to_upstream = 1;
 static int signoff;
 static const char *sign_commit;
-static int verify_msg = 1;
+static int no_verify;
 
 static struct strategy all_strategy[] = {
 	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },

@@ -287,7 +287,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, "verify", &verify_msg, N_("verify commit-msg hook")),
+	OPT_BOOL(0, "no-verify", &no_verify, N_("bypass commit-msg hook")),
 	OPT_END()
 };
 

@@ -842,7 +842,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 			abort_commit(remoteheads, NULL);
 	}
 
-	if (verify_msg && run_commit_hook(0 < option_edit, get_index_file(),
+	if (!no_verify && run_commit_hook(0 < option_edit, get_index_file(),
 					  "commit-msg",
 					  git_path_merge_msg(the_repository), NULL))
 		abort_commit(remoteheads, NULL);

2.22.0.770.g0f2c4a37fd-goog

git-merge does not honor the pre-commit hook when doing automatic merge commits, and for compatibility reasons this is going to stay.

Introduce a pre-merge-commit hook which is called for an automatic merge commit just like pre-commit is called for a non-automatic merge commit (or any other commit).

[js: * renamed hook from "pre-merge" to "pre-merge-commit"
* only discard the index if the hook is actually present
* clarified that hook should write messages to stderr
* squashed test changes from the original series' patch 4/4
* modified tests to follow new pattern from this series' patch 1/4
* reworded commit message
]

---
 Documentation/githooks.txt                    |  7 +++
 builtin/merge.c                               | 12 +++++
 ...-pre-commit-and-pre-merge-commit-hooks.sh} | 45 ++++++++++++++++++-
 templates/hooks--pre-merge-commit.sample      | 13 ++++++
 4 files changed, 76 insertions(+), 1 deletion(-)
 rename t/{t7503-pre-commit-hook.sh => t7503-pre-commit-and-pre-merge-commit-hooks.sh} (78%)
 create mode 100755 templates/hooks--pre-merge-commit.sample

index 82cd573776..7c4c994858 100644
@@ -103,6 +103,13 @@ The default 'pre-commit' hook, when enabled--and with the
 `hooks.allownonascii` config option unset or set to false--prevents
 the use of non-ASCII filenames.
 
+pre-merge-commit
+~~~~~~~~~~~~~~~~
+
+This hook is invoked by 'git merge' when doing an automatic merge
+commit; it is equivalent to 'pre-commit' for a non-automatic commit
+for a merge.
+
 prepare-commit-msg
 ~~~~~~~~~~~~~~~~~~
 

index 4425a7a12e..bf0ae68c40 100644
@@ -816,6 +816,18 @@ static void write_merge_heads(struct commit_list *);
 static void prepare_to_commit(struct commit_list *remoteheads)
 {
 	struct strbuf msg = STRBUF_INIT;
+	const char *index_file = get_index_file();
+
+	if (run_commit_hook(0 < option_edit, index_file, "pre-merge-commit", NULL))
+		abort_commit(remoteheads, NULL);
+	/*
+	 * Re-read the index as pre-merge-commit hook could have updated it,
+	 * and write it out as a tree.  We must do this before we invoke
+	 * the editor and after we invoke run_status above.
+	 */
+	if (find_hook("pre-merge-commit"))
+		discard_cache();
+	read_cache_from(index_file);
 	strbuf_addbuf(&msg, &merge_msg);
 	if (squash)
 		BUG("the control must not reach here under --squash");

new file mode 100755
index 0000000000..399eab1924
@@ -0,0 +1,13 @@
+#!/bin/sh
+#
+# An example hook script to verify what is about to be committed.
+# Called by "git merge" with no arguments.  The hook should
+# exit with non-zero status after issuing an appropriate message to
+# stderr if it wants to stop the merge commit.
+#
+# To enable this hook, rename this file to "pre-merge-commit".
+
+. git-sh-setup
+test -x "$GIT_DIR/hooks/pre-commit" &&
+        exec "$GIT_DIR/hooks/pre-commit"
+:

2.22.0.770.g0f2c4a37fd-goog

This looks good to me at first glance. I may reword a bit in V4, but probably not much.

Thanks for the catch, will fix in V4.

Analogous to commit, introduce a '--no-verify' option which bypasses the pre-merge-commit hook. The shorthand '-n' is taken by '--no-stat' already.

[js: * reworded commit message to reflect current state of --no-stat flag
and new hook name
* fixed flag documentation to reflect new hook name
* cleaned up trailing whitespace
* squashed test changes from the original series' patch 4/4
* modified tests to follow pattern from this series' patch 1/4
]

---
 Documentation/githooks.txt                    |  2 +-
 builtin/merge.c                               |  4 +--
 ...3-pre-commit-and-pre-merge-commit-hooks.sh | 29 +++++++++++++++++++
 3 files changed, 32 insertions(+), 3 deletions(-)

index 7c4c994858..a934553509 100644
@@ -108,7 +108,7 @@ pre-merge-commit
 
 This hook is invoked by 'git merge' when doing an automatic merge
 commit; it is equivalent to 'pre-commit' for a non-automatic commit
-for a merge.
+for a merge, and can be bypassed with the `--no-verify` option.
 
 prepare-commit-msg
 ~~~~~~~~~~~~~~~~~~

index bf0ae68c40..c9746e37b8 100644
@@ -287,7 +287,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 commit-msg hook")),
+	OPT_BOOL(0, "no-verify", &no_verify, N_("bypass pre-merge-commit and commit-msg hooks")),
 	OPT_END()
 };
 

@@ -818,7 +818,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 	struct strbuf msg = STRBUF_INIT;
 	const char *index_file = get_index_file();
 
-	if (run_commit_hook(0 < option_edit, index_file, "pre-merge-commit", NULL))
+	if (!no_verify && run_commit_hook(0 < option_edit, index_file, "pre-merge-commit", NULL))
 		abort_commit(remoteheads, NULL);
 	/*
 	 * Re-read the index as pre-merge-commit hook could have updated it,

index 040dfa0175..86a375ab3e 100755
@@ -68,6 +68,15 @@ test_expect_success '--no-verify with no hook' '
 	test_cmp expected_hooks actual_hooks
 '
 
+test_expect_success '--no-verify with no hook (merge)' '
+	test_when_finished "rm -f expected_hooks actual_hooks" &&
+	touch expected_hooks actual_hooks &&
+	git checkout side &&
+	git merge --no-verify -m "merge master" master &&
+	git checkout master &&
+	test_cmp expected_hooks actual_hooks
+'
+
 test_expect_success 'with succeeding hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
 	ln -s "success.sample" "$PRECOMMIT" &&

@@ -100,6 +109,16 @@ test_expect_success '--no-verify with succeeding hook' '
 	test_cmp expected_hooks actual_hooks
 '
 
+test_expect_success '--no-verify with succeeding hook (merge)' '
+	test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" &&
+	ln -s "success.sample" "$PREMERGE" &&
+	touch expected_hooks actual_hooks &&
+	git checkout side &&
+	git merge --no-verify -m "merge master" master &&
+	git checkout master &&
+	test_cmp expected_hooks actual_hooks
+'
+
 test_expect_success 'with failing hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
 	ln -s "fail.sample" "$PRECOMMIT" &&

@@ -132,6 +151,16 @@ test_expect_success 'with failing hook (merge)' '
 	test_cmp expected_hooks actual_hooks
 '
 
+test_expect_success '--no-verify with failing hook (merge)' '
+	test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" &&
+	ln -s "fail.sample" "$PREMERGE" &&
+	touch expected_hooks actual_hooks &&
+	git checkout side &&
+	git merge --no-verify -m "merge master" master &&
+	git checkout master &&
+	test_cmp expected_hooks actual_hooks
+'
+
 test_expect_success POSIXPERM 'with non-executable hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks; chmod +x \"$HOOKDIR/fail.sample\"" &&
 	ln -s "fail.sample" "$PRECOMMIT" &&

2.22.0.770.g0f2c4a37fd-goog

[Dropped cc-list the first time around. Apologies to those who receive this twice...]

I really like those test improvements. Now it should be harder to mess up a future refactoring and run the wrong hook. These hooks are "very related" so I think this is important.

I've messed with the test a bit and offer these potential improvements for your consideration. I was lazy and just built this on top of your series -- if you agree to some or all of these, you'll probably need to squash them into a few individual patches.

The first four are perhaps more or less a matter of opinion, although I do think that patch 2/5 is based on an opinion shared by others. ;-) Patch 5/5 or something like it seems pretty important to me to make sure that of these two "similar"/"related" hooks with some "backwards-compatibility-and-code-copying-history" around them, we'd better pick the right one when they're both available.

Feel free to pick and squash as you see fit. (I don't think it makes sense to have these go in as-are. They really are meant for squashing.)

Martin

Martin Ågren (5):
t7503: use "&&" in "test_when_finished" rather than ";"
t7503: avoid touch when mtime doesn't matter
t7503: simplify file-juggling
t7503: don't create "actual_hooks" for later appending
t7503: test failing merge with both hooks available

 ...3-pre-commit-and-pre-merge-commit-hooks.sh | 84 +++++++++++--------
 1 file changed, 50 insertions(+), 34 deletions(-)

2.23.0.rc0.30.g51cf315870

---
 t/t7503-pre-commit-and-pre-merge-commit-hooks.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 86a375ab3e..5cc6c98375 100755
@@ -162,7 +162,7 @@ test_expect_success '--no-verify with failing hook (merge)' '
 '
 
 test_expect_success POSIXPERM 'with non-executable hook' '
-	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks; chmod +x \"$HOOKDIR/fail.sample\"" &&
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks && chmod +x \"$HOOKDIR/fail.sample\"" &&
 	ln -s "fail.sample" "$PRECOMMIT" &&
 	chmod -x "$HOOKDIR/fail.sample" &&
 	touch expected_hooks actual_hooks &&

@@ -173,7 +173,7 @@ test_expect_success POSIXPERM 'with non-executable hook' '
 '
 
 test_expect_success POSIXPERM '--no-verify with non-executable hook' '
-	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks; chmod +x \"$HOOKDIR/fail.sample\"" &&
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks && chmod +x \"$HOOKDIR/fail.sample\"" &&
 	ln -s "fail.sample" "$PRECOMMIT" &&
 	chmod -x "$HOOKDIR/fail.sample" &&
 	touch expected_hooks actual_hooks &&

2.23.0.rc0.30.g51cf315870

AFAIU, we avoid using touch unless we really do want to update the mtime. Here, we just want to create empty files, so write >foo instead.

---
 ...3-pre-commit-and-pre-merge-commit-hooks.sh | 38 ++++++++++++-------
 1 file changed, 24 insertions(+), 14 deletions(-)

index 5cc6c98375..a6f1240aa2 100755
@@ -43,7 +43,8 @@ test_expect_success 'root commit' '
 
 test_expect_success 'with no hook' '
 	test_when_finished "rm -f expected_hooks actual_hooks" &&
-	touch expected_hooks actual_hooks &&
+	>expected_hooks &&
+	>actual_hooks &&
 	echo "foo" >file &&
 	git add file &&
 	git commit -m "first" &&

@@ -52,7 +53,8 @@ test_expect_success 'with no hook' '
 
 test_expect_success 'with no hook (merge)' '
 	test_when_finished "rm -f expected_hooks actual_hooks" &&
-	touch expected_hooks actual_hooks &&
+	>expected_hooks &&
+	>actual_hooks &&
 	git checkout side &&
 	git merge -m "merge master" master &&
 	git checkout master &&

@@ -61,7 +63,8 @@ test_expect_success 'with no hook (merge)' '
 
 test_expect_success '--no-verify with no hook' '
 	test_when_finished "rm -f expected_hooks actual_hooks" &&
-	touch expected_hooks actual_hooks &&
+	>expected_hooks &&
+	>actual_hooks &&
 	echo "bar" >file &&
 	git add file &&
 	git commit --no-verify -m "bar" &&

@@ -70,7 +73,8 @@ test_expect_success '--no-verify with no hook' '
 
 test_expect_success '--no-verify with no hook (merge)' '
 	test_when_finished "rm -f expected_hooks actual_hooks" &&
-	touch expected_hooks actual_hooks &&
+	>expected_hooks &&
+	>actual_hooks &&
 	git checkout side &&
 	git merge --no-verify -m "merge master" master &&
 	git checkout master &&

@@ -80,7 +84,7 @@ test_expect_success '--no-verify with no hook (merge)' '
 test_expect_success 'with succeeding hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
 	ln -s "success.sample" "$PRECOMMIT" &&
-	touch actual_hooks &&
+	>actual_hooks &&
 	echo "$PRECOMMIT" >expected_hooks &&
 	echo "more" >>file &&
 	git add file &&

@@ -91,7 +95,7 @@ test_expect_success 'with succeeding hook' '
 test_expect_success 'with succeeding hook (merge)' '
 	test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" &&
 	ln -s "success.sample" "$PREMERGE" &&
-	touch actual_hooks &&
+	>actual_hooks &&
 	echo "$PREMERGE" >expected_hooks &&
 	git checkout side &&
 	git merge -m "merge master" master &&

@@ -102,7 +106,8 @@ test_expect_success 'with succeeding hook (merge)' '
 test_expect_success '--no-verify with succeeding hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
 	ln -s "success.sample" "$PRECOMMIT" &&
-	touch expected_hooks actual_hooks &&
+	>expected_hooks &&
+	>actual_hooks &&
 	echo "even more" >>file &&
 	git add file &&
 	git commit --no-verify -m "even more" &&

@@ -112,7 +117,8 @@ test_expect_success '--no-verify with succeeding hook' '
 test_expect_success '--no-verify with succeeding hook (merge)' '
 	test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" &&
 	ln -s "success.sample" "$PREMERGE" &&
-	touch expected_hooks actual_hooks &&
+	>expected_hooks &&
+	>actual_hooks &&
 	git checkout side &&
 	git merge --no-verify -m "merge master" master &&
 	git checkout master &&

@@ -122,7 +128,7 @@ test_expect_success '--no-verify with succeeding hook (merge)' '
 test_expect_success 'with failing hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
 	ln -s "fail.sample" "$PRECOMMIT" &&
-	touch actual_hooks &&
+	>actual_hooks &&
 	echo "$PRECOMMIT" >expected_hooks &&
 	echo "another" >>file &&
 	git add file &&

@@ -133,7 +139,8 @@ test_expect_success 'with failing hook' '
 test_expect_success '--no-verify with failing hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
 	ln -s "fail.sample" "$PRECOMMIT" &&
-	touch expected_hooks actual_hooks &&
+	>expected_hooks &&
+	>actual_hooks &&
 	echo "stuff" >>file &&
 	git add file &&
 	git commit --no-verify -m "stuff" &&

@@ -143,7 +150,7 @@ test_expect_success '--no-verify with failing hook' '
 test_expect_success 'with failing hook (merge)' '
 	test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" &&
 	ln -s "fail.sample" "$PREMERGE" &&
-	touch actual_hooks &&
+	>actual_hooks &&
 	echo "$PREMERGE" >expected_hooks &&
 	git checkout side &&
 	test_must_fail git merge -m "merge master" master &&

@@ -154,7 +161,8 @@ test_expect_success 'with failing hook (merge)' '
 test_expect_success '--no-verify with failing hook (merge)' '
 	test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" &&
 	ln -s "fail.sample" "$PREMERGE" &&
-	touch expected_hooks actual_hooks &&
+	>expected_hooks &&
+	>actual_hooks &&
 	git checkout side &&
 	git merge --no-verify -m "merge master" master &&
 	git checkout master &&

@@ -165,7 +173,8 @@ test_expect_success POSIXPERM 'with non-executable hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks && chmod +x \"$HOOKDIR/fail.sample\"" &&
 	ln -s "fail.sample" "$PRECOMMIT" &&
 	chmod -x "$HOOKDIR/fail.sample" &&
-	touch expected_hooks actual_hooks &&
+	>expected_hooks &&
+	>actual_hooks &&
 	echo "content" >>file &&
 	git add file &&
 	git commit -m "content" &&

@@ -176,7 +185,8 @@ test_expect_success POSIXPERM '--no-verify with non-executable hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks && chmod +x \"$HOOKDIR/fail.sample\"" &&
 	ln -s "fail.sample" "$PRECOMMIT" &&
 	chmod -x "$HOOKDIR/fail.sample" &&
-	touch expected_hooks actual_hooks &&
+	>expected_hooks &&
+	>actual_hooks &&
 	echo "more content" >>file &&
 	git add file &&
 	git commit --no-verify -m "more content" &&

2.23.0.rc0.30.g51cf315870

Rather than creating an empty "expected_hooks" file and another empty file "actual_hooks" (which we want to not be appended to), just verify that "actual_hooks" isn't created. Do still clean up the "actual_hooks" file, so that a failing test won't start affecting later tests too.

---
 ...3-pre-commit-and-pre-merge-commit-hooks.sh | 60 +++++++------------
 1 file changed, 20 insertions(+), 40 deletions(-)

index a6f1240aa2..477207cb5c 100755
@@ -42,43 +42,35 @@ test_expect_success 'root commit' '
 '
 
 test_expect_success 'with no hook' '
-	test_when_finished "rm -f expected_hooks actual_hooks" &&
-	>expected_hooks &&
-	>actual_hooks &&
+	test_when_finished "rm -f actual_hooks" &&
 	echo "foo" >file &&
 	git add file &&
 	git commit -m "first" &&
-	test_cmp expected_hooks actual_hooks
+	test_path_is_missing actual_hooks
 '
 
 test_expect_success 'with no hook (merge)' '
-	test_when_finished "rm -f expected_hooks actual_hooks" &&
-	>expected_hooks &&
-	>actual_hooks &&
+	test_when_finished "rm -f actual_hooks" &&
 	git checkout side &&
 	git merge -m "merge master" master &&
 	git checkout master &&
-	test_cmp expected_hooks actual_hooks
+	test_path_is_missing actual_hooks
 '
 
 test_expect_success '--no-verify with no hook' '
-	test_when_finished "rm -f expected_hooks actual_hooks" &&
-	>expected_hooks &&
-	>actual_hooks &&
+	test_when_finished "rm -f actual_hooks" &&
 	echo "bar" >file &&
 	git add file &&
 	git commit --no-verify -m "bar" &&
-	test_cmp expected_hooks actual_hooks
+	test_path_is_missing actual_hooks
 '
 
 test_expect_success '--no-verify with no hook (merge)' '
-	test_when_finished "rm -f expected_hooks actual_hooks" &&
-	>expected_hooks &&
-	>actual_hooks &&
+	test_when_finished "rm -f actual_hooks" &&
 	git checkout side &&
 	git merge --no-verify -m "merge master" master &&
 	git checkout master &&
-	test_cmp expected_hooks actual_hooks
+	test_path_is_missing actual_hooks
 '
 
 test_expect_success 'with succeeding hook' '

@@ -104,25 +96,21 @@ test_expect_success 'with succeeding hook (merge)' '
 '
 
 test_expect_success '--no-verify with succeeding hook' '
-	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
+	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
 	ln -s "success.sample" "$PRECOMMIT" &&
-	>expected_hooks &&
-	>actual_hooks &&
 	echo "even more" >>file &&
 	git add file &&
 	git commit --no-verify -m "even more" &&
-	test_cmp expected_hooks actual_hooks
+	test_path_is_missing actual_hooks
 '
 
 test_expect_success '--no-verify with succeeding hook (merge)' '
-	test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" &&
+	test_when_finished "rm -f \"$PREMERGE\" actual_hooks" &&
 	ln -s "success.sample" "$PREMERGE" &&
-	>expected_hooks &&
-	>actual_hooks &&
 	git checkout side &&
 	git merge --no-verify -m "merge master" master &&
 	git checkout master &&
-	test_cmp expected_hooks actual_hooks
+	test_path_is_missing actual_hooks
 '
 
 test_expect_success 'with failing hook' '

@@ -137,14 +125,12 @@ test_expect_success 'with failing hook' '
 '
 
 test_expect_success '--no-verify with failing hook' '
-	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
+	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
 	ln -s "fail.sample" "$PRECOMMIT" &&
-	>expected_hooks &&
-	>actual_hooks &&
 	echo "stuff" >>file &&
 	git add file &&
 	git commit --no-verify -m "stuff" &&
-	test_cmp expected_hooks actual_hooks
+	test_path_is_missing actual_hooks
 '
 
 test_expect_success 'with failing hook (merge)' '

@@ -159,38 +145,32 @@ test_expect_success 'with failing hook (merge)' '
 '
 
 test_expect_success '--no-verify with failing hook (merge)' '
-	test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" &&
+	test_when_finished "rm -f \"$PREMERGE\" actual_hooks" &&
 	ln -s "fail.sample" "$PREMERGE" &&
-	>expected_hooks &&
-	>actual_hooks &&
 	git checkout side &&
 	git merge --no-verify -m "merge master" master &&
 	git checkout master &&
-	test_cmp expected_hooks actual_hooks
+	test_path_is_missing actual_hooks
 '
 
 test_expect_success POSIXPERM 'with non-executable hook' '
-	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks && chmod +x \"$HOOKDIR/fail.sample\"" &&
+	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks && chmod +x \"$HOOKDIR/fail.sample\"" &&
 	ln -s "fail.sample" "$PRECOMMIT" &&
 	chmod -x "$HOOKDIR/fail.sample" &&
-	>expected_hooks &&
-	>actual_hooks &&
 	echo "content" >>file &&
 	git add file &&
 	git commit -m "content" &&
-	test_cmp expected_hooks actual_hooks
+	test_path_is_missing actual_hooks
 '
 
 test_expect_success POSIXPERM '--no-verify with non-executable hook' '
-	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks && chmod +x \"$HOOKDIR/fail.sample\"" &&
+	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks && chmod +x \"$HOOKDIR/fail.sample\"" &&
 	ln -s "fail.sample" "$PRECOMMIT" &&
 	chmod -x "$HOOKDIR/fail.sample" &&
-	>expected_hooks &&
-	>actual_hooks &&
 	echo "more content" >>file &&
 	git add file &&
 	git commit --no-verify -m "more content" &&
-	test_cmp expected_hooks actual_hooks
+	test_path_is_missing actual_hooks
 '
 
 test_expect_success 'with hook requiring GIT_PREFIX' '

2.23.0.rc0.30.g51cf315870

If we fail to call the hook, we won't append to "actual_hooks". "test_cmp" is able to handle a missing file just fine, so these "pre-creations" are mostly confusing. Let's drop them.

--
This "pre-creation" does protect against the file already existing, i.e., it could be seen as a "clearing". But since we use "test_when_finished" in this script... Also, these were "touch" before another of my suggestions, so these really were "pre-creating" these files if needed.

 t/t7503-pre-commit-and-pre-merge-commit-hooks.sh | 4 ----
 1 file changed, 4 deletions(-)

index 477207cb5c..f0c73fd58d 100755
@@ -76,7 +76,6 @@ test_expect_success '--no-verify with no hook (merge)' '
 test_expect_success 'with succeeding hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
 	ln -s "success.sample" "$PRECOMMIT" &&
-	>actual_hooks &&
 	echo "$PRECOMMIT" >expected_hooks &&
 	echo "more" >>file &&
 	git add file &&

@@ -87,7 +86,6 @@ test_expect_success 'with succeeding hook' '
 test_expect_success 'with succeeding hook (merge)' '
 	test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" &&
 	ln -s "success.sample" "$PREMERGE" &&
-	>actual_hooks &&
 	echo "$PREMERGE" >expected_hooks &&
 	git checkout side &&
 	git merge -m "merge master" master &&

@@ -116,7 +114,6 @@ test_expect_success '--no-verify with succeeding hook (merge)' '
 test_expect_success 'with failing hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
 	ln -s "fail.sample" "$PRECOMMIT" &&
-	>actual_hooks &&
 	echo "$PRECOMMIT" >expected_hooks &&
 	echo "another" >>file &&
 	git add file &&

@@ -136,7 +133,6 @@ test_expect_success '--no-verify with failing hook' '
 test_expect_success 'with failing hook (merge)' '
 	test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" &&
 	ln -s "fail.sample" "$PREMERGE" &&
-	>actual_hooks &&
 	echo "$PREMERGE" >expected_hooks &&
 	git checkout side &&
 	test_must_fail git merge -m "merge master" master &&

2.23.0.rc0.30.g51cf315870

With a failing automatic merge, we want to make sure that we *don't* call the pre-merge-commit hook and that we (eventually) *do* call the pre-commit hook.

---
 ...3-pre-commit-and-pre-merge-commit-hooks.sh | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

index f0c73fd58d..4bf359c097 100755
@@ -41,6 +41,18 @@ test_expect_success 'root commit' '
 	git checkout master
 '
 
+test_expect_success 'setup conflicting branches' '
+	test_when_finished "git checkout master" &&
+	git checkout -b conflicting-a master &&
+	echo a >conflicting &&
+	git add conflicting &&
+	git commit -m conflicting-a &&
+	git checkout -b conflicting-b master &&
+	echo b >conflicting &&
+	git add conflicting &&
+	git commit -m conflicting-b
+'
+
 test_expect_success 'with no hook' '
 	test_when_finished "rm -f actual_hooks" &&
 	echo "foo" >file &&

@@ -93,6 +105,24 @@ test_expect_success 'with succeeding hook (merge)' '
 	test_cmp expected_hooks actual_hooks
 '
 
+test_expect_success 'automatic merge fails; both hooks are available' '
+	test_when_finished "rm -f \"$PREMERGE\" \"$PRECOMMIT\"" &&
+	test_when_finished "rm -f expected_hooks actual_hooks" &&
+	test_when_finished "git checkout master" &&
+	ln -s "success.sample" "$PREMERGE" &&
+	ln -s "success.sample" "$PRECOMMIT" &&
+
+	git checkout conflicting-a &&
+	test_must_fail git merge -m "merge conflicting-b" conflicting-b &&
+	test_path_is_missing actual_hooks &&
+
+	echo "$PRECOMMIT" >expected_hooks &&
+	echo a+b >conflicting &&
+	git add conflicting &&
+	git commit -m "resolve conflict" &&
+	test_cmp expected_hooks actual_hooks
+'
+
 test_expect_success '--no-verify with succeeding hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
 	ln -s "success.sample" "$PRECOMMIT" &&

2.23.0.rc0.30.g51cf315870

These all look good to me, thank you for the suggestions! I'll include them in V4.

This series adds a new pre-merge-commit hook, similar in usage to pre-commit. It also improves hook testing in t7503, by verifying that the correct hooks are run or bypassed as expected.

The original series was done by Michael J Gruber <[email protected]>. I have addressed the outstanding review comments, and noted my changes in the commit messages in "[js: ...]" blocks.

Changes since V3:
* Applied several test style fixes suggested by Martin (thanks!).
* Clarified the documentation for pre-merge-commit hook.
* Fixed a few cases where testing that the merge hook did not run might erroneously succeed if we don't have any merge to actually perform.
* Simplified test cleanup by adding a new non-executable sample hook script.
* Added test cases for non-executable pre-merge-commit hooks.

Changes since V2:
* Renamed the hook from "pre-merge" to "pre-merge-commit".
* Added a new patch (1/4) to improve t7503 by verifying that the expected hooks are (or are not) run.
* Squashed test changes (from V2's patch 4/4) into patch 3/4. Modified the tests to follow the example set in patch 1/4.
* Reworded commit messages to match with the current state of certain flags, which changed in between V1 and V2 of this series.

Josh Steadmon (1):
t7503: verify proper hook execution

Michael J Gruber (3):
merge: do no-verify like commit
git-merge: honor pre-merge-commit hook
merge: --no-verify to bypass pre-merge-commit hook

 Documentation/git-merge.txt                   |   2 +-
 Documentation/githooks.txt                    |  22 ++
 Documentation/merge-options.txt               |   4 +
 builtin/merge.c                               |  18 +-
 ...3-pre-commit-and-pre-merge-commit-hooks.sh | 281 ++++++++++++++++++
 t/t7503-pre-commit-hook.sh                    | 139 ---------
 templates/hooks--pre-merge-commit.sample      |  13 +
 7 files changed, 336 insertions(+), 143 deletions(-)
 create mode 100755 t/t7503-pre-commit-and-pre-merge-commit-hooks.sh
 delete mode 100755 t/t7503-pre-commit-hook.sh
 create mode 100755 templates/hooks--pre-merge-commit.sample

Range-diff against v3:
1: f0476b2b1e ! 1: 5085729095 t7503: verify proper hook execution

    @@ Commit message
         write_script() and doing setup inside a test_expect_success block.
     
    +    Improved-by: Martin Ågren <[email protected]>
    +    Signed-off-by: Martin Ågren <[email protected]>
     
      ## t/t7503-pre-commit-hook.sh ##
     @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'
    @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'
     +	echo $0 >>actual_hooks
     +	exit 1
     +	EOF
    ++	write_script "$HOOKDIR/non-exec.sample" <<-\EOF &&
    ++	echo $0 >>actual_hooks
    ++	exit 1
    ++	EOF
    ++	chmod -x "$HOOKDIR/non-exec.sample" &&
     +	write_script "$HOOKDIR/require-prefix.sample" <<-\EOF &&
     +	echo $0 >>actual_hooks
     +	test $GIT_PREFIX = "success/"
    @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'
      
     -	echo "foo" > file &&
     +test_expect_success 'with no hook' '
    -+	test_when_finished "rm -f expected_hooks actual_hooks" &&
    -+	touch expected_hooks actual_hooks &&
    ++	test_when_finished "rm -f actual_hooks" &&
     +	echo "foo" >file &&
      	git add file &&
     -	git commit -m "first"
     -
     +	git commit -m "first" &&
    -+	test_cmp expected_hooks actual_hooks
    ++	test_path_is_missing actual_hooks
      '
      
      test_expect_success '--no-verify with no hook' '
     -
     -	echo "bar" > file &&
    -+	test_when_finished "rm -f expected_hooks actual_hooks" &&
    -+	touch expected_hooks actual_hooks &&
    ++	test_when_finished "rm -f actual_hooks" &&
     +	echo "bar" >file &&
      	git add file &&
     -	git commit --no-verify -m "bar"
     -
     +	git commit --no-verify -m "bar" &&
    -+	test_cmp expected_hooks actual_hooks
    ++	test_path_is_missing actual_hooks
      '
      
     -# now install hook that always succeeds
    @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'
     -	echo "more" >> file &&
     +	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
     +	ln -s "success.sample" "$PRECOMMIT" &&
    -+	touch actual_hooks &&
     +	echo "$PRECOMMIT" >expected_hooks &&
     +	echo "more" >>file &&
      	git add file &&
    @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'
      test_expect_success '--no-verify with succeeding hook' '
     -
     -	echo "even more" >> file &&
    -+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
    ++	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
     +	ln -s "success.sample" "$PRECOMMIT" &&
    -+	touch expected_hooks actual_hooks &&
     +	echo "even more" >>file &&
      	git add file &&
     -	git commit --no-verify -m "even more"
     -
     +	git commit --no-verify -m "even more" &&
    -+	test_cmp expected_hooks actual_hooks
    ++	test_path_is_missing actual_hooks
      '
      
     -# now a hook that fails
    @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'
     -	echo "another" >> file &&
     +	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
     +	ln -s "fail.sample" "$PRECOMMIT" &&
    -+	touch actual_hooks &&
     +	echo "$PRECOMMIT" >expected_hooks &&
     +	echo "another" >>file &&
      	git add file &&
    @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'
      test_expect_success '--no-verify with failing hook' '
     -
     -	echo "stuff" >> file &&
    -+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
    ++	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
     +	ln -s "fail.sample" "$PRECOMMIT" &&
    -+	touch expected_hooks actual_hooks &&
     +	echo "stuff" >>file &&
      	git add file &&
     -	git commit --no-verify -m "stuff"
     -
     +	git commit --no-verify -m "stuff" &&
    -+	test_cmp expected_hooks actual_hooks
    ++	test_path_is_missing actual_hooks
      '
      
     -chmod -x "$HOOK"
      test_expect_success POSIXPERM 'with non-executable hook' '
     -
     -	echo "content" >> file &&
    -+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks; chmod +x \"$HOOKDIR/fail.sample\"" &&
    -+	ln -s "fail.sample" "$PRECOMMIT" &&
    -+	chmod -x "$HOOKDIR/fail.sample" &&
    -+	touch expected_hooks actual_hooks &&
    ++	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
    ++	ln -s "non-exec.sample" "$PRECOMMIT" &&
     +	echo "content" >>file &&
      	git add file &&
     -	git commit -m "content"
     -
     +	git commit -m "content" &&
    -+	test_cmp expected_hooks actual_hooks
    ++	test_path_is_missing actual_hooks
      '
      
      test_expect_success POSIXPERM '--no-verify with non-executable hook' '
     -
     -	echo "more content" >> file &&
    -+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks; chmod +x \"$HOOKDIR/fail.sample\"" &&
    -+	ln -s "fail.sample" "$PRECOMMIT" &&
    -+	chmod -x "$HOOKDIR/fail.sample" &&
    -+	touch expected_hooks actual_hooks &&
    ++	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
    ++	ln -s "non-exec.sample" "$PRECOMMIT" &&
     +	echo "more content" >>file &&
      	git add file &&
     -	git commit --no-verify -m "more content"
     -
     +	git commit --no-verify -m "more content" &&
    -+	test_cmp expected_hooks actual_hooks
    ++	test_path_is_missing actual_hooks
      '
     -chmod +x "$HOOK"
     -
2:  89ddbf410f = 2:  3b701a5c41 merge: do no-verify like commit
3:  61b989ff16 ! 3:  9210421fbb git-merge: honor pre-merge-commit hook
    @@ Commit message
     
         [js: * renamed hook from "pre-merge" to "pre-merge-commit"
              * only discard the index if the hook is actually present
    +         * expanded githooks documentation entry
              * clarified that hook should write messages to stderr
              * squashed test changes from the original series' patch 4/4
              * modified tests to follow new pattern from this series' patch 1/4
    +         * added a test case for non-executable merge hooks
    +         * added a test case for failed merges
    +         * when testing that the merge hook did not run, make sure we
    +           actually have a merge to perform (by resetting the "side" branch
    +           to its original state).
              * reworded commit message
         ]
     
    +    Improved-by: Martin Ågren <[email protected]>
         Signed-off-by: Michael J Gruber <[email protected]>
    +    Signed-off-by: Martin Ågren <[email protected]>
     
      ## Documentation/githooks.txt ##
    @@ Documentation/githooks.txt: The default 'pre-commit' hook, when enabled--and wit
     +pre-merge-commit
     +~~~~~~~~~~~~~~~~
     +
    -+This hook is invoked by 'git merge' when doing an automatic merge
    -+commit; it is equivalent to 'pre-commit' for a non-automatic commit
    -+for a merge.
    ++This hook is invoked by linkgit:git-merge[1]. It takes no parameters, and is
    ++invoked after the merge has been carried out successfully and before
    ++obtaining the proposed commit log message to
    ++make a commit.  Exiting with a non-zero status from this script
    ++causes the `git merge` command to abort before creating a commit.
    ++
    ++The default 'pre-merge-commit' hook, when enabled, runs the
    ++'pre-commit' hook, if the latter is enabled.
    ++
    ++This hook is invoked with the environment variable
    ++`GIT_EDITOR=:` if the command will not bring up an editor
    ++to modify the commit message.
    ++
    ++If the merge cannot be carried out automatically, the conflicts
    ++need to be resolved and the result committed separately (see
    ++linkgit:git-merge[1]). At that point, this hook will not be executed,
    ++but the 'pre-commit' hook will, if it is enabled.
     +
      prepare-commit-msg
      ~~~~~~~~~~~~~~~~~~
    @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'sample sc
      '
      
     +test_expect_success 'root commit' '
    -+	echo "root" > file &&
    ++	echo "root" >file &&
     +	git add file &&
     +	git commit -m "zeroth" &&
     +	git checkout -b side &&
    -+	echo "foo" > foo &&
    ++	echo "foo" >foo &&
     +	git add foo &&
     +	git commit -m "make it non-ff" &&
    ++	git branch side-orig side &&
     +	git checkout master
     +'
    ++
    ++test_expect_success 'setup conflicting branches' '
    ++	test_when_finished "git checkout master" &&
    ++	git checkout -b conflicting-a master &&
    ++	echo a >conflicting &&
    ++	git add conflicting &&
    ++	git commit -m conflicting-a &&
    ++	git checkout -b conflicting-b master &&
    ++	echo b >conflicting &&
    ++	git add conflicting &&
    ++	git commit -m conflicting-b
    ++'
     +
      test_expect_success 'with no hook' '
    - 	test_when_finished "rm -f expected_hooks actual_hooks" &&
    - 	touch expected_hooks actual_hooks &&
    + 	test_when_finished "rm -f actual_hooks" &&
    + 	echo "foo" >file &&
     @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with no hook' '
    - 	test_cmp expected_hooks actual_hooks
    + 	test_path_is_missing actual_hooks
      '
      
     +test_expect_success 'with no hook (merge)' '
    -+	test_when_finished "rm -f expected_hooks actual_hooks" &&
    -+	touch expected_hooks actual_hooks &&
    ++	test_when_finished "rm -f actual_hooks" &&
    ++	git branch -f side side-orig &&
     +	git checkout side &&
     +	git merge -m "merge master" master &&
     +	git checkout master &&
    -+	test_cmp expected_hooks actual_hooks
    ++	test_path_is_missing actual_hooks
     +'
     +
      test_expect_success '--no-verify with no hook' '
    - 	test_when_finished "rm -f expected_hooks actual_hooks" &&
    - 	touch expected_hooks actual_hooks &&
    + 	test_when_finished "rm -f actual_hooks" &&
    + 	echo "bar" >file &&
     @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with succeeding hook' '
      	test_cmp expected_hooks actual_hooks
      '
    @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with succ
     +test_expect_success 'with succeeding hook (merge)' '
     +	test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" &&
     +	ln -s "success.sample" "$PREMERGE" &&
    -+	touch actual_hooks &&
     +	echo "$PREMERGE" >expected_hooks &&
     +	git checkout side &&
     +	git merge -m "merge master" master &&
     +	git checkout master &&
     +	test_cmp expected_hooks actual_hooks
     +'
    ++
    ++test_expect_success 'automatic merge fails; both hooks are available' '
    ++	test_when_finished "rm -f \"$PREMERGE\" \"$PRECOMMIT\"" &&
    ++	test_when_finished "rm -f expected_hooks actual_hooks" &&
    ++	test_when_finished "git checkout master" &&
    ++	ln -s "success.sample" "$PREMERGE" &&
    ++	ln -s "success.sample" "$PRECOMMIT" &&
    ++
    ++	git checkout conflicting-a &&
    ++	test_must_fail git merge -m "merge conflicting-b" conflicting-b &&
    ++	test_path_is_missing actual_hooks &&
    ++
    ++	echo "$PRECOMMIT" >expected_hooks &&
    ++	echo a+b >conflicting &&
    ++	git add conflicting &&
    ++	git commit -m "resolve conflict" &&
    ++	test_cmp expected_hooks actual_hooks
    ++'
     +
      test_expect_success '--no-verify with succeeding hook' '
    - 	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
    + 	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
      	ln -s "success.sample" "$PRECOMMIT" &&
     @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-verify with failing hook' '
    - 	test_cmp expected_hooks actual_hooks
    + 	test_path_is_missing actual_hooks
      '
      
     +test_expect_success 'with failing hook (merge)' '
     +	test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" &&
     +	ln -s "fail.sample" "$PREMERGE" &&
    -+	touch actual_hooks &&
     +	echo "$PREMERGE" >expected_hooks &&
     +	git checkout side &&
     +	test_must_fail git merge -m "merge master" master &&
    @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-veri
     +'
     +
      test_expect_success POSIXPERM 'with non-executable hook' '
    - 	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks; chmod +x \"$HOOKDIR/fail.sample\"" &&
    - 	ln -s "fail.sample" "$PRECOMMIT" &&
    + 	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
    + 	ln -s "non-exec.sample" "$PRECOMMIT" &&
    +@@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success POSIXPERM '--no-verify with non-executable hook' '
    + 	test_path_is_missing actual_hooks
    + '
    + 
    ++test_expect_success POSIXPERM 'with non-executable hook (merge)' '
    ++	test_when_finished "rm -f \"$PREMERGE\" actual_hooks" &&
    ++	ln -s "non-exec.sample" "$PREMERGE" &&
    ++	git branch -f side side-orig &&
    ++	git checkout side &&
    ++	git merge -m "merge master" master &&
    ++	git checkout master &&
    ++	test_path_is_missing actual_hooks
    ++'
    ++
    + test_expect_success 'with hook requiring GIT_PREFIX' '
    + 	test_when_finished "rm -rf \"$PRECOMMIT\" expected_hooks actual_hooks success" &&
    + 	ln -s "require-prefix.sample" "$PRECOMMIT" &&
     
      ## templates/hooks--pre-merge-commit.sample (new) ##
     @@

4: 45828c56fc ! 4: 96c54883d3 merge: --no-verify to bypass pre-merge-commit hook
@@ Commit message
* cleaned up trailing whitespace
* squashed test changes from the original series' patch 4/4
* modified tests to follow pattern from this series' patch 1/4
+ * added a test case for --no-verify with non-executable hook
+ * when testing that the merge hook did not run, make sure we
+ actually have a merge to perform (by resetting the "side" branch
+ to its original state).
+
]

+ Improved-by: Martin Ågren <[email protected]>

+ Signed-off-by: Martin Ågren <[email protected]>

## Documentation/githooks.txt ##
-@@ Documentation/githooks.txt: pre-merge-commit
-
- This hook is invoked by 'git merge' when doing an automatic merge
- commit; it is equivalent to 'pre-commit' for a non-automatic commit --for a merge.
-+for a merge, and can be bypassed with the --no-verify option. +@@ Documentation/githooks.txt: the use of non-ASCII filenames.
+ pre-merge-commit
+ ~~~~~~~~~~~~~~~~

- prepare-commit-msg
- ~~~~~~~~~~~~~~~~~~
+-This hook is invoked by linkgit:git-merge[1]. It takes no parameters, and is ++This hook is invoked by linkgit:git-merge[1], and can be bypassed ++with the --no-verify option. It takes no parameters, and is
+ invoked after the merge has been carried out successfully and before
+ obtaining the proposed commit log message to
+ make a commit. Exiting with a non-zero status from this script

## builtin/merge.c ##
@@ builtin/merge.c: static struct option builtin_merge_options[] = {
@@ builtin/merge.c: static void prepare_to_commit(struct commit_list *remoteheads)

## t/t7503-pre-commit-and-pre-merge-commit-hooks.sh ##
@@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-verify with no hook' '
- test_cmp expected_hooks actual_hooks
+ test_path_is_missing actual_hooks
'

+test_expect_success '--no-verify with no hook (merge)' '
-+ test_when_finished "rm -f expected_hooks actual_hooks" &&
-+ touch expected_hooks actual_hooks &&
++ test_when_finished "rm -f actual_hooks" &&
++ git branch -f side side-orig &&
+ git checkout side &&
+ git merge --no-verify -m "merge master" master &&
+ git checkout master &&
-+ test_cmp expected_hooks actual_hooks
++ test_path_is_missing actual_hooks
+'
+
test_expect_success 'with succeeding hook' '
test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
ln -s "success.sample" "$PRECOMMIT" &&
@@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-verify with succeeding hook' '
- test_cmp expected_hooks actual_hooks
+ test_path_is_missing actual_hooks
'

+test_expect_success '--no-verify with succeeding hook (merge)' '
-+ test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" &&
++ test_when_finished "rm -f \"$PREMERGE\" actual_hooks" &&
+ ln -s "success.sample" "$PREMERGE" &&
-+ touch expected_hooks actual_hooks &&
++ git branch -f side side-orig &&
+ git checkout side &&
+ git merge --no-verify -m "merge master" master &&
+ git checkout master &&
-+ test_cmp expected_hooks actual_hooks
++ test_path_is_missing actual_hooks
+'
+
test_expect_success 'with failing hook' '
@@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with fail
'

+test_expect_success '--no-verify with failing hook (merge)' '
-+ test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" &&
++ test_when_finished "rm -f \"$PREMERGE\" actual_hooks" &&
+ ln -s "fail.sample" "$PREMERGE" &&
-+ touch expected_hooks actual_hooks &&
++ git branch -f side side-orig &&
+ git checkout side &&
+ git merge --no-verify -m "merge master" master &&
+ git checkout master &&
-+ test_cmp expected_hooks actual_hooks
++ test_path_is_missing actual_hooks
+'
+
test_expect_success POSIXPERM 'with non-executable hook' '
- test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks; chmod +x \"$HOOKDIR/fail.sample\"" &&
- ln -s "fail.sample" "$PRECOMMIT" &&
+ test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
+ ln -s "non-exec.sample" "$PRECOMMIT" &&
+@@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success POSIXPERM 'with non-executable hook (merge)' '
+ test_path_is_missing actual_hooks
+ '
+
++test_expect_success POSIXPERM '--no-verify with non-executable hook (merge)' '
++ test_when_finished "rm -f \"$PREMERGE\" actual_hooks" &&
++ ln -s "non-exec.sample" "$PREMERGE" &&
++ git branch -f side side-orig &&
++ git checkout side &&
++ git merge --no-verify -m "merge master" master &&
++ git checkout master &&
++ test_path_is_missing actual_hooks
++'
++
+ test_expect_success 'with hook requiring GIT_PREFIX' '
+ test_when_finished "rm -rf \"$PRECOMMIT\" expected_hooks actual_hooks success" &&
+ ln -s "require-prefix.sample" "$PRECOMMIT" &&

2.22.0.770.g0f2c4a37fd-goog

t7503 did not verify that the expected hooks actually ran during testing. Fix that by making the hook scripts write their $0 into a file so that we can compare actual execution vs. expected execution.

While we're at it, do some test style cleanups, such as using write_script() and doing setup inside a test_expect_success block.

---
 t/t7503-pre-commit-hook.sh | 157 +++++++++++++++++++++----------------
 1 file changed, 89 insertions(+), 68 deletions(-)

index 984889b39d..a71ec31222 100755
@@ -4,124 +4,144 @@ test_description='pre-commit hook'
 
 . ./test-lib.sh
 
-test_expect_success 'with no hook' '
+HOOKDIR="$(git rev-parse --git-dir)/hooks"
+PRECOMMIT="$HOOKDIR/pre-commit"
+
+# Prepare sample scripts that write their $0 to actual_hooks
+test_expect_success 'sample script setup' '
+	mkdir -p "$HOOKDIR" &&
+	write_script "$HOOKDIR/success.sample" <<-\EOF &&
+	echo $0 >>actual_hooks
+	exit 0
+	EOF
+	write_script "$HOOKDIR/fail.sample" <<-\EOF &&
+	echo $0 >>actual_hooks
+	exit 1
+	EOF
+	write_script "$HOOKDIR/non-exec.sample" <<-\EOF &&
+	echo $0 >>actual_hooks
+	exit 1
+	EOF
+	chmod -x "$HOOKDIR/non-exec.sample" &&
+	write_script "$HOOKDIR/require-prefix.sample" <<-\EOF &&
+	echo $0 >>actual_hooks
+	test $GIT_PREFIX = "success/"
+	EOF
+	write_script "$HOOKDIR/check-author.sample" <<-\EOF
+	echo $0 >>actual_hooks
+	test "$GIT_AUTHOR_NAME" = "New Author" &&
+	test "$GIT_AUTHOR_EMAIL" = "[email protected]"
+	EOF
+'
 
-	echo "foo" > file &&
+test_expect_success 'with no hook' '
+	test_when_finished "rm -f actual_hooks" &&
+	echo "foo" >file &&
 	git add file &&
-	git commit -m "first"
-
+	git commit -m "first" &&
+	test_path_is_missing actual_hooks
 '
 
 test_expect_success '--no-verify with no hook' '
-
-	echo "bar" > file &&
+	test_when_finished "rm -f actual_hooks" &&
+	echo "bar" >file &&
 	git add file &&
-	git commit --no-verify -m "bar"
-
+	git commit --no-verify -m "bar" &&
+	test_path_is_missing actual_hooks
 '
 
-# now install hook that always succeeds
-HOOKDIR="$(git rev-parse --git-dir)/hooks"
-HOOK="$HOOKDIR/pre-commit"
-mkdir -p "$HOOKDIR"
-cat > "$HOOK" <<EOF
-#!/bin/sh
-exit 0
-EOF
-chmod +x "$HOOK"
-
 test_expect_success 'with succeeding hook' '
-
-	echo "more" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
+	ln -s "success.sample" "$PRECOMMIT" &&
+	echo "$PRECOMMIT" >expected_hooks &&
+	echo "more" >>file &&
 	git add file &&
-	git commit -m "more"
-
+	git commit -m "more" &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success '--no-verify with succeeding hook' '
-
-	echo "even more" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
+	ln -s "success.sample" "$PRECOMMIT" &&
+	echo "even more" >>file &&
 	git add file &&
-	git commit --no-verify -m "even more"
-
+	git commit --no-verify -m "even more" &&
+	test_path_is_missing actual_hooks
 '
 
-# now a hook that fails
-cat > "$HOOK" <<EOF
-#!/bin/sh
-exit 1
-EOF
-
 test_expect_success 'with failing hook' '
-
-	echo "another" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
+	ln -s "fail.sample" "$PRECOMMIT" &&
+	echo "$PRECOMMIT" >expected_hooks &&
+	echo "another" >>file &&
 	git add file &&
-	test_must_fail git commit -m "another"
-
+	test_must_fail git commit -m "another" &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success '--no-verify with failing hook' '
-
-	echo "stuff" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
+	ln -s "fail.sample" "$PRECOMMIT" &&
+	echo "stuff" >>file &&
 	git add file &&
-	git commit --no-verify -m "stuff"
-
+	git commit --no-verify -m "stuff" &&
+	test_path_is_missing actual_hooks
 '
 
-chmod -x "$HOOK"
 test_expect_success POSIXPERM 'with non-executable hook' '
-
-	echo "content" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
+	ln -s "non-exec.sample" "$PRECOMMIT" &&
+	echo "content" >>file &&
 	git add file &&
-	git commit -m "content"
-
+	git commit -m "content" &&
+	test_path_is_missing actual_hooks
 '
 
 test_expect_success POSIXPERM '--no-verify with non-executable hook' '
-
-	echo "more content" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
+	ln -s "non-exec.sample" "$PRECOMMIT" &&
+	echo "more content" >>file &&
 	git add file &&
-	git commit --no-verify -m "more content"
-
+	git commit --no-verify -m "more content" &&
+	test_path_is_missing actual_hooks
 '
-chmod +x "$HOOK"
-
-# a hook that checks $GIT_PREFIX and succeeds inside the
-# success/ subdirectory only
-cat > "$HOOK" <<EOF
-#!/bin/sh
-test \$GIT_PREFIX = success/
-EOF
 
 test_expect_success 'with hook requiring GIT_PREFIX' '
-
-	echo "more content" >> file &&
+	test_when_finished "rm -rf \"$PRECOMMIT\" expected_hooks actual_hooks success" &&
+	ln -s "require-prefix.sample" "$PRECOMMIT" &&
+	echo "$PRECOMMIT" >expected_hooks &&
+	echo "more content" >>file &&
 	git add file &&
 	mkdir success &&
 	(
 		cd success &&
 		git commit -m "hook requires GIT_PREFIX = success/"
 	) &&
-	rmdir success
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success 'with failing hook requiring GIT_PREFIX' '
-
-	echo "more content" >> file &&
+	test_when_finished "rm -rf \"$PRECOMMIT\" expected_hooks actual_hooks fail" &&
+	ln -s "require-prefix.sample" "$PRECOMMIT" &&
+	echo "$PRECOMMIT" >expected_hooks &&
+	echo "more content" >>file &&
 	git add file &&
 	mkdir fail &&
 	(
 		cd fail &&
 		test_must_fail git commit -m "hook must fail"
 	) &&
-	rmdir fail &&
-	git checkout -- file
+	git checkout -- file &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success 'check the author in hook' '
-	write_script "$HOOK" <<-\EOF &&
-	test "$GIT_AUTHOR_NAME" = "New Author" &&
-	test "$GIT_AUTHOR_EMAIL" = "[email protected]"
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
+	ln -s "check-author.sample" "$PRECOMMIT" &&
+	cat >expected_hooks <<-EOF &&
+	$PRECOMMIT
+	$PRECOMMIT
+	$PRECOMMIT
 	EOF
 	test_must_fail git commit --allow-empty -m "by a.u.thor" &&
 	(

@@ -133,7 +153,8 @@ test_expect_success 'check the author in hook' '
 	) &&
 	git commit --author="New Author <[email protected]>" \
 		--allow-empty -m "by new.author via command line" &&
-	git show -s
+	git show -s &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_done

2.22.0.770.g0f2c4a37fd-goog

The result mostly looks more pleasant compared to the original, modulo a few nits.

Yeah, "cp" works here. Fixed in V5.

From: Michael J Gruber <[email protected]>

f8b863598c ("builtin/merge: honor commit-msg hook for merges", 2017-09-07) introduced the no-verify flag to merge for bypassing the commit-msg hook, though in a different way from the implementation in commit.c.

Change the implementation in merge.c to be the same as in commit.c so that both do the same in the same way. This also changes the output of "git merge --help" to be more clear that the hook return code is respected by default.

[js: * reworded commit message
* squashed documentation changes from original series' patch 3/4
]

---
 Documentation/git-merge.txt     | 2 +-
 Documentation/merge-options.txt | 4 ++++
 builtin/merge.c                 | 6 +++---
 3 files changed, 8 insertions(+), 4 deletions(-)

index 01fd52dc70..092529c619 100644
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
-	[-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
+	[--no-verify] [-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
 	[--[no-]allow-unrelated-histories]
 	[--[no-]rerere-autoupdate] [-m <msg>] [-F <file>] [<commit>...]
 'git merge' (--continue | --abort | --quit)

index 79a00d2a4a..d6a9f4b96f 100644
@@ -105,6 +105,10 @@ option can be used to override --squash.
 +
 With --squash, --commit is not allowed, and will fail.
 
+--no-verify::
+	This option bypasses the pre-merge and commit-msg hooks.
+	See also linkgit:githooks[5].
+
 -s <strategy>::
 --strategy=<strategy>::
 	Use the given merge strategy; can be supplied more than

index e2ccbc44e2..4425a7a12e 100644
@@ -81,7 +81,7 @@ static int show_progress = -1;
 static int default_to_upstream = 1;
 static int signoff;
 static const char *sign_commit;
-static int verify_msg = 1;
+static int no_verify;
 
 static struct strategy all_strategy[] = {
 	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },

@@ -287,7 +287,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, "verify", &verify_msg, N_("verify commit-msg hook")),
+	OPT_BOOL(0, "no-verify", &no_verify, N_("bypass commit-msg hook")),
 	OPT_END()
 };
 

@@ -842,7 +842,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 			abort_commit(remoteheads, NULL);
 	}
 
-	if (verify_msg && run_commit_hook(0 < option_edit, get_index_file(),
+	if (!no_verify && run_commit_hook(0 < option_edit, get_index_file(),
 					  "commit-msg",
 					  git_path_merge_msg(the_repository), NULL))
 		abort_commit(remoteheads, NULL);

2.22.0.770.g0f2c4a37fd-goog

From: Michael J Gruber <[email protected]>

git-merge does not honor the pre-commit hook when doing automatic merge commits, and for compatibility reasons this is going to stay.

Introduce a pre-merge-commit hook which is called for an automatic merge commit just like pre-commit is called for a non-automatic merge commit (or any other commit).

[js: * renamed hook from "pre-merge" to "pre-merge-commit"
* only discard the index if the hook is actually present
* expanded githooks documentation entry
* clarified that hook should write messages to stderr
* squashed test changes from the original series' patch 4/4
* modified tests to follow new pattern from this series' patch 1/4
* added a test case for non-executable merge hooks
* added a test case for failed merges
* when testing that the merge hook did not run, make sure we actually have a merge to perform (by resetting the "side" branch to its original state).
* reworded commit message
]

---
 Documentation/githooks.txt                    | 21 +++++
 builtin/merge.c                               | 12 +++
 ...-pre-commit-and-pre-merge-commit-hooks.sh} | 84 ++++++++++++++++++-
 templates/hooks--pre-merge-commit.sample      | 13 +++
 4 files changed, 129 insertions(+), 1 deletion(-)
 rename t/{t7503-pre-commit-hook.sh => t7503-pre-commit-and-pre-merge-commit-hooks.sh} (63%)
 create mode 100755 templates/hooks--pre-merge-commit.sample

index 82cd573776..d9da474fb0 100644
@@ -103,6 +103,27 @@ The default 'pre-commit' hook, when enabled--and with the
 `hooks.allownonascii` config option unset or set to false--prevents
 the use of non-ASCII filenames.
 
+pre-merge-commit
+~~~~~~~~~~~~~~~~
+
+This hook is invoked by linkgit:git-merge[1]. It takes no parameters, and is
+invoked after the merge has been carried out successfully and before
+obtaining the proposed commit log message to
+make a commit.  Exiting with a non-zero status from this script
+causes the `git merge` command to abort before creating a commit.
+
+The default 'pre-merge-commit' hook, when enabled, runs the
+'pre-commit' hook, if the latter is enabled.
+
+This hook is invoked with the environment variable
+`GIT_EDITOR=:` if the command will not bring up an editor
+to modify the commit message.
+
+If the merge cannot be carried out automatically, the conflicts
+need to be resolved and the result committed separately (see
+linkgit:git-merge[1]). At that point, this hook will not be executed,
+but the 'pre-commit' hook will, if it is enabled.
+
 prepare-commit-msg
 ~~~~~~~~~~~~~~~~~~
 

index 4425a7a12e..bf0ae68c40 100644
@@ -816,6 +816,18 @@ static void write_merge_heads(struct commit_list *);
 static void prepare_to_commit(struct commit_list *remoteheads)
 {
 	struct strbuf msg = STRBUF_INIT;
+	const char *index_file = get_index_file();
+
+	if (run_commit_hook(0 < option_edit, index_file, "pre-merge-commit", NULL))
+		abort_commit(remoteheads, NULL);
+	/*
+	 * Re-read the index as pre-merge-commit hook could have updated it,
+	 * and write it out as a tree.  We must do this before we invoke
+	 * the editor and after we invoke run_status above.
+	 */
+	if (find_hook("pre-merge-commit"))
+		discard_cache();
+	read_cache_from(index_file);
 	strbuf_addbuf(&msg, &merge_msg);
 	if (squash)
 		BUG("the control must not reach here under --squash");

new file mode 100755
index 0000000000..399eab1924
@@ -0,0 +1,13 @@
+#!/bin/sh
+#
+# An example hook script to verify what is about to be committed.
+# Called by "git merge" with no arguments.  The hook should
+# exit with non-zero status after issuing an appropriate message to
+# stderr if it wants to stop the merge commit.
+#
+# To enable this hook, rename this file to "pre-merge-commit".
+
+. git-sh-setup
+test -x "$GIT_DIR/hooks/pre-commit" &&
+        exec "$GIT_DIR/hooks/pre-commit"
+:

2.22.0.770.g0f2c4a37fd-goog

From: Michael J Gruber <[email protected]>

Analogous to commit, introduce a '--no-verify' option which bypasses the pre-merge-commit hook. The shorthand '-n' is taken by '--no-stat' already.

[js: * reworded commit message to reflect current state of --no-stat flag
and new hook name
* fixed flag documentation to reflect new hook name
* cleaned up trailing whitespace
* squashed test changes from the original series' patch 4/4
* modified tests to follow pattern from this series' patch 1/4
* added a test case for --no-verify with non-executable hook
* when testing that the merge hook did not run, make sure we actually have a merge to perform (by resetting the "side" branch to its original state).

]

---
 Documentation/githooks.txt                    |  3 +-
 builtin/merge.c                               |  4 +-
 ...3-pre-commit-and-pre-merge-commit-hooks.sh | 39 +++++++++++++++++++
 3 files changed, 43 insertions(+), 3 deletions(-)

index d9da474fb0..57d6e2b98d 100644
@@ -106,7 +106,8 @@ the use of non-ASCII filenames.
 pre-merge-commit
 ~~~~~~~~~~~~~~~~
 
-This hook is invoked by linkgit:git-merge[1]. It takes no parameters, and is
+This hook is invoked by linkgit:git-merge[1], and can be bypassed
+with the `--no-verify` option.  It takes no parameters, and is
 invoked after the merge has been carried out successfully and before
 obtaining the proposed commit log message to
 make a commit.  Exiting with a non-zero status from this script

index bf0ae68c40..c9746e37b8 100644
@@ -287,7 +287,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 commit-msg hook")),
+	OPT_BOOL(0, "no-verify", &no_verify, N_("bypass pre-merge-commit and commit-msg hooks")),
 	OPT_END()
 };
 

@@ -818,7 +818,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 	struct strbuf msg = STRBUF_INIT;
 	const char *index_file = get_index_file();
 
-	if (run_commit_hook(0 < option_edit, index_file, "pre-merge-commit", NULL))
+	if (!no_verify && run_commit_hook(0 < option_edit, index_file, "pre-merge-commit", NULL))
 		abort_commit(remoteheads, NULL);
 	/*
 	 * Re-read the index as pre-merge-commit hook could have updated it,

index 0700604f03..d900c3a696 100755
@@ -84,6 +84,15 @@ test_expect_success '--no-verify with no hook' '
 	test_path_is_missing actual_hooks
 '
 
+test_expect_success '--no-verify with no hook (merge)' '
+	test_when_finished "rm -f actual_hooks" &&
+	git branch -f side side-orig &&
+	git checkout side &&
+	git merge --no-verify -m "merge master" master &&
+	git checkout master &&
+	test_path_is_missing actual_hooks
+'
+
 test_expect_success 'with succeeding hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
 	ln -s "success.sample" "$PRECOMMIT" &&

@@ -131,6 +140,16 @@ test_expect_success '--no-verify with succeeding hook' '
 	test_path_is_missing actual_hooks
 '
 
+test_expect_success '--no-verify with succeeding hook (merge)' '
+	test_when_finished "rm -f \"$PREMERGE\" actual_hooks" &&
+	ln -s "success.sample" "$PREMERGE" &&
+	git branch -f side side-orig &&
+	git checkout side &&
+	git merge --no-verify -m "merge master" master &&
+	git checkout master &&
+	test_path_is_missing actual_hooks
+'
+
 test_expect_success 'with failing hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
 	ln -s "fail.sample" "$PRECOMMIT" &&

@@ -160,6 +179,16 @@ test_expect_success 'with failing hook (merge)' '
 	test_cmp expected_hooks actual_hooks
 '
 
+test_expect_success '--no-verify with failing hook (merge)' '
+	test_when_finished "rm -f \"$PREMERGE\" actual_hooks" &&
+	ln -s "fail.sample" "$PREMERGE" &&
+	git branch -f side side-orig &&
+	git checkout side &&
+	git merge --no-verify -m "merge master" master &&
+	git checkout master &&
+	test_path_is_missing actual_hooks
+'
+
 test_expect_success POSIXPERM 'with non-executable hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
 	ln -s "non-exec.sample" "$PRECOMMIT" &&

@@ -188,6 +217,16 @@ test_expect_success POSIXPERM 'with non-executable hook (merge)' '
 	test_path_is_missing actual_hooks
 '
 
+test_expect_success POSIXPERM '--no-verify with non-executable hook (merge)' '
+	test_when_finished "rm -f \"$PREMERGE\" actual_hooks" &&
+	ln -s "non-exec.sample" "$PREMERGE" &&
+	git branch -f side side-orig &&
+	git checkout side &&
+	git merge --no-verify -m "merge master" master &&
+	git checkout master &&
+	test_path_is_missing actual_hooks
+'
+
 test_expect_success 'with hook requiring GIT_PREFIX' '
 	test_when_finished "rm -rf \"$PRECOMMIT\" expected_hooks actual_hooks success" &&
 	ln -s "require-prefix.sample" "$PRECOMMIT" &&

2.22.0.770.g0f2c4a37fd-goog

This series adds a new pre-merge-commit hook, similar in usage to pre-commit. It also improves hook testing in t7503, by verifying that the correct hooks are run or bypassed as expected.

The original series was done by Michael J Gruber <[email protected]>. I have addressed the outstanding review comments, and noted my changes in the commit messages in "[js: ...]" blocks.

Changes since V4:
* Used "cp" instead of "ln -s" in test hook setup to make tests more portable.

Changes since V3:
* Applied several test style fixes suggested by Martin (thanks!).
* Clarified the documentation for pre-merge-commit hook.
* Fixed a few cases where testing that the merge hook did not run might erroneously succeed if we don't have any merge to actually perform.
* Simplified test cleanup by adding a new non-executable sample hook script.
* Added test cases for non-executable pre-merge-commit hooks.

Changes since V2:
* Renamed the hook from "pre-merge" to "pre-merge-commit".
* Added a new patch (1/4) to improve t7503 by verifying that the expected hooks are (or are not) run.
* Squashed test changes (from V2's patch 4/4) into patch 3/4. Modified the tests to follow the example set in patch 1/4.
* Reworded commit messages to match with the current state of certain flags, which changed in between V1 and V2 of this series.

Josh Steadmon (1):
t7503: verify proper hook execution

Michael J Gruber (3):
merge: do no-verify like commit
git-merge: honor pre-merge-commit hook
merge: --no-verify to bypass pre-merge-commit hook

 Documentation/git-merge.txt                   |   2 +-
 Documentation/githooks.txt                    |  22 ++
 Documentation/merge-options.txt               |   4 +
 builtin/merge.c                               |  18 +-
 ...3-pre-commit-and-pre-merge-commit-hooks.sh | 281 ++++++++++++++++++
 t/t7503-pre-commit-hook.sh                    | 139 ---------
 templates/hooks--pre-merge-commit.sample      |  13 +
 7 files changed, 336 insertions(+), 143 deletions(-)
 create mode 100755 t/t7503-pre-commit-and-pre-merge-commit-hooks.sh
 delete mode 100755 t/t7503-pre-commit-hook.sh
 create mode 100755 templates/hooks--pre-merge-commit.sample

Range-diff against v4:
1: 5085729095 ! 1: 60bbbbf9e0 t7503: verify proper hook execution

    @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'
     -
     -	echo "more" >> file &&
     +	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
    -+	ln -s "success.sample" "$PRECOMMIT" &&
    ++	cp "$HOOKDIR/success.sample" "$PRECOMMIT" &&
     +	echo "$PRECOMMIT" >expected_hooks &&
     +	echo "more" >>file &&
      	git add file &&
    @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'
     -
     -	echo "even more" >> file &&
     +	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
    -+	ln -s "success.sample" "$PRECOMMIT" &&
    ++	cp "$HOOKDIR/success.sample" "$PRECOMMIT" &&
     +	echo "even more" >>file &&
      	git add file &&
     -	git commit --no-verify -m "even more"
    @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'
     -
     -	echo "another" >> file &&
     +	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
    -+	ln -s "fail.sample" "$PRECOMMIT" &&
    ++	cp "$HOOKDIR/fail.sample" "$PRECOMMIT" &&
     +	echo "$PRECOMMIT" >expected_hooks &&
     +	echo "another" >>file &&
      	git add file &&
    @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'
     -
     -	echo "stuff" >> file &&
     +	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
    -+	ln -s "fail.sample" "$PRECOMMIT" &&
    ++	cp "$HOOKDIR/fail.sample" "$PRECOMMIT" &&
     +	echo "stuff" >>file &&
      	git add file &&
     -	git commit --no-verify -m "stuff"
    @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'
     -
     -	echo "content" >> file &&
     +	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
    -+	ln -s "non-exec.sample" "$PRECOMMIT" &&
    ++	cp "$HOOKDIR/non-exec.sample" "$PRECOMMIT" &&
     +	echo "content" >>file &&
      	git add file &&
     -	git commit -m "content"
    @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'
     -
     -	echo "more content" >> file &&
     +	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
    -+	ln -s "non-exec.sample" "$PRECOMMIT" &&
    ++	cp "$HOOKDIR/non-exec.sample" "$PRECOMMIT" &&
     +	echo "more content" >>file &&
      	git add file &&
     -	git commit --no-verify -m "more content"
    @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'
     -
     -	echo "more content" >> file &&
     +	test_when_finished "rm -rf \"$PRECOMMIT\" expected_hooks actual_hooks success" &&
    -+	ln -s "require-prefix.sample" "$PRECOMMIT" &&
    ++	cp "$HOOKDIR/require-prefix.sample" "$PRECOMMIT" &&
     +	echo "$PRECOMMIT" >expected_hooks &&
     +	echo "more content" >>file &&
      	git add file &&
    @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'
     -
     -	echo "more content" >> file &&
     +	test_when_finished "rm -rf \"$PRECOMMIT\" expected_hooks actual_hooks fail" &&
    -+	ln -s "require-prefix.sample" "$PRECOMMIT" &&
    ++	cp "$HOOKDIR/require-prefix.sample" "$PRECOMMIT" &&
     +	echo "$PRECOMMIT" >expected_hooks &&
     +	echo "more content" >>file &&
      	git add file &&
    @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'
     -	test "$GIT_AUTHOR_NAME" = "New Author" &&
     -	test "$GIT_AUTHOR_EMAIL" = "[email protected]"
     +	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
    -+	ln -s "check-author.sample" "$PRECOMMIT" &&
    ++	cp "$HOOKDIR/check-author.sample" "$PRECOMMIT" &&
     +	cat >expected_hooks <<-EOF &&
     +	$PRECOMMIT
     +	$PRECOMMIT
2:  3b701a5c41 = 2:  02a97eb369 merge: do no-verify like commit
3:  9210421fbb ! 3:  2d41e0ff79 git-merge: honor pre-merge-commit hook
    @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with succ
      
     +test_expect_success 'with succeeding hook (merge)' '
     +	test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" &&
    -+	ln -s "success.sample" "$PREMERGE" &&
    ++	cp "$HOOKDIR/success.sample" "$PREMERGE" &&
     +	echo "$PREMERGE" >expected_hooks &&
     +	git checkout side &&
     +	git merge -m "merge master" master &&
    @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with succ
     +	test_when_finished "rm -f \"$PREMERGE\" \"$PRECOMMIT\"" &&
     +	test_when_finished "rm -f expected_hooks actual_hooks" &&
     +	test_when_finished "git checkout master" &&
    -+	ln -s "success.sample" "$PREMERGE" &&
    -+	ln -s "success.sample" "$PRECOMMIT" &&
    ++	cp "$HOOKDIR/success.sample" "$PREMERGE" &&
    ++	cp "$HOOKDIR/success.sample" "$PRECOMMIT" &&
     +
     +	git checkout conflicting-a &&
     +	test_must_fail git merge -m "merge conflicting-b" conflicting-b &&
    @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with succ
     +
      test_expect_success '--no-verify with succeeding hook' '
      	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
    - 	ln -s "success.sample" "$PRECOMMIT" &&
    + 	cp "$HOOKDIR/success.sample" "$PRECOMMIT" &&
     @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-verify with failing hook' '
      	test_path_is_missing actual_hooks
      '
      
     +test_expect_success 'with failing hook (merge)' '
     +	test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" &&
    -+	ln -s "fail.sample" "$PREMERGE" &&
    ++	cp "$HOOKDIR/fail.sample" "$PREMERGE" &&
     +	echo "$PREMERGE" >expected_hooks &&
     +	git checkout side &&
     +	test_must_fail git merge -m "merge master" master &&
    @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-veri
     +
      test_expect_success POSIXPERM 'with non-executable hook' '
      	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
    - 	ln -s "non-exec.sample" "$PRECOMMIT" &&
    + 	cp "$HOOKDIR/non-exec.sample" "$PRECOMMIT" &&
     @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success POSIXPERM '--no-verify with non-executable hook' '
      	test_path_is_missing actual_hooks
      '
      
     +test_expect_success POSIXPERM 'with non-executable hook (merge)' '
     +	test_when_finished "rm -f \"$PREMERGE\" actual_hooks" &&
    -+	ln -s "non-exec.sample" "$PREMERGE" &&
    ++	cp "$HOOKDIR/non-exec.sample" "$PREMERGE" &&
     +	git branch -f side side-orig &&
     +	git checkout side &&
     +	git merge -m "merge master" master &&
    @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success POSIXPERM
     +
      test_expect_success 'with hook requiring GIT_PREFIX' '
      	test_when_finished "rm -rf \"$PRECOMMIT\" expected_hooks actual_hooks success" &&
    - 	ln -s "require-prefix.sample" "$PRECOMMIT" &&
    + 	cp "$HOOKDIR/require-prefix.sample" "$PRECOMMIT" &&
     
      ## templates/hooks--pre-merge-commit.sample (new) ##
     @@
4:  96c54883d3 ! 4:  7b81e2717f merge: --no-verify to bypass pre-merge-commit hook
    @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-veri
     +
      test_expect_success 'with succeeding hook' '
      	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
    - 	ln -s "success.sample" "$PRECOMMIT" &&
    + 	cp "$HOOKDIR/success.sample" "$PRECOMMIT" &&
     @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-verify with succeeding hook' '
      	test_path_is_missing actual_hooks
      '
      
     +test_expect_success '--no-verify with succeeding hook (merge)' '
     +	test_when_finished "rm -f \"$PREMERGE\" actual_hooks" &&
    -+	ln -s "success.sample" "$PREMERGE" &&
    ++	cp "$HOOKDIR/success.sample" "$PREMERGE" &&
     +	git branch -f side side-orig &&
     +	git checkout side &&
     +	git merge --no-verify -m "merge master" master &&
    @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-veri
     +
      test_expect_success 'with failing hook' '
      	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
    - 	ln -s "fail.sample" "$PRECOMMIT" &&
    + 	cp "$HOOKDIR/fail.sample" "$PRECOMMIT" &&
     @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with failing hook (merge)' '
      	test_cmp expected_hooks actual_hooks
      '
      
     +test_expect_success '--no-verify with failing hook (merge)' '
     +	test_when_finished "rm -f \"$PREMERGE\" actual_hooks" &&
    -+	ln -s "fail.sample" "$PREMERGE" &&
    ++	cp "$HOOKDIR/fail.sample" "$PREMERGE" &&
     +	git branch -f side side-orig &&
     +	git checkout side &&
     +	git merge --no-verify -m "merge master" master &&
    @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with fail
     +
      test_expect_success POSIXPERM 'with non-executable hook' '
      	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
    - 	ln -s "non-exec.sample" "$PRECOMMIT" &&
    + 	cp "$HOOKDIR/non-exec.sample" "$PRECOMMIT" &&
     @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success POSIXPERM 'with non-executable hook (merge)' '
      	test_path_is_missing actual_hooks
      '
      
     +test_expect_success POSIXPERM '--no-verify with non-executable hook (merge)' '
     +	test_when_finished "rm -f \"$PREMERGE\" actual_hooks" &&
    -+	ln -s "non-exec.sample" "$PREMERGE" &&
    ++	cp "$HOOKDIR/non-exec.sample" "$PREMERGE" &&
     +	git branch -f side side-orig &&
     +	git checkout side &&
     +	git merge --no-verify -m "merge master" master &&
    @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success POSIXPERM
     +
      test_expect_success 'with hook requiring GIT_PREFIX' '
      	test_when_finished "rm -rf \"$PRECOMMIT\" expected_hooks actual_hooks success" &&
    - 	ln -s "require-prefix.sample" "$PRECOMMIT" &&
    + 	cp "$HOOKDIR/require-prefix.sample" "$PRECOMMIT" &&

2.22.0.770.g0f2c4a37fd-goog

t7503 did not verify that the expected hooks actually ran during testing. Fix that by making the hook scripts write their $0 into a file so that we can compare actual execution vs. expected execution.

While we're at it, do some test style cleanups, such as using write_script() and doing setup inside a test_expect_success block.

---
 t/t7503-pre-commit-hook.sh | 157 +++++++++++++++++++++----------------
 1 file changed, 89 insertions(+), 68 deletions(-)

index 984889b39d..6aa83204c2 100755
@@ -4,124 +4,144 @@ test_description='pre-commit hook'
 
 . ./test-lib.sh
 
-test_expect_success 'with no hook' '
+HOOKDIR="$(git rev-parse --git-dir)/hooks"
+PRECOMMIT="$HOOKDIR/pre-commit"
+
+# Prepare sample scripts that write their $0 to actual_hooks
+test_expect_success 'sample script setup' '
+	mkdir -p "$HOOKDIR" &&
+	write_script "$HOOKDIR/success.sample" <<-\EOF &&
+	echo $0 >>actual_hooks
+	exit 0
+	EOF
+	write_script "$HOOKDIR/fail.sample" <<-\EOF &&
+	echo $0 >>actual_hooks
+	exit 1
+	EOF
+	write_script "$HOOKDIR/non-exec.sample" <<-\EOF &&
+	echo $0 >>actual_hooks
+	exit 1
+	EOF
+	chmod -x "$HOOKDIR/non-exec.sample" &&
+	write_script "$HOOKDIR/require-prefix.sample" <<-\EOF &&
+	echo $0 >>actual_hooks
+	test $GIT_PREFIX = "success/"
+	EOF
+	write_script "$HOOKDIR/check-author.sample" <<-\EOF
+	echo $0 >>actual_hooks
+	test "$GIT_AUTHOR_NAME" = "New Author" &&
+	test "$GIT_AUTHOR_EMAIL" = "[email protected]"
+	EOF
+'
 
-	echo "foo" > file &&
+test_expect_success 'with no hook' '
+	test_when_finished "rm -f actual_hooks" &&
+	echo "foo" >file &&
 	git add file &&
-	git commit -m "first"
-
+	git commit -m "first" &&
+	test_path_is_missing actual_hooks
 '
 
 test_expect_success '--no-verify with no hook' '
-
-	echo "bar" > file &&
+	test_when_finished "rm -f actual_hooks" &&
+	echo "bar" >file &&
 	git add file &&
-	git commit --no-verify -m "bar"
-
+	git commit --no-verify -m "bar" &&
+	test_path_is_missing actual_hooks
 '
 
-# now install hook that always succeeds
-HOOKDIR="$(git rev-parse --git-dir)/hooks"
-HOOK="$HOOKDIR/pre-commit"
-mkdir -p "$HOOKDIR"
-cat > "$HOOK" <<EOF
-#!/bin/sh
-exit 0
-EOF
-chmod +x "$HOOK"
-
 test_expect_success 'with succeeding hook' '
-
-	echo "more" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
+	cp "$HOOKDIR/success.sample" "$PRECOMMIT" &&
+	echo "$PRECOMMIT" >expected_hooks &&
+	echo "more" >>file &&
 	git add file &&
-	git commit -m "more"
-
+	git commit -m "more" &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success '--no-verify with succeeding hook' '
-
-	echo "even more" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
+	cp "$HOOKDIR/success.sample" "$PRECOMMIT" &&
+	echo "even more" >>file &&
 	git add file &&
-	git commit --no-verify -m "even more"
-
+	git commit --no-verify -m "even more" &&
+	test_path_is_missing actual_hooks
 '
 
-# now a hook that fails
-cat > "$HOOK" <<EOF
-#!/bin/sh
-exit 1
-EOF
-
 test_expect_success 'with failing hook' '
-
-	echo "another" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
+	cp "$HOOKDIR/fail.sample" "$PRECOMMIT" &&
+	echo "$PRECOMMIT" >expected_hooks &&
+	echo "another" >>file &&
 	git add file &&
-	test_must_fail git commit -m "another"
-
+	test_must_fail git commit -m "another" &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success '--no-verify with failing hook' '
-
-	echo "stuff" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
+	cp "$HOOKDIR/fail.sample" "$PRECOMMIT" &&
+	echo "stuff" >>file &&
 	git add file &&
-	git commit --no-verify -m "stuff"
-
+	git commit --no-verify -m "stuff" &&
+	test_path_is_missing actual_hooks
 '
 
-chmod -x "$HOOK"
 test_expect_success POSIXPERM 'with non-executable hook' '
-
-	echo "content" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
+	cp "$HOOKDIR/non-exec.sample" "$PRECOMMIT" &&
+	echo "content" >>file &&
 	git add file &&
-	git commit -m "content"
-
+	git commit -m "content" &&
+	test_path_is_missing actual_hooks
 '
 
 test_expect_success POSIXPERM '--no-verify with non-executable hook' '
-
-	echo "more content" >> file &&
+	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
+	cp "$HOOKDIR/non-exec.sample" "$PRECOMMIT" &&
+	echo "more content" >>file &&
 	git add file &&
-	git commit --no-verify -m "more content"
-
+	git commit --no-verify -m "more content" &&
+	test_path_is_missing actual_hooks
 '
-chmod +x "$HOOK"
-
-# a hook that checks $GIT_PREFIX and succeeds inside the
-# success/ subdirectory only
-cat > "$HOOK" <<EOF
-#!/bin/sh
-test \$GIT_PREFIX = success/
-EOF
 
 test_expect_success 'with hook requiring GIT_PREFIX' '
-
-	echo "more content" >> file &&
+	test_when_finished "rm -rf \"$PRECOMMIT\" expected_hooks actual_hooks success" &&
+	cp "$HOOKDIR/require-prefix.sample" "$PRECOMMIT" &&
+	echo "$PRECOMMIT" >expected_hooks &&
+	echo "more content" >>file &&
 	git add file &&
 	mkdir success &&
 	(
 		cd success &&
 		git commit -m "hook requires GIT_PREFIX = success/"
 	) &&
-	rmdir success
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success 'with failing hook requiring GIT_PREFIX' '
-
-	echo "more content" >> file &&
+	test_when_finished "rm -rf \"$PRECOMMIT\" expected_hooks actual_hooks fail" &&
+	cp "$HOOKDIR/require-prefix.sample" "$PRECOMMIT" &&
+	echo "$PRECOMMIT" >expected_hooks &&
+	echo "more content" >>file &&
 	git add file &&
 	mkdir fail &&
 	(
 		cd fail &&
 		test_must_fail git commit -m "hook must fail"
 	) &&
-	rmdir fail &&
-	git checkout -- file
+	git checkout -- file &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_expect_success 'check the author in hook' '
-	write_script "$HOOK" <<-\EOF &&
-	test "$GIT_AUTHOR_NAME" = "New Author" &&
-	test "$GIT_AUTHOR_EMAIL" = "[email protected]"
+	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
+	cp "$HOOKDIR/check-author.sample" "$PRECOMMIT" &&
+	cat >expected_hooks <<-EOF &&
+	$PRECOMMIT
+	$PRECOMMIT
+	$PRECOMMIT
 	EOF
 	test_must_fail git commit --allow-empty -m "by a.u.thor" &&
 	(

@@ -133,7 +153,8 @@ test_expect_success 'check the author in hook' '
 	) &&
 	git commit --author="New Author <[email protected]>" \
 		--allow-empty -m "by new.author via command line" &&
-	git show -s
+	git show -s &&
+	test_cmp expected_hooks actual_hooks
 '
 
 test_done

2.22.0.770.g0f2c4a37fd-goog

From: Michael J Gruber <[email protected]>

f8b863598c ("builtin/merge: honor commit-msg hook for merges", 2017-09-07) introduced the no-verify flag to merge for bypassing the commit-msg hook, though in a different way from the implementation in commit.c.

Change the implementation in merge.c to be the same as in commit.c so that both do the same in the same way. This also changes the output of "git merge --help" to be more clear that the hook return code is respected by default.

[js: * reworded commit message
* squashed documentation changes from original series' patch 3/4
]

---
 Documentation/git-merge.txt     | 2 +-
 Documentation/merge-options.txt | 4 ++++
 builtin/merge.c                 | 6 +++---
 3 files changed, 8 insertions(+), 4 deletions(-)

index 01fd52dc70..092529c619 100644
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
-	[-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
+	[--no-verify] [-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
 	[--[no-]allow-unrelated-histories]
 	[--[no-]rerere-autoupdate] [-m <msg>] [-F <file>] [<commit>...]
 'git merge' (--continue | --abort | --quit)

index 79a00d2a4a..d6a9f4b96f 100644
@@ -105,6 +105,10 @@ option can be used to override --squash.
 +
 With --squash, --commit is not allowed, and will fail.
 
+--no-verify::
+	This option bypasses the pre-merge and commit-msg hooks.
+	See also linkgit:githooks[5].
+
 -s <strategy>::
 --strategy=<strategy>::
 	Use the given merge strategy; can be supplied more than

index e2ccbc44e2..4425a7a12e 100644
@@ -81,7 +81,7 @@ static int show_progress = -1;
 static int default_to_upstream = 1;
 static int signoff;
 static const char *sign_commit;
-static int verify_msg = 1;
+static int no_verify;
 
 static struct strategy all_strategy[] = {
 	{ "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },

@@ -287,7 +287,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, "verify", &verify_msg, N_("verify commit-msg hook")),
+	OPT_BOOL(0, "no-verify", &no_verify, N_("bypass commit-msg hook")),
 	OPT_END()
 };
 

@@ -842,7 +842,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 			abort_commit(remoteheads, NULL);
 	}
 
-	if (verify_msg && run_commit_hook(0 < option_edit, get_index_file(),
+	if (!no_verify && run_commit_hook(0 < option_edit, get_index_file(),
 					  "commit-msg",
 					  git_path_merge_msg(the_repository), NULL))
 		abort_commit(remoteheads, NULL);

2.22.0.770.g0f2c4a37fd-goog

From: Michael J Gruber <[email protected]>

git-merge does not honor the pre-commit hook when doing automatic merge commits, and for compatibility reasons this is going to stay.

Introduce a pre-merge-commit hook which is called for an automatic merge commit just like pre-commit is called for a non-automatic merge commit (or any other commit).

[js: * renamed hook from "pre-merge" to "pre-merge-commit"
* only discard the index if the hook is actually present
* expanded githooks documentation entry
* clarified that hook should write messages to stderr
* squashed test changes from the original series' patch 4/4
* modified tests to follow new pattern from this series' patch 1/4
* added a test case for non-executable merge hooks
* added a test case for failed merges
* when testing that the merge hook did not run, make sure we actually have a merge to perform (by resetting the "side" branch to its original state).
* reworded commit message
]

---
 Documentation/githooks.txt                    | 21 +++++
 builtin/merge.c                               | 12 +++
 ...-pre-commit-and-pre-merge-commit-hooks.sh} | 84 ++++++++++++++++++-
 templates/hooks--pre-merge-commit.sample      | 13 +++
 4 files changed, 129 insertions(+), 1 deletion(-)
 rename t/{t7503-pre-commit-hook.sh => t7503-pre-commit-and-pre-merge-commit-hooks.sh} (63%)
 create mode 100755 templates/hooks--pre-merge-commit.sample

index 82cd573776..d9da474fb0 100644
@@ -103,6 +103,27 @@ The default 'pre-commit' hook, when enabled--and with the
 `hooks.allownonascii` config option unset or set to false--prevents
 the use of non-ASCII filenames.
 
+pre-merge-commit
+~~~~~~~~~~~~~~~~
+
+This hook is invoked by linkgit:git-merge[1]. It takes no parameters, and is
+invoked after the merge has been carried out successfully and before
+obtaining the proposed commit log message to
+make a commit.  Exiting with a non-zero status from this script
+causes the `git merge` command to abort before creating a commit.
+
+The default 'pre-merge-commit' hook, when enabled, runs the
+'pre-commit' hook, if the latter is enabled.
+
+This hook is invoked with the environment variable
+`GIT_EDITOR=:` if the command will not bring up an editor
+to modify the commit message.
+
+If the merge cannot be carried out automatically, the conflicts
+need to be resolved and the result committed separately (see
+linkgit:git-merge[1]). At that point, this hook will not be executed,
+but the 'pre-commit' hook will, if it is enabled.
+
 prepare-commit-msg
 ~~~~~~~~~~~~~~~~~~
 

index 4425a7a12e..bf0ae68c40 100644
@@ -816,6 +816,18 @@ static void write_merge_heads(struct commit_list *);
 static void prepare_to_commit(struct commit_list *remoteheads)
 {
 	struct strbuf msg = STRBUF_INIT;
+	const char *index_file = get_index_file();
+
+	if (run_commit_hook(0 < option_edit, index_file, "pre-merge-commit", NULL))
+		abort_commit(remoteheads, NULL);
+	/*
+	 * Re-read the index as pre-merge-commit hook could have updated it,
+	 * and write it out as a tree.  We must do this before we invoke
+	 * the editor and after we invoke run_status above.
+	 */
+	if (find_hook("pre-merge-commit"))
+		discard_cache();
+	read_cache_from(index_file);
 	strbuf_addbuf(&msg, &merge_msg);
 	if (squash)
 		BUG("the control must not reach here under --squash");

new file mode 100755
index 0000000000..399eab1924
@@ -0,0 +1,13 @@
+#!/bin/sh
+#
+# An example hook script to verify what is about to be committed.
+# Called by "git merge" with no arguments.  The hook should
+# exit with non-zero status after issuing an appropriate message to
+# stderr if it wants to stop the merge commit.
+#
+# To enable this hook, rename this file to "pre-merge-commit".
+
+. git-sh-setup
+test -x "$GIT_DIR/hooks/pre-commit" &&
+        exec "$GIT_DIR/hooks/pre-commit"
+:

2.22.0.770.g0f2c4a37fd-goog

From: Michael J Gruber <[email protected]>

Analogous to commit, introduce a '--no-verify' option which bypasses the pre-merge-commit hook. The shorthand '-n' is taken by '--no-stat' already.

[js: * reworded commit message to reflect current state of --no-stat flag
and new hook name
* fixed flag documentation to reflect new hook name
* cleaned up trailing whitespace
* squashed test changes from the original series' patch 4/4
* modified tests to follow pattern from this series' patch 1/4
* added a test case for --no-verify with non-executable hook
* when testing that the merge hook did not run, make sure we actually have a merge to perform (by resetting the "side" branch to its original state).

]

---
 Documentation/githooks.txt                    |  3 +-
 builtin/merge.c                               |  4 +-
 ...3-pre-commit-and-pre-merge-commit-hooks.sh | 39 +++++++++++++++++++
 3 files changed, 43 insertions(+), 3 deletions(-)

index d9da474fb0..57d6e2b98d 100644
@@ -106,7 +106,8 @@ the use of non-ASCII filenames.
 pre-merge-commit
 ~~~~~~~~~~~~~~~~
 
-This hook is invoked by linkgit:git-merge[1]. It takes no parameters, and is
+This hook is invoked by linkgit:git-merge[1], and can be bypassed
+with the `--no-verify` option.  It takes no parameters, and is
 invoked after the merge has been carried out successfully and before
 obtaining the proposed commit log message to
 make a commit.  Exiting with a non-zero status from this script

index bf0ae68c40..c9746e37b8 100644
@@ -287,7 +287,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 commit-msg hook")),
+	OPT_BOOL(0, "no-verify", &no_verify, N_("bypass pre-merge-commit and commit-msg hooks")),
 	OPT_END()
 };
 

@@ -818,7 +818,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 	struct strbuf msg = STRBUF_INIT;
 	const char *index_file = get_index_file();
 
-	if (run_commit_hook(0 < option_edit, index_file, "pre-merge-commit", NULL))
+	if (!no_verify && run_commit_hook(0 < option_edit, index_file, "pre-merge-commit", NULL))
 		abort_commit(remoteheads, NULL);
 	/*
 	 * Re-read the index as pre-merge-commit hook could have updated it,

index 7a5434c7ab..b3485450a2 100755
@@ -84,6 +84,15 @@ test_expect_success '--no-verify with no hook' '
 	test_path_is_missing actual_hooks
 '
 
+test_expect_success '--no-verify with no hook (merge)' '
+	test_when_finished "rm -f actual_hooks" &&
+	git branch -f side side-orig &&
+	git checkout side &&
+	git merge --no-verify -m "merge master" master &&
+	git checkout master &&
+	test_path_is_missing actual_hooks
+'
+
 test_expect_success 'with succeeding hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
 	cp "$HOOKDIR/success.sample" "$PRECOMMIT" &&

@@ -131,6 +140,16 @@ test_expect_success '--no-verify with succeeding hook' '
 	test_path_is_missing actual_hooks
 '
 
+test_expect_success '--no-verify with succeeding hook (merge)' '
+	test_when_finished "rm -f \"$PREMERGE\" actual_hooks" &&
+	cp "$HOOKDIR/success.sample" "$PREMERGE" &&
+	git branch -f side side-orig &&
+	git checkout side &&
+	git merge --no-verify -m "merge master" master &&
+	git checkout master &&
+	test_path_is_missing actual_hooks
+'
+
 test_expect_success 'with failing hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" expected_hooks actual_hooks" &&
 	cp "$HOOKDIR/fail.sample" "$PRECOMMIT" &&

@@ -160,6 +179,16 @@ test_expect_success 'with failing hook (merge)' '
 	test_cmp expected_hooks actual_hooks
 '
 
+test_expect_success '--no-verify with failing hook (merge)' '
+	test_when_finished "rm -f \"$PREMERGE\" actual_hooks" &&
+	cp "$HOOKDIR/fail.sample" "$PREMERGE" &&
+	git branch -f side side-orig &&
+	git checkout side &&
+	git merge --no-verify -m "merge master" master &&
+	git checkout master &&
+	test_path_is_missing actual_hooks
+'
+
 test_expect_success POSIXPERM 'with non-executable hook' '
 	test_when_finished "rm -f \"$PRECOMMIT\" actual_hooks" &&
 	cp "$HOOKDIR/non-exec.sample" "$PRECOMMIT" &&

@@ -188,6 +217,16 @@ test_expect_success POSIXPERM 'with non-executable hook (merge)' '
 	test_path_is_missing actual_hooks
 '
 
+test_expect_success POSIXPERM '--no-verify with non-executable hook (merge)' '
+	test_when_finished "rm -f \"$PREMERGE\" actual_hooks" &&
+	cp "$HOOKDIR/non-exec.sample" "$PREMERGE" &&
+	git branch -f side side-orig &&
+	git checkout side &&
+	git merge --no-verify -m "merge master" master &&
+	git checkout master &&
+	test_path_is_missing actual_hooks
+'
+
 test_expect_success 'with hook requiring GIT_PREFIX' '
 	test_when_finished "rm -rf \"$PRECOMMIT\" expected_hooks actual_hooks success" &&
 	cp "$HOOKDIR/require-prefix.sample" "$PRECOMMIT" &&

2.22.0.770.g0f2c4a37fd-goog

I don't have any more nits to pick ;-) so this looks good to me, FWIW.

Martin