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

[Junio C Hamano <gitster@pobox.com>]

Adrian Ratiu <adrian.ratiu@collabora.com> writes:

> The suspicion is correct: I did not intend to disable the hookdir hooks
> via configs when I wrote that initially.
>
> My reasoning was something like "configs affect just the config-defined
> hooks, the traditional hookdir hooks behave the same as before". 
>
> So currently the doc is indeed just too loose.

Thanks for confirming my suspicion.  After re-reading the document,
I am not sure if it was overly loose (see below).

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

Thanks.