git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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


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