Fix two hook configuration bugs

Two small fixes for hook configuration. With the first, hook.pre-commit.enabled=false also stops the pre-commit hook in the hooks directory, as the documentation says. With the second, hook.*.enabled and hook.*.parallel can be given without a value, which means true, as for any other boolean.

Thomas Bachem (2):
hook: honor hook.<event>.enabled for the hookdir hook
hook: accept .enabled and .parallel given without a value

 builtin/hook.c  |  6 ++++--
 hook.c          | 17 +++++++++++------
 hook.h          |  8 +++++++-
 t/t1800-hook.sh | 51 +++++++++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 71 insertions(+), 11 deletions(-)

base-commit: 6de20f6092dcf9bdb1c8efe03db4b70c82b423dd
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2272%2Fthomasbachem%2Fhook-event-enabled-hookdir-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2272/thomasbachem/hook-event-enabled-hookdir-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2272

gitgitgadget

From: Thomas Bachem <[email protected]>

The documentation added by dcfb5af67e (hook: add hook.<event>.enabled switch, 2026-04-10) says that with hook.<event>.enabled set to false "no hooks fire for that event", but only hooks defined in the configuration are skipped. With hook.pre-commit.enabled=false, "git commit" still runs .git/hooks/pre-commit, and "git hook list" shows that hook without the "event-disabled" mark.

Move the event_disabled flag out of u.configured in struct hook, set it for the hookdir hook too, and check it for both kinds of hooks.

The test for hook.<event>.enabled=false looks only at the standard output, but "git hook run" sends hook output to the standard error, so the test passes even when a disabled hook runs. Add a hookdir hook to it and check the standard error as well.

---
 builtin/hook.c  |  6 ++++--
 hook.c          | 14 +++++++++-----
 hook.h          |  8 +++++++-
 t/t1800-hook.sh | 29 +++++++++++++++++++++++++++--
 4 files changed, 47 insertions(+), 10 deletions(-)

