Re: [PATCH-v5 4/4] git-commit: add a prepare-commit-msg hook

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 6, 2008, 10:58 UTC
Message-ID: <7v3as6z700.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <E1JMgdC-00019t-HY@fencepost.gnu.org>

Paolo Bonzini <bonzini@gnu.org> writes:

> If the exit status is non-zero, git-commit will abort.  The hook is
> not suppressed by the --no-verify option, so it should not be used
> as a replacement for the pre-commit hook.

I see this matches _exactly_ what I wanted to say. I however am suspecting that it may not match what you think the hook should do, though.

For example,

> The hook is allowed to edit the message file in place.  It can also be
> used for the same purpose as the pre-commit message, if the verification
> should not be skipped for automatic commits (e.g. during interactive
> rebasing).

I, who is claiming that the purpose of this hook is about preparing the message and not about validating and interfering with the commit, would never say that. It's not just "allowed"; editing is the _sole_ reason for its existence.

I'd make it even stronger and clearer:

    The purpose of the hook is to edit the message file in place
    before it gets presented to the user for further editing.
    It is not suppressed by the --no-verify option, because it
    is not about validating what is to be committed.  A non-zero
    exit from the hook means the hook failed, and aborts the
    commit (in other words, non-zero exit from the hook is an
    abnormal condition, perhaps a bug in the hook itself, and
    should never be about the hook not liking the commit being
    created).

But that is apparently quite different from what you wrote. So I am sensing some miscommunication here. I suspect that you are not convinced by my claim that the hook's sole purpose should be preparing the commit message to be edited, and that it should never be used for validation. Yet an early part of your commit log message pretends that you are agreeing with me. The rest of the patch however still seems to think differently.

And if you think the hook should actively interfere with the commit, please argue for that case first, without sending an incoherent patch. As I often say, unlike Linus, I am not always right.

Show 7 quoted lines

> +If the exit status is non-zero, `git-commit` will abort.
> +The hook is not suppressed by the `\--no-verify` option.
> +
> +The hook is allowed to edit the message file in place.  It can also be
> +used for the same purpose as the pre-commit message, if the verification
> +should not be skipped for automatic commits (e.g. during interactive
> +rebasing).

The same comment applies here.

Show 10 quoted lines

> +++ b/templates/hooks--prepare-commit-msg
> @@ -0,0 +1,36 @@
> +#!/bin/sh
> +#
> +# An example hook script to prepare the commit log message.
> +# Called by git-commit with the name of the file that has the
> +# commit message, followed by the description of the commit
> +# message's source.  The hook should exit with non-zero
> +# status after issuing an appropriate message if it wants to stop the
> +# commit.  The hook is allowed to edit the commit message file.

The same comment applies here as well.

There may be cases where an unmaskable validation hook is desired. But even if that is the case, I see little reason to tie it to the act of commit message preparation.