From c3764d15e61ab622628b94fec204bc2acfb8aee2 Mon Sep 17 00:00:00 2001 From: Thomas Bachem Date: Fri, 23 Oct 2026 13:19:01 +0000 Subject: [PATCH] hook: make hook..enabled cover the hookdir hook hook..enabled, added by dcfb5af67e (hook: add hook..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..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. Assisted-by: Claude Fable 5.1 Signed-off-by: Thomas Bachem Signed-off-by: Junio C Hamano --- 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(-) diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc index 083dc60a13..332657e70f 100644 --- a/Documentation/config/hook.adoc +++ b/Documentation/config/hook.adoc @@ -47,7 +47,8 @@ hook..parallel:: the effective job count is greater than 1. {see-git-hook} hook..enabled:: - Switch to enable or disable all hooks for the `` hook event. + Switch to enable or disable all hooks for the `` 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..enabled` settings. Defaults to `true`. {see-git-hook} diff --git a/builtin/hook.c b/builtin/hook.c index cceeb3586e..1ea698ecc2 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -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) diff --git a/hook.c b/hook.c index d10eef4763..5e756cb353 100644 --- a/hook.c +++ b/hook.c @@ -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); diff --git a/hook.h b/hook.h index 27bb1aeb2e..6af24cb331 100644 --- a/hook.h +++ b/hook.h @@ -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..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 diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 7d4b2982b4..45b99a7f04 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -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..jobs rejects values less than -1' ' test_expect_success 'hook..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..enabled=true does not suppress hooks' ' @@ -1126,9 +1139,13 @@ test_expect_success 'hook..enabled=true does not suppress hooks' ' test_expect_success 'hook..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..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" &&