From: Derrick Stolee <stolee@gmail.com>
To: "brian m. carlson" <sandals@crustytoothpaste.net>,
Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org, gitster@pobox.com, james@jamesliu.io
Subject: Re: [PATCH] git: add --no-hooks global option
Date: Thu, 3 Apr 2025 20:40:07 -0400 [thread overview]
Message-ID: <c1312cb3-dd2b-4b04-8c6f-2d787125448d@gmail.com> (raw)
In-Reply-To: <Z-8Rxy4IK5EHKzLW@tapette.crustytoothpaste.net>
On 4/3/2025 6:55 PM, brian m. carlson wrote:
> On 2025-04-03 at 22:38:08, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <stolee@gmail.com>
>> 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.
>> This is hopefully a helpful feature to more than just the experts I've
>> been hearing from.
>
> I think this is functionality that Jenkins wants because they've
> configured `core.hooksPath` to `/dev/null`, allegedly for security
> reasons. Of course, enabling this feature will also break Git LFS, but
> in a less noticeable and detectable way (currently, Git LFS will fail on
> attempting to install hooks with that setting of `core.hooksPath`, which
> is at least noticeable).
I agree that some hooks are necessary for certain repositories to keep
their data consistent. Another example is the pre- and post-command hooks
that are in the microsoft/git fork that are necessary to avoid issues with
the virtualization layer of VFS for Git. If a user disables those hooks
during a command that changes the index, then they are going to have a bad
time. But maybe they want to disable hooks because all they need is a
'git rev-parse HEAD' or similarly read-only operation.
> I do think that in general certain types of hooks, such as pre-commit
> hooks, should absolutely be optional. There are lots of reasons to
> commit WIP data that doesn't meet whatever standard and we shouldn't
> impede expert users from expert workflows, even if there are many fewer
> reasons to do things like bypass pushing Git LFS objects (which are
> important for integrity).
My intention is definitely more on the side of these optional hooks.
> So I can see the utility of this feature but I can also see how it can
> break lots of things when handled poorly. Of course, we also have `git
> reset --hard` and there's lots of hand-wringing on Stack Overflow about
> having deleted important data, so we have some precedent for expert
> features that could break things badly.
>
> I don't otherwise have a strong opinion either way, although I'd lean
> slightly in favour of this series. I'd of course welcome other people's
> thoughts here.
Thank you for chiming in! I look forward to other thoughts and whetherthe warning in the docs should be strengthened.
Thanks,
-Stolee
next prev parent reply other threads:[~2025-04-04 0:40 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-03 22:38 [PATCH] git: add --no-hooks global option Derrick Stolee via GitGitGadget
2025-04-03 22:55 ` brian m. carlson
2025-04-04 0:40 ` Derrick Stolee [this message]
2025-04-04 14:15 ` Phillip Wood
2025-04-14 10:59 ` Derrick Stolee
2025-04-16 10:29 ` Phillip Wood
2025-04-16 14:28 ` Junio C Hamano
2025-04-16 15:30 ` Derrick Stolee
2025-04-14 15:25 ` Lucas Seiki Oshiro
2025-04-14 21:05 ` brian m. carlson
2025-04-11 18:53 ` D. Ben Knoble
2025-04-16 15:42 ` [PATCH v2] docs: document core.hooksPath=/dev/null Derrick Stolee via GitGitGadget
2025-04-16 16:53 ` Lucas Seiki Oshiro
2025-04-17 0:25 ` Derrick Stolee
2025-04-17 0:39 ` brian m. carlson
2025-04-17 3:42 ` Junio C Hamano
2025-04-17 16:02 ` Lucas Seiki Oshiro
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:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c1312cb3-dd2b-4b04-8c6f-2d787125448d@gmail.com \
--to=stolee@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=james@jamesliu.io \
--cc=sandals@crustytoothpaste.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* 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.
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 NNTP newsgroup(s).