index cceeb3586e..1ea698ecc2 100644
@@ -82,7 +82,9 @@ static int list(int argc, const char **argv, const char *prefix,
 
 		switch (h->kind) {
 		case HOOK_TRADITIONAL:
-			printf("%s%c", _("hook from hookdir"), line_terminator);
+			printf("%s%s%c",
+			       h->event_disabled ? "event-disabled\t" : "",
+			       _("hook from hookdir"), line_terminator);
 			break;
 		case HOOK_CONFIGURED: {
 			const char *name = h->u.configured.friendly_name;

@@ -95,7 +97,7 @@ static int list(int argc, const char **argv, const char *prefix,
 			 * "disabled" surfaces once the event is re-enabled.
 			 */
 			const char *disability =
-				h->u.configured.event_disabled ? "event-disabled\t" :
+				h->event_disabled              ? "event-disabled\t" :
 				h->u.configured.disabled       ? "disabled\t"       :
 								 "";
 			if (scope)

index d10eef4763..5e756cb353 100644
@@ -115,6 +115,9 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 
 	h->kind = HOOK_TRADITIONAL;
 	h->u.traditional.path = xstrdup(hook_path);
+	/* list_hooks_add_configured() has loaded r->disabled_events */
+	h->event_disabled = !!unsorted_string_list_lookup(&r->disabled_events,
+							  hookname);
 
 	string_list_append(hook_list, hook_path)->util = h;
 }

@@ -514,7 +517,7 @@ static void list_hooks_add_configured(struct repository *r,
 			entry->command ? xstrdup(entry->command) : NULL;
 		hook->u.configured.scope = entry->scope;
 		hook->u.configured.disabled = entry->disabled;
-		hook->u.configured.event_disabled = event_is_disabled;
+		hook->event_disabled = event_is_disabled;
 		hook->parallel = entry->parallel;
 
 		string_list_append(list, friendly_name)->util = hook;

@@ -559,8 +562,9 @@ int hook_exists(struct repository *r, const char *name)
 
 	for (size_t i = 0; i < hooks->nr; i++) {
 		struct hook *h = hooks->items[i].util;
-		if (h->kind == HOOK_TRADITIONAL ||
-		    (!h->u.configured.disabled && !h->u.configured.event_disabled)) {
+		if (!h->event_disabled &&
+		    (h->kind == HOOK_TRADITIONAL ||
+		     !h->u.configured.disabled)) {
 			exists = 1;
 			break;
 		}

@@ -583,8 +587,8 @@ static int pick_next_hook(struct child_process *cp,
 		if (hook_cb->hook_to_run_index >= hook_list->nr)
 			return 0;
 		h = hook_list->items[hook_cb->hook_to_run_index++].util;
-	} while (h->kind == HOOK_CONFIGURED &&
-		 (h->u.configured.disabled || h->u.configured.event_disabled));
+	} while (h->event_disabled ||
+		 (h->kind == HOOK_CONFIGURED && h->u.configured.disabled));
 
 	cp->no_stdin = 1;
 	strvec_pushv(&cp->env, hook_cb->options->env.v);

index 27bb1aeb2e..6af24cb331 100644
@@ -32,10 +32,16 @@ struct hook {
 			const char *command;
 			enum config_scope scope;
 			bool disabled;
-			bool event_disabled;
 		} configured;
 	} u;
 
+	/**
+	 * Whether this hook's event is disabled with
+	 * `hook.<event>.enabled = false`. This applies to configured hooks
+	 * and the hookdir hook alike.
+	 */
+	bool event_disabled;
+
 	/**
 	 * Whether this hook may run in parallel with other hooks for the same
 	 * event. Only useful for configured (named) hooks. Traditional hooks

index 7d4b2982b4..27cb0eb01c 100755
@@ -496,6 +496,15 @@ test_expect_success 'disabled configured hook is not reported as existing by hoo
 	test_grep ! "pre-commit" git-bugreport-hook-exists-test.txt
 '
 
+test_expect_success 'event-disabled hookdir hook is not reported as existing by hook_exists' '
+	test_when_finished "rm -f git-bugreport-hook-exists-test.txt" &&
+	setup_hookdir &&
+	test_config hook.pre-commit.enabled false &&
+
+	git bugreport -s hook-exists-test &&
+	test_grep ! "pre-commit" git-bugreport-hook-exists-test.txt
+'
+
 test_expect_success 'globally disabled hook can be re-enabled locally' '
 	test_config_global hook.global-hook.event "test-hook" &&
 	test_config_global hook.global-hook.command "echo \"global-hook ran\"" &&

@@ -1110,9 +1119,13 @@ test_expect_success 'hook.<event>.jobs rejects values less than -1' '
 test_expect_success 'hook.<event>.enabled=false skips all hooks for event' '
 	test_config hook.hook-1.event test-hook &&
 	test_config hook.hook-1.command "echo ran" &&
+	test_hook test-hook <<-\EOF &&
+	echo ran
+	EOF
 	test_config hook.test-hook.enabled false &&
 	git hook run --allow-unknown-hook-name test-hook >out 2>err &&
-	test_must_be_empty out
+	test_must_be_empty out &&
+	test_must_be_empty err
 '
 
 test_expect_success 'hook.<event>.enabled=true does not suppress hooks' '

@@ -1147,10 +1160,14 @@ test_expect_success 'git hook list shows event-disabled hooks as event-disabled'
 	test_config hook.hook-1.command "echo ran" &&
 	test_config hook.hook-2.event test-hook &&
 	test_config hook.hook-2.command "echo ran" &&
+	test_hook test-hook <<-\EOF &&
+	echo ran
+	EOF
 	test_config hook.test-hook.enabled false &&
 	git hook list --allow-unknown-hook-name test-hook >actual &&
 	test_grep "^event-disabled	hook-1$" actual &&
-	test_grep "^event-disabled	hook-2$" actual
+	test_grep "^event-disabled	hook-2$" actual &&
+	test_grep "^event-disabled	hook from hookdir$" actual
 '
 
 test_expect_success 'git hook list shows scope with event-disabled' '

@@ -1169,6 +1186,14 @@ test_expect_success 'git hook list still shows hooks when event is disabled' '
 	test_grep "event-disabled" actual
 '
 
+test_expect_success 'commit skips the hookdir hook with hook.pre-commit.enabled=false' '
+	test_hook pre-commit <<-\EOF &&
+	exit 1
+	EOF
+	test_must_fail git commit --allow-empty -m blocked &&
+	git -c hook.pre-commit.enabled=false commit --allow-empty -m allowed
+'
+
 test_expect_success 'friendly-name matching known event name is rejected' '
 	test_config hook.pre-commit.event pre-commit &&
 	test_config hook.pre-commit.command "echo oops" &&

gitgitgadget

It is good to see that the original author and those who helped were CC'ed.

I have a suspicion that the current behavior may be very much deliberate, and the documentation was written too loosely. In other words, the author may have been so excited about their shiny new toy, the configuration-defined hook mechanism, that they wrote as if that were the only kind of hook that existed in the world, even though that is not true.

Let's see what others want to happen.

Thanks.

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.

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

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

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.

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

From: Thomas Bachem <[email protected]>

hook.<name>.enabled and hook.<name>.parallel are booleans, and git-config(1) documents a boolean without a value as true. But hook_config_lookup_all() rejects every hook.<name>.<key> that has no value, so "git -c hook.lint.parallel hook run pre-commit" fails with "missing value for 'hook.lint.parallel'", and so does a bare "parallel" under [hook "lint"]. The same happens with .enabled, for a hook as well as for an event.

Keep rejecting a missing value for the other keys. For .enabled and .parallel, pass it to git_parse_maybe_bool(), which takes it as true.

---
 hook.c          |  3 ++-
 t/t1800-hook.sh | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

index 5e756cb353..c500fec33e 100644
@@ -184,7 +184,8 @@ static int hook_config_lookup_all(const char *key, const char *value,
 		return 0;
 	}
 
-	if (!value)
+	/* .enabled and .parallel are bools, where a missing value means true */
+	if (!value && strcmp(subkey, "enabled") && strcmp(subkey, "parallel"))
 		return config_error_nonbool(key);
 
 	/* Extract name, ensuring it is null-terminated. */

index 27cb0eb01c..223c399ef0 100755
@@ -516,6 +516,17 @@ test_expect_success 'globally disabled hook can be re-enabled locally' '
 	test_cmp expected actual
 '
 
+test_expect_success 'hook.<friendly-name>.enabled without a value means true' '
+	test_config_global hook.global-hook.event "test-hook" &&
+	test_config_global hook.global-hook.command "echo \"global-hook ran\"" &&
+	test_config_global hook.global-hook.enabled false &&
+
+	echo "global-hook ran" >expected &&
+	git -c hook.global-hook.enabled \
+		hook run --allow-unknown-hook-name test-hook 2>actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'configured hooks run before hookdir hook' '
 	setup_hookdir &&
 	test_config hook.first.event "pre-commit" &&

@@ -1080,6 +1091,17 @@ test_expect_success 'hook.jobs=-1 resolves to online_cpus()' '
 	test_grep "\"region_enter\".*\"hook\".*\"test-hook\".*\"max:$cpus\"" trace.txt
 '
 
+test_expect_success 'hook.<friendly-name>.parallel without a value means true' '
+	test_config hook.hook-1.event test-hook &&
+	test_config hook.hook-1.command "true" &&
+	test_config hook.jobs 2 &&
+
+	GIT_TRACE2_EVENT="$(pwd)/trace-parallel.txt" \
+		git -c hook.hook-1.parallel \
+		hook run --allow-unknown-hook-name test-hook >out 2>err &&
+	test_grep "\"region_enter\".*\"hook\".*\"test-hook\".*\"max:2\"" trace-parallel.txt
+'
+
 test_expect_success 'hook.<event>.jobs=-1 resolves to online_cpus()' '
 	test_config hook.hook-1.event test-hook &&
 	test_config hook.hook-1.command "true" &&

gitgitgadget

Two small changes to hook configuration. With the first, hook.pre-commit.enabled=false also stops the pre-commit hook in the hooks directory. With the second, hook.*.enabled and hook.*.parallel can be given without a value, which means true, as for any other boolean.

Changes since v1:

* The documentation of hook.<event>.enabled now says that it covers configured hooks and the hook in the hooks directory alike. The subject and message of patch 1 present this as a change in behavior rather than a fix (Adrian, Junio).

* The test that hook.<event>.enabled=false leaves the hooks of other events alone now has a hookdir hook too.

Thomas Bachem (2):
hook: make hook.<event>.enabled cover the hookdir hook
hook: accept .enabled and .parallel given without a value

 Documentation/config/hook.adoc |  3 +-
 builtin/hook.c                 |  6 ++--
 hook.c                         | 17 ++++++----
 hook.h                         |  8 ++++-
 t/t1800-hook.sh                | 57 ++++++++++++++++++++++++++++++++--
 5 files changed, 78 insertions(+), 13 deletions(-)

base-commit: 6de20f6092dcf9bdb1c8efe03db4b70c82b423dd
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2272%2Fthomasbachem%2Fhook-event-enabled-hookdir-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2272/thomasbachem/hook-event-enabled-hookdir-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2272

Range-diff vs v1:

1: 1a7d060fae ! 1: cfcbe80ba5 hook: honor hook.<event>.enabled for the hookdir hook
@@ Metadata
Author: Thomas Bachem <[email protected]>

## Commit message ##
- hook: honor hook.<event>.enabled for the hookdir hook
+ hook: make hook.<event>.enabled cover the hookdir hook

- The documentation added by dcfb5af67e (hook: add hook.<event>.enabled
- switch, 2026-04-10) says that with hook.<event>.enabled set to false
- "no hooks fire for that event", but only hooks defined in the
- configuration are skipped. With hook.pre-commit.enabled=false,
- "git commit" still runs .git/hooks/pre-commit, and "git hook list"
- shows that hook without the "event-disabled" mark.
+ hook.<event>.enabled, added by dcfb5af67e (hook: add
+ hook.<event>.enabled switch, 2026-04-10), skips only the hooks
+ defined in the configuration when set to false. With
+ hook.pre-commit.enabled=false, "git commit" still runs
+ .git/hooks/pre-commit, and "git hook list" shows that hook without
+ the "event-disabled" mark.

- Move the event_disabled flag out of u.configured in struct hook, set
- it for the hookdir hook too, and check it for both kinds of hooks.
+ That was the intent, but the documentation says "no hooks fire for
+ that event", which reads as covering the hookdir hook too. No other
+ setting turns off the hookdir hook of a single event either:
+ core.hooksPath=/dev/null turns off the hookdir hooks of all events.
+ Make the switch cover the hookdir hook as well, and say so in the
+ documentation. This changes behavior for anyone who set the switch
+ and relied on the hookdir hook still running.

The test for hook.<event>.enabled=false looks only at the standard output, but "git hook run" sends hook output to the standard error,
@@ Commit message

+ ## Documentation/config/hook.adoc ##
+@@ Documentation/config/hook.adoc: hook.<friendly-name>.parallel::
+ the effective job count is greater than 1. {see-git-hook}
+
+ hook.<event>.enabled::
+- Switch to enable or disable all hooks for the <event> hook event.
++ Switch to enable or disable all hooks for the <event> hook event,
++ configured hooks and the traditional hook in the hooks directory alike.
+ When set to false, no hooks fire for that event, regardless of any
+ per-hook hook.<friendly-name>.enabled settings. Defaults to true.
+ {see-git-hook}
+
## builtin/hook.c ##
@@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix,

@@ t/t1800-hook.sh: test_expect_success 'hook.<event>.jobs rejects values less than
'

test_expect_success 'hook.<event>.enabled=true does not suppress hooks' '
+@@ t/t1800-hook.sh: test_expect_success 'hook.<event>.enabled=true does not suppress hooks' '
+ test_expect_success 'hook.<event>.enabled=false does not affect other events' '
+ test_config hook.hook-1.event test-hook &&
+ test_config hook.hook-1.command "echo ran" &&
++ test_hook test-hook <<-\EOF &&
++ echo hookdir ran
++ EOF
+ test_config hook.other-event.enabled false &&
+ git hook run --allow-unknown-hook-name test-hook >out 2>err &&
+- test_grep "ran" err
++ test_grep "^ran$" err &&
++ test_grep "^hookdir ran$" err
+ '
+
+ test_expect_success 'hook.<friendly-name>.enabled=false still disables that hook' ' @@ t/t1800-hook.sh: test_expect_success 'git hook list shows event-disabled hooks as event-disabled' test_config hook.hook-1.command "echo ran" &&
test_config hook.hook-2.event test-hook &&
2: 3db167c9d1 = 2: 53a5a1787a hook: accept .enabled and .parallel given without a value

gitgitgadget

From: Thomas Bachem <[email protected]>

hook.<event>.enabled, added by dcfb5af67e (hook: add hook.<event>.enabled switch, 2026-04-10), skips only the hooks defined in the configuration when set to false. With hook.pre-commit.enabled=false, "git commit" still runs .git/hooks/pre-commit, and "git hook list" shows that hook without the "event-disabled" mark.

That was the intent, but the documentation says "no hooks fire for that event", which reads as covering the hookdir hook too. No other setting turns off the hookdir hook of a single event either:
core.hooksPath=/dev/null turns off the hookdir hooks of all events. Make the switch cover the hookdir hook as well, and say so in the documentation. This changes behavior for anyone who set the switch and relied on the hookdir hook still running.

The test for hook.<event>.enabled=false looks only at the standard output, but "git hook run" sends hook output to the standard error, so the test passes even when a disabled hook runs. Add a hookdir hook to it and check the standard error as well.

---
 Documentation/config/hook.adoc |  3 ++-
 builtin/hook.c                 |  6 ++++--
 hook.c                         | 14 +++++++++-----
 hook.h                         |  8 +++++++-
 t/t1800-hook.sh                | 35 +++++++++++++++++++++++++++++++---
 5 files changed, 54 insertions(+), 12 deletions(-)

index 083dc60a13..332657e70f 100644
@@ -47,7 +47,8 @@ hook.<friendly-name>.parallel::
 	the effective job count is greater than 1. {see-git-hook}
 
 hook.<event>.enabled::
-	Switch to enable or disable all hooks for the `<event>` hook event.
+	Switch to enable or disable all hooks for the `<event>` hook event,
+	configured hooks and the traditional hook in the hooks directory alike.
 	When set to `false`, no hooks fire for that event, regardless of any
 	per-hook `hook.<friendly-name>.enabled` settings. Defaults to `true`.
 	{see-git-hook}

index cceeb3586e..1ea698ecc2 100644
@@ -82,7 +82,9 @@ static int list(int argc, const char **argv, const char *prefix,
 
 		switch (h->kind) {
 		case HOOK_TRADITIONAL:
-			printf("%s%c", _("hook from hookdir"), line_terminator);
+			printf("%s%s%c",
+			       h->event_disabled ? "event-disabled\t" : "",
+			       _("hook from hookdir"), line_terminator);
 			break;
 		case HOOK_CONFIGURED: {
 			const char *name = h->u.configured.friendly_name;

@@ -95,7 +97,7 @@ static int list(int argc, const char **argv, const char *prefix,
 			 * "disabled" surfaces once the event is re-enabled.
 			 */
 			const char *disability =
-				h->u.configured.event_disabled ? "event-disabled\t" :
+				h->event_disabled              ? "event-disabled\t" :
 				h->u.configured.disabled       ? "disabled\t"       :
 								 "";
 			if (scope)

index d10eef4763..5e756cb353 100644
@@ -115,6 +115,9 @@ static void list_hooks_add_default(struct repository *r, const char *hookname,
 
 	h->kind = HOOK_TRADITIONAL;
 	h->u.traditional.path = xstrdup(hook_path);
+	/* list_hooks_add_configured() has loaded r->disabled_events */
+	h->event_disabled = !!unsorted_string_list_lookup(&r->disabled_events,
+							  hookname);
 
 	string_list_append(hook_list, hook_path)->util = h;
 }

@@ -514,7 +517,7 @@ static void list_hooks_add_configured(struct repository *r,
 			entry->command ? xstrdup(entry->command) : NULL;
 		hook->u.configured.scope = entry->scope;
 		hook->u.configured.disabled = entry->disabled;
-		hook->u.configured.event_disabled = event_is_disabled;
+		hook->event_disabled = event_is_disabled;
 		hook->parallel = entry->parallel;
 
 		string_list_append(list, friendly_name)->util = hook;

@@ -559,8 +562,9 @@ int hook_exists(struct repository *r, const char *name)
 
 	for (size_t i = 0; i < hooks->nr; i++) {
 		struct hook *h = hooks->items[i].util;
-		if (h->kind == HOOK_TRADITIONAL ||
-		    (!h->u.configured.disabled && !h->u.configured.event_disabled)) {
+		if (!h->event_disabled &&
+		    (h->kind == HOOK_TRADITIONAL ||
+		     !h->u.configured.disabled)) {
 			exists = 1;
 			break;
 		}

@@ -583,8 +587,8 @@ static int pick_next_hook(struct child_process *cp,
 		if (hook_cb->hook_to_run_index >= hook_list->nr)
 			return 0;
 		h = hook_list->items[hook_cb->hook_to_run_index++].util;
-	} while (h->kind == HOOK_CONFIGURED &&
-		 (h->u.configured.disabled || h->u.configured.event_disabled));
+	} while (h->event_disabled ||
+		 (h->kind == HOOK_CONFIGURED && h->u.configured.disabled));
 
 	cp->no_stdin = 1;
 	strvec_pushv(&cp->env, hook_cb->options->env.v);

index 27bb1aeb2e..6af24cb331 100644
@@ -32,10 +32,16 @@ struct hook {
 			const char *command;
 			enum config_scope scope;
 			bool disabled;
-			bool event_disabled;
 		} configured;
 	} u;
 
+	/**
+	 * Whether this hook's event is disabled with
+	 * `hook.<event>.enabled = false`. This applies to configured hooks
+	 * and the hookdir hook alike.
+	 */
+	bool event_disabled;
+
 	/**
 	 * Whether this hook may run in parallel with other hooks for the same
 	 * event. Only useful for configured (named) hooks. Traditional hooks

index 7d4b2982b4..45b99a7f04 100755
@@ -496,6 +496,15 @@ test_expect_success 'disabled configured hook is not reported as existing by hoo
 	test_grep ! "pre-commit" git-bugreport-hook-exists-test.txt
 '
 
+test_expect_success 'event-disabled hookdir hook is not reported as existing by hook_exists' '
+	test_when_finished "rm -f git-bugreport-hook-exists-test.txt" &&
+	setup_hookdir &&
+	test_config hook.pre-commit.enabled false &&
+
+	git bugreport -s hook-exists-test &&
+	test_grep ! "pre-commit" git-bugreport-hook-exists-test.txt
+'
+
 test_expect_success 'globally disabled hook can be re-enabled locally' '
 	test_config_global hook.global-hook.event "test-hook" &&
 	test_config_global hook.global-hook.command "echo \"global-hook ran\"" &&

@@ -1110,9 +1119,13 @@ test_expect_success 'hook.<event>.jobs rejects values less than -1' '
 test_expect_success 'hook.<event>.enabled=false skips all hooks for event' '
 	test_config hook.hook-1.event test-hook &&
 	test_config hook.hook-1.command "echo ran" &&
+	test_hook test-hook <<-\EOF &&
+	echo ran
+	EOF
 	test_config hook.test-hook.enabled false &&
 	git hook run --allow-unknown-hook-name test-hook >out 2>err &&
-	test_must_be_empty out
+	test_must_be_empty out &&
+	test_must_be_empty err
 '
 
 test_expect_success 'hook.<event>.enabled=true does not suppress hooks' '

@@ -1126,9 +1139,13 @@ test_expect_success 'hook.<event>.enabled=true does not suppress hooks' '
 test_expect_success 'hook.<event>.enabled=false does not affect other events' '
 	test_config hook.hook-1.event test-hook &&
 	test_config hook.hook-1.command "echo ran" &&
+	test_hook test-hook <<-\EOF &&
+	echo hookdir ran
+	EOF
 	test_config hook.other-event.enabled false &&
 	git hook run --allow-unknown-hook-name test-hook >out 2>err &&
-	test_grep "ran" err
+	test_grep "^ran$" err &&
+	test_grep "^hookdir ran$" err
 '
 
 test_expect_success 'hook.<friendly-name>.enabled=false still disables that hook' '

@@ -1147,10 +1164,14 @@ test_expect_success 'git hook list shows event-disabled hooks as event-disabled'
 	test_config hook.hook-1.command "echo ran" &&
 	test_config hook.hook-2.event test-hook &&
 	test_config hook.hook-2.command "echo ran" &&
+	test_hook test-hook <<-\EOF &&
+	echo ran
+	EOF
 	test_config hook.test-hook.enabled false &&
 	git hook list --allow-unknown-hook-name test-hook >actual &&
 	test_grep "^event-disabled	hook-1$" actual &&
-	test_grep "^event-disabled	hook-2$" actual
+	test_grep "^event-disabled	hook-2$" actual &&
+	test_grep "^event-disabled	hook from hookdir$" actual
 '
 
 test_expect_success 'git hook list shows scope with event-disabled' '

@@ -1169,6 +1190,14 @@ test_expect_success 'git hook list still shows hooks when event is disabled' '
 	test_grep "event-disabled" actual
 '
 
+test_expect_success 'commit skips the hookdir hook with hook.pre-commit.enabled=false' '
+	test_hook pre-commit <<-\EOF &&
+	exit 1
+	EOF
+	test_must_fail git commit --allow-empty -m blocked &&
+	git -c hook.pre-commit.enabled=false commit --allow-empty -m allowed
+'
+
 test_expect_success 'friendly-name matching known event name is rejected' '
 	test_config hook.pre-commit.event pre-commit &&
 	test_config hook.pre-commit.command "echo oops" &&

gitgitgadget

From: Thomas Bachem <[email protected]>

hook.<name>.enabled and hook.<name>.parallel are booleans, and git-config(1) documents a boolean without a value as true. But hook_config_lookup_all() rejects every hook.<name>.<key> that has no value, so "git -c hook.lint.parallel hook run pre-commit" fails with "missing value for 'hook.lint.parallel'", and so does a bare "parallel" under [hook "lint"]. The same happens with .enabled, for a hook as well as for an event.

Keep rejecting a missing value for the other keys. For .enabled and .parallel, pass it to git_parse_maybe_bool(), which takes it as true.

---
 hook.c          |  3 ++-
 t/t1800-hook.sh | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

index 5e756cb353..c500fec33e 100644
@@ -184,7 +184,8 @@ static int hook_config_lookup_all(const char *key, const char *value,
 		return 0;
 	}
 
-	if (!value)
+	/* .enabled and .parallel are bools, where a missing value means true */
+	if (!value && strcmp(subkey, "enabled") && strcmp(subkey, "parallel"))
 		return config_error_nonbool(key);
 
 	/* Extract name, ensuring it is null-terminated. */

index 45b99a7f04..fca4cfc712 100755
@@ -516,6 +516,17 @@ test_expect_success 'globally disabled hook can be re-enabled locally' '
 	test_cmp expected actual
 '
 
+test_expect_success 'hook.<friendly-name>.enabled without a value means true' '
+	test_config_global hook.global-hook.event "test-hook" &&
+	test_config_global hook.global-hook.command "echo \"global-hook ran\"" &&
+	test_config_global hook.global-hook.enabled false &&
+
+	echo "global-hook ran" >expected &&
+	git -c hook.global-hook.enabled \
+		hook run --allow-unknown-hook-name test-hook 2>actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'configured hooks run before hookdir hook' '
 	setup_hookdir &&
 	test_config hook.first.event "pre-commit" &&

@@ -1080,6 +1091,17 @@ test_expect_success 'hook.jobs=-1 resolves to online_cpus()' '
 	test_grep "\"region_enter\".*\"hook\".*\"test-hook\".*\"max:$cpus\"" trace.txt
 '
 
+test_expect_success 'hook.<friendly-name>.parallel without a value means true' '
+	test_config hook.hook-1.event test-hook &&
+	test_config hook.hook-1.command "true" &&
+	test_config hook.jobs 2 &&
+
+	GIT_TRACE2_EVENT="$(pwd)/trace-parallel.txt" \
+		git -c hook.hook-1.parallel \
+		hook run --allow-unknown-hook-name test-hook >out 2>err &&
+	test_grep "\"region_enter\".*\"hook\".*\"test-hook\".*\"max:2\"" trace-parallel.txt
+'
+
 test_expect_success 'hook.<event>.jobs=-1 resolves to online_cpus()' '
 	test_config hook.hook-1.event test-hook &&
 	test_config hook.hook-1.command "true" &&

gitgitgadget

Thanks, the range-diff looks good. Will queue.