Re: [PATCH v3 3/4] git-merge: honor pre-merge-commit hook

["Martin Ågren" <martin.agren@gmail.com>]

On Fri, 2 Aug 2019 at 00:20, Josh Steadmon <steadmon@google.com> wrote:

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

I'm not sure everyone understands what an "automatic merge commit" is.
(Is it an automatic "merge commit", or an "automatic merge" commit? Or
sort of both?) And I'm not sure exactly what to infer from the
"equivalence". I happen to know that the statement about the default
hook can only be half-carried over. And I'm not sure what to infer from
"All the git commit hooks are invoked with the environment variable
...".

Is the below suggestion 1) correct, 2) readable?

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

  The default 'pre-merge-commit' hook, when enabled, runs the
  'pre-commit' hook, if the latter is enabled.

  This hook is invoked with the environment variable
  `GIT_EDITOR=:` if the command will not bring up an editor
  to modify the commit message.

  If the merge cannot be carried out automatically, the conflicts
  need to be resolved and the result committed separately (see
  linkgit:git-merge[1]). At that point, this hook will not be executed,
  but the 'pre-commit' hook will, if it is enabled.

(If you use this or something like it, notice how this already mentions
`--no-verify`...)

> +test_expect_success 'root commit' '
> +       echo "root" > file &&
> +       git add file &&
> +       git commit -m "zeroth" &&
> +       git checkout -b side &&
> +       echo "foo" > foo &&
> +       git add foo &&
> +       git commit -m "make it non-ff" &&
> +       git checkout master
> +'

You got rid of loads of "> file" in patch 1/4, so it seems unfortunate
to introduce a few here. ;-)


Martin