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

[Stefan Beller <sbeller@google.com>]

On Fri, Sep 22, 2017 at 5:04 AM, Michael J Gruber <git@grubix.eu> wrote:
> From: Michael J Gruber <git@drmicha.warpmail.net>
>
> git-merge does not honor the pre-commit hook when doing automatic merge
> commits, and for compatibility reasons this is going to stay.
>
> Introduce a pre-merge hook which is called for an automatic merge commit
> just like pre-commit is called for a non-automatic merge commit (or any
> other commit).
>
> Signed-off-by: Michael J Gruber <git@grubix.eu>
> ---
>  Documentation/githooks.txt        |  7 +++++++
>  builtin/merge.c                   | 11 +++++++++++
>  templates/hooks--pre-merge.sample | 13 +++++++++++++
>  3 files changed, 31 insertions(+)
>  create mode 100755 templates/hooks--pre-merge.sample
>
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index 1bb4f92d4d..85bedd208c 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -99,6 +99,13 @@ All the 'git commit' hooks are invoked with the environment
>  variable `GIT_EDITOR=:` if the command will not bring up an editor
>  to modify the commit message.
>
> +pre-merge
> +~~~~~~~~~
> +
> +This hook is invoked by 'git merge' when doing an automatic merge
> +commit; it is equivalent to 'pre-commit' for a non-automatic commit
> +for a merge.
> +
>  prepare-commit-msg
>  ~~~~~~~~~~~~~~~~~~
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index ab5ffe85e8..de254d466b 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -769,6 +769,17 @@ static void write_merge_heads(struct commit_list *);
>  static void prepare_to_commit(struct commit_list *remoteheads)
>  {
>         struct strbuf msg = STRBUF_INIT;
> +       const char *index_file = get_index_file();
> +
> +       if (run_commit_hook(0 < option_edit, index_file, "pre-merge", NULL))
> +               abort_commit(remoteheads, NULL);
> +       /*
> +        * Re-read the index as pre-merge hook could have updated it,
> +        * and write it out as a tree.  We must do this before we invoke
> +        * the editor and after we invoke run_status above.
> +        */
> +       discard_cache();
> +       read_cache_from(index_file);

Please see 680ee550d7 (commit: skip discarding the index
if there is no pre-commit hook, 2017-08-14), maybe we can do it similarly.
Dropping and rereading the index may be expensive for large repos.

>         strbuf_addbuf(&msg, &merge_msg);
>         strbuf_addch(&msg, '\n');
>         if (squash)
> diff --git a/templates/hooks--pre-merge.sample b/templates/hooks--pre-merge.sample
> new file mode 100755
> index 0000000000..a6313e6d5c
> --- /dev/null
> +++ b/templates/hooks--pre-merge.sample
> @@ -0,0 +1,13 @@
> +#!/bin/sh
> +#
> +# An example hook script to verify what is about to be committed.
> +# Called by "git merge" with no arguments.  The hook should
> +# exit with non-zero status after issuing an appropriate message if

The message goes to stdout or sterr or both?

> +# it wants to stop the commit.

nit: s/commit/merge commit/ maybe?

Thanks,
Stefan