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

From: Derrick Stolee <stolee@gmail.com>
Date: Apr 14, 2025, 10:59 UTC
Message-ID: <cb3d080b-51a3-4096-9650-34ec4ed68f1d@gmail.com>
In-Reply-To: <2c7994bc-2be0-43a5-9627-0d530746b3ab@gmail.com>

On 4/4/2025 10:15 AM, Phillip Wood wrote:

Show 18 quoted lines

> 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.

Both can also be an option.

> 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.

Show 6 quoted lines

>> 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?

You're right. I was thinking about setting it to a "directory that doesn't exist" (but actually could be a path that exists accidentally like "/bogus") but I forgot that we could use /dev/null.

I'll remove this "(seems unsafe)" wording.

Show 16 quoted lines

>> 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.

I don't read a strong reason in your message that this is a _bad_ idea either. As in, there's nothing that hints that this will cause significant harm to users other than providing a new footgun (and we have plenty of those for folks willing to look, including the _existence_ of hooks).

Thanks, -Stolee