Re: [PATCH 1/2] hook: honor hook.<event>.enabled for the hookdir hook

[Adrian Ratiu <adrian.ratiu@collabora.com>]

On Wed, 21 Oct 2026, Junio C Hamano <gitster@pobox.com> wrote:
> Adrian Ratiu <adrian.ratiu@collabora.com> writes:
>> However, having said that, I don't have a strong opinion and am ok
>> either way, so if others think it's a good idea, we could accept this
>> patch instead of fixing the doc. :)
>
> I am undecided if we should treat it as a backward incompatible
> change or if we should just label this change as a "bugfix".
>
> The additional documentation in question said
>
>     hook.<event>.enabled::
>        Switch to enable or disable all hooks for the `<event>` hook event.
>        When set to `false`, no hooks fire for that event, regardless of any
>        per-hook `hook.<friendly-name>.enabled` settings. Defaults to `true`.
>        See linkgit:git-hook[1].
>
> which talks about "hook.<friendly-name>.enabled" setting, hinting
> that this is also about the hooks that can be controlled by that
> other setting, which was added by 1ecce722cd (hook: allow disabling
> config hooks, 2026-02-19), which added "diable" specifically for
> "config hooks", and the documentation from that addition says:
>
>     hook.<friendly-name>.enabled::
> 	Whether the hook `hook.<name>` is enabled. Defaults to `true`.
> 	Set to `false` to disable the hook without removing its
> 	configuration. This is particularly useful when a hook is defined
> 	in a system or global config file and needs to be disabled for a
> 	specific repository. See linkgit:git-hook[1].
>
> mentioning "removing its configuration", and "defined in ... config
> file", to me, it is very clear that this configuration variable is
> only about the hooks defined in the configuration system.
>
> Which tempts me to say that the change being discussed is a backward
> incompatible breakage, rather than a bugfix.
>
> But casual end-users would of course do not know how the feature
> evolved, and may not read the documentation of the feature they use
> carefully enough.  So I dunno.  Perhaps changing the behaviour at
> the same time updating the documentation to make it clear these
> cover all kinds of hooks, not just the configuration-defined ones,
> may be sufficient.

Agreed, let's keep it simple.

@Thomas, can you please send a v2 to update the documentation to make it
clear that disabling the hook event disables both the config-defined
hooks and the hookdir hooks for that event?

Just a short mention like you specified in hook.h, to avoid any
ambiguity.

Many thanks,
Adrian