On 4/4/2025 10:15 AM, Phillip Wood wrote: > Hi Stolee > > On 03/04/2025 23:38, Derrick Stolee via GitGitGadget wrote: >> From: Derrick Stolee <stolee@xxxxxxxxx> >> >> 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. >> 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. >> 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
