mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <>
To: Thierry Reding <>
Cc:, Taylor Blau <>
Subject: Re: [PATCH v2] am: Allow passing --no-verify flag
Date: Mon, 21 Nov 2022 14:33:06 +0900	[thread overview]
Message-ID: <xmqqilj8yir1.fsf@gitster.g> (raw)
In-Reply-To: <> (Thierry Reding's message of "Sat, 19 Nov 2022 01:50:31 +0100")

Thierry Reding <> writes:

> From: Thierry Reding <>
> The git-am --no-verify flag is analogous to the same flag passed to
> git-commit. It bypasses the pre-applypatch and applypatch-msg hooks
> if they are enabled.
> Signed-off-by: Thierry Reding <>
> ---
> Changes in v2:
> - add test to verify that the new option works

> +-n::
> +--no-verify::
> +	By default, the pre-applypatch and applypatch-msg hooks are run.
> +	When any of `--no-verify` or `-n` is given, these are bypassed.
> +	See also linkgit:githooks[5].

I think the goal of this topic is to allow bypassing the checks made
by these two hooks (and possibly future ones that validate the input
to "am"), and there are at least two possible implementations to
achieve that goal.  You can still run the hook and ignore its
failure exit, or you can skip running the hook and pretend as if
hook succeeded.

As it is documented that applypatch-msg is allowed to edit the
message file to normalize the message, between the two, running the
hook (to allow the hook to automatically edit the message) but
ignoring its failure would be a more intuitive approach to "bypass"
the check.  If the option were called --no-hook or --bypass-hooks
then it would be a different story, though.

>  	assert(state->msg);
> -	ret = run_hooks_l("applypatch-msg", am_path(state, "final-commit"), NULL);
> +
> +	if (!state->no_verify)
> +		ret = run_hooks_l("applypatch-msg", am_path(state, "final-commit"), NULL);

And it seems that this took a less intuitive avenue of bypassing the
hook completely.  I am not 100% convinced that this is the better
choice (but I am not convinced it is the worse one, either).

> diff --git a/t/ b/t/
> new file mode 100755
> index 000000000000..fbf45998243f
> --- /dev/null
> +++ b/t/
> @@ -0,0 +1,60 @@
> +#!/bin/sh
> +

It is surprising, and I am not enthused to see, that this needs an
entirely new script.

Don't we already have a script or two to test "am", among which the
invocation of hooks is already tested?

  reply	other threads:[~2022-11-21  5:33 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-19  0:50 [PATCH v2] am: Allow passing --no-verify flag Thierry Reding
2022-11-21  5:33 ` Junio C Hamano [this message]
2022-11-25 17:41   ` Thierry Reding
2022-11-27  1:27     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqilj8yir1.fsf@gitster.g \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).