[PATCH 0/3] pre-merge-hook

DORMANTno replies

12 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH 0/3] pre-merge-hook

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:38

The pre-commit hook is often used to ensure certain properties of each
comitted tree like formatting or coding standards, validity (lint/make)
or code quality (make test). But merges introduce new commits unless
they are fast forwards, and therefore they can break these properties
because the pre-commit hook is not run by "git merge".

Introduce a pre-merge hook which works for (non ff, automatic) merges
like pre-commit does for commits. Typically this will just call the
pre-commit hook (like in the sample hook), but it does not need to.

Michael J Gruber (3):
  git-merge: Honor pre-merge hook
  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                   | 15 ++++++++-
 t/t7503-pre-commit-hook.sh        | 66 ++++++++++++++++++++++++++++++++++++++-
 templates/hooks--pre-merge.sample | 13 ++++++++
 6 files changed, 104 insertions(+), 3 deletions(-)
 create mode 100755 templates/hooks--pre-merge.sample

-- 
1.7.12.406.gafd3f81

[PATCH 3/3] t7503: add tests for pre-merge-hook

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:38

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".

Signed-off-by: Michael J Gruber <redacted>
---
 t/t7503-pre-commit-hook.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 65 insertions(+), 1 deletion(-)
diff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-hook.sh
index 984889b..36ae87f 100755
--- a/t/t7503-pre-commit-hook.sh
+++ b/t/t7503-pre-commit-hook.sh
@@ -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' '
 
-- 
1.7.12.406.gafd3f81

[PATCH 1/3] git-merge: Honor pre-merge hook

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:38

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).

Signed-off-by: Michael J Gruber <redacted>
---
 Documentation/githooks.txt        |  7 +++++++
 builtin/merge.c                   | 13 ++++++++++++-
 templates/hooks--pre-merge.sample | 13 +++++++++++++
 3 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100755 templates/hooks--pre-merge.sample
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index b9003fe..3fae643 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -86,6 +86,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
 ~~~~~~~~~~~~~~~~~~
 
