From: Adrian Ratiu <adrian.ratiu@collabora.com>
Date: Fri, 20 Feb 2026 14:47:13 GMT
Subject: Re: [PATCH v2 5/8] hook: allow disabling config hooks
Message-ID: <87qzqfwjym.fsf@collabora.com>
In-Reply-To: <aZhXom2cD8juWYs4@pks.im>

On Fri, 20 Feb 2026, Patrick Steinhardt <ps@pks.im> wrote:
> On Thu, Feb 19, 2026 at 12:23:49AM +0200, Adrian Ratiu wrote:
>> diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc
>> index 9faafe3016..0cda4745a6 100644
>> --- a/Documentation/config/hook.adoc
>> +++ b/Documentation/config/hook.adoc
>> @@ -13,3 +13,10 @@ hook.<name>.event::
>>  	specified event, the associated `hook.<name>.command` is executed.
>>  	This is a multi-valued key. To run `hook.<name>` on multiple
>>  	events, specify the key more than once. See linkgit:git-hook[1].
>> +
>> +hook.<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].
>
> Nice.
>
>> diff --git a/hook.c b/hook.c
>> index 8a9b405f76..35c24bf33d 100644
>> --- a/hook.c
>> +++ b/hook.c
>> @@ -164,6 +164,21 @@ static int hook_config_lookup_all(const char *key, const char *value,
>>  		char *old = strmap_put(&data->commands, hook_name,
>>  				       xstrdup(value));
>>  		free(old);
>> +	} else if (!strcmp(subkey, "enabled")) {
>> +		switch (git_parse_maybe_bool(value)) {
>> +		case 0: /* disabled */
>> +			if (!unsorted_string_list_lookup(&data->disabled_hooks,
>> +							 hook_name))
>> +				string_list_append(&data->disabled_hooks,
>> +						   hook_name);
>> +			break;
>> +		case 1: /* enabled: undo a prior disabled entry */
>> +			unsorted_string_list_remove(&data->disabled_hooks,
>> +						    hook_name);
>> +			break;
>> +		default:
>> +			break; /* ignore unrecognised values */
>> +		}
>>  	}
>
> Somewhat similar to my preceding questions: why don't we store the
> enabled state in the `struct hook` structure itself? Like that we can
> for example even list disabled hooks in `git hooks list --disabled`, if
> we ever wanted to do something like that.

The answer to the preceeding question is unfortunately no (I tried it),
however the answer to this question is YES.

I took the design decision to "filter" disabled hooks at config parsing
time, however I had no idea when doing it that we might want to run
something like "git hooks list --disabled".

I'll change this in v3 and cache the disabled config state instead of
filtering out disabled hooks completely.

This way we could still use their information as you suggested and we
could even enable/disable config hooks via git commands, however that is
yet-another-new-feature and this series is big enough and should be just
focused on adding the "base" functionality.

So in v3 I will cache the disabled state add the `git hooks list
--disabled` feature as well, but stop there. :)