{"thread":{"id":"46808","subject":"[PATCH 2/4] merge: do no-verify like commit","startedAt":"2017-09-22T12:04:27Z","lastAt":"2019-08-08T13:08:23Z","messageCount":59,"participants":["Michael J Gruber","Stefan Beller","Martin Ågren","Junio C Hamano","Josh Steadmon"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"328631","messageId":"1ec69134cad991f01f6a48b57fbc7047cc226965.1506081120.git.git@grubix.eu","threadId":"46808","inReplyTo":"cover.1506081120.git.git@grubix.eu","subject":"[PATCH 2/4] merge: do no-verify like commit","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-09-22T12:04:13Z","receivedAt":"2017-09-22T12:04:27Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"f8b863598c (\"builtin/merge: honor commit-msg hook for merges\", 2017-09-07)\nintroduced the no-verify to merge for bypassing the commit-msg hook,\nthough in a different way from the implementation in commit.c.\n\nChange the implementation in merge.c to be the same as in merge.c so\nthat both do the same in the same way.\n\nSigned-off-by: Michael J Gruber \n---\n builtin/merge.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex de254d466b..7ba094ee87 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -73,7 +73,7 @@ static int show_progress = -1;\n static int default_to_upstream = 1;\n static int signoff;\n static const char *sign_commit;\n-static int verify_msg = 1;\n+static int no_verify;\n \n static struct strategy all_strategy[] = {\n \t{ \"recursive\", DEFAULT_TWOHEAD | NO_TRIVIAL },\n@@ -237,7 +237,7 @@ static struct option builtin_merge_options[] = {\n \t N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n \tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n-\tOPT_BOOL(0, \"verify\", &verify_msg, N_(\"verify commit-msg hook\")),\n+\tOPT_BOOL(0, \"no-verify\", &no_verify, N_(\"bypass commit-msg hook\")),\n \tOPT_END()\n };\n \n@@ -798,7 +798,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \t\t\tabort_commit(remoteheads, NULL);\n \t}\n \n-\tif (verify_msg && run_commit_hook(0 < option_edit, get_index_file(),\n+\tif (!no_verify && run_commit_hook(0 < option_edit, get_index_file(),\n \t\t\t\t\t \"commit-msg\",\n \t\t\t\t\t git_path_merge_msg(), NULL))\n \t\tabort_commit(remoteheads, NULL);\n-- \n2.14.1.909.g0fa57a0913\n\n"},{"id":"328632","messageId":"053ce92e978aff511667669682c9a71830008605.1506081120.git.git@grubix.eu","threadId":"46808","inReplyTo":"cover.1506081120.git.git@grubix.eu","subject":"[PATCH 1/4] git-merge: Honor pre-merge hook","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-09-22T12:04:12Z","receivedAt":"2017-09-22T12:04:29Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"From: Michael J Gruber \n\ngit-merge does not honor the pre-commit hook when doing automatic merge\ncommits, and for compatibility reasons this is going to stay.\n\nIntroduce a pre-merge hook which is called for an automatic merge commit\njust like pre-commit is called for a non-automatic merge commit (or any\nother commit).\n\nSigned-off-by: Michael J Gruber \n---\n Documentation/githooks.txt | 7 +++++++\n builtin/merge.c | 11 +++++++++++\n templates/hooks--pre-merge.sample | 13 +++++++++++++\n 3 files changed, 31 insertions(+)\n create mode 100755 templates/hooks--pre-merge.sample\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 1bb4f92d4d..85bedd208c 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -99,6 +99,13 @@ All the 'git commit' hooks are invoked with the environment\n variable `GIT_EDITOR=:` if the command will not bring up an editor\n to modify the commit message.\n \n+pre-merge\n+~~~~~~~~~\n+\n+This hook is invoked by 'git merge' when doing an automatic merge\n+commit; it is equivalent to 'pre-commit' for a non-automatic commit\n+for a merge.\n+\n prepare-commit-msg\n ~~~~~~~~~~~~~~~~~~\n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex ab5ffe85e8..de254d466b 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -769,6 +769,17 @@ static void write_merge_heads(struct commit_list *);\n static void prepare_to_commit(struct commit_list *remoteheads)\n {\n \tstruct strbuf msg = STRBUF_INIT;\n+\tconst char *index_file = get_index_file();\n+\n+\tif (run_commit_hook(0 < option_edit, index_file, \"pre-merge\", NULL))\n+\t\tabort_commit(remoteheads, NULL);\n+\t/*\n+\t * Re-read the index as pre-merge hook could have updated it,\n+\t * and write it out as a tree. We must do this before we invoke\n+\t * the editor and after we invoke run_status above.\n+\t */\n+\tdiscard_cache();\n+\tread_cache_from(index_file);\n \tstrbuf_addbuf(&msg, &merge_msg);\n \tstrbuf_addch(&msg, '\\n');\n \tif (squash)\ndiff --git a/templates/hooks--pre-merge.sample b/templates/hooks--pre-merge.sample\nnew file mode 100755\nindex 0000000000..a6313e6d5c\n--- /dev/null\n+++ b/templates/hooks--pre-merge.sample\n@@ -0,0 +1,13 @@\n+#!/bin/sh\n+#\n+# An example hook script to verify what is about to be committed.\n+# Called by \"git merge\" with no arguments. The hook should\n+# exit with non-zero status after issuing an appropriate message if\n+# it wants to stop the commit.\n+#\n+# To enable this hook, rename this file to \"pre-merge\".\n+\n+. git-sh-setup\n+test -x \"$GIT_DIR/hooks/pre-commit\" &&\n+ exec \"$GIT_DIR/hooks/pre-commit\"\n+:\n-- \n2.14.1.909.g0fa57a0913\n\n"},{"id":"328633","messageId":"bdc45cade9a8d108b7d11d66c0eecb27c2f23af6.1506081120.git.git@grubix.eu","threadId":"46808","inReplyTo":"cover.1506081120.git.git@grubix.eu","subject":"[PATCH 3/4] merge: --no-verify to bypass pre-merge hook","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-09-22T12:04:14Z","receivedAt":"2017-09-22T12:04:33Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"From: Michael J Gruber \n\nAnalogous to commit, introduce a '--no-verify' option which bypasses the\npre-merge hook. The shorthand '-n' is taken by the (non-existing)\n'--no-stat' already.\n\nSigned-off-by: Michael J Gruber \n---\n Documentation/git-merge.txt | 2 +-\n Documentation/githooks.txt | 2 +-\n Documentation/merge-options.txt | 4 ++++\n builtin/merge.c | 4 ++--\n 4 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\nindex 4df6431c34..5f0e1768e1 100644\n--- a/Documentation/git-merge.txt\n+++ b/Documentation/git-merge.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n --------\n [verse]\n 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]\n-\t[-s ] [-X ] [-S[]]\n+\t[--no-verify] [-s ] [-X ] [-S[]]\n \t[--[no-]allow-unrelated-histories]\n \t[--[no-]rerere-autoupdate] [-m ] [...]\n 'git merge' --abort\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 85bedd208c..969441d7a2 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -104,7 +104,7 @@ pre-merge\n \n This hook is invoked by 'git merge' when doing an automatic merge\n commit; it is equivalent to 'pre-commit' for a non-automatic commit\n-for a merge.\n+for a merge, and can be bypassed with the `--no-verify` option. \n \n prepare-commit-msg\n ~~~~~~~~~~~~~~~~~~\ndiff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt\nindex 4e32304301..75019d5919 100644\n--- a/Documentation/merge-options.txt\n+++ b/Documentation/merge-options.txt\n@@ -74,6 +74,10 @@ merge.\n With --no-squash perform the merge and commit the result. This\n option can be used to override --squash.\n \n+--no-verify::\n+\tThis option bypasses the pre-merge and commit-msg hooks.\n+\tSee also linkgit:githooks[5].\n+\n -s ::\n --strategy=::\n \tUse the given merge strategy; can be supplied more than\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 7ba094ee87..c63510c199 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -237,7 +237,7 @@ static struct option builtin_merge_options[] = {\n \t N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n \tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n-\tOPT_BOOL(0, \"no-verify\", &no_verify, N_(\"bypass commit-msg hook\")),\n+\tOPT_BOOL(0, \"no-verify\", &no_verify, N_(\"bypass pre-merge and commit-msg hooks\")),\n \tOPT_END()\n };\n \n@@ -771,7 +771,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \tstruct strbuf msg = STRBUF_INIT;\n \tconst char *index_file = get_index_file();\n \n-\tif (run_commit_hook(0 < option_edit, index_file, \"pre-merge\", NULL))\n+\tif (!no_verify && run_commit_hook(0 < option_edit, index_file, \"pre-merge\", NULL))\n \t\tabort_commit(remoteheads, NULL);\n \t/*\n \t * Re-read the index as pre-merge hook could have updated it,\n-- \n2.14.1.909.g0fa57a0913\n\n"},{"id":"328634","messageId":"381fe8facbbc1667d655572fcff5d6eb5f0e07fc.1506081120.git.git@grubix.eu","threadId":"46808","inReplyTo":"cover.1506081120.git.git@grubix.eu","subject":"[PATCH 4/4] t7503: add tests for pre-merge-hook","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-09-22T12:04:15Z","receivedAt":"2017-09-22T12:04:37Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"From: Michael J Gruber \n\nAdd tests which make sure that the pre-merge-hook is called when\npresent, allows/disallows merge commits depending on its return value\nand is suppressed by \"--no-verify\".\n\nSigned-off-by: Michael J Gruber \n---\n t/t7503-pre-commit-hook.sh | 66 +++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 65 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-hook.sh\nindex 984889b39d..36ae87f7ef 100755\n--- a/t/t7503-pre-commit-hook.sh\n+++ b/t/t7503-pre-commit-hook.sh\n@@ -1,9 +1,22 @@\n #!/bin/sh\n \n-test_description='pre-commit hook'\n+test_description='pre-commit and pre-merge hooks'\n \n . ./test-lib.sh\n \n+test_expect_success 'root commit' '\n+\n+\techo \"root\" > file &&\n+\tgit add file &&\n+\tgit commit -m \"zeroth\" &&\n+\tgit checkout -b side &&\n+\techo \"foo\" > foo &&\n+\tgit add foo &&\n+\tgit commit -m \"make it non-ff\" &&\n+\tgit checkout master\n+\n+'\n+\n test_expect_success 'with no hook' '\n \n \techo \"foo\" > file &&\n@@ -12,6 +25,14 @@ test_expect_success 'with no hook' '\n \n '\n \n+test_expect_success 'with no hook (merge)' '\n+\n+\tgit checkout side &&\n+\tgit merge -m \"merge master\" master &&\n+\tgit checkout master\n+\n+'\n+\n test_expect_success '--no-verify with no hook' '\n \n \techo \"bar\" > file &&\n@@ -20,15 +41,25 @@ test_expect_success '--no-verify with no hook' '\n \n '\n \n+test_expect_success '--no-verify with no hook (merge)' '\n+\n+\tgit checkout side &&\n+\tgit merge --no-verify -m \"merge master\" master &&\n+\tgit checkout master\n+\n+'\n+\n # now install hook that always succeeds\n HOOKDIR=\"$(git rev-parse --git-dir)/hooks\"\n HOOK=\"$HOOKDIR/pre-commit\"\n+MERGEHOOK=\"$HOOKDIR/pre-merge\"\n mkdir -p \"$HOOKDIR\"\n cat > \"$HOOK\" <> file &&\n@@ -46,11 +85,20 @@ test_expect_success '--no-verify with succeeding hook' '\n \n '\n \n+test_expect_success '--no-verify with succeeding hook (merge)' '\n+\n+\tgit checkout side &&\n+\tgit merge --no-verify -m \"merge master\" master &&\n+\tgit checkout master\n+\n+'\n+\n # now a hook that fails\n cat > \"$HOOK\" < wrote:\n> From: Michael J Gruber \n>\n> git-merge does not honor the pre-commit hook when doing automatic merge\n> commits, and for compatibility reasons this is going to stay.\n>\n> Introduce a pre-merge hook which is called for an automatic merge commit\n> just like pre-commit is called for a non-automatic merge commit (or any\n> other commit).\n>\n> Signed-off-by: Michael J Gruber \n> ---\n> Documentation/githooks.txt | 7 +++++++\n> builtin/merge.c | 11 +++++++++++\n> templates/hooks--pre-merge.sample | 13 +++++++++++++\n> 3 files changed, 31 insertions(+)\n> create mode 100755 templates/hooks--pre-merge.sample\n>\n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> index 1bb4f92d4d..85bedd208c 100644\n> --- a/Documentation/githooks.txt\n> +++ b/Documentation/githooks.txt\n> @@ -99,6 +99,13 @@ All the 'git commit' hooks are invoked with the environment\n> variable `GIT_EDITOR=:` if the command will not bring up an editor\n> to modify the commit message.\n>\n> +pre-merge\n> +~~~~~~~~~\n> +\n> +This hook is invoked by 'git merge' when doing an automatic merge\n> +commit; it is equivalent to 'pre-commit' for a non-automatic commit\n> +for a merge.\n> +\n> prepare-commit-msg\n> ~~~~~~~~~~~~~~~~~~\n>\n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index ab5ffe85e8..de254d466b 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -769,6 +769,17 @@ static void write_merge_heads(struct commit_list *);\n> static void prepare_to_commit(struct commit_list *remoteheads)\n> {\n> struct strbuf msg = STRBUF_INIT;\n> + const char *index_file = get_index_file();\n> +\n> + if (run_commit_hook(0 < option_edit, index_file, \"pre-merge\", NULL))\n> + abort_commit(remoteheads, NULL);\n> + /*\n> + * Re-read the index as pre-merge hook could have updated it,\n> + * and write it out as a tree. We must do this before we invoke\n> + * the editor and after we invoke run_status above.\n> + */\n> + discard_cache();\n> + read_cache_from(index_file);\n\nPlease see 680ee550d7 (commit: skip discarding the index\nif there is no pre-commit hook, 2017-08-14), maybe we can do it similarly.\nDropping and rereading the index may be expensive for large repos.\n\n> strbuf_addbuf(&msg, &merge_msg);\n> strbuf_addch(&msg, '\\n');\n> if (squash)\n> diff --git a/templates/hooks--pre-merge.sample b/templates/hooks--pre-merge.sample\n> new file mode 100755\n> index 0000000000..a6313e6d5c\n> --- /dev/null\n> +++ b/templates/hooks--pre-merge.sample\n> @@ -0,0 +1,13 @@\n> +#!/bin/sh\n> +#\n> +# An example hook script to verify what is about to be committed.\n> +# Called by \"git merge\" with no arguments. The hook should\n> +# exit with non-zero status after issuing an appropriate message if\n\nThe message goes to stdout or sterr or both?\n\n> +# it wants to stop the commit.\n\nnit: s/commit/merge commit/ maybe?\n\nThanks,\nStefan\n"},{"id":"328665","messageId":"CAGZ79kZFPG7C8Me8dq6kf17HubMs5hwnCriuWArWUhcap4ushQ@mail.gmail.com","threadId":"46808","inReplyTo":"1ec69134cad991f01f6a48b57fbc7047cc226965.1506081120.git.git@grubix.eu","subject":"Re: [PATCH 2/4] merge: do no-verify like commit","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-09-22T19:55:13Z","receivedAt":"2017-09-22T19:55:20Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Sep 22, 2017 at 5:04 AM, Michael J Gruber wrote:\n> f8b863598c (\"builtin/merge: honor commit-msg hook for merges\", 2017-09-07)\n> introduced the no-verify to merge for bypassing the commit-msg hook,\n> though in a different way from the implementation in commit.c.\n>\n> Change the implementation in merge.c to be the same as in merge.c so\n> that both do the same in the same way.\n>\n> Signed-off-by: Michael J Gruber \n\nThanks for spotting.\nJunio spotted it as well; I assumed I had reverted it in the last\nsent out (accepted) patch.\n\nIf we want to further the commit message:\n This changes the output of 'git merge --help' in a way that\n it suggests that the hook is on by default.\n\nThanks,\nStefan\n\n> ---\n> builtin/merge.c | 6 +++---\n> 1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index de254d466b..7ba094ee87 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -73,7 +73,7 @@ static int show_progress = -1;\n> static int default_to_upstream = 1;\n> static int signoff;\n> static const char *sign_commit;\n> -static int verify_msg = 1;\n> +static int no_verify;\n>\n> static struct strategy all_strategy[] = {\n> { \"recursive\", DEFAULT_TWOHEAD | NO_TRIVIAL },\n> @@ -237,7 +237,7 @@ static struct option builtin_merge_options[] = {\n> N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n> OPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n> OPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n> - OPT_BOOL(0, \"verify\", &verify_msg, N_(\"verify commit-msg hook\")),\n> + OPT_BOOL(0, \"no-verify\", &no_verify, N_(\"bypass commit-msg hook\")),\n> OPT_END()\n> };\n>\n> @@ -798,7 +798,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n> abort_commit(remoteheads, NULL);\n> }\n>\n> - if (verify_msg && run_commit_hook(0 < option_edit, get_index_file(),\n> + if (!no_verify && run_commit_hook(0 < option_edit, get_index_file(),\n> \"commit-msg\",\n> git_path_merge_msg(), NULL))\n> abort_commit(remoteheads, NULL);\n> --\n> 2.14.1.909.g0fa57a0913\n>\n"},{"id":"328666","messageId":"CAGZ79ka3Bejmv=hT2xNAzx0K-r7S8SaTKmgeW9MB1FqhrY31pA@mail.gmail.com","threadId":"46808","inReplyTo":"381fe8facbbc1667d655572fcff5d6eb5f0e07fc.1506081120.git.git@grubix.eu","subject":"Re: [PATCH 4/4] t7503: add tests for pre-merge-hook","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-09-22T20:01:31Z","receivedAt":"2017-09-22T20:01:40Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Sep 22, 2017 at 5:04 AM, Michael J Gruber wrote:\n\n> --- a/t/t7503-pre-commit-hook.sh\n> +++ b/t/t7503-pre-commit-hook.sh\n\n> -test_description='pre-commit hook'\n> +test_description='pre-commit and pre-merge hooks'\n\nTangent: do we also want to rename the file?\n\n> +test_expect_success '--no-verify with succeeding hook (merge)' '\n\nThe return value of the hook ought to not matter, so we'd rather want\nto test for 'no side effect by hook on --no-verify' ?\n\n\n> +test_expect_success '--no-verify with failing hook (merge)' '\n> +\n> + git checkout side &&\n> + git merge --no-verify -m \"merge master\" master &&\n> + git checkout master\n> +\n> +'\n\nI assume the same (unsolved) issues apply to this hook as\nto the commit-msg hook with 'git merge --continue'\nDo we want a test for that (I am torn on that) ?\n"},{"id":"328708","messageId":"CAN0heSo5AE=Z2yoZmtcvnPc_7h3wWn4yMrzB7kDXPyhB_CsLQw@mail.gmail.com","threadId":"46808","inReplyTo":"053ce92e978aff511667669682c9a71830008605.1506081120.git.git@grubix.eu","subject":"Re: [PATCH 1/4] git-merge: Honor pre-merge hook","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2017-09-23T00:04:53Z","receivedAt":"2017-09-23T00:04:59Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On 22 September 2017 at 14:04, Michael J Gruber wrote:\n> From: Michael J Gruber \n>\n> git-merge does not honor the pre-commit hook when doing automatic merge\n> commits, and for compatibility reasons this is going to stay.\n>\n> Introduce a pre-merge hook which is called for an automatic merge commit\n> just like pre-commit is called for a non-automatic merge commit (or any\n> other commit).\n>\n> Signed-off-by: Michael J Gruber \n[...]\n> diff --git a/templates/hooks--pre-merge.sample b/templates/hooks--pre-merge.sample\n> new file mode 100755\n> index 0000000000..a6313e6d5c\n> --- /dev/null\n> +++ b/templates/hooks--pre-merge.sample\n> @@ -0,0 +1,13 @@\n> +#!/bin/sh\n> +#\n> +# An example hook script to verify what is about to be committed.\n> +# Called by \"git merge\" with no arguments. The hook should\n> +# exit with non-zero status after issuing an appropriate message if\n> +# it wants to stop the commit.\n> +#\n> +# To enable this hook, rename this file to \"pre-merge\".\n> +\n> +. git-sh-setup\n> +test -x \"$GIT_DIR/hooks/pre-commit\" &&\n> + exec \"$GIT_DIR/hooks/pre-commit\"\n> +:\n\nWon't this always exit successfully? Is that wanted? If this sample hook\nis activated but can't find an executable pre-commit hook to call out\nto, should it complain in some helpful way rather than exiting silently\n(whether successfully or not)?\n\nMartin\n"},{"id":"328750","messageId":"xmqq60c9j7sx.fsf@gitster.mtv.corp.google.com","threadId":"46808","inReplyTo":"bdc45cade9a8d108b7d11d66c0eecb27c2f23af6.1506081120.git.git@grubix.eu","subject":"Re: [PATCH 3/4] merge: --no-verify to bypass pre-merge hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-09-23T23:48:14Z","receivedAt":"2017-09-23T23:49:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> From: Michael J Gruber \n>\n> Analogous to commit, introduce a '--no-verify' option which bypasses the\n> pre-merge hook. The shorthand '-n' is taken by the (non-existing)\n> '--no-stat' already.\n>\n> Signed-off-by: Michael J Gruber \n> ---\n\nIt appears that some of the pieces in this patch, namely,\nD/git-merge.txt and D/merge-options.txt, belong to 2/4, where we are\nfixing up an earlier addition of --[no-]verify option to the\ncommand, to be updated to only add mention of pre-merge hook in this\nstep?\n\nThe end result looks good regardless, I would think, though.\n"},{"id":"328841","messageId":"b5aa26de-46db-271e-e86e-bc9ba9c67fb6@grubix.eu","threadId":"46808","inReplyTo":"xmqq60c9j7sx.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/4] merge: --no-verify to bypass pre-merge hook","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2017-09-25T10:54:22Z","receivedAt":"2017-09-25T10:54:35Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 24.09.2017 01:48:\n> Michael J Gruber writes:\n> \n>> From: Michael J Gruber \n>>\n>> Analogous to commit, introduce a '--no-verify' option which bypasses the\n>> pre-merge hook. The shorthand '-n' is taken by the (non-existing)\n>> '--no-stat' already.\n>>\n>> Signed-off-by: Michael J Gruber \n>> ---\n> \n> It appears that some of the pieces in this patch, namely,\n> D/git-merge.txt and D/merge-options.txt, belong to 2/4, where we are\n> fixing up an earlier addition of --[no-]verify option to the\n> command, to be updated to only add mention of pre-merge hook in this\n> step?\n\nOh, sorry, rebaser error. I should also rewrite all commits to my\ncurrent e-mail.\n\n> The end result looks good regardless, I would think, though.\n\nPending restructuring and attending to the other comments...\n"},{"id":"329409","messageId":"xmqqfub2p014.fsf@gitster.mtv.corp.google.com","threadId":"46808","inReplyTo":"cover.1506081120.git.git@grubix.eu","subject":"Re: [PATCH 0/4] pre-merge hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-02T05:54:31Z","receivedAt":"2017-10-02T05:54:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Now that f8b863598c (\"builtin/merge: honor commit-msg hook for merges\",\n> 2017-09-07) has landed, merge is getting closer to behaving like commit,\n> which is important because both are used to complete merges (automatic\n> vs. non-automatic).\n\nJust a gentle ping to the thread to see if it is still alive.\n\nI think people agree that this is a good thing to do, but it seems\nthat the execution leaves a few loose ends, judging from the review\ncomments that have yet to be answered.\n\nThanks.\n"},{"id":"329462","messageId":"CAGZ79kaf9Me+zR+ij4h=0tzULU1qmB+rbjVkK3naXyvn+WTq8w@mail.gmail.com","threadId":"46808","inReplyTo":"xmqqfub2p014.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/4] pre-merge hook","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-10-02T16:59:43Z","receivedAt":"2017-10-02T16:59:49Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sun, Oct 1, 2017 at 10:54 PM, Junio C Hamano wrote:\n> Michael J Gruber writes:\n>\n>> Now that f8b863598c (\"builtin/merge: honor commit-msg hook for merges\",\n>> 2017-09-07) has landed, merge is getting closer to behaving like commit,\n>> which is important because both are used to complete merges (automatic\n>> vs. non-automatic).\n>\n> Just a gentle ping to the thread to see if it is still alive.\n>\n> I think people agree that this is a good thing to do, but it seems\n> that the execution leaves a few loose ends, judging from the review\n> comments that have yet to be answered.\n>\n> Thanks.\n\nI agree. IIRC the series was ok in design and goal, just minor comments\nfor coding.\n\nThanks,\nStefan\n"},{"id":"330514","messageId":"xmqqbml6pgfm.fsf@gitster.mtv.corp.google.com","threadId":"46808","inReplyTo":"cover.1506081120.git.git@grubix.eu","subject":"Re: [PATCH 0/4] pre-merge hook","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-17T04:05:33Z","receivedAt":"2017-10-17T04:05:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> This series revives an old suggestion of mine to make merge honor\n> pre-commit hook or a separate pre-merge hook....\n\nThis seems to have become an abandoned loose end, so I'll drop the\ntopic from my tree for now; revival of the discussion is _not_\nunwelcome (aka \"dropping without prejudice\").\n\n\n"},{"id":"379084","messageId":"cover.1563489311.git.steadmon@google.com","threadId":"46808","inReplyTo":"xmqqbml6pgfm.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v2 0/4] pre-merge hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-07-18T22:57:36Z","receivedAt":"2019-07-18T22:57:45Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"I would like to revive discussion on this series; I have addressed most\nof the review comments on the original series sent by Michael, with the\nfollowing exceptions:\n\n* Martin's objection on 1/4 that the sample hook would always exit\n successfully is (I believe) incorrect. To test this, I ran\n \"/bin/true && exec test 0 == 1\" in a /bin/sh subshell, and it\n correctly had a non-zero exit status.\n\n* I'm not sure that I understand Stefan's objections on 4/4 regarding\n the hook's exit status. I am also not familiar with \"the same\n (unsolved) issues [as] commit-msg\".\n\nI have noted my changes in each commit message in \"[js: ...]\" sections.\n\nMichael's original series description is below:\n\nNow that f8b863598c (\"builtin/merge: honor commit-msg hook for merges\",\n2017-09-07) has landed, merge is getting closer to behaving like commit,\nwhich is important because both are used to complete merges (automatic\nvs. non-automatic).\n\nThis series revives an old suggestion of mine to make merge honor\npre-commit hook or a separate pre-merge hook. Since pre-commit does not\nreceive any arguments (which the hook could use tell between commit and\nmerge) and in order to keep existing behaviour (as opposed to the other\npatch) this series introduces a pre-merge hook, with a sample hook that\nsimply calls the pre-commit hook.\n\ncommit and merge have very similar code paths. f8b863598c implemented\n\"--no-verify\" for merge differently from the existing implementation in\ncommit. 2/4 changes that and could be first in this series, with the\nremaining 3 squashed into 2 commits or 1. I've been rebasing and using\nthis series for years now, 2/4 is the new-comer which fixed the breakage\nfrom f8b863598c that I encountered because of the no-verify\nimplementation.\n\nMichael J Gruber (4):\n git-merge: Honor pre-merge hook\n merge: do no-verify like commit\n merge: --no-verify to bypass pre-merge hook\n t7503: add tests for pre-merge-hook\n\n Documentation/git-merge.txt | 2 +-\n Documentation/githooks.txt | 7 ++\n Documentation/merge-options.txt | 4 ++\n builtin/merge.c | 18 ++++-\n ...> t7503-pre-commit-and-pre-merge-hooks.sh} | 66 ++++++++++++++++++-\n templates/hooks--pre-merge.sample | 13 ++++\n 6 files changed, 105 insertions(+), 5 deletions(-)\n rename t/{t7503-pre-commit-hook.sh => t7503-pre-commit-and-pre-merge-hooks.sh} (67%)\n create mode 100755 templates/hooks--pre-merge.sample\n\n-- \n2.22.0.657.g960e92d24f-goog\n\n"},{"id":"379085","messageId":"e8b3fd8a5bf8f77795454a901fecaa585701eeb2.1563490164.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1563490164.git.steadmon@google.com","subject":"[PATCH v2 1/4] git-merge: Honor pre-merge hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-07-18T22:57:37Z","receivedAt":"2019-07-18T22:57:47Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"From: Michael J Gruber \n\ngit-merge does not honor the pre-commit hook when doing automatic merge\ncommits, and for compatibility reasons this is going to stay.\n\nIntroduce a pre-merge hook which is called for an automatic merge commit\njust like pre-commit is called for a non-automatic merge commit (or any\nother commit).\n\n[js: addressed review comments:\n * clarified that hook should write messages to stderr\n * only discard the index if the hook is actually present\n]\n\nSigned-off-by: Michael J Gruber \nSigned-off-by: Josh Steadmon \n---\n Documentation/githooks.txt | 7 +++++++\n builtin/merge.c | 12 ++++++++++++\n templates/hooks--pre-merge.sample | 13 +++++++++++++\n 3 files changed, 32 insertions(+)\n create mode 100755 templates/hooks--pre-merge.sample\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 786e778ab8..dcc6606d44 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -103,6 +103,13 @@ The default 'pre-commit' hook, when enabled--and with the\n `hooks.allownonascii` config option unset or set to false--prevents\n the use of non-ASCII filenames.\n \n+pre-merge\n+~~~~~~~~~\n+\n+This hook is invoked by 'git merge' when doing an automatic merge\n+commit; it is equivalent to 'pre-commit' for a non-automatic commit\n+for a merge.\n+\n prepare-commit-msg\n ~~~~~~~~~~~~~~~~~~\n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 6e99aead46..5cd7752191 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -816,6 +816,18 @@ static void write_merge_heads(struct commit_list *);\n static void prepare_to_commit(struct commit_list *remoteheads)\n {\n \tstruct strbuf msg = STRBUF_INIT;\n+\tconst char *index_file = get_index_file();\n+\n+\tif (run_commit_hook(0 < option_edit, index_file, \"pre-merge\", NULL))\n+\t\tabort_commit(remoteheads, NULL);\n+\t/*\n+\t * Re-read the index as pre-merge hook could have updated it,\n+\t * and write it out as a tree. We must do this before we invoke\n+\t * the editor and after we invoke run_status above.\n+\t */\n+\tif (find_hook(\"pre-merge\"))\n+\t\tdiscard_cache();\n+\tread_cache_from(index_file);\n \tstrbuf_addbuf(&msg, &merge_msg);\n \tif (squash)\n \t\tBUG(\"the control must not reach here under --squash\");\ndiff --git a/templates/hooks--pre-merge.sample b/templates/hooks--pre-merge.sample\nnew file mode 100755\nindex 0000000000..f459b3db44\n--- /dev/null\n+++ b/templates/hooks--pre-merge.sample\n@@ -0,0 +1,13 @@\n+#!/bin/sh\n+#\n+# An example hook script to verify what is about to be committed.\n+# Called by \"git merge\" with no arguments. The hook should\n+# exit with non-zero status after issuing an appropriate message to\n+# stderr if it wants to stop the merge commit.\n+#\n+# To enable this hook, rename this file to \"pre-merge\".\n+\n+. git-sh-setup\n+test -x \"$GIT_DIR/hooks/pre-commit\" &&\n+ exec \"$GIT_DIR/hooks/pre-commit\"\n+:\n-- \n2.22.0.657.g960e92d24f-goog\n\n"},{"id":"379086","messageId":"36406a85beaf63d356fa9818463f73140acf1ec9.1563490164.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1563490164.git.steadmon@google.com","subject":"[PATCH v2 2/4] merge: do no-verify like commit","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-07-18T22:57:38Z","receivedAt":"2019-07-18T22:57:50Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"From: Michael J Gruber \n\nf8b863598c (\"builtin/merge: honor commit-msg hook for merges\", 2017-09-07)\nintroduced the no-verify flag to merge for bypassing the commit-msg\nhook, though in a different way from the implementation in commit.c.\n\nChange the implementation in merge.c to be the same as in commit.c so\nthat both do the same in the same way. This also changes the output of\n\"git merge --help\" to be more clear that the hook return code is\nrespected by default.\n\n[js: reworded commit message, and moved documentation changes from patch\n 3/4 to this commit.]\n\nSigned-off-by: Michael J Gruber \nSigned-off-by: Josh Steadmon \n---\n Documentation/git-merge.txt | 2 +-\n Documentation/merge-options.txt | 4 ++++\n builtin/merge.c | 6 +++---\n 3 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\nindex c01cfa6595..f71ed1d0f9 100644\n--- a/Documentation/git-merge.txt\n+++ b/Documentation/git-merge.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n --------\n [verse]\n 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]\n-\t[-s ] [-X ] [-S[]]\n+\t[--no-verify] [-s ] [-X ] [-S[]]\n \t[--[no-]allow-unrelated-histories]\n \t[--[no-]rerere-autoupdate] [-m ] [-F ] [...]\n 'git merge' --abort\ndiff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt\nindex 79a00d2a4a..d6a9f4b96f 100644\n--- a/Documentation/merge-options.txt\n+++ b/Documentation/merge-options.txt\n@@ -105,6 +105,10 @@ option can be used to override --squash.\n +\n With --squash, --commit is not allowed, and will fail.\n \n+--no-verify::\n+\tThis option bypasses the pre-merge and commit-msg hooks.\n+\tSee also linkgit:githooks[5].\n+\n -s ::\n --strategy=::\n \tUse the given merge strategy; can be supplied more than\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 5cd7752191..5bbef203f3 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -81,7 +81,7 @@ static int show_progress = -1;\n static int default_to_upstream = 1;\n static int signoff;\n static const char *sign_commit;\n-static int verify_msg = 1;\n+static int no_verify;\n \n static struct strategy all_strategy[] = {\n \t{ \"recursive\", DEFAULT_TWOHEAD | NO_TRIVIAL },\n@@ -287,7 +287,7 @@ static struct option builtin_merge_options[] = {\n \t N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n \tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n-\tOPT_BOOL(0, \"verify\", &verify_msg, N_(\"verify commit-msg hook\")),\n+\tOPT_BOOL(0, \"no-verify\", &no_verify, N_(\"bypass commit-msg hook\")),\n \tOPT_END()\n };\n \n@@ -854,7 +854,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \t\t\tabort_commit(remoteheads, NULL);\n \t}\n \n-\tif (verify_msg && run_commit_hook(0 < option_edit, get_index_file(),\n+\tif (!no_verify && run_commit_hook(0 < option_edit, get_index_file(),\n \t\t\t\t\t \"commit-msg\",\n \t\t\t\t\t git_path_merge_msg(the_repository), NULL))\n \t\tabort_commit(remoteheads, NULL);\n-- \n2.22.0.657.g960e92d24f-goog\n\n"},{"id":"379087","messageId":"2440ad35e420db868596435dea3d634e37dbe671.1563490164.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1563490164.git.steadmon@google.com","subject":"[PATCH v2 3/4] merge: --no-verify to bypass pre-merge hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-07-18T22:57:39Z","receivedAt":"2019-07-18T22:57:53Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"From: Michael J Gruber \n\nAnalogous to commit, introduce a '--no-verify' option which bypasses the\npre-merge hook. The shorthand '-n' is taken by the (non-existing)\n'--no-stat' already.\n\n[js: cleaned up trailing whitespace, moved some documentation changes\n from this commit to 2/4.]\n\nSigned-off-by: Michael J Gruber \nSigned-off-by: Josh Steadmon \n---\n Documentation/githooks.txt | 2 +-\n builtin/merge.c | 4 ++--\n 2 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex dcc6606d44..6a93478dcf 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -108,7 +108,7 @@ pre-merge\n \n This hook is invoked by 'git merge' when doing an automatic merge\n commit; it is equivalent to 'pre-commit' for a non-automatic commit\n-for a merge.\n+for a merge, and can be bypassed with the `--no-verify` option.\n \n prepare-commit-msg\n ~~~~~~~~~~~~~~~~~~\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 5bbef203f3..5ed3472c84 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -287,7 +287,7 @@ static struct option builtin_merge_options[] = {\n \t N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n \tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n-\tOPT_BOOL(0, \"no-verify\", &no_verify, N_(\"bypass commit-msg hook\")),\n+\tOPT_BOOL(0, \"no-verify\", &no_verify, N_(\"bypass pre-merge and commit-msg hooks\")),\n \tOPT_END()\n };\n \n@@ -818,7 +818,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \tstruct strbuf msg = STRBUF_INIT;\n \tconst char *index_file = get_index_file();\n \n-\tif (run_commit_hook(0 < option_edit, index_file, \"pre-merge\", NULL))\n+\tif (!no_verify && run_commit_hook(0 < option_edit, index_file, \"pre-merge\", NULL))\n \t\tabort_commit(remoteheads, NULL);\n \t/*\n \t * Re-read the index as pre-merge hook could have updated it,\n-- \n2.22.0.657.g960e92d24f-goog\n\n"},{"id":"379088","messageId":"69dc3696e715f9be9e545e0142244e16bdd489cc.1563490164.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1563490164.git.steadmon@google.com","subject":"[PATCH v2 4/4] t7503: add tests for pre-merge-hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-07-18T22:57:40Z","receivedAt":"2019-07-18T22:57:55Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"From: Michael J Gruber \n\nAdd tests which make sure that the pre-merge-hook is called when\npresent, allows/disallows merge commits depending on its return value\nand is suppressed by \"--no-verify\".\n\n[js: renamed test as suggested in review comments]\n\nSigned-off-by: Michael J Gruber \nSigned-off-by: Josh Steadmon \n---\n ...> t7503-pre-commit-and-pre-merge-hooks.sh} | 66 ++++++++++++++++++-\n 1 file changed, 65 insertions(+), 1 deletion(-)\n rename t/{t7503-pre-commit-hook.sh => t7503-pre-commit-and-pre-merge-hooks.sh} (67%)\n\ndiff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-and-pre-merge-hooks.sh\nsimilarity index 67%\nrename from t/t7503-pre-commit-hook.sh\nrename to t/t7503-pre-commit-and-pre-merge-hooks.sh\nindex 984889b39d..36ae87f7ef 100755\n--- a/t/t7503-pre-commit-hook.sh\n+++ b/t/t7503-pre-commit-and-pre-merge-hooks.sh\n@@ -1,9 +1,22 @@\n #!/bin/sh\n \n-test_description='pre-commit hook'\n+test_description='pre-commit and pre-merge hooks'\n \n . ./test-lib.sh\n \n+test_expect_success 'root commit' '\n+\n+\techo \"root\" > file &&\n+\tgit add file &&\n+\tgit commit -m \"zeroth\" &&\n+\tgit checkout -b side &&\n+\techo \"foo\" > foo &&\n+\tgit add foo &&\n+\tgit commit -m \"make it non-ff\" &&\n+\tgit checkout master\n+\n+'\n+\n test_expect_success 'with no hook' '\n \n \techo \"foo\" > file &&\n@@ -12,6 +25,14 @@ test_expect_success 'with no hook' '\n \n '\n \n+test_expect_success 'with no hook (merge)' '\n+\n+\tgit checkout side &&\n+\tgit merge -m \"merge master\" master &&\n+\tgit checkout master\n+\n+'\n+\n test_expect_success '--no-verify with no hook' '\n \n \techo \"bar\" > file &&\n@@ -20,15 +41,25 @@ test_expect_success '--no-verify with no hook' '\n \n '\n \n+test_expect_success '--no-verify with no hook (merge)' '\n+\n+\tgit checkout side &&\n+\tgit merge --no-verify -m \"merge master\" master &&\n+\tgit checkout master\n+\n+'\n+\n # now install hook that always succeeds\n HOOKDIR=\"$(git rev-parse --git-dir)/hooks\"\n HOOK=\"$HOOKDIR/pre-commit\"\n+MERGEHOOK=\"$HOOKDIR/pre-merge\"\n mkdir -p \"$HOOKDIR\"\n cat > \"$HOOK\" <> file &&\n@@ -46,11 +85,20 @@ test_expect_success '--no-verify with succeeding hook' '\n \n '\n \n+test_expect_success '--no-verify with succeeding hook (merge)' '\n+\n+\tgit checkout side &&\n+\tgit merge --no-verify -m \"merge master\" master &&\n+\tgit checkout master\n+\n+'\n+\n # now a hook that fails\n cat > \"$HOOK\" < t7503-pre-commit-and-pre-merge-hooks.sh} | 66 ++++++++++++++++++-\n templates/hooks--pre-merge.sample | 13 ++++\n 6 files changed, 105 insertions(+), 5 deletions(-)\n rename t/{t7503-pre-commit-hook.sh => t7503-pre-commit-and-pre-merge-hooks.sh} (67%)\n create mode 100755 templates/hooks--pre-merge.sample\n\n-- \n2.22.0.657.g960e92d24f-goog\n\n"},{"id":"379517","messageId":"CAN0heSpi3rYAR5XmT4FM_YB746JjYHxmQHwASk4778saBKoUUA@mail.gmail.com","threadId":"46808","inReplyTo":"e8b3fd8a5bf8f77795454a901fecaa585701eeb2.1563490164.git.steadmon@google.com","subject":"Re: [PATCH v2 1/4] git-merge: Honor pre-merge hook","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2019-07-29T20:00:32Z","receivedAt":"2019-07-29T20:00:53Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Fri, 19 Jul 2019 at 00:57, Josh Steadmon wrote:\n> --- /dev/null\n> +++ b/templates/hooks--pre-merge.sample\n> @@ -0,0 +1,13 @@\n> +#!/bin/sh\n> +#\n> +# An example hook script to verify what is about to be committed.\n> +# Called by \"git merge\" with no arguments. The hook should\n> +# exit with non-zero status after issuing an appropriate message to\n> +# stderr if it wants to stop the merge commit.\n> +#\n> +# To enable this hook, rename this file to \"pre-merge\".\n> +\n> +. git-sh-setup\n> +test -x \"$GIT_DIR/hooks/pre-commit\" &&\n> + exec \"$GIT_DIR/hooks/pre-commit\"\n> +:\n\nI'm pretty certain that my comment here was with respect to the \":\",\nwhich made me think \"hmmm, won't : always exit successfully?\". My\nslightly less ignorant self of today answers \"sure, but we won't always\nget that far\" -- the \"exec\" is pretty important, and it will cause the\ncurrent shell to be replaced with the hook invocation. So however the\npre-commit hook decides to exit, that'll be our exit status.\n\nI retract my comment from back then.\n\n\nMartin\n"},{"id":"379519","messageId":"CAN0heSqNEgFXLOTBssW6wSnqqvWDGfOU+TW7_5T2V6t+GCAm5A@mail.gmail.com","threadId":"46808","inReplyTo":"2440ad35e420db868596435dea3d634e37dbe671.1563490164.git.steadmon@google.com","subject":"Re: [PATCH v2 3/4] merge: --no-verify to bypass pre-merge hook","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2019-07-29T20:02:49Z","receivedAt":"2019-07-29T20:03:04Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Fri, 19 Jul 2019 at 00:57, Josh Steadmon wrote:\n> From: Michael J Gruber \n>\n> Analogous to commit, introduce a '--no-verify' option which bypasses the\n> pre-merge hook. The shorthand '-n' is taken by the (non-existing)\n> '--no-stat' already.\n\nI don't understand this \"(non-existing)\". I realize this message is two\nyears old, maybe lots older still, and I haven't dug. But at least\n*now*, \"-n\" seems to behave like \"--no-stat\", suppressing \"--stat\", from\nmy simple testing.\n\n\nMartin\n"},{"id":"379520","messageId":"CAN0heSoU7XUN8GOFzzBrLG0RbY65hhYUn_-_Ax1DfFsNZ64hpA@mail.gmail.com","threadId":"46808","inReplyTo":"69dc3696e715f9be9e545e0142244e16bdd489cc.1563490164.git.steadmon@google.com","subject":"Re: [PATCH v2 4/4] t7503: add tests for pre-merge-hook","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2019-07-29T20:04:49Z","receivedAt":"2019-07-29T20:05:04Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Fri, 19 Jul 2019 at 00:57, Josh Steadmon wrote:\n> +test_expect_success '--no-verify with succeeding hook (merge)' '\n> +\n> + git checkout side &&\n> + git merge --no-verify -m \"merge master\" master &&\n> + git checkout master\n> +\n> +'\n\nThis test doesn't even try to verify that the hook was actually ignored.\nThat left me puzzled for a while...\n\n> +test_expect_success '--no-verify with failing hook (merge)' '\n> +\n> + git checkout side &&\n> + git merge --no-verify -m \"merge master\" master &&\n> + git checkout master\n> +\n> +'\n\n... but this would then (most likely) fail, so we would notice\nsomething's wrong.\n\nThis script seems to me like if it passes 100%, we can be fairly sure\nwe're ok, but if some individual test fails, we shouldn't be surprised\nif its oneline description is a bit off compared to the bug. Similarly,\nquite a few tests could pass, despite their oneline description inducing\nthe thought of \"but surely, if /that/ were the problem, /those/ tests\nwould fail\".\n\nAnyway, I realize this is just following the existing approach. I guess\nyou could argue it has served us well for a long time.\n\nI would probably prefer seeing the various hunks in this patch being\nsquashed into the relevant commits (1/4 vs 3/4) to make those patches\nmore self-describing.\n\n\nMartin\n"},{"id":"379545","messageId":"CAN0heSppzb9MCcR2296y1FFBoYyOs16e73DFhgV1bjkkxemRww@mail.gmail.com","threadId":"46808","inReplyTo":"cover.1563490164.git.steadmon@google.com","subject":"Re: [PATCH v2 0/4] pre-merge hook","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2019-07-29T20:09:07Z","receivedAt":"2019-07-29T20:09:22Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Fri, 19 Jul 2019 at 01:56, Josh Steadmon wrote:\n> * Martin's objection on 1/4 that the sample hook would always exit\n> successfully is (I believe) incorrect. To test this, I ran\n> \"/bin/true && exec test 0 == 1\" in a /bin/sh subshell, and it\n> correctly had a non-zero exit status.\n\nI retract my comment on this. It was incorrect, indeed.\n\n> git-merge: Honor pre-merge hook\n\nNit: s/H/h/\n\n> merge: do no-verify like commit\n\nNit (maybe just me): this could be patch 1/N, before getting started\nwith the real focus of this series.\n\n\nMartin\n"},{"id":"379549","messageId":"CAN0heSr4N-+fjn3+58oUvvv57vDXtA2tR-NEEyoS4toj36V2-Q@mail.gmail.com","threadId":"46808","inReplyTo":"cover.1563490164.git.steadmon@google.com","subject":"Re: [PATCH v2 0/4] pre-merge hook","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2019-07-29T20:29:07Z","receivedAt":"2019-07-29T20:29:21Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Fri, 19 Jul 2019 at 01:56, Josh Steadmon wrote:\n> This series revives an old suggestion [...] to make merge honor\n> pre-commit hook or a separate pre-merge hook. Since pre-commit does not\n> receive any arguments (which the hook could use tell between commit and\n> merge) and in order to keep existing behaviour (as opposed to the other\n> patch) this series introduces a pre-merge hook, with a sample hook that\n> simply calls the pre-commit hook.\n\nArgh, I wanted to comment on this in the cover letter, but forgot and\njust left a bunch of nits.\n\nI wonder if we might ever regret naming this \"pre-merge\" and not, e.g.,\n\"pre-merge-commit\". There's the pre-rebase hook which runs before\ngit-rebase even starts actually rebasing, so I could well imagine a\n\"pre-merge\" hook which would get called before merging starts.\n\nI'm also pondering whether there should be an \"automatic\" in there, but\n\"pre-automatic-merge-commit\" looks a bit awkward. Anyway, I'm not even\nsure an \"automatic merge commit\" is a well-defined thing. I can figure\nout what it pretty certainly means, but it's not crystal clear. There\nmight be a need for some more discussion in the added docs for what this\nnew hook is for and how it compares to pre-commit. Right now, the\nproposed docs suggests they're equivalent in a way, but I think that's a\nbit confusing -- they are certainly not synonyms, and they'd never both\nbe called, for example. They can be used for the same purpose in\ndifferent scenarios, sure.\n\nI tried coming up with some proposed docs for githooks.txt, but didn't\nfeel I achieved much of value. :-/\n\nMartin\n"},{"id":"379561","messageId":"20190729232955.GG43313@google.com","threadId":"46808","inReplyTo":"CAN0heSppzb9MCcR2296y1FFBoYyOs16e73DFhgV1bjkkxemRww@mail.gmail.com","subject":"Re: [PATCH v2 0/4] pre-merge hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-07-29T23:29:55Z","receivedAt":"2019-07-29T23:30:09Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2019.07.29 22:09, Martin Ågren wrote:\n> On Fri, 19 Jul 2019 at 01:56, Josh Steadmon wrote:\n> > * Martin's objection on 1/4 that the sample hook would always exit\n> > successfully is (I believe) incorrect. To test this, I ran\n> > \"/bin/true && exec test 0 == 1\" in a /bin/sh subshell, and it\n> > correctly had a non-zero exit status.\n> \n> I retract my comment on this. It was incorrect, indeed.\n> \n> > git-merge: Honor pre-merge hook\n> \n> Nit: s/H/h/\n\nFixed in V3.\n\n> > merge: do no-verify like commit\n> \n> Nit (maybe just me): this could be patch 1/N, before getting started\n> with the real focus of this series.\n\nAgreed, I found the ordering strange as well, but wanted to keep it the\nsame as V1 to start with. Fixed in V3.\n\n> \n> \n> Martin\n"},{"id":"379562","messageId":"20190729233349.GH43313@google.com","threadId":"46808","inReplyTo":"CAN0heSqNEgFXLOTBssW6wSnqqvWDGfOU+TW7_5T2V6t+GCAm5A@mail.gmail.com","subject":"Re: [PATCH v2 3/4] merge: --no-verify to bypass pre-merge hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-07-29T23:33:49Z","receivedAt":"2019-07-29T23:33:58Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2019.07.29 22:02, Martin Ågren wrote:\n> On Fri, 19 Jul 2019 at 00:57, Josh Steadmon wrote:\n> > From: Michael J Gruber \n> >\n> > Analogous to commit, introduce a '--no-verify' option which bypasses the\n> > pre-merge hook. The shorthand '-n' is taken by the (non-existing)\n> > '--no-stat' already.\n> \n> I don't understand this \"(non-existing)\". I realize this message is two\n> years old, maybe lots older still, and I haven't dug. But at least\n> *now*, \"-n\" seems to behave like \"--no-stat\", suppressing \"--stat\", from\n> my simple testing.\n\nFixed in V3.\n"},{"id":"379563","messageId":"20190729233911.GI43313@google.com","threadId":"46808","inReplyTo":"CAN0heSr4N-+fjn3+58oUvvv57vDXtA2tR-NEEyoS4toj36V2-Q@mail.gmail.com","subject":"Re: [PATCH v2 0/4] pre-merge hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-07-29T23:39:11Z","receivedAt":"2019-07-29T23:39:19Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2019.07.29 22:29, Martin Ågren wrote:\n> On Fri, 19 Jul 2019 at 01:56, Josh Steadmon wrote:\n> > This series revives an old suggestion [...] to make merge honor\n> > pre-commit hook or a separate pre-merge hook. Since pre-commit does not\n> > receive any arguments (which the hook could use tell between commit and\n> > merge) and in order to keep existing behaviour (as opposed to the other\n> > patch) this series introduces a pre-merge hook, with a sample hook that\n> > simply calls the pre-commit hook.\n> \n> Argh, I wanted to comment on this in the cover letter, but forgot and\n> just left a bunch of nits.\n> \n> I wonder if we might ever regret naming this \"pre-merge\" and not, e.g.,\n> \"pre-merge-commit\". There's the pre-rebase hook which runs before\n> git-rebase even starts actually rebasing, so I could well imagine a\n> \"pre-merge\" hook which would get called before merging starts.\n\n\"pre-merge-commit\" sounds reasonable to me. I'll wait for a bit before\nsending out V3 in case anyone else wants to chime in here though.\n\n> I'm also pondering whether there should be an \"automatic\" in there, but\n> \"pre-automatic-merge-commit\" looks a bit awkward. Anyway, I'm not even\n> sure an \"automatic merge commit\" is a well-defined thing. I can figure\n> out what it pretty certainly means, but it's not crystal clear. There\n> might be a need for some more discussion in the added docs for what this\n> new hook is for and how it compares to pre-commit. Right now, the\n> proposed docs suggests they're equivalent in a way, but I think that's a\n> bit confusing -- they are certainly not synonyms, and they'd never both\n> be called, for example. They can be used for the same purpose in\n> different scenarios, sure.\n> \n> I tried coming up with some proposed docs for githooks.txt, but didn't\n> feel I achieved much of value. :-/\n> \n> Martin\n"},{"id":"379564","messageId":"20190729234319.GJ43313@google.com","threadId":"46808","inReplyTo":"CAN0heSoU7XUN8GOFzzBrLG0RbY65hhYUn_-_Ax1DfFsNZ64hpA@mail.gmail.com","subject":"Re: [PATCH v2 4/4] t7503: add tests for pre-merge-hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-07-29T23:43:19Z","receivedAt":"2019-07-29T23:43:28Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2019.07.29 22:04, Martin Ågren wrote:\n> On Fri, 19 Jul 2019 at 00:57, Josh Steadmon wrote:\n> > +test_expect_success '--no-verify with succeeding hook (merge)' '\n> > +\n> > + git checkout side &&\n> > + git merge --no-verify -m \"merge master\" master &&\n> > + git checkout master\n> > +\n> > +'\n> \n> This test doesn't even try to verify that the hook was actually ignored.\n> That left me puzzled for a while...\n> \n> > +test_expect_success '--no-verify with failing hook (merge)' '\n> > +\n> > + git checkout side &&\n> > + git merge --no-verify -m \"merge master\" master &&\n> > + git checkout master\n> > +\n> > +'\n> \n> ... but this would then (most likely) fail, so we would notice\n> something's wrong.\n> \n> This script seems to me like if it passes 100%, we can be fairly sure\n> we're ok, but if some individual test fails, we shouldn't be surprised\n> if its oneline description is a bit off compared to the bug. Similarly,\n> quite a few tests could pass, despite their oneline description inducing\n> the thought of \"but surely, if /that/ were the problem, /those/ tests\n> would fail\".\n> \n> Anyway, I realize this is just following the existing approach. I guess\n> you could argue it has served us well for a long time.\n> \n> I would probably prefer seeing the various hunks in this patch being\n> squashed into the relevant commits (1/4 vs 3/4) to make those patches\n> more self-describing.\n\nWill squash these as you said in V3. Will also think about whether\nanother test approach would make more sense here.\n"},{"id":"379572","messageId":"CAN0heSodNUQ4eoipj7dVi7kr-NhHNyppcMm_SYpGTr_xZbgZHA@mail.gmail.com","threadId":"46808","inReplyTo":"20190729234319.GJ43313@google.com","subject":"Re: [PATCH v2 4/4] t7503: add tests for pre-merge-hook","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2019-07-30T07:13:26Z","receivedAt":"2019-07-30T07:13:40Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Tue, 30 Jul 2019 at 01:43, Josh Steadmon wrote:\n>\n> On 2019.07.29 22:04, Martin Ågren wrote:\n> > This script seems to me like if it passes 100%, we can be fairly sure\n> > we're ok, but [...]\n\n> Will squash these as you said in V3. Will also think about whether\n> another test approach would make more sense here.\n\nThinking a bit more about this, this test uses two identical hooks, runs\nsome commands and verifies that \"the\" hook was run (or not, with\n--no-verify). If the implementation started calling the wrong hook\n(pre-commit / pre-merge) or both hooks, we wouldn't notice.\n\nPlease forgive my braindump below, I'm on the run so I'm just throwing\nthis out there:\n\nPerhaps (first do some modernizing of this script, to protect various\nsetup steps, use \"write_script\", etc, then) make the existing hook a\ntiny bit pre-commit-specific, e.g., by doing something like \"echo\npre-commit >>executed-hooks\", then at select places check \"test_cmp\nexecuted-hooks pre-commit\" (against \"echo pre-commit >pre-commit\"),\n\"test_path_is_missing executed-hooks\", and so on, coupled with some\n\"test_when_finished 'rm -f executed_hooks'\". Then the tests added for\nthis series would use a very similar hook, appending and checking for\n\"pre-merge[-commit]\", That should make us fairly certain that we're\nrunning precisely the wanted hook, I think.\n\nMartin\n"},{"id":"379700","messageId":"20190731183420.GO43313@google.com","threadId":"46808","inReplyTo":"CAN0heSodNUQ4eoipj7dVi7kr-NhHNyppcMm_SYpGTr_xZbgZHA@mail.gmail.com","subject":"Re: [PATCH v2 4/4] t7503: add tests for pre-merge-hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-07-31T18:34:20Z","receivedAt":"2019-07-31T18:34:30Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2019.07.30 09:13, Martin Ågren wrote:\n> On Tue, 30 Jul 2019 at 01:43, Josh Steadmon wrote:\n> >\n> > On 2019.07.29 22:04, Martin Ågren wrote:\n> > > This script seems to me like if it passes 100%, we can be fairly sure\n> > > we're ok, but [...]\n> \n> > Will squash these as you said in V3. Will also think about whether\n> > another test approach would make more sense here.\n> \n> Thinking a bit more about this, this test uses two identical hooks, runs\n> some commands and verifies that \"the\" hook was run (or not, with\n> --no-verify). If the implementation started calling the wrong hook\n> (pre-commit / pre-merge) or both hooks, we wouldn't notice.\n> \n> Please forgive my braindump below, I'm on the run so I'm just throwing\n> this out there:\n> \n> Perhaps (first do some modernizing of this script, to protect various\n> setup steps, use \"write_script\", etc, then) make the existing hook a\n> tiny bit pre-commit-specific, e.g., by doing something like \"echo\n> pre-commit >>executed-hooks\", then at select places check \"test_cmp\n> executed-hooks pre-commit\" (against \"echo pre-commit >pre-commit\"),\n> \"test_path_is_missing executed-hooks\", and so on, coupled with some\n> \"test_when_finished 'rm -f executed_hooks'\". Then the tests added for\n> this series would use a very similar hook, appending and checking for\n> \"pre-merge[-commit]\", That should make us fairly certain that we're\n> running precisely the wanted hook, I think.\n> \n> Martin\n\nThat sounds like a reasonable approach, thank you for the suggestions. I\nwill work on this for V3.\n"},{"id":"379789","messageId":"cover.1564695892.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1563490164.git.steadmon@google.com","subject":"[PATCH v3 0/4] pre-merge-commit hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-01T22:20:42Z","receivedAt":"2019-08-01T22:20:51Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"This series adds a new pre-merge-commit hook, similar in usage to\npre-commit. It also improves hook testing in t7503, by verifying that\nthe correct hooks are run or bypassed as expected.\n\nThe original series was done by Michael J Gruber . I have\naddressed the outstanding review comments, and noted my changes in the\ncommit messages in \"[js: ...]\" blocks.\n\nChanges since V2:\n* Renamed the hook from \"pre-merge\" to \"pre-merge-commit\".\n* Added a new patch (1/4) to improve t7503 by verifying that the\n expected hooks are (or are not) run.\n* Squashed test changes (from V2's patch 4/4) into patch 3/4.\n Modified the tests to follow the example set in patch 1/4.\n* Reworded commit messages to match with the current state of certain\n flags, which changed in between V1 and V2 of this series.\n\nJosh Steadmon (1):\n t7503: verify proper hook execution\n\nMichael J Gruber (3):\n merge: do no-verify like commit\n git-merge: honor pre-merge-commit hook\n merge: --no-verify to bypass pre-merge-commit hook\n\n Documentation/git-merge.txt | 2 +-\n Documentation/githooks.txt | 7 +\n Documentation/merge-options.txt | 4 +\n builtin/merge.c | 18 +-\n ...3-pre-commit-and-pre-merge-commit-hooks.sh | 237 ++++++++++++++++++\n t/t7503-pre-commit-hook.sh | 139 ----------\n templates/hooks--pre-merge-commit.sample | 13 +\n 7 files changed, 277 insertions(+), 143 deletions(-)\n create mode 100755 t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n delete mode 100755 t/t7503-pre-commit-hook.sh\n create mode 100755 templates/hooks--pre-merge-commit.sample\n\nRange-diff against v2:\n1: e8b3fd8a5b < -: ---------- git-merge: Honor pre-merge hook\n-: ---------- > 1: f0476b2b1e t7503: verify proper hook execution\n2: 36406a85be ! 2: 89ddbf410f merge: do no-verify like commit\n @@ Commit message\n \"git merge --help\" to be more clear that the hook return code is\n respected by default.\n \n - [js: reworded commit message, and moved documentation changes from patch\n - 3/4 to this commit.]\n + [js: * reworded commit message\n + * squashed documentation changes from original series' patch 3/4\n + ]\n \n Signed-off-by: Michael J Gruber \n @@ Documentation/git-merge.txt: SYNOPSIS\n + [--no-verify] [-s ] [-X ] [-S[]]\n [--[no-]allow-unrelated-histories]\n [--[no-]rerere-autoupdate] [-m ] [-F ] [...]\n - 'git merge' --abort\n + 'git merge' (--continue | --abort | --quit)\n \n ## Documentation/merge-options.txt ##\n @@ Documentation/merge-options.txt: option can be used to override --squash.\n3: 2440ad35e4 < -: ---------- merge: --no-verify to bypass pre-merge hook\n4: 69dc3696e7 < -: ---------- t7503: add tests for pre-merge-hook\n-: ---------- > 3: 61b989ff16 git-merge: honor pre-merge-commit hook\n-: ---------- > 4: 45828c56fc merge: --no-verify to bypass pre-merge-commit hook\n\n-- \n2.22.0.770.g0f2c4a37fd-goog\n\n"},{"id":"379790","messageId":"f0476b2b1e1e1f1d528b94523e86f210ccbd7dab.1564695893.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1564695892.git.steadmon@google.com","subject":"[PATCH v3 1/4] t7503: verify proper hook execution","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-01T22:20:43Z","receivedAt":"2019-08-01T22:20:54Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"t7503 did not verify that the expected hooks actually ran during\ntesting. Fix that by making the hook scripts write their $0 into a file\nso that we can compare actual execution vs. expected execution.\n\nWhile we're at it, do some test style cleanups, such as using\nwrite_script() and doing setup inside a test_expect_success block.\n\nSigned-off-by: Josh Steadmon \n---\n t/t7503-pre-commit-hook.sh | 162 +++++++++++++++++++++----------------\n 1 file changed, 94 insertions(+), 68 deletions(-)\n\ndiff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-hook.sh\nindex 984889b39d..500bdd97c2 100755\n--- a/t/t7503-pre-commit-hook.sh\n+++ b/t/t7503-pre-commit-hook.sh\n@@ -4,124 +4,149 @@ test_description='pre-commit hook'\n \n . ./test-lib.sh\n \n-test_expect_success 'with no hook' '\n+HOOKDIR=\"$(git rev-parse --git-dir)/hooks\"\n+PRECOMMIT=\"$HOOKDIR/pre-commit\"\n+\n+# Prepare sample scripts that write their $0 to actual_hooks\n+test_expect_success 'sample script setup' '\n+\tmkdir -p \"$HOOKDIR\" &&\n+\twrite_script \"$HOOKDIR/success.sample\" <<-\\EOF &&\n+\techo $0 >>actual_hooks\n+\texit 0\n+\tEOF\n+\twrite_script \"$HOOKDIR/fail.sample\" <<-\\EOF &&\n+\techo $0 >>actual_hooks\n+\texit 1\n+\tEOF\n+\twrite_script \"$HOOKDIR/require-prefix.sample\" <<-\\EOF &&\n+\techo $0 >>actual_hooks\n+\ttest $GIT_PREFIX = \"success/\"\n+\tEOF\n+\twrite_script \"$HOOKDIR/check-author.sample\" <<-\\EOF\n+\techo $0 >>actual_hooks\n+\ttest \"$GIT_AUTHOR_NAME\" = \"New Author\" &&\n+\ttest \"$GIT_AUTHOR_EMAIL\" = \"newauthor@example.com\"\n+\tEOF\n+'\n \n-\techo \"foo\" > file &&\n+test_expect_success 'with no hook' '\n+\ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n+\ttouch expected_hooks actual_hooks &&\n+\techo \"foo\" >file &&\n \tgit add file &&\n-\tgit commit -m \"first\"\n-\n+\tgit commit -m \"first\" &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n test_expect_success '--no-verify with no hook' '\n-\n-\techo \"bar\" > file &&\n+\ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n+\ttouch expected_hooks actual_hooks &&\n+\techo \"bar\" >file &&\n \tgit add file &&\n-\tgit commit --no-verify -m \"bar\"\n-\n+\tgit commit --no-verify -m \"bar\" &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n-# now install hook that always succeeds\n-HOOKDIR=\"$(git rev-parse --git-dir)/hooks\"\n-HOOK=\"$HOOKDIR/pre-commit\"\n-mkdir -p \"$HOOKDIR\"\n-cat > \"$HOOK\" <> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n+\tln -s \"success.sample\" \"$PRECOMMIT\" &&\n+\ttouch actual_hooks &&\n+\techo \"$PRECOMMIT\" >expected_hooks &&\n+\techo \"more\" >>file &&\n \tgit add file &&\n-\tgit commit -m \"more\"\n-\n+\tgit commit -m \"more\" &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n test_expect_success '--no-verify with succeeding hook' '\n-\n-\techo \"even more\" >> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n+\tln -s \"success.sample\" \"$PRECOMMIT\" &&\n+\ttouch expected_hooks actual_hooks &&\n+\techo \"even more\" >>file &&\n \tgit add file &&\n-\tgit commit --no-verify -m \"even more\"\n-\n+\tgit commit --no-verify -m \"even more\" &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n-# now a hook that fails\n-cat > \"$HOOK\" <> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n+\tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n+\ttouch actual_hooks &&\n+\techo \"$PRECOMMIT\" >expected_hooks &&\n+\techo \"another\" >>file &&\n \tgit add file &&\n-\ttest_must_fail git commit -m \"another\"\n-\n+\ttest_must_fail git commit -m \"another\" &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n test_expect_success '--no-verify with failing hook' '\n-\n-\techo \"stuff\" >> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n+\tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n+\ttouch expected_hooks actual_hooks &&\n+\techo \"stuff\" >>file &&\n \tgit add file &&\n-\tgit commit --no-verify -m \"stuff\"\n-\n+\tgit commit --no-verify -m \"stuff\" &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n-chmod -x \"$HOOK\"\n test_expect_success POSIXPERM 'with non-executable hook' '\n-\n-\techo \"content\" >> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks; chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n+\tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n+\tchmod -x \"$HOOKDIR/fail.sample\" &&\n+\ttouch expected_hooks actual_hooks &&\n+\techo \"content\" >>file &&\n \tgit add file &&\n-\tgit commit -m \"content\"\n-\n+\tgit commit -m \"content\" &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n test_expect_success POSIXPERM '--no-verify with non-executable hook' '\n-\n-\techo \"more content\" >> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks; chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n+\tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n+\tchmod -x \"$HOOKDIR/fail.sample\" &&\n+\ttouch expected_hooks actual_hooks &&\n+\techo \"more content\" >>file &&\n \tgit add file &&\n-\tgit commit --no-verify -m \"more content\"\n-\n+\tgit commit --no-verify -m \"more content\" &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n-chmod +x \"$HOOK\"\n-\n-# a hook that checks $GIT_PREFIX and succeeds inside the\n-# success/ subdirectory only\n-cat > \"$HOOK\" <> file &&\n+\ttest_when_finished \"rm -rf \\\"$PRECOMMIT\\\" expected_hooks actual_hooks success\" &&\n+\tln -s \"require-prefix.sample\" \"$PRECOMMIT\" &&\n+\techo \"$PRECOMMIT\" >expected_hooks &&\n+\techo \"more content\" >>file &&\n \tgit add file &&\n \tmkdir success &&\n \t(\n \t\tcd success &&\n \t\tgit commit -m \"hook requires GIT_PREFIX = success/\"\n \t) &&\n-\trmdir success\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n test_expect_success 'with failing hook requiring GIT_PREFIX' '\n-\n-\techo \"more content\" >> file &&\n+\ttest_when_finished \"rm -rf \\\"$PRECOMMIT\\\" expected_hooks actual_hooks fail\" &&\n+\tln -s \"require-prefix.sample\" \"$PRECOMMIT\" &&\n+\techo \"$PRECOMMIT\" >expected_hooks &&\n+\techo \"more content\" >>file &&\n \tgit add file &&\n \tmkdir fail &&\n \t(\n \t\tcd fail &&\n \t\ttest_must_fail git commit -m \"hook must fail\"\n \t) &&\n-\trmdir fail &&\n-\tgit checkout -- file\n+\tgit checkout -- file &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n test_expect_success 'check the author in hook' '\n-\twrite_script \"$HOOK\" <<-\\EOF &&\n-\ttest \"$GIT_AUTHOR_NAME\" = \"New Author\" &&\n-\ttest \"$GIT_AUTHOR_EMAIL\" = \"newauthor@example.com\"\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n+\tln -s \"check-author.sample\" \"$PRECOMMIT\" &&\n+\tcat >expected_hooks <<-EOF &&\n+\t$PRECOMMIT\n+\t$PRECOMMIT\n+\t$PRECOMMIT\n \tEOF\n \ttest_must_fail git commit --allow-empty -m \"by a.u.thor\" &&\n \t(\n@@ -133,7 +158,8 @@ test_expect_success 'check the author in hook' '\n \t) &&\n \tgit commit --author=\"New Author \" \\\n \t\t--allow-empty -m \"by new.author via command line\" &&\n-\tgit show -s\n+\tgit show -s &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n test_done\n-- \n2.22.0.770.g0f2c4a37fd-goog\n\n"},{"id":"379791","messageId":"89ddbf410f71bbd0dc556297913561c8492a4084.1564695893.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1564695892.git.steadmon@google.com","subject":"[PATCH v3 2/4] merge: do no-verify like commit","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-01T22:20:44Z","receivedAt":"2019-08-01T22:20:56Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"f8b863598c (\"builtin/merge: honor commit-msg hook for merges\", 2017-09-07)\nintroduced the no-verify flag to merge for bypassing the commit-msg\nhook, though in a different way from the implementation in commit.c.\n\nChange the implementation in merge.c to be the same as in commit.c so\nthat both do the same in the same way. This also changes the output of\n\"git merge --help\" to be more clear that the hook return code is\nrespected by default.\n\n[js: * reworded commit message\n * squashed documentation changes from original series' patch 3/4\n]\n\nSigned-off-by: Michael J Gruber \nSigned-off-by: Josh Steadmon \n---\n Documentation/git-merge.txt | 2 +-\n Documentation/merge-options.txt | 4 ++++\n builtin/merge.c | 6 +++---\n 3 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\nindex 01fd52dc70..092529c619 100644\n--- a/Documentation/git-merge.txt\n+++ b/Documentation/git-merge.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n --------\n [verse]\n 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]\n-\t[-s ] [-X ] [-S[]]\n+\t[--no-verify] [-s ] [-X ] [-S[]]\n \t[--[no-]allow-unrelated-histories]\n \t[--[no-]rerere-autoupdate] [-m ] [-F ] [...]\n 'git merge' (--continue | --abort | --quit)\ndiff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt\nindex 79a00d2a4a..d6a9f4b96f 100644\n--- a/Documentation/merge-options.txt\n+++ b/Documentation/merge-options.txt\n@@ -105,6 +105,10 @@ option can be used to override --squash.\n +\n With --squash, --commit is not allowed, and will fail.\n \n+--no-verify::\n+\tThis option bypasses the pre-merge and commit-msg hooks.\n+\tSee also linkgit:githooks[5].\n+\n -s ::\n --strategy=::\n \tUse the given merge strategy; can be supplied more than\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex e2ccbc44e2..4425a7a12e 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -81,7 +81,7 @@ static int show_progress = -1;\n static int default_to_upstream = 1;\n static int signoff;\n static const char *sign_commit;\n-static int verify_msg = 1;\n+static int no_verify;\n \n static struct strategy all_strategy[] = {\n \t{ \"recursive\", DEFAULT_TWOHEAD | NO_TRIVIAL },\n@@ -287,7 +287,7 @@ static struct option builtin_merge_options[] = {\n \t N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n \tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n-\tOPT_BOOL(0, \"verify\", &verify_msg, N_(\"verify commit-msg hook\")),\n+\tOPT_BOOL(0, \"no-verify\", &no_verify, N_(\"bypass commit-msg hook\")),\n \tOPT_END()\n };\n \n@@ -842,7 +842,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \t\t\tabort_commit(remoteheads, NULL);\n \t}\n \n-\tif (verify_msg && run_commit_hook(0 < option_edit, get_index_file(),\n+\tif (!no_verify && run_commit_hook(0 < option_edit, get_index_file(),\n \t\t\t\t\t \"commit-msg\",\n \t\t\t\t\t git_path_merge_msg(the_repository), NULL))\n \t\tabort_commit(remoteheads, NULL);\n-- \n2.22.0.770.g0f2c4a37fd-goog\n\n"},{"id":"379792","messageId":"61b989ff16eadfd0508e10f71c9b318eb15ce2a7.1564695893.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1564695892.git.steadmon@google.com","subject":"[PATCH v3 3/4] git-merge: honor pre-merge-commit hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-01T22:20:45Z","receivedAt":"2019-08-01T22:20:59Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"git-merge does not honor the pre-commit hook when doing automatic merge\ncommits, and for compatibility reasons this is going to stay.\n\nIntroduce a pre-merge-commit hook which is called for an automatic merge\ncommit just like pre-commit is called for a non-automatic merge commit\n(or any other commit).\n\n[js: * renamed hook from \"pre-merge\" to \"pre-merge-commit\"\n * only discard the index if the hook is actually present\n * clarified that hook should write messages to stderr\n * squashed test changes from the original series' patch 4/4\n * modified tests to follow new pattern from this series' patch 1/4\n * reworded commit message\n]\n\nSigned-off-by: Michael J Gruber \nSigned-off-by: Josh Steadmon \n---\n Documentation/githooks.txt | 7 +++\n builtin/merge.c | 12 +++++\n ...-pre-commit-and-pre-merge-commit-hooks.sh} | 45 ++++++++++++++++++-\n templates/hooks--pre-merge-commit.sample | 13 ++++++\n 4 files changed, 76 insertions(+), 1 deletion(-)\n rename t/{t7503-pre-commit-hook.sh => t7503-pre-commit-and-pre-merge-commit-hooks.sh} (78%)\n create mode 100755 templates/hooks--pre-merge-commit.sample\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 82cd573776..7c4c994858 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -103,6 +103,13 @@ The default 'pre-commit' hook, when enabled--and with the\n `hooks.allownonascii` config option unset or set to false--prevents\n the use of non-ASCII filenames.\n \n+pre-merge-commit\n+~~~~~~~~~~~~~~~~\n+\n+This hook is invoked by 'git merge' when doing an automatic merge\n+commit; it is equivalent to 'pre-commit' for a non-automatic commit\n+for a merge.\n+\n prepare-commit-msg\n ~~~~~~~~~~~~~~~~~~\n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 4425a7a12e..bf0ae68c40 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -816,6 +816,18 @@ static void write_merge_heads(struct commit_list *);\n static void prepare_to_commit(struct commit_list *remoteheads)\n {\n \tstruct strbuf msg = STRBUF_INIT;\n+\tconst char *index_file = get_index_file();\n+\n+\tif (run_commit_hook(0 < option_edit, index_file, \"pre-merge-commit\", NULL))\n+\t\tabort_commit(remoteheads, NULL);\n+\t/*\n+\t * Re-read the index as pre-merge-commit hook could have updated it,\n+\t * and write it out as a tree. We must do this before we invoke\n+\t * the editor and after we invoke run_status above.\n+\t */\n+\tif (find_hook(\"pre-merge-commit\"))\n+\t\tdiscard_cache();\n+\tread_cache_from(index_file);\n \tstrbuf_addbuf(&msg, &merge_msg);\n \tif (squash)\n \t\tBUG(\"the control must not reach here under --squash\");\ndiff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\nsimilarity index 78%\nrename from t/t7503-pre-commit-hook.sh\nrename to t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\nindex 500bdd97c2..040dfa0175 100755\n--- a/t/t7503-pre-commit-hook.sh\n+++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n@@ -1,11 +1,12 @@\n #!/bin/sh\n \n-test_description='pre-commit hook'\n+test_description='pre-commit and pre-merge-commit hooks'\n \n . ./test-lib.sh\n \n HOOKDIR=\"$(git rev-parse --git-dir)/hooks\"\n PRECOMMIT=\"$HOOKDIR/pre-commit\"\n+PREMERGE=\"$HOOKDIR/pre-merge-commit\"\n \n # Prepare sample scripts that write their $0 to actual_hooks\n test_expect_success 'sample script setup' '\n@@ -29,6 +30,17 @@ test_expect_success 'sample script setup' '\n \tEOF\n '\n \n+test_expect_success 'root commit' '\n+\techo \"root\" > file &&\n+\tgit add file &&\n+\tgit commit -m \"zeroth\" &&\n+\tgit checkout -b side &&\n+\techo \"foo\" > foo &&\n+\tgit add foo &&\n+\tgit commit -m \"make it non-ff\" &&\n+\tgit checkout master\n+'\n+\n test_expect_success 'with no hook' '\n \ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n \ttouch expected_hooks actual_hooks &&\n@@ -38,6 +50,15 @@ test_expect_success 'with no hook' '\n \ttest_cmp expected_hooks actual_hooks\n '\n \n+test_expect_success 'with no hook (merge)' '\n+\ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n+\ttouch expected_hooks actual_hooks &&\n+\tgit checkout side &&\n+\tgit merge -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_cmp expected_hooks actual_hooks\n+'\n+\n test_expect_success '--no-verify with no hook' '\n \ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n \ttouch expected_hooks actual_hooks &&\n@@ -58,6 +79,17 @@ test_expect_success 'with succeeding hook' '\n \ttest_cmp expected_hooks actual_hooks\n '\n \n+test_expect_success 'with succeeding hook (merge)' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n+\tln -s \"success.sample\" \"$PREMERGE\" &&\n+\ttouch actual_hooks &&\n+\techo \"$PREMERGE\" >expected_hooks &&\n+\tgit checkout side &&\n+\tgit merge -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_cmp expected_hooks actual_hooks\n+'\n+\n test_expect_success '--no-verify with succeeding hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n \tln -s \"success.sample\" \"$PRECOMMIT\" &&\n@@ -89,6 +121,17 @@ test_expect_success '--no-verify with failing hook' '\n \ttest_cmp expected_hooks actual_hooks\n '\n \n+test_expect_success 'with failing hook (merge)' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n+\tln -s \"fail.sample\" \"$PREMERGE\" &&\n+\ttouch actual_hooks &&\n+\techo \"$PREMERGE\" >expected_hooks &&\n+\tgit checkout side &&\n+\ttest_must_fail git merge -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_cmp expected_hooks actual_hooks\n+'\n+\n test_expect_success POSIXPERM 'with non-executable hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks; chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n \tln -s \"fail.sample\" \"$PRECOMMIT\" &&\ndiff --git a/templates/hooks--pre-merge-commit.sample b/templates/hooks--pre-merge-commit.sample\nnew file mode 100755\nindex 0000000000..399eab1924\n--- /dev/null\n+++ b/templates/hooks--pre-merge-commit.sample\n@@ -0,0 +1,13 @@\n+#!/bin/sh\n+#\n+# An example hook script to verify what is about to be committed.\n+# Called by \"git merge\" with no arguments. The hook should\n+# exit with non-zero status after issuing an appropriate message to\n+# stderr if it wants to stop the merge commit.\n+#\n+# To enable this hook, rename this file to \"pre-merge-commit\".\n+\n+. git-sh-setup\n+test -x \"$GIT_DIR/hooks/pre-commit\" &&\n+ exec \"$GIT_DIR/hooks/pre-commit\"\n+:\n-- \n2.22.0.770.g0f2c4a37fd-goog\n\n"},{"id":"379793","messageId":"45828c56fcd672193096ad767eab24730a0e47ec.1564695893.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1564695892.git.steadmon@google.com","subject":"[PATCH v3 4/4] merge: --no-verify to bypass pre-merge-commit hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-01T22:20:46Z","receivedAt":"2019-08-01T22:21:02Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"Analogous to commit, introduce a '--no-verify' option which bypasses the\npre-merge-commit hook. The shorthand '-n' is taken by '--no-stat'\nalready.\n\n[js: * reworded commit message to reflect current state of --no-stat flag\n and new hook name\n * fixed flag documentation to reflect new hook name\n * cleaned up trailing whitespace\n * squashed test changes from the original series' patch 4/4\n * modified tests to follow pattern from this series' patch 1/4\n]\n\nSigned-off-by: Michael J Gruber \nSigned-off-by: Josh Steadmon \n---\n Documentation/githooks.txt | 2 +-\n builtin/merge.c | 4 +--\n ...3-pre-commit-and-pre-merge-commit-hooks.sh | 29 +++++++++++++++++++\n 3 files changed, 32 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 7c4c994858..a934553509 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -108,7 +108,7 @@ pre-merge-commit\n \n This hook is invoked by 'git merge' when doing an automatic merge\n commit; it is equivalent to 'pre-commit' for a non-automatic commit\n-for a merge.\n+for a merge, and can be bypassed with the `--no-verify` option.\n \n prepare-commit-msg\n ~~~~~~~~~~~~~~~~~~\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex bf0ae68c40..c9746e37b8 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -287,7 +287,7 @@ static struct option builtin_merge_options[] = {\n \t N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n \tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n-\tOPT_BOOL(0, \"no-verify\", &no_verify, N_(\"bypass commit-msg hook\")),\n+\tOPT_BOOL(0, \"no-verify\", &no_verify, N_(\"bypass pre-merge-commit and commit-msg hooks\")),\n \tOPT_END()\n };\n \n@@ -818,7 +818,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \tstruct strbuf msg = STRBUF_INIT;\n \tconst char *index_file = get_index_file();\n \n-\tif (run_commit_hook(0 < option_edit, index_file, \"pre-merge-commit\", NULL))\n+\tif (!no_verify && run_commit_hook(0 < option_edit, index_file, \"pre-merge-commit\", NULL))\n \t\tabort_commit(remoteheads, NULL);\n \t/*\n \t * Re-read the index as pre-merge-commit hook could have updated it,\ndiff --git a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\nindex 040dfa0175..86a375ab3e 100755\n--- a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n+++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n@@ -68,6 +68,15 @@ test_expect_success '--no-verify with no hook' '\n \ttest_cmp expected_hooks actual_hooks\n '\n \n+test_expect_success '--no-verify with no hook (merge)' '\n+\ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n+\ttouch expected_hooks actual_hooks &&\n+\tgit checkout side &&\n+\tgit merge --no-verify -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_cmp expected_hooks actual_hooks\n+'\n+\n test_expect_success 'with succeeding hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n \tln -s \"success.sample\" \"$PRECOMMIT\" &&\n@@ -100,6 +109,16 @@ test_expect_success '--no-verify with succeeding hook' '\n \ttest_cmp expected_hooks actual_hooks\n '\n \n+test_expect_success '--no-verify with succeeding hook (merge)' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n+\tln -s \"success.sample\" \"$PREMERGE\" &&\n+\ttouch expected_hooks actual_hooks &&\n+\tgit checkout side &&\n+\tgit merge --no-verify -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_cmp expected_hooks actual_hooks\n+'\n+\n test_expect_success 'with failing hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n \tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n@@ -132,6 +151,16 @@ test_expect_success 'with failing hook (merge)' '\n \ttest_cmp expected_hooks actual_hooks\n '\n \n+test_expect_success '--no-verify with failing hook (merge)' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n+\tln -s \"fail.sample\" \"$PREMERGE\" &&\n+\ttouch expected_hooks actual_hooks &&\n+\tgit checkout side &&\n+\tgit merge --no-verify -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_cmp expected_hooks actual_hooks\n+'\n+\n test_expect_success POSIXPERM 'with non-executable hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks; chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n \tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n-- \n2.22.0.770.g0f2c4a37fd-goog\n\n"},{"id":"379806","messageId":"CAN0heSo30-ng223sJTvz5_Go+-Yu=h=qvFg0KOhguLsFVE7b2Q@mail.gmail.com","threadId":"46808","inReplyTo":"61b989ff16eadfd0508e10f71c9b318eb15ce2a7.1564695893.git.steadmon@google.com","subject":"Re: [PATCH v3 3/4] git-merge: honor pre-merge-commit hook","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2019-08-02T09:45:05Z","receivedAt":"2019-08-02T09:51:00Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Fri, 2 Aug 2019 at 00:20, Josh Steadmon wrote:\n\n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> index 82cd573776..7c4c994858 100644\n> --- a/Documentation/githooks.txt\n> +++ b/Documentation/githooks.txt\n> @@ -103,6 +103,13 @@ The default 'pre-commit' hook, when enabled--and with the\n> `hooks.allownonascii` config option unset or set to false--prevents\n> the use of non-ASCII filenames.\n>\n> +pre-merge-commit\n> +~~~~~~~~~~~~~~~~\n> +\n> +This hook is invoked by 'git merge' when doing an automatic merge\n> +commit; it is equivalent to 'pre-commit' for a non-automatic commit\n> +for a merge.\n> +\n\nI'm not sure everyone understands what an \"automatic merge commit\" is.\n(Is it an automatic \"merge commit\", or an \"automatic merge\" commit? Or\nsort of both?) And I'm not sure exactly what to infer from the\n\"equivalence\". I happen to know that the statement about the default\nhook can only be half-carried over. And I'm not sure what to infer from\n\"All the git commit hooks are invoked with the environment variable\n...\".\n\nIs the below suggestion 1) correct, 2) readable?\n\n This hook is invoked by linkgit:git-merge[1], and can be bypassed\n with the `--no-verify` option. It takes no parameters, and is\n invoked after the merge has been carried out successfully and before\n obtaining the proposed commit log message to\n make a commit. Exiting with a non-zero status from this script\n causes the `git merge` command to abort before creating a commit.\n\n The default 'pre-merge-commit' hook, when enabled, runs the\n 'pre-commit' hook, if the latter is enabled.\n\n This hook is invoked with the environment variable\n `GIT_EDITOR=:` if the command will not bring up an editor\n to modify the commit message.\n\n If the merge cannot be carried out automatically, the conflicts\n need to be resolved and the result committed separately (see\n linkgit:git-merge[1]). At that point, this hook will not be executed,\n but the 'pre-commit' hook will, if it is enabled.\n\n(If you use this or something like it, notice how this already mentions\n`--no-verify`...)\n\n> +test_expect_success 'root commit' '\n> + echo \"root\" > file &&\n> + git add file &&\n> + git commit -m \"zeroth\" &&\n> + git checkout -b side &&\n> + echo \"foo\" > foo &&\n> + git add foo &&\n> + git commit -m \"make it non-ff\" &&\n> + git checkout master\n> +'\n\nYou got rid of loads of \"> file\" in patch 1/4, so it seems unfortunate\nto introduce a few here. ;-)\n\n\nMartin\n"},{"id":"379807","messageId":"cover.1564737003.git.martin.agren@gmail.com","threadId":"46808","inReplyTo":"cover.1564695892.git.steadmon@google.com","subject":"Re: [PATCH v3 0/4] pre-merge-commit hook","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2019-08-02T09:56:34Z","receivedAt":"2019-08-02T09:57:20Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"[Dropped cc-list the first time around. Apologies to those who receive\nthis twice...]\n\nOn Fri, 2 Aug 2019 at 00:20, Josh Steadmon wrote:\n>\n> This series adds a new pre-merge-commit hook, similar in usage to\n> pre-commit. It also improves hook testing in t7503, by verifying that\n> the correct hooks are run or bypassed as expected.\n\nI really like those test improvements. Now it should be harder to mess\nup a future refactoring and run the wrong hook. These hooks are \"very\nrelated\" so I think this is important.\n\nI've messed with the test a bit and offer these potential improvements\nfor your consideration. I was lazy and just built this on top of your\nseries -- if you agree to some or all of these, you'll probably need to\nsquash them into a few individual patches.\n\nThe first four are perhaps more or less a matter of opinion, although I\ndo think that patch 2/5 is based on an opinion shared by others. ;-)\nPatch 5/5 or something like it seems pretty important to me to make\nsure that of these two \"similar\"/\"related\" hooks with some\n\"backwards-compatibility-and-code-copying-history\" around them, we'd\nbetter pick the right one when they're both available.\n \nFeel free to pick and squash as you see fit. (I don't think it makes\nsense to have these go in as-are. They really are meant for squashing.)\n\nMartin\n\nMartin Ågren (5):\n t7503: use \"&&\" in \"test_when_finished\" rather than \";\"\n t7503: avoid touch when mtime doesn't matter\n t7503: simplify file-juggling\n t7503: don't create \"actual_hooks\" for later appending\n t7503: test failing merge with both hooks available\n\n ...3-pre-commit-and-pre-merge-commit-hooks.sh | 84 +++++++++++--------\n 1 file changed, 50 insertions(+), 34 deletions(-)\n\n-- \n2.23.0.rc0.30.g51cf315870\n\n"},{"id":"379808","messageId":"d697767c1814ae77306d520c88f06ded19542ba4.1564737003.git.martin.agren@gmail.com","threadId":"46808","inReplyTo":"cover.1564737003.git.martin.agren@gmail.com","subject":"[PATCH 1/5] t7503: use \"&&\" in \"test_when_finished\" rather than \";\"","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2019-08-02T09:56:35Z","receivedAt":"2019-08-02T09:57:25Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"Signed-off-by: Martin Ågren \n---\n t/t7503-pre-commit-and-pre-merge-commit-hooks.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\nindex 86a375ab3e..5cc6c98375 100755\n--- a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n+++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n@@ -162,7 +162,7 @@ test_expect_success '--no-verify with failing hook (merge)' '\n '\n \n test_expect_success POSIXPERM 'with non-executable hook' '\n-\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks; chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks && chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n \tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n \tchmod -x \"$HOOKDIR/fail.sample\" &&\n \ttouch expected_hooks actual_hooks &&\n@@ -173,7 +173,7 @@ test_expect_success POSIXPERM 'with non-executable hook' '\n '\n \n test_expect_success POSIXPERM '--no-verify with non-executable hook' '\n-\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks; chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks && chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n \tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n \tchmod -x \"$HOOKDIR/fail.sample\" &&\n \ttouch expected_hooks actual_hooks &&\n-- \n2.23.0.rc0.30.g51cf315870\n\n"},{"id":"379809","messageId":"fa13d568021cdcaf66365b783218270dcfb0c79a.1564737003.git.martin.agren@gmail.com","threadId":"46808","inReplyTo":"cover.1564737003.git.martin.agren@gmail.com","subject":"[PATCH 2/5] t7503: avoid touch when mtime doesn't matter","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2019-08-02T09:56:36Z","receivedAt":"2019-08-02T09:57:31Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"AFAIU, we avoid using `touch` unless we really do want to update the\nmtime. Here, we just want to create empty files, so write `>foo`\ninstead.\n\nSigned-off-by: Martin Ågren \n---\n ...3-pre-commit-and-pre-merge-commit-hooks.sh | 38 ++++++++++++-------\n 1 file changed, 24 insertions(+), 14 deletions(-)\n\ndiff --git a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\nindex 5cc6c98375..a6f1240aa2 100755\n--- a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n+++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n@@ -43,7 +43,8 @@ test_expect_success 'root commit' '\n \n test_expect_success 'with no hook' '\n \ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n-\ttouch expected_hooks actual_hooks &&\n+\t>expected_hooks &&\n+\t>actual_hooks &&\n \techo \"foo\" >file &&\n \tgit add file &&\n \tgit commit -m \"first\" &&\n@@ -52,7 +53,8 @@ test_expect_success 'with no hook' '\n \n test_expect_success 'with no hook (merge)' '\n \ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n-\ttouch expected_hooks actual_hooks &&\n+\t>expected_hooks &&\n+\t>actual_hooks &&\n \tgit checkout side &&\n \tgit merge -m \"merge master\" master &&\n \tgit checkout master &&\n@@ -61,7 +63,8 @@ test_expect_success 'with no hook (merge)' '\n \n test_expect_success '--no-verify with no hook' '\n \ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n-\ttouch expected_hooks actual_hooks &&\n+\t>expected_hooks &&\n+\t>actual_hooks &&\n \techo \"bar\" >file &&\n \tgit add file &&\n \tgit commit --no-verify -m \"bar\" &&\n@@ -70,7 +73,8 @@ test_expect_success '--no-verify with no hook' '\n \n test_expect_success '--no-verify with no hook (merge)' '\n \ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n-\ttouch expected_hooks actual_hooks &&\n+\t>expected_hooks &&\n+\t>actual_hooks &&\n \tgit checkout side &&\n \tgit merge --no-verify -m \"merge master\" master &&\n \tgit checkout master &&\n@@ -80,7 +84,7 @@ test_expect_success '--no-verify with no hook (merge)' '\n test_expect_success 'with succeeding hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n \tln -s \"success.sample\" \"$PRECOMMIT\" &&\n-\ttouch actual_hooks &&\n+\t>actual_hooks &&\n \techo \"$PRECOMMIT\" >expected_hooks &&\n \techo \"more\" >>file &&\n \tgit add file &&\n@@ -91,7 +95,7 @@ test_expect_success 'with succeeding hook' '\n test_expect_success 'with succeeding hook (merge)' '\n \ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n \tln -s \"success.sample\" \"$PREMERGE\" &&\n-\ttouch actual_hooks &&\n+\t>actual_hooks &&\n \techo \"$PREMERGE\" >expected_hooks &&\n \tgit checkout side &&\n \tgit merge -m \"merge master\" master &&\n@@ -102,7 +106,8 @@ test_expect_success 'with succeeding hook (merge)' '\n test_expect_success '--no-verify with succeeding hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n \tln -s \"success.sample\" \"$PRECOMMIT\" &&\n-\ttouch expected_hooks actual_hooks &&\n+\t>expected_hooks &&\n+\t>actual_hooks &&\n \techo \"even more\" >>file &&\n \tgit add file &&\n \tgit commit --no-verify -m \"even more\" &&\n@@ -112,7 +117,8 @@ test_expect_success '--no-verify with succeeding hook' '\n test_expect_success '--no-verify with succeeding hook (merge)' '\n \ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n \tln -s \"success.sample\" \"$PREMERGE\" &&\n-\ttouch expected_hooks actual_hooks &&\n+\t>expected_hooks &&\n+\t>actual_hooks &&\n \tgit checkout side &&\n \tgit merge --no-verify -m \"merge master\" master &&\n \tgit checkout master &&\n@@ -122,7 +128,7 @@ test_expect_success '--no-verify with succeeding hook (merge)' '\n test_expect_success 'with failing hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n \tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n-\ttouch actual_hooks &&\n+\t>actual_hooks &&\n \techo \"$PRECOMMIT\" >expected_hooks &&\n \techo \"another\" >>file &&\n \tgit add file &&\n@@ -133,7 +139,8 @@ test_expect_success 'with failing hook' '\n test_expect_success '--no-verify with failing hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n \tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n-\ttouch expected_hooks actual_hooks &&\n+\t>expected_hooks &&\n+\t>actual_hooks &&\n \techo \"stuff\" >>file &&\n \tgit add file &&\n \tgit commit --no-verify -m \"stuff\" &&\n@@ -143,7 +150,7 @@ test_expect_success '--no-verify with failing hook' '\n test_expect_success 'with failing hook (merge)' '\n \ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n \tln -s \"fail.sample\" \"$PREMERGE\" &&\n-\ttouch actual_hooks &&\n+\t>actual_hooks &&\n \techo \"$PREMERGE\" >expected_hooks &&\n \tgit checkout side &&\n \ttest_must_fail git merge -m \"merge master\" master &&\n@@ -154,7 +161,8 @@ test_expect_success 'with failing hook (merge)' '\n test_expect_success '--no-verify with failing hook (merge)' '\n \ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n \tln -s \"fail.sample\" \"$PREMERGE\" &&\n-\ttouch expected_hooks actual_hooks &&\n+\t>expected_hooks &&\n+\t>actual_hooks &&\n \tgit checkout side &&\n \tgit merge --no-verify -m \"merge master\" master &&\n \tgit checkout master &&\n@@ -165,7 +173,8 @@ test_expect_success POSIXPERM 'with non-executable hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks && chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n \tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n \tchmod -x \"$HOOKDIR/fail.sample\" &&\n-\ttouch expected_hooks actual_hooks &&\n+\t>expected_hooks &&\n+\t>actual_hooks &&\n \techo \"content\" >>file &&\n \tgit add file &&\n \tgit commit -m \"content\" &&\n@@ -176,7 +185,8 @@ test_expect_success POSIXPERM '--no-verify with non-executable hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks && chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n \tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n \tchmod -x \"$HOOKDIR/fail.sample\" &&\n-\ttouch expected_hooks actual_hooks &&\n+\t>expected_hooks &&\n+\t>actual_hooks &&\n \techo \"more content\" >>file &&\n \tgit add file &&\n \tgit commit --no-verify -m \"more content\" &&\n-- \n2.23.0.rc0.30.g51cf315870\n\n"},{"id":"379810","messageId":"1a65bc5519e59934bf665175b4377c5d6427b77a.1564737003.git.martin.agren@gmail.com","threadId":"46808","inReplyTo":"cover.1564737003.git.martin.agren@gmail.com","subject":"[PATCH 5/5] t7503: test failing merge with both hooks available","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2019-08-02T09:56:39Z","receivedAt":"2019-08-02T09:57:45Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"With a failing automatic merge, we want to make sure that we *don't*\ncall the pre-merge-commit hook and that we (eventually) *do* call the\npre-commit hook.\n\nSigned-off-by: Martin Ågren \n---\n ...3-pre-commit-and-pre-merge-commit-hooks.sh | 30 +++++++++++++++++++\n 1 file changed, 30 insertions(+)\n\ndiff --git a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\nindex f0c73fd58d..4bf359c097 100755\n--- a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n+++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n@@ -41,6 +41,18 @@ test_expect_success 'root commit' '\n \tgit checkout master\n '\n \n+test_expect_success 'setup conflicting branches' '\n+\ttest_when_finished \"git checkout master\" &&\n+\tgit checkout -b conflicting-a master &&\n+\techo a >conflicting &&\n+\tgit add conflicting &&\n+\tgit commit -m conflicting-a &&\n+\tgit checkout -b conflicting-b master &&\n+\techo b >conflicting &&\n+\tgit add conflicting &&\n+\tgit commit -m conflicting-b\n+'\n+\n test_expect_success 'with no hook' '\n \ttest_when_finished \"rm -f actual_hooks\" &&\n \techo \"foo\" >file &&\n@@ -93,6 +105,24 @@ test_expect_success 'with succeeding hook (merge)' '\n \ttest_cmp expected_hooks actual_hooks\n '\n \n+test_expect_success 'automatic merge fails; both hooks are available' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" \\\"$PRECOMMIT\\\"\" &&\n+\ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n+\ttest_when_finished \"git checkout master\" &&\n+\tln -s \"success.sample\" \"$PREMERGE\" &&\n+\tln -s \"success.sample\" \"$PRECOMMIT\" &&\n+\n+\tgit checkout conflicting-a &&\n+\ttest_must_fail git merge -m \"merge conflicting-b\" conflicting-b &&\n+\ttest_path_is_missing actual_hooks &&\n+\n+\techo \"$PRECOMMIT\" >expected_hooks &&\n+\techo a+b >conflicting &&\n+\tgit add conflicting &&\n+\tgit commit -m \"resolve conflict\" &&\n+\ttest_cmp expected_hooks actual_hooks\n+'\n+\n test_expect_success '--no-verify with succeeding hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n \tln -s \"success.sample\" \"$PRECOMMIT\" &&\n-- \n2.23.0.rc0.30.g51cf315870\n\n"},{"id":"379811","messageId":"4901b00e7c96452c030354e4e582f55f5d6b1de1.1564737003.git.martin.agren@gmail.com","threadId":"46808","inReplyTo":"cover.1564737003.git.martin.agren@gmail.com","subject":"[PATCH 4/5] t7503: don't create \"actual_hooks\" for later appending","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2019-08-02T09:56:38Z","receivedAt":"2019-08-02T09:58:42Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"If we fail to call the hook, we won't append to \"actual_hooks\".\n\"test_cmp\" is able to handle a missing file just fine, so these\n\"pre-creations\" are mostly confusing. Let's drop them.\n\nSigned-off-by: Martin Ågren \n--\n This \"pre-creation\" does protect against the file already\n existing, i.e., it could be seen as a \"clearing\". But since\n we use \"test_when_finished\" in this script... Also, these\n were \"touch\" before another of my suggestions, so these\n really were \"pre-creating\" these files if needed.\n\n t/t7503-pre-commit-and-pre-merge-commit-hooks.sh | 4 ----\n 1 file changed, 4 deletions(-)\n\ndiff --git a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\nindex 477207cb5c..f0c73fd58d 100755\n--- a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n+++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n@@ -76,7 +76,6 @@ test_expect_success '--no-verify with no hook (merge)' '\n test_expect_success 'with succeeding hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n \tln -s \"success.sample\" \"$PRECOMMIT\" &&\n-\t>actual_hooks &&\n \techo \"$PRECOMMIT\" >expected_hooks &&\n \techo \"more\" >>file &&\n \tgit add file &&\n@@ -87,7 +86,6 @@ test_expect_success 'with succeeding hook' '\n test_expect_success 'with succeeding hook (merge)' '\n \ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n \tln -s \"success.sample\" \"$PREMERGE\" &&\n-\t>actual_hooks &&\n \techo \"$PREMERGE\" >expected_hooks &&\n \tgit checkout side &&\n \tgit merge -m \"merge master\" master &&\n@@ -116,7 +114,6 @@ test_expect_success '--no-verify with succeeding hook (merge)' '\n test_expect_success 'with failing hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n \tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n-\t>actual_hooks &&\n \techo \"$PRECOMMIT\" >expected_hooks &&\n \techo \"another\" >>file &&\n \tgit add file &&\n@@ -136,7 +133,6 @@ test_expect_success '--no-verify with failing hook' '\n test_expect_success 'with failing hook (merge)' '\n \ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n \tln -s \"fail.sample\" \"$PREMERGE\" &&\n-\t>actual_hooks &&\n \techo \"$PREMERGE\" >expected_hooks &&\n \tgit checkout side &&\n \ttest_must_fail git merge -m \"merge master\" master &&\n-- \n2.23.0.rc0.30.g51cf315870\n\n"},{"id":"379812","messageId":"12cae1e82eceb0586bbc132bf8f883bb862189d7.1564737003.git.martin.agren@gmail.com","threadId":"46808","inReplyTo":"cover.1564737003.git.martin.agren@gmail.com","subject":"[PATCH 3/5] t7503: simplify file-juggling","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2019-08-02T09:56:37Z","receivedAt":"2019-08-02T09:58:47Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"Rather than creating an empty \"expected_hooks\" file and another empty\nfile \"actual_hooks\" (which we want to not be appended to), just verify\nthat \"actual_hooks\" isn't created. Do still clean up the \"actual_hooks\"\nfile, so that a failing test won't start affecting later tests too.\n\nSigned-off-by: Martin Ågren \n---\n ...3-pre-commit-and-pre-merge-commit-hooks.sh | 60 +++++++------------\n 1 file changed, 20 insertions(+), 40 deletions(-)\n\ndiff --git a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\nindex a6f1240aa2..477207cb5c 100755\n--- a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n+++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n@@ -42,43 +42,35 @@ test_expect_success 'root commit' '\n '\n \n test_expect_success 'with no hook' '\n-\ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n-\t>expected_hooks &&\n-\t>actual_hooks &&\n+\ttest_when_finished \"rm -f actual_hooks\" &&\n \techo \"foo\" >file &&\n \tgit add file &&\n \tgit commit -m \"first\" &&\n-\ttest_cmp expected_hooks actual_hooks\n+\ttest_path_is_missing actual_hooks\n '\n \n test_expect_success 'with no hook (merge)' '\n-\ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n-\t>expected_hooks &&\n-\t>actual_hooks &&\n+\ttest_when_finished \"rm -f actual_hooks\" &&\n \tgit checkout side &&\n \tgit merge -m \"merge master\" master &&\n \tgit checkout master &&\n-\ttest_cmp expected_hooks actual_hooks\n+\ttest_path_is_missing actual_hooks\n '\n \n test_expect_success '--no-verify with no hook' '\n-\ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n-\t>expected_hooks &&\n-\t>actual_hooks &&\n+\ttest_when_finished \"rm -f actual_hooks\" &&\n \techo \"bar\" >file &&\n \tgit add file &&\n \tgit commit --no-verify -m \"bar\" &&\n-\ttest_cmp expected_hooks actual_hooks\n+\ttest_path_is_missing actual_hooks\n '\n \n test_expect_success '--no-verify with no hook (merge)' '\n-\ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n-\t>expected_hooks &&\n-\t>actual_hooks &&\n+\ttest_when_finished \"rm -f actual_hooks\" &&\n \tgit checkout side &&\n \tgit merge --no-verify -m \"merge master\" master &&\n \tgit checkout master &&\n-\ttest_cmp expected_hooks actual_hooks\n+\ttest_path_is_missing actual_hooks\n '\n \n test_expect_success 'with succeeding hook' '\n@@ -104,25 +96,21 @@ test_expect_success 'with succeeding hook (merge)' '\n '\n \n test_expect_success '--no-verify with succeeding hook' '\n-\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n \tln -s \"success.sample\" \"$PRECOMMIT\" &&\n-\t>expected_hooks &&\n-\t>actual_hooks &&\n \techo \"even more\" >>file &&\n \tgit add file &&\n \tgit commit --no-verify -m \"even more\" &&\n-\ttest_cmp expected_hooks actual_hooks\n+\ttest_path_is_missing actual_hooks\n '\n \n test_expect_success '--no-verify with succeeding hook (merge)' '\n-\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n \tln -s \"success.sample\" \"$PREMERGE\" &&\n-\t>expected_hooks &&\n-\t>actual_hooks &&\n \tgit checkout side &&\n \tgit merge --no-verify -m \"merge master\" master &&\n \tgit checkout master &&\n-\ttest_cmp expected_hooks actual_hooks\n+\ttest_path_is_missing actual_hooks\n '\n \n test_expect_success 'with failing hook' '\n@@ -137,14 +125,12 @@ test_expect_success 'with failing hook' '\n '\n \n test_expect_success '--no-verify with failing hook' '\n-\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n \tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n-\t>expected_hooks &&\n-\t>actual_hooks &&\n \techo \"stuff\" >>file &&\n \tgit add file &&\n \tgit commit --no-verify -m \"stuff\" &&\n-\ttest_cmp expected_hooks actual_hooks\n+\ttest_path_is_missing actual_hooks\n '\n \n test_expect_success 'with failing hook (merge)' '\n@@ -159,38 +145,32 @@ test_expect_success 'with failing hook (merge)' '\n '\n \n test_expect_success '--no-verify with failing hook (merge)' '\n-\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n \tln -s \"fail.sample\" \"$PREMERGE\" &&\n-\t>expected_hooks &&\n-\t>actual_hooks &&\n \tgit checkout side &&\n \tgit merge --no-verify -m \"merge master\" master &&\n \tgit checkout master &&\n-\ttest_cmp expected_hooks actual_hooks\n+\ttest_path_is_missing actual_hooks\n '\n \n test_expect_success POSIXPERM 'with non-executable hook' '\n-\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks && chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks && chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n \tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n \tchmod -x \"$HOOKDIR/fail.sample\" &&\n-\t>expected_hooks &&\n-\t>actual_hooks &&\n \techo \"content\" >>file &&\n \tgit add file &&\n \tgit commit -m \"content\" &&\n-\ttest_cmp expected_hooks actual_hooks\n+\ttest_path_is_missing actual_hooks\n '\n \n test_expect_success POSIXPERM '--no-verify with non-executable hook' '\n-\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks && chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks && chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n \tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n \tchmod -x \"$HOOKDIR/fail.sample\" &&\n-\t>expected_hooks &&\n-\t>actual_hooks &&\n \techo \"more content\" >>file &&\n \tgit add file &&\n \tgit commit --no-verify -m \"more content\" &&\n-\ttest_cmp expected_hooks actual_hooks\n+\ttest_path_is_missing actual_hooks\n '\n \n test_expect_success 'with hook requiring GIT_PREFIX' '\n-- \n2.23.0.rc0.30.g51cf315870\n\n"},{"id":"379813","messageId":"CAN0heSpyS6T-T8ueZc6L_zrgLrutf6P_pNYe=KJ+SpF+ycgfmA@mail.gmail.com","threadId":"46808","inReplyTo":"f0476b2b1e1e1f1d528b94523e86f210ccbd7dab.1564695893.git.steadmon@google.com","subject":"Re: [PATCH v3 1/4] t7503: verify proper hook execution","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2019-08-02T09:43:00Z","receivedAt":"2019-08-02T10:01:26Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Fri, 2 Aug 2019 at 00:20, Josh Steadmon wrote:\n>\n> t7503 did not verify that the expected hooks actually ran during\n> testing. Fix that by making the hook scripts write their $0 into a file\n> so that we can compare actual execution vs. expected execution.\n\nIt could be argued that this test is perfectly fine as long as there is\njust one hook to choose from, and as long as we're fine with failing\ntests being a bit non-obvious to debug. Since we'll soon have two hooks\nto choose from in this test, we really want to make sure that we're\nexecuting the right one, not just \"at least one\". (No need to reroll,\nthis is just me thinking out loud.)\n\n\nMartin\n"},{"id":"379864","messageId":"20190802221855.GR43313@google.com","threadId":"46808","inReplyTo":"cover.1564737003.git.martin.agren@gmail.com","subject":"Re: [PATCH v3 0/4] pre-merge-commit hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-02T22:18:55Z","receivedAt":"2019-08-02T22:19:04Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2019.08.02 11:56, Martin Ågren wrote:\n> [Dropped cc-list the first time around. Apologies to those who receive\n> this twice...]\n> \n> On Fri, 2 Aug 2019 at 00:20, Josh Steadmon wrote:\n> >\n> > This series adds a new pre-merge-commit hook, similar in usage to\n> > pre-commit. It also improves hook testing in t7503, by verifying that\n> > the correct hooks are run or bypassed as expected.\n> \n> I really like those test improvements. Now it should be harder to mess\n> up a future refactoring and run the wrong hook. These hooks are \"very\n> related\" so I think this is important.\n> \n> I've messed with the test a bit and offer these potential improvements\n> for your consideration. I was lazy and just built this on top of your\n> series -- if you agree to some or all of these, you'll probably need to\n> squash them into a few individual patches.\n> \n> The first four are perhaps more or less a matter of opinion, although I\n> do think that patch 2/5 is based on an opinion shared by others. ;-)\n> Patch 5/5 or something like it seems pretty important to me to make\n> sure that of these two \"similar\"/\"related\" hooks with some\n> \"backwards-compatibility-and-code-copying-history\" around them, we'd\n> better pick the right one when they're both available.\n> \n> Feel free to pick and squash as you see fit. (I don't think it makes\n> sense to have these go in as-are. They really are meant for squashing.)\n> \n> Martin\n> \n> Martin Ågren (5):\n> t7503: use \"&&\" in \"test_when_finished\" rather than \";\"\n> t7503: avoid touch when mtime doesn't matter\n> t7503: simplify file-juggling\n> t7503: don't create \"actual_hooks\" for later appending\n> t7503: test failing merge with both hooks available\n> \n> ...3-pre-commit-and-pre-merge-commit-hooks.sh | 84 +++++++++++--------\n> 1 file changed, 50 insertions(+), 34 deletions(-)\n> \n> -- \n> 2.23.0.rc0.30.g51cf315870\n\nThese all look good to me, thank you for the suggestions! I'll include\nthem in V4.\n"},{"id":"379865","messageId":"20190802222039.GS43313@google.com","threadId":"46808","inReplyTo":"CAN0heSo30-ng223sJTvz5_Go+-Yu=h=qvFg0KOhguLsFVE7b2Q@mail.gmail.com","subject":"Re: [PATCH v3 3/4] git-merge: honor pre-merge-commit hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-02T22:20:39Z","receivedAt":"2019-08-02T22:20:47Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2019.08.02 11:45, Martin Ågren wrote:\n> On Fri, 2 Aug 2019 at 00:20, Josh Steadmon wrote:\n> \n> > diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> > index 82cd573776..7c4c994858 100644\n> > --- a/Documentation/githooks.txt\n> > +++ b/Documentation/githooks.txt\n> > @@ -103,6 +103,13 @@ The default 'pre-commit' hook, when enabled--and with the\n> > `hooks.allownonascii` config option unset or set to false--prevents\n> > the use of non-ASCII filenames.\n> >\n> > +pre-merge-commit\n> > +~~~~~~~~~~~~~~~~\n> > +\n> > +This hook is invoked by 'git merge' when doing an automatic merge\n> > +commit; it is equivalent to 'pre-commit' for a non-automatic commit\n> > +for a merge.\n> > +\n> \n> I'm not sure everyone understands what an \"automatic merge commit\" is.\n> (Is it an automatic \"merge commit\", or an \"automatic merge\" commit? Or\n> sort of both?) And I'm not sure exactly what to infer from the\n> \"equivalence\". I happen to know that the statement about the default\n> hook can only be half-carried over. And I'm not sure what to infer from\n> \"All the git commit hooks are invoked with the environment variable\n> ...\".\n> \n> Is the below suggestion 1) correct, 2) readable?\n> \n> This hook is invoked by linkgit:git-merge[1], and can be bypassed\n> with the `--no-verify` option. It takes no parameters, and is\n> invoked after the merge has been carried out successfully and before\n> obtaining the proposed commit log message to\n> make a commit. Exiting with a non-zero status from this script\n> causes the `git merge` command to abort before creating a commit.\n> \n> The default 'pre-merge-commit' hook, when enabled, runs the\n> 'pre-commit' hook, if the latter is enabled.\n> \n> This hook is invoked with the environment variable\n> `GIT_EDITOR=:` if the command will not bring up an editor\n> to modify the commit message.\n> \n> If the merge cannot be carried out automatically, the conflicts\n> need to be resolved and the result committed separately (see\n> linkgit:git-merge[1]). At that point, this hook will not be executed,\n> but the 'pre-commit' hook will, if it is enabled.\n> \n> (If you use this or something like it, notice how this already mentions\n> `--no-verify`...)\n\nThis looks good to me at first glance. I may reword a bit in V4, but\nprobably not much.\n\n> > +test_expect_success 'root commit' '\n> > + echo \"root\" > file &&\n> > + git add file &&\n> > + git commit -m \"zeroth\" &&\n> > + git checkout -b side &&\n> > + echo \"foo\" > foo &&\n> > + git add foo &&\n> > + git commit -m \"make it non-ff\" &&\n> > + git checkout master\n> > +'\n> \n> You got rid of loads of \"> file\" in patch 1/4, so it seems unfortunate\n> to introduce a few here. ;-)\n\nThanks for the catch, will fix in V4.\n"},{"id":"379962","messageId":"cover.1565044345.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1564695892.git.steadmon@google.com","subject":"[PATCH v4 0/4] pre-merge-commit hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-05T22:43:35Z","receivedAt":"2019-08-05T22:43:44Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"This series adds a new pre-merge-commit hook, similar in usage to\npre-commit. It also improves hook testing in t7503, by verifying that\nthe correct hooks are run or bypassed as expected.\n\nThe original series was done by Michael J Gruber . I have\naddressed the outstanding review comments, and noted my changes in the\ncommit messages in \"[js: ...]\" blocks.\n\nChanges since V3:\n* Applied several test style fixes suggested by Martin (thanks!).\n* Clarified the documentation for pre-merge-commit hook.\n* Fixed a few cases where testing that the merge hook did not run might\n erroneously succeed if we don't have any merge to actually perform.\n* Simplified test cleanup by adding a new non-executable sample hook\n script.\n* Added test cases for non-executable pre-merge-commit hooks.\n\nChanges since V2:\n* Renamed the hook from \"pre-merge\" to \"pre-merge-commit\".\n* Added a new patch (1/4) to improve t7503 by verifying that the\n expected hooks are (or are not) run.\n* Squashed test changes (from V2's patch 4/4) into patch 3/4.\n Modified the tests to follow the example set in patch 1/4.\n* Reworded commit messages to match with the current state of certain\n flags, which changed in between V1 and V2 of this series.\n\nJosh Steadmon (1):\n t7503: verify proper hook execution\n\nMichael J Gruber (3):\n merge: do no-verify like commit\n git-merge: honor pre-merge-commit hook\n merge: --no-verify to bypass pre-merge-commit hook\n\n Documentation/git-merge.txt | 2 +-\n Documentation/githooks.txt | 22 ++\n Documentation/merge-options.txt | 4 +\n builtin/merge.c | 18 +-\n ...3-pre-commit-and-pre-merge-commit-hooks.sh | 281 ++++++++++++++++++\n t/t7503-pre-commit-hook.sh | 139 ---------\n templates/hooks--pre-merge-commit.sample | 13 +\n 7 files changed, 336 insertions(+), 143 deletions(-)\n create mode 100755 t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n delete mode 100755 t/t7503-pre-commit-hook.sh\n create mode 100755 templates/hooks--pre-merge-commit.sample\n\nRange-diff against v3:\n1: f0476b2b1e ! 1: 5085729095 t7503: verify proper hook execution\n @@ Commit message\n write_script() and doing setup inside a test_expect_success block.\n \n + Improved-by: Martin Ågren \n + Signed-off-by: Martin Ågren \n \n ## t/t7503-pre-commit-hook.sh ##\n @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'\n @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'\n +\techo $0 >>actual_hooks\n +\texit 1\n +\tEOF\n ++\twrite_script \"$HOOKDIR/non-exec.sample\" <<-\\EOF &&\n ++\techo $0 >>actual_hooks\n ++\texit 1\n ++\tEOF\n ++\tchmod -x \"$HOOKDIR/non-exec.sample\" &&\n +\twrite_script \"$HOOKDIR/require-prefix.sample\" <<-\\EOF &&\n +\techo $0 >>actual_hooks\n +\ttest $GIT_PREFIX = \"success/\"\n @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'\n \n -\techo \"foo\" > file &&\n +test_expect_success 'with no hook' '\n -+\ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n -+\ttouch expected_hooks actual_hooks &&\n ++\ttest_when_finished \"rm -f actual_hooks\" &&\n +\techo \"foo\" >file &&\n \tgit add file &&\n -\tgit commit -m \"first\"\n -\n +\tgit commit -m \"first\" &&\n -+\ttest_cmp expected_hooks actual_hooks\n ++\ttest_path_is_missing actual_hooks\n '\n \n test_expect_success '--no-verify with no hook' '\n -\n -\techo \"bar\" > file &&\n -+\ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n -+\ttouch expected_hooks actual_hooks &&\n ++\ttest_when_finished \"rm -f actual_hooks\" &&\n +\techo \"bar\" >file &&\n \tgit add file &&\n -\tgit commit --no-verify -m \"bar\"\n -\n +\tgit commit --no-verify -m \"bar\" &&\n -+\ttest_cmp expected_hooks actual_hooks\n ++\ttest_path_is_missing actual_hooks\n '\n \n -# now install hook that always succeeds\n @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'\n -\techo \"more\" >> file &&\n +\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n +\tln -s \"success.sample\" \"$PRECOMMIT\" &&\n -+\ttouch actual_hooks &&\n +\techo \"$PRECOMMIT\" >expected_hooks &&\n +\techo \"more\" >>file &&\n \tgit add file &&\n @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'\n test_expect_success '--no-verify with succeeding hook' '\n -\n -\techo \"even more\" >> file &&\n -+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n ++\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n +\tln -s \"success.sample\" \"$PRECOMMIT\" &&\n -+\ttouch expected_hooks actual_hooks &&\n +\techo \"even more\" >>file &&\n \tgit add file &&\n -\tgit commit --no-verify -m \"even more\"\n -\n +\tgit commit --no-verify -m \"even more\" &&\n -+\ttest_cmp expected_hooks actual_hooks\n ++\ttest_path_is_missing actual_hooks\n '\n \n -# now a hook that fails\n @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'\n -\techo \"another\" >> file &&\n +\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n +\tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n -+\ttouch actual_hooks &&\n +\techo \"$PRECOMMIT\" >expected_hooks &&\n +\techo \"another\" >>file &&\n \tgit add file &&\n @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'\n test_expect_success '--no-verify with failing hook' '\n -\n -\techo \"stuff\" >> file &&\n -+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n ++\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n +\tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n -+\ttouch expected_hooks actual_hooks &&\n +\techo \"stuff\" >>file &&\n \tgit add file &&\n -\tgit commit --no-verify -m \"stuff\"\n -\n +\tgit commit --no-verify -m \"stuff\" &&\n -+\ttest_cmp expected_hooks actual_hooks\n ++\ttest_path_is_missing actual_hooks\n '\n \n -chmod -x \"$HOOK\"\n test_expect_success POSIXPERM 'with non-executable hook' '\n -\n -\techo \"content\" >> file &&\n -+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks; chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n -+\tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n -+\tchmod -x \"$HOOKDIR/fail.sample\" &&\n -+\ttouch expected_hooks actual_hooks &&\n ++\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n ++\tln -s \"non-exec.sample\" \"$PRECOMMIT\" &&\n +\techo \"content\" >>file &&\n \tgit add file &&\n -\tgit commit -m \"content\"\n -\n +\tgit commit -m \"content\" &&\n -+\ttest_cmp expected_hooks actual_hooks\n ++\ttest_path_is_missing actual_hooks\n '\n \n test_expect_success POSIXPERM '--no-verify with non-executable hook' '\n -\n -\techo \"more content\" >> file &&\n -+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks; chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n -+\tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n -+\tchmod -x \"$HOOKDIR/fail.sample\" &&\n -+\ttouch expected_hooks actual_hooks &&\n ++\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n ++\tln -s \"non-exec.sample\" \"$PRECOMMIT\" &&\n +\techo \"more content\" >>file &&\n \tgit add file &&\n -\tgit commit --no-verify -m \"more content\"\n -\n +\tgit commit --no-verify -m \"more content\" &&\n -+\ttest_cmp expected_hooks actual_hooks\n ++\ttest_path_is_missing actual_hooks\n '\n -chmod +x \"$HOOK\"\n -\n2: 89ddbf410f = 2: 3b701a5c41 merge: do no-verify like commit\n3: 61b989ff16 ! 3: 9210421fbb git-merge: honor pre-merge-commit hook\n @@ Commit message\n \n [js: * renamed hook from \"pre-merge\" to \"pre-merge-commit\"\n * only discard the index if the hook is actually present\n + * expanded githooks documentation entry\n * clarified that hook should write messages to stderr\n * squashed test changes from the original series' patch 4/4\n * modified tests to follow new pattern from this series' patch 1/4\n + * added a test case for non-executable merge hooks\n + * added a test case for failed merges\n + * when testing that the merge hook did not run, make sure we\n + actually have a merge to perform (by resetting the \"side\" branch\n + to its original state).\n * reworded commit message\n ]\n \n + Improved-by: Martin Ågren \n Signed-off-by: Michael J Gruber \n + Signed-off-by: Martin Ågren \n \n ## Documentation/githooks.txt ##\n @@ Documentation/githooks.txt: The default 'pre-commit' hook, when enabled--and wit\n +pre-merge-commit\n +~~~~~~~~~~~~~~~~\n +\n -+This hook is invoked by 'git merge' when doing an automatic merge\n -+commit; it is equivalent to 'pre-commit' for a non-automatic commit\n -+for a merge.\n ++This hook is invoked by linkgit:git-merge[1]. It takes no parameters, and is\n ++invoked after the merge has been carried out successfully and before\n ++obtaining the proposed commit log message to\n ++make a commit. Exiting with a non-zero status from this script\n ++causes the `git merge` command to abort before creating a commit.\n ++\n ++The default 'pre-merge-commit' hook, when enabled, runs the\n ++'pre-commit' hook, if the latter is enabled.\n ++\n ++This hook is invoked with the environment variable\n ++`GIT_EDITOR=:` if the command will not bring up an editor\n ++to modify the commit message.\n ++\n ++If the merge cannot be carried out automatically, the conflicts\n ++need to be resolved and the result committed separately (see\n ++linkgit:git-merge[1]). At that point, this hook will not be executed,\n ++but the 'pre-commit' hook will, if it is enabled.\n +\n prepare-commit-msg\n ~~~~~~~~~~~~~~~~~~\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'sample sc\n '\n \n +test_expect_success 'root commit' '\n -+\techo \"root\" > file &&\n ++\techo \"root\" >file &&\n +\tgit add file &&\n +\tgit commit -m \"zeroth\" &&\n +\tgit checkout -b side &&\n -+\techo \"foo\" > foo &&\n ++\techo \"foo\" >foo &&\n +\tgit add foo &&\n +\tgit commit -m \"make it non-ff\" &&\n ++\tgit branch side-orig side &&\n +\tgit checkout master\n +'\n ++\n ++test_expect_success 'setup conflicting branches' '\n ++\ttest_when_finished \"git checkout master\" &&\n ++\tgit checkout -b conflicting-a master &&\n ++\techo a >conflicting &&\n ++\tgit add conflicting &&\n ++\tgit commit -m conflicting-a &&\n ++\tgit checkout -b conflicting-b master &&\n ++\techo b >conflicting &&\n ++\tgit add conflicting &&\n ++\tgit commit -m conflicting-b\n ++'\n +\n test_expect_success 'with no hook' '\n - \ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n - \ttouch expected_hooks actual_hooks &&\n + \ttest_when_finished \"rm -f actual_hooks\" &&\n + \techo \"foo\" >file &&\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with no hook' '\n - \ttest_cmp expected_hooks actual_hooks\n + \ttest_path_is_missing actual_hooks\n '\n \n +test_expect_success 'with no hook (merge)' '\n -+\ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n -+\ttouch expected_hooks actual_hooks &&\n ++\ttest_when_finished \"rm -f actual_hooks\" &&\n ++\tgit branch -f side side-orig &&\n +\tgit checkout side &&\n +\tgit merge -m \"merge master\" master &&\n +\tgit checkout master &&\n -+\ttest_cmp expected_hooks actual_hooks\n ++\ttest_path_is_missing actual_hooks\n +'\n +\n test_expect_success '--no-verify with no hook' '\n - \ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n - \ttouch expected_hooks actual_hooks &&\n + \ttest_when_finished \"rm -f actual_hooks\" &&\n + \techo \"bar\" >file &&\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with succeeding hook' '\n \ttest_cmp expected_hooks actual_hooks\n '\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with succ\n +test_expect_success 'with succeeding hook (merge)' '\n +\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n +\tln -s \"success.sample\" \"$PREMERGE\" &&\n -+\ttouch actual_hooks &&\n +\techo \"$PREMERGE\" >expected_hooks &&\n +\tgit checkout side &&\n +\tgit merge -m \"merge master\" master &&\n +\tgit checkout master &&\n +\ttest_cmp expected_hooks actual_hooks\n +'\n ++\n ++test_expect_success 'automatic merge fails; both hooks are available' '\n ++\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" \\\"$PRECOMMIT\\\"\" &&\n ++\ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n ++\ttest_when_finished \"git checkout master\" &&\n ++\tln -s \"success.sample\" \"$PREMERGE\" &&\n ++\tln -s \"success.sample\" \"$PRECOMMIT\" &&\n ++\n ++\tgit checkout conflicting-a &&\n ++\ttest_must_fail git merge -m \"merge conflicting-b\" conflicting-b &&\n ++\ttest_path_is_missing actual_hooks &&\n ++\n ++\techo \"$PRECOMMIT\" >expected_hooks &&\n ++\techo a+b >conflicting &&\n ++\tgit add conflicting &&\n ++\tgit commit -m \"resolve conflict\" &&\n ++\ttest_cmp expected_hooks actual_hooks\n ++'\n +\n test_expect_success '--no-verify with succeeding hook' '\n - \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n + \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n \tln -s \"success.sample\" \"$PRECOMMIT\" &&\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-verify with failing hook' '\n - \ttest_cmp expected_hooks actual_hooks\n + \ttest_path_is_missing actual_hooks\n '\n \n +test_expect_success 'with failing hook (merge)' '\n +\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n +\tln -s \"fail.sample\" \"$PREMERGE\" &&\n -+\ttouch actual_hooks &&\n +\techo \"$PREMERGE\" >expected_hooks &&\n +\tgit checkout side &&\n +\ttest_must_fail git merge -m \"merge master\" master &&\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-veri\n +'\n +\n test_expect_success POSIXPERM 'with non-executable hook' '\n - \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks; chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n - \tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n + \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n + \tln -s \"non-exec.sample\" \"$PRECOMMIT\" &&\n +@@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success POSIXPERM '--no-verify with non-executable hook' '\n + \ttest_path_is_missing actual_hooks\n + '\n + \n ++test_expect_success POSIXPERM 'with non-executable hook (merge)' '\n ++\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n ++\tln -s \"non-exec.sample\" \"$PREMERGE\" &&\n ++\tgit branch -f side side-orig &&\n ++\tgit checkout side &&\n ++\tgit merge -m \"merge master\" master &&\n ++\tgit checkout master &&\n ++\ttest_path_is_missing actual_hooks\n ++'\n ++\n + test_expect_success 'with hook requiring GIT_PREFIX' '\n + \ttest_when_finished \"rm -rf \\\"$PRECOMMIT\\\" expected_hooks actual_hooks success\" &&\n + \tln -s \"require-prefix.sample\" \"$PRECOMMIT\" &&\n \n ## templates/hooks--pre-merge-commit.sample (new) ##\n @@\n4: 45828c56fc ! 4: 96c54883d3 merge: --no-verify to bypass pre-merge-commit hook\n @@ Commit message\n * cleaned up trailing whitespace\n * squashed test changes from the original series' patch 4/4\n * modified tests to follow pattern from this series' patch 1/4\n + * added a test case for --no-verify with non-executable hook\n + * when testing that the merge hook did not run, make sure we\n + actually have a merge to perform (by resetting the \"side\" branch\n + to its original state).\n +\n ]\n \n + Improved-by: Martin Ågren \n Signed-off-by: Michael J Gruber \n + Signed-off-by: Martin Ågren \n \n ## Documentation/githooks.txt ##\n -@@ Documentation/githooks.txt: pre-merge-commit\n - \n - This hook is invoked by 'git merge' when doing an automatic merge\n - commit; it is equivalent to 'pre-commit' for a non-automatic commit\n --for a merge.\n -+for a merge, and can be bypassed with the `--no-verify` option.\n +@@ Documentation/githooks.txt: the use of non-ASCII filenames.\n + pre-merge-commit\n + ~~~~~~~~~~~~~~~~\n \n - prepare-commit-msg\n - ~~~~~~~~~~~~~~~~~~\n +-This hook is invoked by linkgit:git-merge[1]. It takes no parameters, and is\n ++This hook is invoked by linkgit:git-merge[1], and can be bypassed\n ++with the `--no-verify` option. It takes no parameters, and is\n + invoked after the merge has been carried out successfully and before\n + obtaining the proposed commit log message to\n + make a commit. Exiting with a non-zero status from this script\n \n ## builtin/merge.c ##\n @@ builtin/merge.c: static struct option builtin_merge_options[] = {\n @@ builtin/merge.c: static void prepare_to_commit(struct commit_list *remoteheads)\n \n ## t/t7503-pre-commit-and-pre-merge-commit-hooks.sh ##\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-verify with no hook' '\n - \ttest_cmp expected_hooks actual_hooks\n + \ttest_path_is_missing actual_hooks\n '\n \n +test_expect_success '--no-verify with no hook (merge)' '\n -+\ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n -+\ttouch expected_hooks actual_hooks &&\n ++\ttest_when_finished \"rm -f actual_hooks\" &&\n ++\tgit branch -f side side-orig &&\n +\tgit checkout side &&\n +\tgit merge --no-verify -m \"merge master\" master &&\n +\tgit checkout master &&\n -+\ttest_cmp expected_hooks actual_hooks\n ++\ttest_path_is_missing actual_hooks\n +'\n +\n test_expect_success 'with succeeding hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n \tln -s \"success.sample\" \"$PRECOMMIT\" &&\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-verify with succeeding hook' '\n - \ttest_cmp expected_hooks actual_hooks\n + \ttest_path_is_missing actual_hooks\n '\n \n +test_expect_success '--no-verify with succeeding hook (merge)' '\n -+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n ++\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n +\tln -s \"success.sample\" \"$PREMERGE\" &&\n -+\ttouch expected_hooks actual_hooks &&\n ++\tgit branch -f side side-orig &&\n +\tgit checkout side &&\n +\tgit merge --no-verify -m \"merge master\" master &&\n +\tgit checkout master &&\n -+\ttest_cmp expected_hooks actual_hooks\n ++\ttest_path_is_missing actual_hooks\n +'\n +\n test_expect_success 'with failing hook' '\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with fail\n '\n \n +test_expect_success '--no-verify with failing hook (merge)' '\n -+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n ++\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n +\tln -s \"fail.sample\" \"$PREMERGE\" &&\n -+\ttouch expected_hooks actual_hooks &&\n ++\tgit branch -f side side-orig &&\n +\tgit checkout side &&\n +\tgit merge --no-verify -m \"merge master\" master &&\n +\tgit checkout master &&\n -+\ttest_cmp expected_hooks actual_hooks\n ++\ttest_path_is_missing actual_hooks\n +'\n +\n test_expect_success POSIXPERM 'with non-executable hook' '\n - \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks; chmod +x \\\"$HOOKDIR/fail.sample\\\"\" &&\n - \tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n + \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n + \tln -s \"non-exec.sample\" \"$PRECOMMIT\" &&\n +@@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success POSIXPERM 'with non-executable hook (merge)' '\n + \ttest_path_is_missing actual_hooks\n + '\n + \n ++test_expect_success POSIXPERM '--no-verify with non-executable hook (merge)' '\n ++\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n ++\tln -s \"non-exec.sample\" \"$PREMERGE\" &&\n ++\tgit branch -f side side-orig &&\n ++\tgit checkout side &&\n ++\tgit merge --no-verify -m \"merge master\" master &&\n ++\tgit checkout master &&\n ++\ttest_path_is_missing actual_hooks\n ++'\n ++\n + test_expect_success 'with hook requiring GIT_PREFIX' '\n + \ttest_when_finished \"rm -rf \\\"$PRECOMMIT\\\" expected_hooks actual_hooks success\" &&\n + \tln -s \"require-prefix.sample\" \"$PRECOMMIT\" &&\n-- \n2.22.0.770.g0f2c4a37fd-goog\n\n"},{"id":"379963","messageId":"50857290955c5467ef6f75083395c695340fa284.1565044345.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1565044345.git.steadmon@google.com","subject":"[PATCH v4 1/4] t7503: verify proper hook execution","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-05T22:43:36Z","receivedAt":"2019-08-05T22:43:47Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"t7503 did not verify that the expected hooks actually ran during\ntesting. Fix that by making the hook scripts write their $0 into a file\nso that we can compare actual execution vs. expected execution.\n\nWhile we're at it, do some test style cleanups, such as using\nwrite_script() and doing setup inside a test_expect_success block.\n\nImproved-by: Martin Ågren \nSigned-off-by: Martin Ågren \nSigned-off-by: Josh Steadmon \n---\n t/t7503-pre-commit-hook.sh | 157 +++++++++++++++++++++----------------\n 1 file changed, 89 insertions(+), 68 deletions(-)\n\ndiff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-hook.sh\nindex 984889b39d..a71ec31222 100755\n--- a/t/t7503-pre-commit-hook.sh\n+++ b/t/t7503-pre-commit-hook.sh\n@@ -4,124 +4,144 @@ test_description='pre-commit hook'\n \n . ./test-lib.sh\n \n-test_expect_success 'with no hook' '\n+HOOKDIR=\"$(git rev-parse --git-dir)/hooks\"\n+PRECOMMIT=\"$HOOKDIR/pre-commit\"\n+\n+# Prepare sample scripts that write their $0 to actual_hooks\n+test_expect_success 'sample script setup' '\n+\tmkdir -p \"$HOOKDIR\" &&\n+\twrite_script \"$HOOKDIR/success.sample\" <<-\\EOF &&\n+\techo $0 >>actual_hooks\n+\texit 0\n+\tEOF\n+\twrite_script \"$HOOKDIR/fail.sample\" <<-\\EOF &&\n+\techo $0 >>actual_hooks\n+\texit 1\n+\tEOF\n+\twrite_script \"$HOOKDIR/non-exec.sample\" <<-\\EOF &&\n+\techo $0 >>actual_hooks\n+\texit 1\n+\tEOF\n+\tchmod -x \"$HOOKDIR/non-exec.sample\" &&\n+\twrite_script \"$HOOKDIR/require-prefix.sample\" <<-\\EOF &&\n+\techo $0 >>actual_hooks\n+\ttest $GIT_PREFIX = \"success/\"\n+\tEOF\n+\twrite_script \"$HOOKDIR/check-author.sample\" <<-\\EOF\n+\techo $0 >>actual_hooks\n+\ttest \"$GIT_AUTHOR_NAME\" = \"New Author\" &&\n+\ttest \"$GIT_AUTHOR_EMAIL\" = \"newauthor@example.com\"\n+\tEOF\n+'\n \n-\techo \"foo\" > file &&\n+test_expect_success 'with no hook' '\n+\ttest_when_finished \"rm -f actual_hooks\" &&\n+\techo \"foo\" >file &&\n \tgit add file &&\n-\tgit commit -m \"first\"\n-\n+\tgit commit -m \"first\" &&\n+\ttest_path_is_missing actual_hooks\n '\n \n test_expect_success '--no-verify with no hook' '\n-\n-\techo \"bar\" > file &&\n+\ttest_when_finished \"rm -f actual_hooks\" &&\n+\techo \"bar\" >file &&\n \tgit add file &&\n-\tgit commit --no-verify -m \"bar\"\n-\n+\tgit commit --no-verify -m \"bar\" &&\n+\ttest_path_is_missing actual_hooks\n '\n \n-# now install hook that always succeeds\n-HOOKDIR=\"$(git rev-parse --git-dir)/hooks\"\n-HOOK=\"$HOOKDIR/pre-commit\"\n-mkdir -p \"$HOOKDIR\"\n-cat > \"$HOOK\" <> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n+\tln -s \"success.sample\" \"$PRECOMMIT\" &&\n+\techo \"$PRECOMMIT\" >expected_hooks &&\n+\techo \"more\" >>file &&\n \tgit add file &&\n-\tgit commit -m \"more\"\n-\n+\tgit commit -m \"more\" &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n test_expect_success '--no-verify with succeeding hook' '\n-\n-\techo \"even more\" >> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n+\tln -s \"success.sample\" \"$PRECOMMIT\" &&\n+\techo \"even more\" >>file &&\n \tgit add file &&\n-\tgit commit --no-verify -m \"even more\"\n-\n+\tgit commit --no-verify -m \"even more\" &&\n+\ttest_path_is_missing actual_hooks\n '\n \n-# now a hook that fails\n-cat > \"$HOOK\" <> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n+\tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n+\techo \"$PRECOMMIT\" >expected_hooks &&\n+\techo \"another\" >>file &&\n \tgit add file &&\n-\ttest_must_fail git commit -m \"another\"\n-\n+\ttest_must_fail git commit -m \"another\" &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n test_expect_success '--no-verify with failing hook' '\n-\n-\techo \"stuff\" >> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n+\tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n+\techo \"stuff\" >>file &&\n \tgit add file &&\n-\tgit commit --no-verify -m \"stuff\"\n-\n+\tgit commit --no-verify -m \"stuff\" &&\n+\ttest_path_is_missing actual_hooks\n '\n \n-chmod -x \"$HOOK\"\n test_expect_success POSIXPERM 'with non-executable hook' '\n-\n-\techo \"content\" >> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n+\tln -s \"non-exec.sample\" \"$PRECOMMIT\" &&\n+\techo \"content\" >>file &&\n \tgit add file &&\n-\tgit commit -m \"content\"\n-\n+\tgit commit -m \"content\" &&\n+\ttest_path_is_missing actual_hooks\n '\n \n test_expect_success POSIXPERM '--no-verify with non-executable hook' '\n-\n-\techo \"more content\" >> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n+\tln -s \"non-exec.sample\" \"$PRECOMMIT\" &&\n+\techo \"more content\" >>file &&\n \tgit add file &&\n-\tgit commit --no-verify -m \"more content\"\n-\n+\tgit commit --no-verify -m \"more content\" &&\n+\ttest_path_is_missing actual_hooks\n '\n-chmod +x \"$HOOK\"\n-\n-# a hook that checks $GIT_PREFIX and succeeds inside the\n-# success/ subdirectory only\n-cat > \"$HOOK\" <> file &&\n+\ttest_when_finished \"rm -rf \\\"$PRECOMMIT\\\" expected_hooks actual_hooks success\" &&\n+\tln -s \"require-prefix.sample\" \"$PRECOMMIT\" &&\n+\techo \"$PRECOMMIT\" >expected_hooks &&\n+\techo \"more content\" >>file &&\n \tgit add file &&\n \tmkdir success &&\n \t(\n \t\tcd success &&\n \t\tgit commit -m \"hook requires GIT_PREFIX = success/\"\n \t) &&\n-\trmdir success\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n test_expect_success 'with failing hook requiring GIT_PREFIX' '\n-\n-\techo \"more content\" >> file &&\n+\ttest_when_finished \"rm -rf \\\"$PRECOMMIT\\\" expected_hooks actual_hooks fail\" &&\n+\tln -s \"require-prefix.sample\" \"$PRECOMMIT\" &&\n+\techo \"$PRECOMMIT\" >expected_hooks &&\n+\techo \"more content\" >>file &&\n \tgit add file &&\n \tmkdir fail &&\n \t(\n \t\tcd fail &&\n \t\ttest_must_fail git commit -m \"hook must fail\"\n \t) &&\n-\trmdir fail &&\n-\tgit checkout -- file\n+\tgit checkout -- file &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n test_expect_success 'check the author in hook' '\n-\twrite_script \"$HOOK\" <<-\\EOF &&\n-\ttest \"$GIT_AUTHOR_NAME\" = \"New Author\" &&\n-\ttest \"$GIT_AUTHOR_EMAIL\" = \"newauthor@example.com\"\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n+\tln -s \"check-author.sample\" \"$PRECOMMIT\" &&\n+\tcat >expected_hooks <<-EOF &&\n+\t$PRECOMMIT\n+\t$PRECOMMIT\n+\t$PRECOMMIT\n \tEOF\n \ttest_must_fail git commit --allow-empty -m \"by a.u.thor\" &&\n \t(\n@@ -133,7 +153,8 @@ test_expect_success 'check the author in hook' '\n \t) &&\n \tgit commit --author=\"New Author \" \\\n \t\t--allow-empty -m \"by new.author via command line\" &&\n-\tgit show -s\n+\tgit show -s &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n test_done\n-- \n2.22.0.770.g0f2c4a37fd-goog\n\n"},{"id":"379964","messageId":"3b701a5c41da4f64e0b1cd1ccc94ec8b6aaffd2b.1565044345.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1565044345.git.steadmon@google.com","subject":"[PATCH v4 2/4] merge: do no-verify like commit","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-05T22:43:37Z","receivedAt":"2019-08-05T22:43:49Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"From: Michael J Gruber \n\nf8b863598c (\"builtin/merge: honor commit-msg hook for merges\", 2017-09-07)\nintroduced the no-verify flag to merge for bypassing the commit-msg\nhook, though in a different way from the implementation in commit.c.\n\nChange the implementation in merge.c to be the same as in commit.c so\nthat both do the same in the same way. This also changes the output of\n\"git merge --help\" to be more clear that the hook return code is\nrespected by default.\n\n[js: * reworded commit message\n * squashed documentation changes from original series' patch 3/4\n]\n\nSigned-off-by: Michael J Gruber \nSigned-off-by: Josh Steadmon \n---\n Documentation/git-merge.txt | 2 +-\n Documentation/merge-options.txt | 4 ++++\n builtin/merge.c | 6 +++---\n 3 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\nindex 01fd52dc70..092529c619 100644\n--- a/Documentation/git-merge.txt\n+++ b/Documentation/git-merge.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n --------\n [verse]\n 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]\n-\t[-s ] [-X ] [-S[]]\n+\t[--no-verify] [-s ] [-X ] [-S[]]\n \t[--[no-]allow-unrelated-histories]\n \t[--[no-]rerere-autoupdate] [-m ] [-F ] [...]\n 'git merge' (--continue | --abort | --quit)\ndiff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt\nindex 79a00d2a4a..d6a9f4b96f 100644\n--- a/Documentation/merge-options.txt\n+++ b/Documentation/merge-options.txt\n@@ -105,6 +105,10 @@ option can be used to override --squash.\n +\n With --squash, --commit is not allowed, and will fail.\n \n+--no-verify::\n+\tThis option bypasses the pre-merge and commit-msg hooks.\n+\tSee also linkgit:githooks[5].\n+\n -s ::\n --strategy=::\n \tUse the given merge strategy; can be supplied more than\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex e2ccbc44e2..4425a7a12e 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -81,7 +81,7 @@ static int show_progress = -1;\n static int default_to_upstream = 1;\n static int signoff;\n static const char *sign_commit;\n-static int verify_msg = 1;\n+static int no_verify;\n \n static struct strategy all_strategy[] = {\n \t{ \"recursive\", DEFAULT_TWOHEAD | NO_TRIVIAL },\n@@ -287,7 +287,7 @@ static struct option builtin_merge_options[] = {\n \t N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n \tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n-\tOPT_BOOL(0, \"verify\", &verify_msg, N_(\"verify commit-msg hook\")),\n+\tOPT_BOOL(0, \"no-verify\", &no_verify, N_(\"bypass commit-msg hook\")),\n \tOPT_END()\n };\n \n@@ -842,7 +842,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \t\t\tabort_commit(remoteheads, NULL);\n \t}\n \n-\tif (verify_msg && run_commit_hook(0 < option_edit, get_index_file(),\n+\tif (!no_verify && run_commit_hook(0 < option_edit, get_index_file(),\n \t\t\t\t\t \"commit-msg\",\n \t\t\t\t\t git_path_merge_msg(the_repository), NULL))\n \t\tabort_commit(remoteheads, NULL);\n-- \n2.22.0.770.g0f2c4a37fd-goog\n\n"},{"id":"379965","messageId":"9210421fbb74c43a8b5e48478229a77ddd50ed3e.1565044345.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1565044345.git.steadmon@google.com","subject":"[PATCH v4 3/4] git-merge: honor pre-merge-commit hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-05T22:43:38Z","receivedAt":"2019-08-05T22:43:51Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"From: Michael J Gruber \n\ngit-merge does not honor the pre-commit hook when doing automatic merge\ncommits, and for compatibility reasons this is going to stay.\n\nIntroduce a pre-merge-commit hook which is called for an automatic merge\ncommit just like pre-commit is called for a non-automatic merge commit\n(or any other commit).\n\n[js: * renamed hook from \"pre-merge\" to \"pre-merge-commit\"\n * only discard the index if the hook is actually present\n * expanded githooks documentation entry\n * clarified that hook should write messages to stderr\n * squashed test changes from the original series' patch 4/4\n * modified tests to follow new pattern from this series' patch 1/4\n * added a test case for non-executable merge hooks\n * added a test case for failed merges\n * when testing that the merge hook did not run, make sure we\n actually have a merge to perform (by resetting the \"side\" branch\n to its original state).\n * reworded commit message\n]\n\nImproved-by: Martin Ågren \nSigned-off-by: Michael J Gruber \nSigned-off-by: Martin Ågren \nSigned-off-by: Josh Steadmon \n---\n Documentation/githooks.txt | 21 +++++\n builtin/merge.c | 12 +++\n ...-pre-commit-and-pre-merge-commit-hooks.sh} | 84 ++++++++++++++++++-\n templates/hooks--pre-merge-commit.sample | 13 +++\n 4 files changed, 129 insertions(+), 1 deletion(-)\n rename t/{t7503-pre-commit-hook.sh => t7503-pre-commit-and-pre-merge-commit-hooks.sh} (63%)\n create mode 100755 templates/hooks--pre-merge-commit.sample\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 82cd573776..d9da474fb0 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -103,6 +103,27 @@ The default 'pre-commit' hook, when enabled--and with the\n `hooks.allownonascii` config option unset or set to false--prevents\n the use of non-ASCII filenames.\n \n+pre-merge-commit\n+~~~~~~~~~~~~~~~~\n+\n+This hook is invoked by linkgit:git-merge[1]. It takes no parameters, and is\n+invoked after the merge has been carried out successfully and before\n+obtaining the proposed commit log message to\n+make a commit. Exiting with a non-zero status from this script\n+causes the `git merge` command to abort before creating a commit.\n+\n+The default 'pre-merge-commit' hook, when enabled, runs the\n+'pre-commit' hook, if the latter is enabled.\n+\n+This hook is invoked with the environment variable\n+`GIT_EDITOR=:` if the command will not bring up an editor\n+to modify the commit message.\n+\n+If the merge cannot be carried out automatically, the conflicts\n+need to be resolved and the result committed separately (see\n+linkgit:git-merge[1]). At that point, this hook will not be executed,\n+but the 'pre-commit' hook will, if it is enabled.\n+\n prepare-commit-msg\n ~~~~~~~~~~~~~~~~~~\n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 4425a7a12e..bf0ae68c40 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -816,6 +816,18 @@ static void write_merge_heads(struct commit_list *);\n static void prepare_to_commit(struct commit_list *remoteheads)\n {\n \tstruct strbuf msg = STRBUF_INIT;\n+\tconst char *index_file = get_index_file();\n+\n+\tif (run_commit_hook(0 < option_edit, index_file, \"pre-merge-commit\", NULL))\n+\t\tabort_commit(remoteheads, NULL);\n+\t/*\n+\t * Re-read the index as pre-merge-commit hook could have updated it,\n+\t * and write it out as a tree. We must do this before we invoke\n+\t * the editor and after we invoke run_status above.\n+\t */\n+\tif (find_hook(\"pre-merge-commit\"))\n+\t\tdiscard_cache();\n+\tread_cache_from(index_file);\n \tstrbuf_addbuf(&msg, &merge_msg);\n \tif (squash)\n \t\tBUG(\"the control must not reach here under --squash\");\ndiff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\nsimilarity index 63%\nrename from t/t7503-pre-commit-hook.sh\nrename to t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\nindex a71ec31222..0700604f03 100755\n--- a/t/t7503-pre-commit-hook.sh\n+++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n@@ -1,11 +1,12 @@\n #!/bin/sh\n \n-test_description='pre-commit hook'\n+test_description='pre-commit and pre-merge-commit hooks'\n \n . ./test-lib.sh\n \n HOOKDIR=\"$(git rev-parse --git-dir)/hooks\"\n PRECOMMIT=\"$HOOKDIR/pre-commit\"\n+PREMERGE=\"$HOOKDIR/pre-merge-commit\"\n \n # Prepare sample scripts that write their $0 to actual_hooks\n test_expect_success 'sample script setup' '\n@@ -34,6 +35,30 @@ test_expect_success 'sample script setup' '\n \tEOF\n '\n \n+test_expect_success 'root commit' '\n+\techo \"root\" >file &&\n+\tgit add file &&\n+\tgit commit -m \"zeroth\" &&\n+\tgit checkout -b side &&\n+\techo \"foo\" >foo &&\n+\tgit add foo &&\n+\tgit commit -m \"make it non-ff\" &&\n+\tgit branch side-orig side &&\n+\tgit checkout master\n+'\n+\n+test_expect_success 'setup conflicting branches' '\n+\ttest_when_finished \"git checkout master\" &&\n+\tgit checkout -b conflicting-a master &&\n+\techo a >conflicting &&\n+\tgit add conflicting &&\n+\tgit commit -m conflicting-a &&\n+\tgit checkout -b conflicting-b master &&\n+\techo b >conflicting &&\n+\tgit add conflicting &&\n+\tgit commit -m conflicting-b\n+'\n+\n test_expect_success 'with no hook' '\n \ttest_when_finished \"rm -f actual_hooks\" &&\n \techo \"foo\" >file &&\n@@ -42,6 +67,15 @@ test_expect_success 'with no hook' '\n \ttest_path_is_missing actual_hooks\n '\n \n+test_expect_success 'with no hook (merge)' '\n+\ttest_when_finished \"rm -f actual_hooks\" &&\n+\tgit branch -f side side-orig &&\n+\tgit checkout side &&\n+\tgit merge -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_path_is_missing actual_hooks\n+'\n+\n test_expect_success '--no-verify with no hook' '\n \ttest_when_finished \"rm -f actual_hooks\" &&\n \techo \"bar\" >file &&\n@@ -60,6 +94,34 @@ test_expect_success 'with succeeding hook' '\n \ttest_cmp expected_hooks actual_hooks\n '\n \n+test_expect_success 'with succeeding hook (merge)' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n+\tln -s \"success.sample\" \"$PREMERGE\" &&\n+\techo \"$PREMERGE\" >expected_hooks &&\n+\tgit checkout side &&\n+\tgit merge -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_cmp expected_hooks actual_hooks\n+'\n+\n+test_expect_success 'automatic merge fails; both hooks are available' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" \\\"$PRECOMMIT\\\"\" &&\n+\ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n+\ttest_when_finished \"git checkout master\" &&\n+\tln -s \"success.sample\" \"$PREMERGE\" &&\n+\tln -s \"success.sample\" \"$PRECOMMIT\" &&\n+\n+\tgit checkout conflicting-a &&\n+\ttest_must_fail git merge -m \"merge conflicting-b\" conflicting-b &&\n+\ttest_path_is_missing actual_hooks &&\n+\n+\techo \"$PRECOMMIT\" >expected_hooks &&\n+\techo a+b >conflicting &&\n+\tgit add conflicting &&\n+\tgit commit -m \"resolve conflict\" &&\n+\ttest_cmp expected_hooks actual_hooks\n+'\n+\n test_expect_success '--no-verify with succeeding hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n \tln -s \"success.sample\" \"$PRECOMMIT\" &&\n@@ -88,6 +150,16 @@ test_expect_success '--no-verify with failing hook' '\n \ttest_path_is_missing actual_hooks\n '\n \n+test_expect_success 'with failing hook (merge)' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n+\tln -s \"fail.sample\" \"$PREMERGE\" &&\n+\techo \"$PREMERGE\" >expected_hooks &&\n+\tgit checkout side &&\n+\ttest_must_fail git merge -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_cmp expected_hooks actual_hooks\n+'\n+\n test_expect_success POSIXPERM 'with non-executable hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n \tln -s \"non-exec.sample\" \"$PRECOMMIT\" &&\n@@ -106,6 +178,16 @@ test_expect_success POSIXPERM '--no-verify with non-executable hook' '\n \ttest_path_is_missing actual_hooks\n '\n \n+test_expect_success POSIXPERM 'with non-executable hook (merge)' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n+\tln -s \"non-exec.sample\" \"$PREMERGE\" &&\n+\tgit branch -f side side-orig &&\n+\tgit checkout side &&\n+\tgit merge -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_path_is_missing actual_hooks\n+'\n+\n test_expect_success 'with hook requiring GIT_PREFIX' '\n \ttest_when_finished \"rm -rf \\\"$PRECOMMIT\\\" expected_hooks actual_hooks success\" &&\n \tln -s \"require-prefix.sample\" \"$PRECOMMIT\" &&\ndiff --git a/templates/hooks--pre-merge-commit.sample b/templates/hooks--pre-merge-commit.sample\nnew file mode 100755\nindex 0000000000..399eab1924\n--- /dev/null\n+++ b/templates/hooks--pre-merge-commit.sample\n@@ -0,0 +1,13 @@\n+#!/bin/sh\n+#\n+# An example hook script to verify what is about to be committed.\n+# Called by \"git merge\" with no arguments. The hook should\n+# exit with non-zero status after issuing an appropriate message to\n+# stderr if it wants to stop the merge commit.\n+#\n+# To enable this hook, rename this file to \"pre-merge-commit\".\n+\n+. git-sh-setup\n+test -x \"$GIT_DIR/hooks/pre-commit\" &&\n+ exec \"$GIT_DIR/hooks/pre-commit\"\n+:\n-- \n2.22.0.770.g0f2c4a37fd-goog\n\n"},{"id":"379966","messageId":"96c54883d3adbe314461c4bf70e7edc9d2ee52d3.1565044345.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1565044345.git.steadmon@google.com","subject":"[PATCH v4 4/4] merge: --no-verify to bypass pre-merge-commit hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-05T22:43:39Z","receivedAt":"2019-08-05T22:43:54Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"From: Michael J Gruber \n\nAnalogous to commit, introduce a '--no-verify' option which bypasses the\npre-merge-commit hook. The shorthand '-n' is taken by '--no-stat'\nalready.\n\n[js: * reworded commit message to reflect current state of --no-stat flag\n and new hook name\n * fixed flag documentation to reflect new hook name\n * cleaned up trailing whitespace\n * squashed test changes from the original series' patch 4/4\n * modified tests to follow pattern from this series' patch 1/4\n * added a test case for --no-verify with non-executable hook\n * when testing that the merge hook did not run, make sure we\n actually have a merge to perform (by resetting the \"side\" branch\n to its original state).\n\n]\n\nImproved-by: Martin Ågren \nSigned-off-by: Michael J Gruber \nSigned-off-by: Martin Ågren \nSigned-off-by: Josh Steadmon \n---\n Documentation/githooks.txt | 3 +-\n builtin/merge.c | 4 +-\n ...3-pre-commit-and-pre-merge-commit-hooks.sh | 39 +++++++++++++++++++\n 3 files changed, 43 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex d9da474fb0..57d6e2b98d 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -106,7 +106,8 @@ the use of non-ASCII filenames.\n pre-merge-commit\n ~~~~~~~~~~~~~~~~\n \n-This hook is invoked by linkgit:git-merge[1]. It takes no parameters, and is\n+This hook is invoked by linkgit:git-merge[1], and can be bypassed\n+with the `--no-verify` option. It takes no parameters, and is\n invoked after the merge has been carried out successfully and before\n obtaining the proposed commit log message to\n make a commit. Exiting with a non-zero status from this script\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex bf0ae68c40..c9746e37b8 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -287,7 +287,7 @@ static struct option builtin_merge_options[] = {\n \t N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n \tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n-\tOPT_BOOL(0, \"no-verify\", &no_verify, N_(\"bypass commit-msg hook\")),\n+\tOPT_BOOL(0, \"no-verify\", &no_verify, N_(\"bypass pre-merge-commit and commit-msg hooks\")),\n \tOPT_END()\n };\n \n@@ -818,7 +818,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \tstruct strbuf msg = STRBUF_INIT;\n \tconst char *index_file = get_index_file();\n \n-\tif (run_commit_hook(0 < option_edit, index_file, \"pre-merge-commit\", NULL))\n+\tif (!no_verify && run_commit_hook(0 < option_edit, index_file, \"pre-merge-commit\", NULL))\n \t\tabort_commit(remoteheads, NULL);\n \t/*\n \t * Re-read the index as pre-merge-commit hook could have updated it,\ndiff --git a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\nindex 0700604f03..d900c3a696 100755\n--- a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n+++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n@@ -84,6 +84,15 @@ test_expect_success '--no-verify with no hook' '\n \ttest_path_is_missing actual_hooks\n '\n \n+test_expect_success '--no-verify with no hook (merge)' '\n+\ttest_when_finished \"rm -f actual_hooks\" &&\n+\tgit branch -f side side-orig &&\n+\tgit checkout side &&\n+\tgit merge --no-verify -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_path_is_missing actual_hooks\n+'\n+\n test_expect_success 'with succeeding hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n \tln -s \"success.sample\" \"$PRECOMMIT\" &&\n@@ -131,6 +140,16 @@ test_expect_success '--no-verify with succeeding hook' '\n \ttest_path_is_missing actual_hooks\n '\n \n+test_expect_success '--no-verify with succeeding hook (merge)' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n+\tln -s \"success.sample\" \"$PREMERGE\" &&\n+\tgit branch -f side side-orig &&\n+\tgit checkout side &&\n+\tgit merge --no-verify -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_path_is_missing actual_hooks\n+'\n+\n test_expect_success 'with failing hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n \tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n@@ -160,6 +179,16 @@ test_expect_success 'with failing hook (merge)' '\n \ttest_cmp expected_hooks actual_hooks\n '\n \n+test_expect_success '--no-verify with failing hook (merge)' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n+\tln -s \"fail.sample\" \"$PREMERGE\" &&\n+\tgit branch -f side side-orig &&\n+\tgit checkout side &&\n+\tgit merge --no-verify -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_path_is_missing actual_hooks\n+'\n+\n test_expect_success POSIXPERM 'with non-executable hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n \tln -s \"non-exec.sample\" \"$PRECOMMIT\" &&\n@@ -188,6 +217,16 @@ test_expect_success POSIXPERM 'with non-executable hook (merge)' '\n \ttest_path_is_missing actual_hooks\n '\n \n+test_expect_success POSIXPERM '--no-verify with non-executable hook (merge)' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n+\tln -s \"non-exec.sample\" \"$PREMERGE\" &&\n+\tgit branch -f side side-orig &&\n+\tgit checkout side &&\n+\tgit merge --no-verify -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_path_is_missing actual_hooks\n+'\n+\n test_expect_success 'with hook requiring GIT_PREFIX' '\n \ttest_when_finished \"rm -rf \\\"$PRECOMMIT\\\" expected_hooks actual_hooks success\" &&\n \tln -s \"require-prefix.sample\" \"$PRECOMMIT\" &&\n-- \n2.22.0.770.g0f2c4a37fd-goog\n\n"},{"id":"380034","messageId":"xmqqk1bqjh4s.fsf@gitster-ct.c.googlers.com","threadId":"46808","inReplyTo":"50857290955c5467ef6f75083395c695340fa284.1565044345.git.steadmon@google.com","subject":"Re: [PATCH v4 1/4] t7503: verify proper hook execution","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-06T18:14:27Z","receivedAt":"2019-08-06T18:14:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh Steadmon writes:\n\n> t7503 did not verify that the expected hooks actually ran during\n> testing. Fix that by making the hook scripts write their $0 into a file\n> so that we can compare actual execution vs. expected execution.\n>\n> While we're at it, do some test style cleanups, such as using\n> write_script() and doing setup inside a test_expect_success block.\n> ...\n\nThe result mostly looks more pleasant compared to the original,\nmodulo a few nits.\n\n> test_expect_success 'with succeeding hook' '\n> -\n> -\techo \"more\" >> file &&\n> +\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n> +\tln -s \"success.sample\" \"$PRECOMMIT\" &&\n\nNot all systems that want to run Git are capable of symbolic links.\nWe should be able to do a copy here, perhaps? Just the most basic\nform of \"cp\" without even \"-p\" would be sufficient as the target is\nexpected to be missing (i.e. \"cp\" or \"ln -s\" would be creating the\n\"$PRECOMMIT\"), right?\n\n"},{"id":"380084","messageId":"20190807181144.GV43313@google.com","threadId":"46808","inReplyTo":"xmqqk1bqjh4s.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4 1/4] t7503: verify proper hook execution","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-07T18:11:44Z","receivedAt":"2019-08-07T18:11:56Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2019.08.06 11:14, Junio C Hamano wrote:\n> Josh Steadmon writes:\n> \n> > t7503 did not verify that the expected hooks actually ran during\n> > testing. Fix that by making the hook scripts write their $0 into a file\n> > so that we can compare actual execution vs. expected execution.\n> >\n> > While we're at it, do some test style cleanups, such as using\n> > write_script() and doing setup inside a test_expect_success block.\n> > ...\n> \n> The result mostly looks more pleasant compared to the original,\n> modulo a few nits.\n> \n> > test_expect_success 'with succeeding hook' '\n> > -\n> > -\techo \"more\" >> file &&\n> > +\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n> > +\tln -s \"success.sample\" \"$PRECOMMIT\" &&\n> \n> Not all systems that want to run Git are capable of symbolic links.\n> We should be able to do a copy here, perhaps? Just the most basic\n> form of \"cp\" without even \"-p\" would be sufficient as the target is\n> expected to be missing (i.e. \"cp\" or \"ln -s\" would be creating the\n> \"$PRECOMMIT\"), right?\n\nYeah, \"cp\" works here. Fixed in V5.\n"},{"id":"380086","messageId":"cover.1565203880.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1565044345.git.steadmon@google.com","subject":"[PATCH v5 0/4] pre-merge-commit hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-07T18:57:04Z","receivedAt":"2019-08-07T18:57:23Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"This series adds a new pre-merge-commit hook, similar in usage to\npre-commit. It also improves hook testing in t7503, by verifying that\nthe correct hooks are run or bypassed as expected.\n\nThe original series was done by Michael J Gruber . I have\naddressed the outstanding review comments, and noted my changes in the\ncommit messages in \"[js: ...]\" blocks.\n\nChanges since V4:\n* Used \"cp\" instead of \"ln -s\" in test hook setup to make tests more\n portable.\n\nChanges since V3:\n* Applied several test style fixes suggested by Martin (thanks!).\n* Clarified the documentation for pre-merge-commit hook.\n* Fixed a few cases where testing that the merge hook did not run might\n erroneously succeed if we don't have any merge to actually perform.\n* Simplified test cleanup by adding a new non-executable sample hook\n script.\n* Added test cases for non-executable pre-merge-commit hooks.\n\nChanges since V2:\n* Renamed the hook from \"pre-merge\" to \"pre-merge-commit\".\n* Added a new patch (1/4) to improve t7503 by verifying that the\n expected hooks are (or are not) run.\n* Squashed test changes (from V2's patch 4/4) into patch 3/4.\n Modified the tests to follow the example set in patch 1/4.\n* Reworded commit messages to match with the current state of certain\n flags, which changed in between V1 and V2 of this series.\n\nJosh Steadmon (1):\n t7503: verify proper hook execution\n\nMichael J Gruber (3):\n merge: do no-verify like commit\n git-merge: honor pre-merge-commit hook\n merge: --no-verify to bypass pre-merge-commit hook\n\n Documentation/git-merge.txt | 2 +-\n Documentation/githooks.txt | 22 ++\n Documentation/merge-options.txt | 4 +\n builtin/merge.c | 18 +-\n ...3-pre-commit-and-pre-merge-commit-hooks.sh | 281 ++++++++++++++++++\n t/t7503-pre-commit-hook.sh | 139 ---------\n templates/hooks--pre-merge-commit.sample | 13 +\n 7 files changed, 336 insertions(+), 143 deletions(-)\n create mode 100755 t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n delete mode 100755 t/t7503-pre-commit-hook.sh\n create mode 100755 templates/hooks--pre-merge-commit.sample\n\nRange-diff against v4:\n1: 5085729095 ! 1: 60bbbbf9e0 t7503: verify proper hook execution\n @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'\n -\n -\techo \"more\" >> file &&\n +\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n -+\tln -s \"success.sample\" \"$PRECOMMIT\" &&\n ++\tcp \"$HOOKDIR/success.sample\" \"$PRECOMMIT\" &&\n +\techo \"$PRECOMMIT\" >expected_hooks &&\n +\techo \"more\" >>file &&\n \tgit add file &&\n @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'\n -\n -\techo \"even more\" >> file &&\n +\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n -+\tln -s \"success.sample\" \"$PRECOMMIT\" &&\n ++\tcp \"$HOOKDIR/success.sample\" \"$PRECOMMIT\" &&\n +\techo \"even more\" >>file &&\n \tgit add file &&\n -\tgit commit --no-verify -m \"even more\"\n @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'\n -\n -\techo \"another\" >> file &&\n +\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n -+\tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n ++\tcp \"$HOOKDIR/fail.sample\" \"$PRECOMMIT\" &&\n +\techo \"$PRECOMMIT\" >expected_hooks &&\n +\techo \"another\" >>file &&\n \tgit add file &&\n @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'\n -\n -\techo \"stuff\" >> file &&\n +\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n -+\tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n ++\tcp \"$HOOKDIR/fail.sample\" \"$PRECOMMIT\" &&\n +\techo \"stuff\" >>file &&\n \tgit add file &&\n -\tgit commit --no-verify -m \"stuff\"\n @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'\n -\n -\techo \"content\" >> file &&\n +\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n -+\tln -s \"non-exec.sample\" \"$PRECOMMIT\" &&\n ++\tcp \"$HOOKDIR/non-exec.sample\" \"$PRECOMMIT\" &&\n +\techo \"content\" >>file &&\n \tgit add file &&\n -\tgit commit -m \"content\"\n @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'\n -\n -\techo \"more content\" >> file &&\n +\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n -+\tln -s \"non-exec.sample\" \"$PRECOMMIT\" &&\n ++\tcp \"$HOOKDIR/non-exec.sample\" \"$PRECOMMIT\" &&\n +\techo \"more content\" >>file &&\n \tgit add file &&\n -\tgit commit --no-verify -m \"more content\"\n @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'\n -\n -\techo \"more content\" >> file &&\n +\ttest_when_finished \"rm -rf \\\"$PRECOMMIT\\\" expected_hooks actual_hooks success\" &&\n -+\tln -s \"require-prefix.sample\" \"$PRECOMMIT\" &&\n ++\tcp \"$HOOKDIR/require-prefix.sample\" \"$PRECOMMIT\" &&\n +\techo \"$PRECOMMIT\" >expected_hooks &&\n +\techo \"more content\" >>file &&\n \tgit add file &&\n @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'\n -\n -\techo \"more content\" >> file &&\n +\ttest_when_finished \"rm -rf \\\"$PRECOMMIT\\\" expected_hooks actual_hooks fail\" &&\n -+\tln -s \"require-prefix.sample\" \"$PRECOMMIT\" &&\n ++\tcp \"$HOOKDIR/require-prefix.sample\" \"$PRECOMMIT\" &&\n +\techo \"$PRECOMMIT\" >expected_hooks &&\n +\techo \"more content\" >>file &&\n \tgit add file &&\n @@ t/t7503-pre-commit-hook.sh: test_description='pre-commit hook'\n -\ttest \"$GIT_AUTHOR_NAME\" = \"New Author\" &&\n -\ttest \"$GIT_AUTHOR_EMAIL\" = \"newauthor@example.com\"\n +\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n -+\tln -s \"check-author.sample\" \"$PRECOMMIT\" &&\n ++\tcp \"$HOOKDIR/check-author.sample\" \"$PRECOMMIT\" &&\n +\tcat >expected_hooks <<-EOF &&\n +\t$PRECOMMIT\n +\t$PRECOMMIT\n2: 3b701a5c41 = 2: 02a97eb369 merge: do no-verify like commit\n3: 9210421fbb ! 3: 2d41e0ff79 git-merge: honor pre-merge-commit hook\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with succ\n \n +test_expect_success 'with succeeding hook (merge)' '\n +\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n -+\tln -s \"success.sample\" \"$PREMERGE\" &&\n ++\tcp \"$HOOKDIR/success.sample\" \"$PREMERGE\" &&\n +\techo \"$PREMERGE\" >expected_hooks &&\n +\tgit checkout side &&\n +\tgit merge -m \"merge master\" master &&\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with succ\n +\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" \\\"$PRECOMMIT\\\"\" &&\n +\ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n +\ttest_when_finished \"git checkout master\" &&\n -+\tln -s \"success.sample\" \"$PREMERGE\" &&\n -+\tln -s \"success.sample\" \"$PRECOMMIT\" &&\n ++\tcp \"$HOOKDIR/success.sample\" \"$PREMERGE\" &&\n ++\tcp \"$HOOKDIR/success.sample\" \"$PRECOMMIT\" &&\n +\n +\tgit checkout conflicting-a &&\n +\ttest_must_fail git merge -m \"merge conflicting-b\" conflicting-b &&\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with succ\n +\n test_expect_success '--no-verify with succeeding hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n - \tln -s \"success.sample\" \"$PRECOMMIT\" &&\n + \tcp \"$HOOKDIR/success.sample\" \"$PRECOMMIT\" &&\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-verify with failing hook' '\n \ttest_path_is_missing actual_hooks\n '\n \n +test_expect_success 'with failing hook (merge)' '\n +\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n -+\tln -s \"fail.sample\" \"$PREMERGE\" &&\n ++\tcp \"$HOOKDIR/fail.sample\" \"$PREMERGE\" &&\n +\techo \"$PREMERGE\" >expected_hooks &&\n +\tgit checkout side &&\n +\ttest_must_fail git merge -m \"merge master\" master &&\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-veri\n +\n test_expect_success POSIXPERM 'with non-executable hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n - \tln -s \"non-exec.sample\" \"$PRECOMMIT\" &&\n + \tcp \"$HOOKDIR/non-exec.sample\" \"$PRECOMMIT\" &&\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success POSIXPERM '--no-verify with non-executable hook' '\n \ttest_path_is_missing actual_hooks\n '\n \n +test_expect_success POSIXPERM 'with non-executable hook (merge)' '\n +\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n -+\tln -s \"non-exec.sample\" \"$PREMERGE\" &&\n ++\tcp \"$HOOKDIR/non-exec.sample\" \"$PREMERGE\" &&\n +\tgit branch -f side side-orig &&\n +\tgit checkout side &&\n +\tgit merge -m \"merge master\" master &&\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success POSIXPERM\n +\n test_expect_success 'with hook requiring GIT_PREFIX' '\n \ttest_when_finished \"rm -rf \\\"$PRECOMMIT\\\" expected_hooks actual_hooks success\" &&\n - \tln -s \"require-prefix.sample\" \"$PRECOMMIT\" &&\n + \tcp \"$HOOKDIR/require-prefix.sample\" \"$PRECOMMIT\" &&\n \n ## templates/hooks--pre-merge-commit.sample (new) ##\n @@\n4: 96c54883d3 ! 4: 7b81e2717f merge: --no-verify to bypass pre-merge-commit hook\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-veri\n +\n test_expect_success 'with succeeding hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n - \tln -s \"success.sample\" \"$PRECOMMIT\" &&\n + \tcp \"$HOOKDIR/success.sample\" \"$PRECOMMIT\" &&\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-verify with succeeding hook' '\n \ttest_path_is_missing actual_hooks\n '\n \n +test_expect_success '--no-verify with succeeding hook (merge)' '\n +\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n -+\tln -s \"success.sample\" \"$PREMERGE\" &&\n ++\tcp \"$HOOKDIR/success.sample\" \"$PREMERGE\" &&\n +\tgit branch -f side side-orig &&\n +\tgit checkout side &&\n +\tgit merge --no-verify -m \"merge master\" master &&\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success '--no-veri\n +\n test_expect_success 'with failing hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n - \tln -s \"fail.sample\" \"$PRECOMMIT\" &&\n + \tcp \"$HOOKDIR/fail.sample\" \"$PRECOMMIT\" &&\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with failing hook (merge)' '\n \ttest_cmp expected_hooks actual_hooks\n '\n \n +test_expect_success '--no-verify with failing hook (merge)' '\n +\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n -+\tln -s \"fail.sample\" \"$PREMERGE\" &&\n ++\tcp \"$HOOKDIR/fail.sample\" \"$PREMERGE\" &&\n +\tgit branch -f side side-orig &&\n +\tgit checkout side &&\n +\tgit merge --no-verify -m \"merge master\" master &&\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with fail\n +\n test_expect_success POSIXPERM 'with non-executable hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n - \tln -s \"non-exec.sample\" \"$PRECOMMIT\" &&\n + \tcp \"$HOOKDIR/non-exec.sample\" \"$PRECOMMIT\" &&\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success POSIXPERM 'with non-executable hook (merge)' '\n \ttest_path_is_missing actual_hooks\n '\n \n +test_expect_success POSIXPERM '--no-verify with non-executable hook (merge)' '\n +\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n -+\tln -s \"non-exec.sample\" \"$PREMERGE\" &&\n ++\tcp \"$HOOKDIR/non-exec.sample\" \"$PREMERGE\" &&\n +\tgit branch -f side side-orig &&\n +\tgit checkout side &&\n +\tgit merge --no-verify -m \"merge master\" master &&\n @@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success POSIXPERM\n +\n test_expect_success 'with hook requiring GIT_PREFIX' '\n \ttest_when_finished \"rm -rf \\\"$PRECOMMIT\\\" expected_hooks actual_hooks success\" &&\n - \tln -s \"require-prefix.sample\" \"$PRECOMMIT\" &&\n + \tcp \"$HOOKDIR/require-prefix.sample\" \"$PRECOMMIT\" &&\n-- \n2.22.0.770.g0f2c4a37fd-goog\n\n"},{"id":"380087","messageId":"60bbbbf9e0cf118945ace358cd85a8631ff7f26a.1565203880.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1565203880.git.steadmon@google.com","subject":"[PATCH v5 1/4] t7503: verify proper hook execution","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-07T18:57:05Z","receivedAt":"2019-08-07T18:57:24Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"t7503 did not verify that the expected hooks actually ran during\ntesting. Fix that by making the hook scripts write their $0 into a file\nso that we can compare actual execution vs. expected execution.\n\nWhile we're at it, do some test style cleanups, such as using\nwrite_script() and doing setup inside a test_expect_success block.\n\nImproved-by: Martin Ågren \nSigned-off-by: Martin Ågren \nSigned-off-by: Josh Steadmon \n---\n t/t7503-pre-commit-hook.sh | 157 +++++++++++++++++++++----------------\n 1 file changed, 89 insertions(+), 68 deletions(-)\n\ndiff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-hook.sh\nindex 984889b39d..6aa83204c2 100755\n--- a/t/t7503-pre-commit-hook.sh\n+++ b/t/t7503-pre-commit-hook.sh\n@@ -4,124 +4,144 @@ test_description='pre-commit hook'\n \n . ./test-lib.sh\n \n-test_expect_success 'with no hook' '\n+HOOKDIR=\"$(git rev-parse --git-dir)/hooks\"\n+PRECOMMIT=\"$HOOKDIR/pre-commit\"\n+\n+# Prepare sample scripts that write their $0 to actual_hooks\n+test_expect_success 'sample script setup' '\n+\tmkdir -p \"$HOOKDIR\" &&\n+\twrite_script \"$HOOKDIR/success.sample\" <<-\\EOF &&\n+\techo $0 >>actual_hooks\n+\texit 0\n+\tEOF\n+\twrite_script \"$HOOKDIR/fail.sample\" <<-\\EOF &&\n+\techo $0 >>actual_hooks\n+\texit 1\n+\tEOF\n+\twrite_script \"$HOOKDIR/non-exec.sample\" <<-\\EOF &&\n+\techo $0 >>actual_hooks\n+\texit 1\n+\tEOF\n+\tchmod -x \"$HOOKDIR/non-exec.sample\" &&\n+\twrite_script \"$HOOKDIR/require-prefix.sample\" <<-\\EOF &&\n+\techo $0 >>actual_hooks\n+\ttest $GIT_PREFIX = \"success/\"\n+\tEOF\n+\twrite_script \"$HOOKDIR/check-author.sample\" <<-\\EOF\n+\techo $0 >>actual_hooks\n+\ttest \"$GIT_AUTHOR_NAME\" = \"New Author\" &&\n+\ttest \"$GIT_AUTHOR_EMAIL\" = \"newauthor@example.com\"\n+\tEOF\n+'\n \n-\techo \"foo\" > file &&\n+test_expect_success 'with no hook' '\n+\ttest_when_finished \"rm -f actual_hooks\" &&\n+\techo \"foo\" >file &&\n \tgit add file &&\n-\tgit commit -m \"first\"\n-\n+\tgit commit -m \"first\" &&\n+\ttest_path_is_missing actual_hooks\n '\n \n test_expect_success '--no-verify with no hook' '\n-\n-\techo \"bar\" > file &&\n+\ttest_when_finished \"rm -f actual_hooks\" &&\n+\techo \"bar\" >file &&\n \tgit add file &&\n-\tgit commit --no-verify -m \"bar\"\n-\n+\tgit commit --no-verify -m \"bar\" &&\n+\ttest_path_is_missing actual_hooks\n '\n \n-# now install hook that always succeeds\n-HOOKDIR=\"$(git rev-parse --git-dir)/hooks\"\n-HOOK=\"$HOOKDIR/pre-commit\"\n-mkdir -p \"$HOOKDIR\"\n-cat > \"$HOOK\" <> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n+\tcp \"$HOOKDIR/success.sample\" \"$PRECOMMIT\" &&\n+\techo \"$PRECOMMIT\" >expected_hooks &&\n+\techo \"more\" >>file &&\n \tgit add file &&\n-\tgit commit -m \"more\"\n-\n+\tgit commit -m \"more\" &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n test_expect_success '--no-verify with succeeding hook' '\n-\n-\techo \"even more\" >> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n+\tcp \"$HOOKDIR/success.sample\" \"$PRECOMMIT\" &&\n+\techo \"even more\" >>file &&\n \tgit add file &&\n-\tgit commit --no-verify -m \"even more\"\n-\n+\tgit commit --no-verify -m \"even more\" &&\n+\ttest_path_is_missing actual_hooks\n '\n \n-# now a hook that fails\n-cat > \"$HOOK\" <> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n+\tcp \"$HOOKDIR/fail.sample\" \"$PRECOMMIT\" &&\n+\techo \"$PRECOMMIT\" >expected_hooks &&\n+\techo \"another\" >>file &&\n \tgit add file &&\n-\ttest_must_fail git commit -m \"another\"\n-\n+\ttest_must_fail git commit -m \"another\" &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n test_expect_success '--no-verify with failing hook' '\n-\n-\techo \"stuff\" >> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n+\tcp \"$HOOKDIR/fail.sample\" \"$PRECOMMIT\" &&\n+\techo \"stuff\" >>file &&\n \tgit add file &&\n-\tgit commit --no-verify -m \"stuff\"\n-\n+\tgit commit --no-verify -m \"stuff\" &&\n+\ttest_path_is_missing actual_hooks\n '\n \n-chmod -x \"$HOOK\"\n test_expect_success POSIXPERM 'with non-executable hook' '\n-\n-\techo \"content\" >> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n+\tcp \"$HOOKDIR/non-exec.sample\" \"$PRECOMMIT\" &&\n+\techo \"content\" >>file &&\n \tgit add file &&\n-\tgit commit -m \"content\"\n-\n+\tgit commit -m \"content\" &&\n+\ttest_path_is_missing actual_hooks\n '\n \n test_expect_success POSIXPERM '--no-verify with non-executable hook' '\n-\n-\techo \"more content\" >> file &&\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n+\tcp \"$HOOKDIR/non-exec.sample\" \"$PRECOMMIT\" &&\n+\techo \"more content\" >>file &&\n \tgit add file &&\n-\tgit commit --no-verify -m \"more content\"\n-\n+\tgit commit --no-verify -m \"more content\" &&\n+\ttest_path_is_missing actual_hooks\n '\n-chmod +x \"$HOOK\"\n-\n-# a hook that checks $GIT_PREFIX and succeeds inside the\n-# success/ subdirectory only\n-cat > \"$HOOK\" <> file &&\n+\ttest_when_finished \"rm -rf \\\"$PRECOMMIT\\\" expected_hooks actual_hooks success\" &&\n+\tcp \"$HOOKDIR/require-prefix.sample\" \"$PRECOMMIT\" &&\n+\techo \"$PRECOMMIT\" >expected_hooks &&\n+\techo \"more content\" >>file &&\n \tgit add file &&\n \tmkdir success &&\n \t(\n \t\tcd success &&\n \t\tgit commit -m \"hook requires GIT_PREFIX = success/\"\n \t) &&\n-\trmdir success\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n test_expect_success 'with failing hook requiring GIT_PREFIX' '\n-\n-\techo \"more content\" >> file &&\n+\ttest_when_finished \"rm -rf \\\"$PRECOMMIT\\\" expected_hooks actual_hooks fail\" &&\n+\tcp \"$HOOKDIR/require-prefix.sample\" \"$PRECOMMIT\" &&\n+\techo \"$PRECOMMIT\" >expected_hooks &&\n+\techo \"more content\" >>file &&\n \tgit add file &&\n \tmkdir fail &&\n \t(\n \t\tcd fail &&\n \t\ttest_must_fail git commit -m \"hook must fail\"\n \t) &&\n-\trmdir fail &&\n-\tgit checkout -- file\n+\tgit checkout -- file &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n test_expect_success 'check the author in hook' '\n-\twrite_script \"$HOOK\" <<-\\EOF &&\n-\ttest \"$GIT_AUTHOR_NAME\" = \"New Author\" &&\n-\ttest \"$GIT_AUTHOR_EMAIL\" = \"newauthor@example.com\"\n+\ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n+\tcp \"$HOOKDIR/check-author.sample\" \"$PRECOMMIT\" &&\n+\tcat >expected_hooks <<-EOF &&\n+\t$PRECOMMIT\n+\t$PRECOMMIT\n+\t$PRECOMMIT\n \tEOF\n \ttest_must_fail git commit --allow-empty -m \"by a.u.thor\" &&\n \t(\n@@ -133,7 +153,8 @@ test_expect_success 'check the author in hook' '\n \t) &&\n \tgit commit --author=\"New Author \" \\\n \t\t--allow-empty -m \"by new.author via command line\" &&\n-\tgit show -s\n+\tgit show -s &&\n+\ttest_cmp expected_hooks actual_hooks\n '\n \n test_done\n-- \n2.22.0.770.g0f2c4a37fd-goog\n\n"},{"id":"380088","messageId":"02a97eb369e7f99c426a4d8fd5efd2c9d1af1e5f.1565203880.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1565203880.git.steadmon@google.com","subject":"[PATCH v5 2/4] merge: do no-verify like commit","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-07T18:57:06Z","receivedAt":"2019-08-07T18:57:26Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"From: Michael J Gruber \n\nf8b863598c (\"builtin/merge: honor commit-msg hook for merges\", 2017-09-07)\nintroduced the no-verify flag to merge for bypassing the commit-msg\nhook, though in a different way from the implementation in commit.c.\n\nChange the implementation in merge.c to be the same as in commit.c so\nthat both do the same in the same way. This also changes the output of\n\"git merge --help\" to be more clear that the hook return code is\nrespected by default.\n\n[js: * reworded commit message\n * squashed documentation changes from original series' patch 3/4\n]\n\nSigned-off-by: Michael J Gruber \nSigned-off-by: Josh Steadmon \n---\n Documentation/git-merge.txt | 2 +-\n Documentation/merge-options.txt | 4 ++++\n builtin/merge.c | 6 +++---\n 3 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\nindex 01fd52dc70..092529c619 100644\n--- a/Documentation/git-merge.txt\n+++ b/Documentation/git-merge.txt\n@@ -10,7 +10,7 @@ SYNOPSIS\n --------\n [verse]\n 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]\n-\t[-s ] [-X ] [-S[]]\n+\t[--no-verify] [-s ] [-X ] [-S[]]\n \t[--[no-]allow-unrelated-histories]\n \t[--[no-]rerere-autoupdate] [-m ] [-F ] [...]\n 'git merge' (--continue | --abort | --quit)\ndiff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt\nindex 79a00d2a4a..d6a9f4b96f 100644\n--- a/Documentation/merge-options.txt\n+++ b/Documentation/merge-options.txt\n@@ -105,6 +105,10 @@ option can be used to override --squash.\n +\n With --squash, --commit is not allowed, and will fail.\n \n+--no-verify::\n+\tThis option bypasses the pre-merge and commit-msg hooks.\n+\tSee also linkgit:githooks[5].\n+\n -s ::\n --strategy=::\n \tUse the given merge strategy; can be supplied more than\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex e2ccbc44e2..4425a7a12e 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -81,7 +81,7 @@ static int show_progress = -1;\n static int default_to_upstream = 1;\n static int signoff;\n static const char *sign_commit;\n-static int verify_msg = 1;\n+static int no_verify;\n \n static struct strategy all_strategy[] = {\n \t{ \"recursive\", DEFAULT_TWOHEAD | NO_TRIVIAL },\n@@ -287,7 +287,7 @@ static struct option builtin_merge_options[] = {\n \t N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n \tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n-\tOPT_BOOL(0, \"verify\", &verify_msg, N_(\"verify commit-msg hook\")),\n+\tOPT_BOOL(0, \"no-verify\", &no_verify, N_(\"bypass commit-msg hook\")),\n \tOPT_END()\n };\n \n@@ -842,7 +842,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \t\t\tabort_commit(remoteheads, NULL);\n \t}\n \n-\tif (verify_msg && run_commit_hook(0 < option_edit, get_index_file(),\n+\tif (!no_verify && run_commit_hook(0 < option_edit, get_index_file(),\n \t\t\t\t\t \"commit-msg\",\n \t\t\t\t\t git_path_merge_msg(the_repository), NULL))\n \t\tabort_commit(remoteheads, NULL);\n-- \n2.22.0.770.g0f2c4a37fd-goog\n\n"},{"id":"380089","messageId":"2d41e0ff79045a3a0abf83d8f580e958023fb331.1565203880.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1565203880.git.steadmon@google.com","subject":"[PATCH v5 3/4] git-merge: honor pre-merge-commit hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-07T18:57:07Z","receivedAt":"2019-08-07T18:57:29Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"From: Michael J Gruber \n\ngit-merge does not honor the pre-commit hook when doing automatic merge\ncommits, and for compatibility reasons this is going to stay.\n\nIntroduce a pre-merge-commit hook which is called for an automatic merge\ncommit just like pre-commit is called for a non-automatic merge commit\n(or any other commit).\n\n[js: * renamed hook from \"pre-merge\" to \"pre-merge-commit\"\n * only discard the index if the hook is actually present\n * expanded githooks documentation entry\n * clarified that hook should write messages to stderr\n * squashed test changes from the original series' patch 4/4\n * modified tests to follow new pattern from this series' patch 1/4\n * added a test case for non-executable merge hooks\n * added a test case for failed merges\n * when testing that the merge hook did not run, make sure we\n actually have a merge to perform (by resetting the \"side\" branch\n to its original state).\n * reworded commit message\n]\n\nImproved-by: Martin Ågren \nSigned-off-by: Michael J Gruber \nSigned-off-by: Martin Ågren \nSigned-off-by: Josh Steadmon \n---\n Documentation/githooks.txt | 21 +++++\n builtin/merge.c | 12 +++\n ...-pre-commit-and-pre-merge-commit-hooks.sh} | 84 ++++++++++++++++++-\n templates/hooks--pre-merge-commit.sample | 13 +++\n 4 files changed, 129 insertions(+), 1 deletion(-)\n rename t/{t7503-pre-commit-hook.sh => t7503-pre-commit-and-pre-merge-commit-hooks.sh} (63%)\n create mode 100755 templates/hooks--pre-merge-commit.sample\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 82cd573776..d9da474fb0 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -103,6 +103,27 @@ The default 'pre-commit' hook, when enabled--and with the\n `hooks.allownonascii` config option unset or set to false--prevents\n the use of non-ASCII filenames.\n \n+pre-merge-commit\n+~~~~~~~~~~~~~~~~\n+\n+This hook is invoked by linkgit:git-merge[1]. It takes no parameters, and is\n+invoked after the merge has been carried out successfully and before\n+obtaining the proposed commit log message to\n+make a commit. Exiting with a non-zero status from this script\n+causes the `git merge` command to abort before creating a commit.\n+\n+The default 'pre-merge-commit' hook, when enabled, runs the\n+'pre-commit' hook, if the latter is enabled.\n+\n+This hook is invoked with the environment variable\n+`GIT_EDITOR=:` if the command will not bring up an editor\n+to modify the commit message.\n+\n+If the merge cannot be carried out automatically, the conflicts\n+need to be resolved and the result committed separately (see\n+linkgit:git-merge[1]). At that point, this hook will not be executed,\n+but the 'pre-commit' hook will, if it is enabled.\n+\n prepare-commit-msg\n ~~~~~~~~~~~~~~~~~~\n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 4425a7a12e..bf0ae68c40 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -816,6 +816,18 @@ static void write_merge_heads(struct commit_list *);\n static void prepare_to_commit(struct commit_list *remoteheads)\n {\n \tstruct strbuf msg = STRBUF_INIT;\n+\tconst char *index_file = get_index_file();\n+\n+\tif (run_commit_hook(0 < option_edit, index_file, \"pre-merge-commit\", NULL))\n+\t\tabort_commit(remoteheads, NULL);\n+\t/*\n+\t * Re-read the index as pre-merge-commit hook could have updated it,\n+\t * and write it out as a tree. We must do this before we invoke\n+\t * the editor and after we invoke run_status above.\n+\t */\n+\tif (find_hook(\"pre-merge-commit\"))\n+\t\tdiscard_cache();\n+\tread_cache_from(index_file);\n \tstrbuf_addbuf(&msg, &merge_msg);\n \tif (squash)\n \t\tBUG(\"the control must not reach here under --squash\");\ndiff --git a/t/t7503-pre-commit-hook.sh b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\nsimilarity index 63%\nrename from t/t7503-pre-commit-hook.sh\nrename to t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\nindex 6aa83204c2..7a5434c7ab 100755\n--- a/t/t7503-pre-commit-hook.sh\n+++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n@@ -1,11 +1,12 @@\n #!/bin/sh\n \n-test_description='pre-commit hook'\n+test_description='pre-commit and pre-merge-commit hooks'\n \n . ./test-lib.sh\n \n HOOKDIR=\"$(git rev-parse --git-dir)/hooks\"\n PRECOMMIT=\"$HOOKDIR/pre-commit\"\n+PREMERGE=\"$HOOKDIR/pre-merge-commit\"\n \n # Prepare sample scripts that write their $0 to actual_hooks\n test_expect_success 'sample script setup' '\n@@ -34,6 +35,30 @@ test_expect_success 'sample script setup' '\n \tEOF\n '\n \n+test_expect_success 'root commit' '\n+\techo \"root\" >file &&\n+\tgit add file &&\n+\tgit commit -m \"zeroth\" &&\n+\tgit checkout -b side &&\n+\techo \"foo\" >foo &&\n+\tgit add foo &&\n+\tgit commit -m \"make it non-ff\" &&\n+\tgit branch side-orig side &&\n+\tgit checkout master\n+'\n+\n+test_expect_success 'setup conflicting branches' '\n+\ttest_when_finished \"git checkout master\" &&\n+\tgit checkout -b conflicting-a master &&\n+\techo a >conflicting &&\n+\tgit add conflicting &&\n+\tgit commit -m conflicting-a &&\n+\tgit checkout -b conflicting-b master &&\n+\techo b >conflicting &&\n+\tgit add conflicting &&\n+\tgit commit -m conflicting-b\n+'\n+\n test_expect_success 'with no hook' '\n \ttest_when_finished \"rm -f actual_hooks\" &&\n \techo \"foo\" >file &&\n@@ -42,6 +67,15 @@ test_expect_success 'with no hook' '\n \ttest_path_is_missing actual_hooks\n '\n \n+test_expect_success 'with no hook (merge)' '\n+\ttest_when_finished \"rm -f actual_hooks\" &&\n+\tgit branch -f side side-orig &&\n+\tgit checkout side &&\n+\tgit merge -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_path_is_missing actual_hooks\n+'\n+\n test_expect_success '--no-verify with no hook' '\n \ttest_when_finished \"rm -f actual_hooks\" &&\n \techo \"bar\" >file &&\n@@ -60,6 +94,34 @@ test_expect_success 'with succeeding hook' '\n \ttest_cmp expected_hooks actual_hooks\n '\n \n+test_expect_success 'with succeeding hook (merge)' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n+\tcp \"$HOOKDIR/success.sample\" \"$PREMERGE\" &&\n+\techo \"$PREMERGE\" >expected_hooks &&\n+\tgit checkout side &&\n+\tgit merge -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_cmp expected_hooks actual_hooks\n+'\n+\n+test_expect_success 'automatic merge fails; both hooks are available' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" \\\"$PRECOMMIT\\\"\" &&\n+\ttest_when_finished \"rm -f expected_hooks actual_hooks\" &&\n+\ttest_when_finished \"git checkout master\" &&\n+\tcp \"$HOOKDIR/success.sample\" \"$PREMERGE\" &&\n+\tcp \"$HOOKDIR/success.sample\" \"$PRECOMMIT\" &&\n+\n+\tgit checkout conflicting-a &&\n+\ttest_must_fail git merge -m \"merge conflicting-b\" conflicting-b &&\n+\ttest_path_is_missing actual_hooks &&\n+\n+\techo \"$PRECOMMIT\" >expected_hooks &&\n+\techo a+b >conflicting &&\n+\tgit add conflicting &&\n+\tgit commit -m \"resolve conflict\" &&\n+\ttest_cmp expected_hooks actual_hooks\n+'\n+\n test_expect_success '--no-verify with succeeding hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n \tcp \"$HOOKDIR/success.sample\" \"$PRECOMMIT\" &&\n@@ -88,6 +150,16 @@ test_expect_success '--no-verify with failing hook' '\n \ttest_path_is_missing actual_hooks\n '\n \n+test_expect_success 'with failing hook (merge)' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" expected_hooks actual_hooks\" &&\n+\tcp \"$HOOKDIR/fail.sample\" \"$PREMERGE\" &&\n+\techo \"$PREMERGE\" >expected_hooks &&\n+\tgit checkout side &&\n+\ttest_must_fail git merge -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_cmp expected_hooks actual_hooks\n+'\n+\n test_expect_success POSIXPERM 'with non-executable hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n \tcp \"$HOOKDIR/non-exec.sample\" \"$PRECOMMIT\" &&\n@@ -106,6 +178,16 @@ test_expect_success POSIXPERM '--no-verify with non-executable hook' '\n \ttest_path_is_missing actual_hooks\n '\n \n+test_expect_success POSIXPERM 'with non-executable hook (merge)' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n+\tcp \"$HOOKDIR/non-exec.sample\" \"$PREMERGE\" &&\n+\tgit branch -f side side-orig &&\n+\tgit checkout side &&\n+\tgit merge -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_path_is_missing actual_hooks\n+'\n+\n test_expect_success 'with hook requiring GIT_PREFIX' '\n \ttest_when_finished \"rm -rf \\\"$PRECOMMIT\\\" expected_hooks actual_hooks success\" &&\n \tcp \"$HOOKDIR/require-prefix.sample\" \"$PRECOMMIT\" &&\ndiff --git a/templates/hooks--pre-merge-commit.sample b/templates/hooks--pre-merge-commit.sample\nnew file mode 100755\nindex 0000000000..399eab1924\n--- /dev/null\n+++ b/templates/hooks--pre-merge-commit.sample\n@@ -0,0 +1,13 @@\n+#!/bin/sh\n+#\n+# An example hook script to verify what is about to be committed.\n+# Called by \"git merge\" with no arguments. The hook should\n+# exit with non-zero status after issuing an appropriate message to\n+# stderr if it wants to stop the merge commit.\n+#\n+# To enable this hook, rename this file to \"pre-merge-commit\".\n+\n+. git-sh-setup\n+test -x \"$GIT_DIR/hooks/pre-commit\" &&\n+ exec \"$GIT_DIR/hooks/pre-commit\"\n+:\n-- \n2.22.0.770.g0f2c4a37fd-goog\n\n"},{"id":"380090","messageId":"7b81e2717f739a92dea10eaf570407c742d30533.1565203880.git.steadmon@google.com","threadId":"46808","inReplyTo":"cover.1565203880.git.steadmon@google.com","subject":"[PATCH v5 4/4] merge: --no-verify to bypass pre-merge-commit hook","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2019-08-07T18:57:08Z","receivedAt":"2019-08-07T18:57:32Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"From: Michael J Gruber \n\nAnalogous to commit, introduce a '--no-verify' option which bypasses the\npre-merge-commit hook. The shorthand '-n' is taken by '--no-stat'\nalready.\n\n[js: * reworded commit message to reflect current state of --no-stat flag\n and new hook name\n * fixed flag documentation to reflect new hook name\n * cleaned up trailing whitespace\n * squashed test changes from the original series' patch 4/4\n * modified tests to follow pattern from this series' patch 1/4\n * added a test case for --no-verify with non-executable hook\n * when testing that the merge hook did not run, make sure we\n actually have a merge to perform (by resetting the \"side\" branch\n to its original state).\n\n]\n\nImproved-by: Martin Ågren \nSigned-off-by: Michael J Gruber \nSigned-off-by: Martin Ågren \nSigned-off-by: Josh Steadmon \n---\n Documentation/githooks.txt | 3 +-\n builtin/merge.c | 4 +-\n ...3-pre-commit-and-pre-merge-commit-hooks.sh | 39 +++++++++++++++++++\n 3 files changed, 43 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex d9da474fb0..57d6e2b98d 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -106,7 +106,8 @@ the use of non-ASCII filenames.\n pre-merge-commit\n ~~~~~~~~~~~~~~~~\n \n-This hook is invoked by linkgit:git-merge[1]. It takes no parameters, and is\n+This hook is invoked by linkgit:git-merge[1], and can be bypassed\n+with the `--no-verify` option. It takes no parameters, and is\n invoked after the merge has been carried out successfully and before\n obtaining the proposed commit log message to\n make a commit. Exiting with a non-zero status from this script\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex bf0ae68c40..c9746e37b8 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -287,7 +287,7 @@ static struct option builtin_merge_options[] = {\n \t N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n \tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n-\tOPT_BOOL(0, \"no-verify\", &no_verify, N_(\"bypass commit-msg hook\")),\n+\tOPT_BOOL(0, \"no-verify\", &no_verify, N_(\"bypass pre-merge-commit and commit-msg hooks\")),\n \tOPT_END()\n };\n \n@@ -818,7 +818,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \tstruct strbuf msg = STRBUF_INIT;\n \tconst char *index_file = get_index_file();\n \n-\tif (run_commit_hook(0 < option_edit, index_file, \"pre-merge-commit\", NULL))\n+\tif (!no_verify && run_commit_hook(0 < option_edit, index_file, \"pre-merge-commit\", NULL))\n \t\tabort_commit(remoteheads, NULL);\n \t/*\n \t * Re-read the index as pre-merge-commit hook could have updated it,\ndiff --git a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\nindex 7a5434c7ab..b3485450a2 100755\n--- a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n+++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh\n@@ -84,6 +84,15 @@ test_expect_success '--no-verify with no hook' '\n \ttest_path_is_missing actual_hooks\n '\n \n+test_expect_success '--no-verify with no hook (merge)' '\n+\ttest_when_finished \"rm -f actual_hooks\" &&\n+\tgit branch -f side side-orig &&\n+\tgit checkout side &&\n+\tgit merge --no-verify -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_path_is_missing actual_hooks\n+'\n+\n test_expect_success 'with succeeding hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n \tcp \"$HOOKDIR/success.sample\" \"$PRECOMMIT\" &&\n@@ -131,6 +140,16 @@ test_expect_success '--no-verify with succeeding hook' '\n \ttest_path_is_missing actual_hooks\n '\n \n+test_expect_success '--no-verify with succeeding hook (merge)' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n+\tcp \"$HOOKDIR/success.sample\" \"$PREMERGE\" &&\n+\tgit branch -f side side-orig &&\n+\tgit checkout side &&\n+\tgit merge --no-verify -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_path_is_missing actual_hooks\n+'\n+\n test_expect_success 'with failing hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" expected_hooks actual_hooks\" &&\n \tcp \"$HOOKDIR/fail.sample\" \"$PRECOMMIT\" &&\n@@ -160,6 +179,16 @@ test_expect_success 'with failing hook (merge)' '\n \ttest_cmp expected_hooks actual_hooks\n '\n \n+test_expect_success '--no-verify with failing hook (merge)' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n+\tcp \"$HOOKDIR/fail.sample\" \"$PREMERGE\" &&\n+\tgit branch -f side side-orig &&\n+\tgit checkout side &&\n+\tgit merge --no-verify -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_path_is_missing actual_hooks\n+'\n+\n test_expect_success POSIXPERM 'with non-executable hook' '\n \ttest_when_finished \"rm -f \\\"$PRECOMMIT\\\" actual_hooks\" &&\n \tcp \"$HOOKDIR/non-exec.sample\" \"$PRECOMMIT\" &&\n@@ -188,6 +217,16 @@ test_expect_success POSIXPERM 'with non-executable hook (merge)' '\n \ttest_path_is_missing actual_hooks\n '\n \n+test_expect_success POSIXPERM '--no-verify with non-executable hook (merge)' '\n+\ttest_when_finished \"rm -f \\\"$PREMERGE\\\" actual_hooks\" &&\n+\tcp \"$HOOKDIR/non-exec.sample\" \"$PREMERGE\" &&\n+\tgit branch -f side side-orig &&\n+\tgit checkout side &&\n+\tgit merge --no-verify -m \"merge master\" master &&\n+\tgit checkout master &&\n+\ttest_path_is_missing actual_hooks\n+'\n+\n test_expect_success 'with hook requiring GIT_PREFIX' '\n \ttest_when_finished \"rm -rf \\\"$PRECOMMIT\\\" expected_hooks actual_hooks success\" &&\n \tcp \"$HOOKDIR/require-prefix.sample\" \"$PRECOMMIT\" &&\n-- \n2.22.0.770.g0f2c4a37fd-goog\n\n"},{"id":"380127","messageId":"CAN0heSqGv3XKx+nE+8QqP6Wr7rpVtBHFqZuz1KaikRmq-yg8+w@mail.gmail.com","threadId":"46808","inReplyTo":"cover.1565203880.git.steadmon@google.com","subject":"Re: [PATCH v5 0/4] pre-merge-commit hook","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2019-08-08T13:08:09Z","receivedAt":"2019-08-08T13:08:23Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Wed, 7 Aug 2019 at 20:57, Josh Steadmon wrote:\n>\n> This series adds a new pre-merge-commit hook, similar in usage to\n> pre-commit. It also improves hook testing in t7503, by verifying that\n> the correct hooks are run or bypassed as expected.\n>\n> The original series was done by Michael J Gruber . I have\n> addressed the outstanding review comments, and noted my changes in the\n> commit messages in \"[js: ...]\" blocks.\n>\n> Changes since V4:\n\nI don't have any more nits to pick ;-) so this looks good to me, FWIW.\n\nMartin\n"}]}