diff --git a/builtin/merge.c b/builtin/merge.c
index 0ec8f0d..7c09e0b 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -898,12 +898,23 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 {
 	struct strbuf msg = STRBUF_INIT;
 	const char *comment = _(merge_editor_comment);
+	const char *index_file = get_index_file();
+
+	if (run_hook(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 (0 < option_edit)
 		strbuf_add_lines(&msg, "# ", comment, strlen(comment));
 	write_merge_msg(&msg);
-	run_hook(get_index_file(), "prepare-commit-msg",
+	run_hook(index_file, "prepare-commit-msg",
 		 git_path("MERGE_MSG"), "merge", NULL, NULL);
 	if (0 < option_edit) {
 		if (launch_editor(git_path("MERGE_MSG"), NULL, NULL))
diff --git a/templates/hooks--pre-merge.sample b/templates/hooks--pre-merge.sample
new file mode 100755
index 0000000..a6313e6
--- /dev/null
+++ b/templates/hooks--pre-merge.sample
@@ -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"
+:
-- 
1.7.12.406.gafd3f81

[PATCH 2/3] merge: --no-verify to bypass pre-merge hook

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:38

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.

Signed-off-by: Michael J Gruber <redacted>
---
 Documentation/git-merge.txt     | 2 +-
 Documentation/githooks.txt      | 2 +-
 Documentation/merge-options.txt | 4 ++++
 builtin/merge.c                 | 4 +++-
 4 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 20f9228..8816865 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
-	[-s <strategy>] [-X <strategy-option>]
+	[--no-verify] [-s <strategy>] [-X <strategy-option>]
 	[--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
 'git merge' <msg> HEAD <commit>...
 'git merge' --abort
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 3fae643..e20bfe0 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -91,7 +91,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
 ~~~~~~~~~~~~~~~~~~
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 0bcbe0a..93e2e61 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -70,6 +70,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
diff --git a/builtin/merge.c b/builtin/merge.c
index 7c09e0b..4d36a00 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -60,6 +60,7 @@ static const char *branch;
 static char *branch_mergeoptions;
 static int option_renormalize;
 static int verbosity;
+static int no_verify;
 static int allow_rerere_auto;
 static int abort_current_merge;
 static int show_progress = -1;
@@ -198,6 +199,7 @@ static struct option builtin_merge_options[] = {
 		N_("allow fast-forward (default)")),
 	OPT_BOOLEAN(0, "ff-only", &fast_forward_only,
 		N_("abort if fast-forward is not possible")),
+	OPT_BOOLEAN(0, "no-verify", &no_verify, "bypass pre-merge hook"),
 	OPT_RERERE_AUTOUPDATE(&allow_rerere_auto),
 	OPT_CALLBACK('s', "strategy", &use_strategies, N_("strategy"),
 		N_("merge strategy to use"), option_parse_strategy),
@@ -900,7 +902,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 	const char *comment = _(merge_editor_comment);
 	const char *index_file = get_index_file();
 
-	if (run_hook(index_file, "pre-merge", NULL))
+	if (!no_verify && run_hook(index_file, "pre-merge", NULL))
 		abort_commit(remoteheads, NULL);
 	/*
 	 * Re-read the index as pre-merge hook could have updated it,
-- 
1.7.12.406.gafd3f81

Re: [PATCH 1/3] git-merge: Honor pre-merge hook

From: Michael Haggerty <hidden>
Date: 2016-06-15 22:54:38

On 09/05/2012 03:39 PM, Michael J Gruber wrote:
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).
What exactly is an "automatic merge commit"?  Is it any merge that
doesn't have a conflict?  A merge that doesn't invoke the editor?  A
merge done as part of another operation (e.g., pull)?  I don't see the
term mentioned in the git-merge or githooks man pages.

I think it would be good if you would define this term in the
documentation files that your patch touched, and perhaps in the githooks
section about "pre-commit" as well.

Secondly, though it is impossible (for backwards compatibility reasons)
for the pre-commit hook to be invoked for automatic merges, no such
considerations prohibit the pre-merge commit from being invoked for
non-automatic merges.  In other words, both hooks, pre-commit *and*
pre-merge, could be invoked for non-automatic merges.  Would this be
preferable?

It depends on what pre-merge scripts are likely to be used for.  If they
will tend to be used for merge-specific actions, then it might be more
convenient for *all* merges to be vetted by them.  On the other hand, if
they tend to do the same actions as pre-commit hooks, then having
non-automatic merge commits go through both hooks would tend to be more
annoying than helpful.  Specifically, one of the scripts would probably
have to check whether the merge is a non-automatic merge, and if so do
nothing (i.e., letting the other script take care of it).  This would
also require an easy way for a script to determine whether a commit is a
non-automatic merge commit.

Have you considered this?

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: [PATCH 1/3] git-merge: Honor pre-merge hook

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:38

Michael Haggerty venit, vidit, dixit 05.09.2012 17:30:
On 09/05/2012 03:39 PM, Michael J Gruber wrote:
quoted
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).
What exactly is an "automatic merge commit"?  Is it any merge that
doesn't have a conflict?  A merge that doesn't invoke the editor?  A
merge done as part of another operation (e.g., pull)?  I don't see the
term mentioned in the git-merge or githooks man pages.

I think it would be good if you would define this term in the
documentation files that your patch touched, and perhaps in the githooks
section about "pre-commit" as well.
"git merge" can go three ways:

F: fast forward: no commit is created, only a ref is changed
A: automatic: true merge (non-ff) without conflicts (i.e. chosen
strategy can perform the merge); a new commit is created
C: merge with conflicts: no commit is created but the index is prepared
(partially) for a merge commit

In case F, no commit hook is run (talking only about pre-commit/pre-merge).

In case A, no commit is run so far but my patch proposes pre-merge to be
run.

In case C, pre-commit (!) is run so far and after my patch.
Secondly, though it is impossible (for backwards compatibility reasons)
for the pre-commit hook to be invoked for automatic merges, no such
considerations prohibit the pre-merge commit from being invoked for
non-automatic merges.  In other words, both hooks, pre-commit *and*
pre-merge, could be invoked for non-automatic merges.  Would this be
preferable?

It depends on what pre-merge scripts are likely to be used for.  If they
will tend to be used for merge-specific actions, then it might be more
convenient for *all* merges to be vetted by them.  On the other hand, if
they tend to do the same actions as pre-commit hooks, then having
non-automatic merge commits go through both hooks would tend to be more
annoying than helpful.  Specifically, one of the scripts would probably
have to check whether the merge is a non-automatic merge, and if so do
nothing (i.e., letting the other script take care of it).  This would
also require an easy way for a script to determine whether a commit is a
non-automatic merge commit.

Have you considered this?
Your second paragraph explains why I did it the way I did. One can
easily have pre-merge call pre-commit, or have them be different. One
can not easily have only pre-merge called for a non-automatic merge
commit, but that is because of backward compatibility. The way *I* would
like it is:

- call pre-merge for any non-ff merge commit (automatic or not)
- call pre-commit for any non-merge commit (#parents <=1)

But that would break compatibility.

So I hope my patch is the best approximation to the above which keeps
compatibility and is simple to handle in most situations.

Cheers
Michael

Re: [PATCH 1/3] git-merge: Honor pre-merge hook

From: Michael Haggerty <hidden>
Date: 2016-06-15 22:54:38

On 09/06/2012 10:16 AM, Michael J Gruber wrote:
Michael Haggerty venit, vidit, dixit 05.09.2012 17:30:
quoted
On 09/05/2012 03:39 PM, Michael J Gruber wrote:
quoted
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).
What exactly is an "automatic merge commit"?  Is it any merge that
doesn't have a conflict?  A merge that doesn't invoke the editor?  A
merge done as part of another operation (e.g., pull)?  I don't see the
term mentioned in the git-merge or githooks man pages.

I think it would be good if you would define this term in the
documentation files that your patch touched, and perhaps in the githooks
section about "pre-commit" as well.
"git merge" can go three ways:

F: fast forward: no commit is created, only a ref is changed
A: automatic: true merge (non-ff) without conflicts (i.e. chosen
strategy can perform the merge); a new commit is created
C: merge with conflicts: no commit is created but the index is prepared
(partially) for a merge commit

In case F, no commit hook is run (talking only about pre-commit/pre-merge).

In case A, no commit is run so far but my patch proposes pre-merge to be
run.

In case C, pre-commit (!) is run so far and after my patch.
Thanks for the explanation.  I hope you will explain this briefly in the
patch to the docs.
quoted
Secondly, though it is impossible (for backwards compatibility reasons)
for the pre-commit hook to be invoked for automatic merges, no such
considerations prohibit the pre-merge commit from being invoked for
non-automatic merges.  In other words, both hooks, pre-commit *and*
pre-merge, could be invoked for non-automatic merges.  Would this be
preferable?

It depends on what pre-merge scripts are likely to be used for.  If they
will tend to be used for merge-specific actions, then it might be more
convenient for *all* merges to be vetted by them.  On the other hand, if
they tend to do the same actions as pre-commit hooks, then having
non-automatic merge commits go through both hooks would tend to be more
annoying than helpful.  Specifically, one of the scripts would probably
have to check whether the merge is a non-automatic merge, and if so do
nothing (i.e., letting the other script take care of it).  This would
also require an easy way for a script to determine whether a commit is a
non-automatic merge commit.

Have you considered this?
Your second paragraph explains why I did it the way I did. One can
easily have pre-merge call pre-commit, or have them be different. One
can not easily have only pre-merge called for a non-automatic merge
commit, but that is because of backward compatibility. The way *I* would
like it is:

- call pre-merge for any non-ff merge commit (automatic or not)
- call pre-commit for any non-merge commit (#parents <=1)

But that would break compatibility.

So I hope my patch is the best approximation to the above which keeps
compatibility and is simple to handle in most situations.
I can understand your reasoning and won't object.  But before I shut up,
I will point out a third alternative that is arguably closer to your
"ideal":

- For non-merge commits, call pre-commit
- For automatic merge commits, call pre-merge
- For non-automatic merge commits:
  if pre-merge exists, call pre-merge (only)
  else if pre-commit exists, call pre-commit (for backwards comptibility)

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

[PATCHv2 0/4] pre-commit hook for merges

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:38

In this second iteration I implement Junio's suggestion: 'git merge' invokes
the pre-commit hook when merge.usePreCommitHook is set to true.

1/4 documents that 'git merge' invokes the prepare-commit-msg hook
unconditionally already.

2-4 are v2 of the previous 1-3.

This leaves aside the issue of commit-msg and post-commit hooks. If we can live
with a bit of incompatibility I would rename the config to merge.useCommitHooks
and call all of them based on that.  This would change the way prepare-commit-msg is
treated now.

We could also introduce 4 config variables, 3 defaulting to false, 1 to true,
of course...

[I had messed up my alias file when adding mhagger, and it seems that tripped up
vger; resending, sorry.]

Michael J Gruber (4):
  merge: document prepare-commit-msg hook usage
  git-merge: Honor pre-commit hook based on config
  merge: --no-verify to bypass pre-commit hook
  t7503: add tests for pre-commit hook (merge)

 Documentation/git-merge.txt     |  7 ++++-
 Documentation/githooks.txt      |  6 ++++
 Documentation/merge-config.txt  |  5 ++++
 Documentation/merge-options.txt |  4 +++
 builtin/merge.c                 | 19 ++++++++++++-
 t/t7503-pre-commit-hook.sh      | 62 +++++++++++++++++++++++++++++++++++++++++
 6 files changed, 101 insertions(+), 2 deletions(-)

-- 
1.7.12.406.gafd3f81

[PATCHv2 1/4] merge: document prepare-commit-msg hook usage

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:38

Signed-off-by: Michael J Gruber <redacted>
---
 Documentation/git-merge.txt | 5 +++++
 Documentation/githooks.txt  | 2 ++
 2 files changed, 7 insertions(+)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 20f9228..b3ba8a8 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -308,6 +308,11 @@ branch.<name>.mergeoptions::
 	supported options are the same as those of 'git merge', but option
 	values containing whitespace characters are currently not supported.
 
+HOOKS
+-----
+This command can run the `prepare-commit-msg` hook.
+See linkgit:githooks[5] for more information.
+
 SEE ALSO
 --------
 linkgit:git-fmt-merge-msg[1], linkgit:git-pull[1],
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index b9003fe..b32134c 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -108,6 +108,8 @@ it is not suppressed by the `--no-verify` option.  A non-zero exit
 means a failure of the hook and aborts the commit.  It should not
 be used as replacement for pre-commit hook.
 
+This hook is also called by 'git merge' when it creates a new commit.
+
 The sample `prepare-commit-msg` hook that comes with git comments
 out the `Conflicts:` part of a merge's commit message.
 
-- 
1.7.12.406.gafd3f81

[PATCHv2 2/4] git-merge: Honor pre-commit hook based on config

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:38

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 merge.usePreCommitHook which controls whether an automatic
merge commit invokes pre-commit.

Signed-off-by: Michael J Gruber <redacted>
---
 Documentation/git-merge.txt    |  2 +-
 Documentation/githooks.txt     |  3 +++
 Documentation/merge-config.txt |  5 +++++
 builtin/merge.c                | 17 ++++++++++++++++-
 4 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index b3ba8a8..3501ae2 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -310,7 +310,7 @@ branch.<name>.mergeoptions::
 
 HOOKS
 -----
-This command can run the `prepare-commit-msg` hook.
+This command can run the `prepare-commit-msg` and `pre-commit` hooks.
 See linkgit:githooks[5] for more information.
 
 SEE ALSO
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index b32134c..d62e02d 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -86,6 +86,9 @@ 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.
 
+If the configuration option `merge.usePreCommitHook` is set to `true`
+then 'git merge' invokes this hook whenever it creates a new commit.
+
 prepare-commit-msg
 ~~~~~~~~~~~~~~~~~~
 
diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt
index 861bd6f..727caad 100644
--- a/Documentation/merge-config.txt
+++ b/Documentation/merge-config.txt
@@ -59,6 +59,11 @@ merge.tool::
 	and "xxdiff".  Any other value is treated is custom merge tool
 	and there must be a corresponding mergetool.<tool>.cmd option.
 
+merge.usePreCommitHook::
+	Controls whether the pre-commit hook is invoked when 'git merge'
+	creates a new commit (true merge without conflicts).  False by
+	default.
+
 merge.verbosity::
 	Controls the amount of output shown by the recursive merge
 	strategy.  Level 0 outputs nothing except a final error
diff --git a/builtin/merge.c b/builtin/merge.c
index 0ec8f0d..a2590a9 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -64,6 +64,7 @@ static int allow_rerere_auto;
 static int abort_current_merge;
 static int show_progress = -1;
 static int default_to_upstream;
+static int use_pre_commit_hook;
 static const char *sign_commit;
 
 static struct strategy all_strategy[] = {
@@ -575,6 +576,9 @@ static int git_merge_config(const char *k, const char *v, void *cb)
 	} else if (!strcmp(k, "merge.defaulttoupstream")) {
 		default_to_upstream = git_config_bool(k, v);
 		return 0;
+	} else if (!strcmp(k, "merge.useprecommithook")) {
+		use_pre_commit_hook = git_config_bool(k, v);
+		return 0;
 	}
 
 	status = fmt_merge_msg_config(k, v, cb);
@@ -898,12 +902,23 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 {
 	struct strbuf msg = STRBUF_INIT;
 	const char *comment = _(merge_editor_comment);
+	const char *index_file = get_index_file();
+
+	if (use_pre_commit_hook && run_hook(index_file, "pre-commit", NULL))
+		abort_commit(remoteheads, NULL);
+	/*
+	 * Re-read the index as pre-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.
+	 */
+	discard_cache();
+	read_cache_from(index_file);
 	strbuf_addbuf(&msg, &merge_msg);
 	strbuf_addch(&msg, '\n');
 	if (0 < option_edit)
 		strbuf_add_lines(&msg, "# ", comment, strlen(comment));
 	write_merge_msg(&msg);
-	run_hook(get_index_file(), "prepare-commit-msg",
+	run_hook(index_file, "prepare-commit-msg",
 		 git_path("MERGE_MSG"), "merge", NULL, NULL);
 	if (0 < option_edit) {
 		if (launch_editor(git_path("MERGE_MSG"), NULL, NULL))
-- 
1.7.12.406.gafd3f81

[PATCHv2 3/4] merge: --no-verify to bypass pre-commit hook

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:38

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

Signed-off-by: Michael J Gruber <redacted>
---
 Documentation/git-merge.txt     | 2 +-
 Documentation/githooks.txt      | 1 +
 Documentation/merge-options.txt | 4 ++++
 builtin/merge.c                 | 4 +++-
 4 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 3501ae2..363fbea 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
-	[-s <strategy>] [-X <strategy-option>]
+	[--no-verify] [-s <strategy>] [-X <strategy-option>]
 	[--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
 'git merge' <msg> HEAD <commit>...
 'git merge' --abort
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index d62e02d..c734e2c 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -88,6 +88,7 @@ to modify the commit message.
 
 If the configuration option `merge.usePreCommitHook` is set to `true`
 then 'git merge' invokes this hook whenever it creates a new commit.
+It can be bypassed with the `\--no-verify` option. 
 
 prepare-commit-msg
 ~~~~~~~~~~~~~~~~~~
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 0bcbe0a..5695fc6 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -70,6 +70,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-commit hook.
+	See also linkgit:githooks[5].
+
 -s <strategy>::
 --strategy=<strategy>::
 	Use the given merge strategy; can be supplied more than
diff --git a/builtin/merge.c b/builtin/merge.c
index a2590a9..58a848f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -60,6 +60,7 @@ static const char *branch;
 static char *branch_mergeoptions;
 static int option_renormalize;
 static int verbosity;
+static int no_verify;
 static int allow_rerere_auto;
 static int abort_current_merge;
 static int show_progress = -1;
@@ -199,6 +200,7 @@ static struct option builtin_merge_options[] = {
 		N_("allow fast-forward (default)")),
 	OPT_BOOLEAN(0, "ff-only", &fast_forward_only,
 		N_("abort if fast-forward is not possible")),
+	OPT_BOOLEAN(0, "no-verify", &no_verify, "bypass pre-merge hook"),
 	OPT_RERERE_AUTOUPDATE(&allow_rerere_auto),
 	OPT_CALLBACK('s', "strategy", &use_strategies, N_("strategy"),
 		N_("merge strategy to use"), option_parse_strategy),
@@ -904,7 +906,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 	const char *comment = _(merge_editor_comment);
 	const char *index_file = get_index_file();
 
-	if (use_pre_commit_hook && run_hook(index_file, "pre-commit", NULL))
+	if (use_pre_commit_hook && !no_verify && run_hook(index_file, "pre-commit", NULL))
 		abort_commit(remoteheads, NULL);
 	/*
 	 * Re-read the index as pre-commit hook could have updated it,
-- 
1.7.12.406.gafd3f81

[PATCHv2 4/4] t7503: add tests for pre-commit hook (merge)

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:38

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

Signed-off-by: Michael J Gruber <redacted>
---
 t/t7503-pre-commit-hook.sh | 62 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)
diff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-hook.sh
index 984889b..e4d324d 100755
--- a/t/t7503-pre-commit-hook.sh
+++ b/t/t7503-pre-commit-hook.sh
@@ -4,6 +4,19 @@ test_description='pre-commit hook'
 
 . ./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,6 +41,14 @@ 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"
@@ -29,6 +58,7 @@ cat > "$HOOK" <<EOF
 exit 0
 EOF
 chmod +x "$HOOK"
+git config merge.usePreCommitHook true
 
 test_expect_success 'with succeeding hook' '
 
@@ -38,6 +68,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,6 +84,14 @@ 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
@@ -68,6 +114,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' '
 
-- 
1.7.12.406.gafd3f81
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help