Re: [PATCH] git: add --no-hooks global option

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Stolee

On 03/04/2025 23:38, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <stolee@gmail.com>
> 
> In some situations, these hooks have poor performance and expert users
> may want to skip the hooks as they don't seem to affect the current
> situation. One example is a pre-commit hook that checks for certain
> structures in the local changes, but expert users are likely to have
> done the right thing in advance.

Next they'll be saying that they never make a mistake when writing a one 
line patch! More seriously I agree there are times when one may want to 
bypass the pre-commit hook but we already have "git commit --no-verify" 
to do that. In general hooks that are so slow that the user wants to 
bypass them are self-defeating and I'd argue that the solution is to fix 
the performance of the hook rather than make it easier to skip it. One 
solution for speeding up pre-commit hooks is to process files in 
parallel. Unfortunately git does not provide support for that but there 
are hook frameworks that do.

> I have come across users who have disabled hooks themselves either by
> deleting hooks (supported, safe) or setting 'core.hooksPath' to some
> bogus path (seems unsafe).

I thought "git -c core.hooksPath=/dev/null" was a fairly standard way of 
disabling hooks on a one-off basis - what makes it unsafe?

> The supported process is painful to swap
> between the hook-enabled scenario and the hook-disabled scenario.
> 
> To that end, add a new --no-hooks global option to allow users to
> disable hooks quickly. This option is modeled similarly to the
> --no-advice option in b79deeb554 (advice: add --no-advice global option,
> 2024-05-03). This uses a GIT_HOOKS environment variable to communicate
> to subprocesses as well as making this a backwards-compatible way for
> tools to signal that they want to disable hooks.
> 
> The critical piece is that all hooks pass through run_hooks_opt() where
> a static int will evaluate the environment variable and store that the
> variable is initialized for faster repeated runs.

That certainly makes the implementation much more viable. However I'm 
not really convinced this is a good idea.

Best Wishes

Phillip

> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
>      git: add --no-hooks global option
>      
>      This is hopefully a helpful feature to more than just the experts I've
>      been hearing from.
>      
>      Thanks,
>      
>       * Stolee
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1899%2Fderrickstolee%2Fno-hooks-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1899/derrickstolee/no-hooks-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1899
> 
>   Documentation/git.adoc       | 13 ++++++++++++-
>   environment.h                |  6 ++++++
>   git.c                        |  6 +++++-
>   hook.c                       |  7 +++++++
>   t/t1350-config-hooks-path.sh | 34 ++++++++++++++++++++++++++++++++++
>   5 files changed, 64 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/git.adoc b/Documentation/git.adoc
> index 743b7b00e4d..a34c8cfbe78 100644
> --- a/Documentation/git.adoc
> +++ b/Documentation/git.adoc
> @@ -14,7 +14,7 @@ SYNOPSIS
>       [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]
>       [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]
>       [--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]
> -    <command> [<args>]
> +    [--no-hooks] <command> [<args>]
>   
>   DESCRIPTION
>   -----------
> @@ -230,6 +230,12 @@ If you just want to run git as if it was started in `<path>` then use
>   	linkgit:gitattributes[5]. This is equivalent to setting the
>   	`GIT_ATTR_SOURCE` environment variable.
>   
> +--no-hooks::
> +	Skip running local Git hooks, even if configured locally. Hooks
> +	are an opt-in feature, so be sure that you know the impact of
> +	ignoring hooks when running with this option. This is equivalent
> +	to setting `GIT_HOOKS=0` environment variable.
> +
>   GIT COMMANDS
>   ------------
>   
> @@ -771,6 +777,11 @@ for further details.
>   	not set, Git will choose buffered or record-oriented flushing
>   	based on whether stdout appears to be redirected to a file or not.
>   
> +`GIT_HOOKS`::
> +	If this Boolean environment variable is set to false, then commands
> +	will ignore any configured hooks as if the `--no-hooks` option was
> +	provided.
> +
>   `GIT_TRACE`::
>   	Enables general trace messages, e.g. alias expansion, built-in
>   	command execution and external command execution.
> diff --git a/environment.h b/environment.h
> index 45e690f203f..22ddf201144 100644
> --- a/environment.h
> +++ b/environment.h
> @@ -50,6 +50,12 @@
>    */
>   #define GIT_ADVICE_ENVIRONMENT "GIT_ADVICE"
>   
> +/*
> + * Environment variable used to propagate the --no-hooks global option to
> + * the hooks layer and to any child processes.
> + */
> +#define GIT_HOOKS "GIT_HOOKS"
> +
>   /*
>    * Environment variable used in handshaking the wire protocol.
>    * Contains a colon ':' separated list of keys with optional values
> diff --git a/git.c b/git.c
> index 77c43595223..d7ebcf60947 100644
> --- a/git.c
> +++ b/git.c
> @@ -41,7 +41,7 @@ const char git_usage_string[] =
>   	   "           [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]\n"
>   	   "           [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]\n"
>   	   "           [--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]\n"
> -	   "           <command> [<args>]");
> +	   "           [--no-hooks] <command> [<args>]");
>   
>   const char git_more_info_string[] =
>   	N_("'git help -a' and 'git help -g' list available subcommands and some\n"
> @@ -349,6 +349,10 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
>   			setenv(GIT_ADVICE_ENVIRONMENT, "0", 1);
>   			if (envchanged)
>   				*envchanged = 1;
> +		} else if (!strcmp(cmd, "--no-hooks")) {
> +			setenv(GIT_HOOKS, "0", 1);
> +			if (envchanged)
> +				*envchanged = 1;
>   		} else {
>   			fprintf(stderr, _("unknown option: %s\n"), cmd);
>   			usage(git_usage_string);
> diff --git a/hook.c b/hook.c
> index b3de1048bf4..b209553d7a8 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -144,6 +144,13 @@ int run_hooks_opt(struct repository *r, const char *hook_name,
>   
>   		.data = &cb_data,
>   	};
> +	static int do_run_hooks = -1;
> +
> +	if (do_run_hooks < 0)
> +		do_run_hooks = git_env_bool(GIT_HOOKS, 1);
> +
> +	if (!do_run_hooks)
> +		goto cleanup;
>   
>   	if (!options)
>   		BUG("a struct run_hooks_opt must be provided to run_hooks");
> diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh
> index 45a04929170..4c6a0eafe4e 100755
> --- a/t/t1350-config-hooks-path.sh
> +++ b/t/t1350-config-hooks-path.sh
> @@ -48,4 +48,38 @@ test_expect_success 'core.hooksPath=/dev/null' '
>   	{ test /dev/null = "$value" || test nul = "$value"; }
>   '
>   
> +test_expect_success '--no-hooks' '
> +	rm -f actual &&
> +	test_might_fail git config --unset core.hooksPath &&
> +
> +	write_script .git/hooks/pre-commit <<-\EOF &&
> +	echo HOOK >>actual
> +	EOF
> +
> +	echo HOOK >expect &&
> +
> +	git commit --allow-empty -m "A" &&
> +	test_cmp expect actual &&
> +
> +	git --no-hooks commit --allow-empty -m "B" &&
> +	test_cmp expect actual
> +'
> +
> +test_expect_success 'GIT_HOOKS' '
> +	rm -f actual &&
> +	test_might_fail git config --unset core.hooksPath &&
> +
> +	write_script .git/hooks/pre-commit <<-\EOF &&
> +	echo HOOK >>actual
> +	EOF
> +
> +	echo HOOK >expect &&
> +
> +	GIT_HOOKS=1 git commit --allow-empty -m "A" &&
> +	test_cmp expect actual &&
> +
> +	GIT_HOOKS=0 git commit --allow-empty -m "B" &&
> +	test_cmp expect actual
> +'
> +
>   test_done
> 
> base-commit: 5b97a56fa0e7d580dc8865b73107407c9b3f0eff