From: Phillip Wood Date: Fri, 04 Apr 2025 14:15:22 GMT Subject: Re: [PATCH] git: add --no-hooks global option Message-ID: <2c7994bc-2be0-43a5-9627-0d530746b3ab@gmail.com> In-Reply-To: Hi Stolee On 03/04/2025 23:38, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee > > 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 > --- > 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=] > [--work-tree=] [--namespace=] [--config-env==] > - [] > + [--no-hooks] [] > > DESCRIPTION > ----------- > @@ -230,6 +230,12 @@ If you just want to run git as if it was started in `` 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=]\n" > " [--work-tree=] [--namespace=] [--config-env==]\n" > - " []"); > + " [--no-hooks] []"); > > 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