From ee2fbfd6b28fba20bc936ad1c2cb2617ba251025 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Thu, 19 Feb 2026 00:23:45 +0200 Subject: [PATCH 1/8] hook: add internal state alloc/free callbacks Some hooks use opaque structs to keep internal state between callbacks. Because hooks ran sequentially (jobs == 1) with one command per hook, these internal states could be allocated on the stack for each hook run. Next commits add the ability to run multiple commands for each hook, so the states cannot be shared or stored on the stack anymore, especially since down the line we will also enable parallel execution (jobs > 1). Add alloc/free helpers for each hook, doing a "deep" alloc/init & free of their internal opaque struct. The alloc callback takes a context pointer, to initialize the struct at at the time of resource acquisition. These callbacks must always be provided together: no alloc without free and no free without alloc, otherwise a BUG() is triggered. Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 33 ++++++++++++++++++++++++++------- hook.c | 13 +++++++++++++ hook.h | 25 ++++++++++++++++++++++++- refs.c | 24 +++++++++++++++++++----- transport.c | 27 ++++++++++++++++++++------- 5 files changed, 102 insertions(+), 20 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index b5379a4895..5259f09788 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -901,6 +901,26 @@ static int feed_receive_hook_cb(int hook_stdin_fd, void *pp_cb UNUSED, void *pp_ return state->cmd ? 0 : 1; /* 0 = more to come, 1 = EOF */ } +static void *receive_hook_feed_state_alloc(void *feed_pipe_ctx) +{ + struct receive_hook_feed_state *init_state = feed_pipe_ctx; + struct receive_hook_feed_state *data = xcalloc(1, sizeof(*data)); + data->report = init_state->report; + data->cmd = init_state->cmd; + data->skip_broken = init_state->skip_broken; + strbuf_init(&data->buf, 0); + return data; +} + +static void receive_hook_feed_state_free(void *data) +{ + struct receive_hook_feed_state *d = data; + if (!d) + return; + strbuf_release(&d->buf); + free(d); +} + static int run_receive_hook(struct command *commands, const char *hook_name, int skip_broken, @@ -908,7 +928,7 @@ static int run_receive_hook(struct command *commands, { struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; struct command *iter = commands; - struct receive_hook_feed_state feed_state; + struct receive_hook_feed_state feed_init_state = { 0 }; struct async sideband_async; int sideband_async_started = 0; int saved_stderr = -1; @@ -938,16 +958,15 @@ static int run_receive_hook(struct command *commands, prepare_sideband_async(&sideband_async, &saved_stderr, &sideband_async_started); /* set up stdin callback */ - feed_state.cmd = commands; - feed_state.skip_broken = skip_broken; - feed_state.report = NULL; - strbuf_init(&feed_state.buf, 0); - opt.feed_pipe_cb_data = &feed_state; + feed_init_state.cmd = commands; + feed_init_state.skip_broken = skip_broken; + opt.feed_pipe_ctx = &feed_init_state; opt.feed_pipe = feed_receive_hook_cb; + opt.feed_pipe_cb_data_alloc = receive_hook_feed_state_alloc; + opt.feed_pipe_cb_data_free = receive_hook_feed_state_free; ret = run_hooks_opt(the_repository, hook_name, &opt); - strbuf_release(&feed_state.buf); finish_sideband_async(&sideband_async, saved_stderr, sideband_async_started); return ret; diff --git a/hook.c b/hook.c index cde7198412..83ff658866 100644 --- a/hook.c +++ b/hook.c @@ -133,6 +133,8 @@ static int notify_hook_finished(int result, static void run_hooks_opt_clear(struct run_hooks_opt *options) { + if (options->feed_pipe_cb_data_free) + options->feed_pipe_cb_data_free(options->feed_pipe_cb_data); strvec_clear(&options->env); strvec_clear(&options->args); } @@ -172,6 +174,17 @@ int run_hooks_opt(struct repository *r, const char *hook_name, if (!options->jobs) BUG("run_hooks_opt must be called with options.jobs >= 1"); + /* + * Ensure cb_data copy and free functions are either provided together, + * or neither one is provided. + */ + if ((options->feed_pipe_cb_data_alloc && !options->feed_pipe_cb_data_free) || + (!options->feed_pipe_cb_data_alloc && options->feed_pipe_cb_data_free)) + BUG("feed_pipe_cb_data_alloc and feed_pipe_cb_data_free must be set together"); + + if (options->feed_pipe_cb_data_alloc) + options->feed_pipe_cb_data = options->feed_pipe_cb_data_alloc(options->feed_pipe_ctx); + if (options->invoked_hook) *options->invoked_hook = 0; diff --git a/hook.h b/hook.h index 20eb56fd63..a6bdc6f90f 100644 --- a/hook.h +++ b/hook.h @@ -5,6 +5,9 @@ struct repository; +typedef void (*cb_data_free_fn)(void *data); +typedef void *(*cb_data_alloc_fn)(void *init_ctx); + struct run_hooks_opt { /* Environment vars to be set for each hook */ @@ -88,10 +91,30 @@ struct run_hooks_opt * It can be accessed directly via the third callback arg 'pp_task_cb': * struct ... *state = pp_task_cb; * - * The caller is responsible for managing the memory for this data. + * The caller is responsible for managing the memory for this data by + * providing alloc/free callbacks to `run_hooks_opt`. + * * Only useful when using `run_hooks_opt.feed_pipe`, otherwise ignore it. */ void *feed_pipe_cb_data; + + /** + * Some hooks need to create a fresh `feed_pipe_cb_data` internal state, + * so they can keep track of progress without affecting one another. + * + * If provided, this function will be called to alloc & initialize the + * `feed_pipe_cb_data` for each hook. + * + * The `feed_pipe_ctx` pointer can be used to pass initialization data. + */ + cb_data_alloc_fn feed_pipe_cb_data_alloc; + + /** + * Called to free the memory initialized by `feed_pipe_cb_data_alloc`. + * + * Must always be provided when `feed_pipe_cb_data_alloc` is provided. + */ + cb_data_free_fn feed_pipe_cb_data_free; }; #define RUN_HOOKS_OPT_INIT { \ diff --git a/refs.c b/refs.c index 1e2ac90018..e06a137c29 100644 --- a/refs.c +++ b/refs.c @@ -2511,24 +2511,38 @@ static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_ return 0; /* no more input to feed */ } +static void *transaction_feed_cb_data_alloc(void *feed_pipe_ctx UNUSED) +{ + struct transaction_feed_cb_data *data = xmalloc(sizeof(*data)); + strbuf_init(&data->buf, 0); + data->index = 0; + return data; +} + +static void transaction_feed_cb_data_free(void *data) +{ + struct transaction_feed_cb_data *d = data; + if (!d) + return; + strbuf_release(&d->buf); + free(d); +} + static int run_transaction_hook(struct ref_transaction *transaction, const char *state) { struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; - struct transaction_feed_cb_data feed_ctx = { 0 }; int ret = 0; strvec_push(&opt.args, state); opt.feed_pipe = transaction_hook_feed_stdin; opt.feed_pipe_ctx = transaction; - opt.feed_pipe_cb_data = &feed_ctx; - - strbuf_init(&feed_ctx.buf, 0); + opt.feed_pipe_cb_data_alloc = transaction_feed_cb_data_alloc; + opt.feed_pipe_cb_data_free = transaction_feed_cb_data_free; ret = run_hooks_opt(transaction->ref_store->repo, "reference-transaction", &opt); - strbuf_release(&feed_ctx.buf); return ret; } diff --git a/transport.c b/transport.c index e876cc9189..d25e9a4fb0 100644 --- a/transport.c +++ b/transport.c @@ -1357,21 +1357,36 @@ static int pre_push_hook_feed_stdin(int hook_stdin_fd, void *pp_cb UNUSED, void return 0; } +static void *pre_push_hook_data_alloc(void *feed_pipe_ctx) +{ + struct feed_pre_push_hook_data *data = xmalloc(sizeof(*data)); + strbuf_init(&data->buf, 0); + data->refs = (struct ref *)feed_pipe_ctx; + return data; +} + +static void pre_push_hook_data_free(void *data) +{ + struct feed_pre_push_hook_data *d = data; + if (!d) + return; + strbuf_release(&d->buf); + free(d); +} + static int run_pre_push_hook(struct transport *transport, struct ref *remote_refs) { struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; - struct feed_pre_push_hook_data data; int ret = 0; strvec_push(&opt.args, transport->remote->name); strvec_push(&opt.args, transport->url); - strbuf_init(&data.buf, 0); - data.refs = remote_refs; - opt.feed_pipe = pre_push_hook_feed_stdin; - opt.feed_pipe_cb_data = &data; + opt.feed_pipe_ctx = remote_refs; + opt.feed_pipe_cb_data_alloc = pre_push_hook_data_alloc; + opt.feed_pipe_cb_data_free = pre_push_hook_data_free; /* * pre-push hooks expect stdout & stderr to be separate, so don't merge @@ -1381,8 +1396,6 @@ static int run_pre_push_hook(struct transport *transport, ret = run_hooks_opt(the_repository, "pre-push", &opt); - strbuf_release(&data.buf); - return ret; } From 4a36cb4c9f0f508db2e5dda75673e0d4b1242007 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Thu, 19 Feb 2026 00:23:46 +0200 Subject: [PATCH 2/8] hook: run a list of hooks to prepare for multihook support Hooks are limited to run one command (the default from the hookdir) for each event. This limitation makes it impossible to run multiple commands via config files, which the next commits will add. Implement the ability to run a list of hooks in hook.[ch]. For now, the list contains only one entry representing the "default" hook from the hookdir, so there is no user-visible change in this commit. All hook commands still run sequentially like before. A separate patch series will enable running them in parallel. Signed-off-by: Emily Shaffer Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- hook.c | 139 ++++++++++++++++++++++++++++++++++++++++++++------------- hook.h | 59 ++++++++++++++++++------ 2 files changed, 153 insertions(+), 45 deletions(-) diff --git a/hook.c b/hook.c index 83ff658866..c008a7232d 100644 --- a/hook.c +++ b/hook.c @@ -47,9 +47,97 @@ const char *find_hook(struct repository *r, const char *name) return path.buf; } +static void hook_clear(struct hook *h, cb_data_free_fn cb_data_free) +{ + if (!h) + return; + + if (h->kind == HOOK_TRADITIONAL) + free((void *)h->u.traditional.path); + + if (cb_data_free) + cb_data_free(h->feed_pipe_cb_data); + + free(h); +} + +static void hook_list_clear(struct string_list *hooks, cb_data_free_fn cb_data_free) +{ + struct string_list_item *item; + + for_each_string_list_item(item, hooks) + hook_clear(item->util, cb_data_free); + + string_list_clear(hooks, 0); +} + +/* Helper to detect and add default "traditional" hooks from the hookdir. */ +static void list_hooks_add_default(struct repository *r, const char *hookname, + struct string_list *hook_list, + struct run_hooks_opt *options) +{ + const char *hook_path = find_hook(r, hookname); + struct hook *h; + + if (!hook_path) + return; + + h = xcalloc(1, sizeof(struct hook)); + + /* + * If the hook is to run in a specific dir, a relative path can + * become invalid in that dir, so convert to an absolute path. + */ + if (options && options->dir) + hook_path = absolute_path(hook_path); + + /* Setup per-hook internal state cb data */ + if (options && options->feed_pipe_cb_data_alloc) + h->feed_pipe_cb_data = options->feed_pipe_cb_data_alloc(options->feed_pipe_ctx); + + h->kind = HOOK_TRADITIONAL; + h->u.traditional.path = xstrdup(hook_path); + + string_list_append(hook_list, hook_path)->util = h; +} + +/* + * Provides a list of hook commands to run for the 'hookname' event. + * + * This function consolidates hooks from two sources: + * 1. The config-based hooks (not yet implemented). + * 2. The "traditional" hook found in the repository hooks directory + * (e.g., .git/hooks/pre-commit). + * + * The list is ordered by execution priority. + * + * The caller is responsible for freeing the memory of the returned list + * using string_list_clear() and free(). + */ +static struct string_list *list_hooks(struct repository *r, const char *hookname, + struct run_hooks_opt *options) +{ + struct string_list *hook_head; + + if (!hookname) + BUG("null hookname was provided to hook_list()!"); + + hook_head = xmalloc(sizeof(struct string_list)); + string_list_init_dup(hook_head); + + /* Add the default "traditional" hooks from hookdir. */ + list_hooks_add_default(r, hookname, hook_head, options); + + return hook_head; +} + int hook_exists(struct repository *r, const char *name) { - return !!find_hook(r, name); + struct string_list *hooks = list_hooks(r, name, NULL); + int exists = hooks->nr > 0; + hook_list_clear(hooks, NULL); + free(hooks); + return exists; } static int pick_next_hook(struct child_process *cp, @@ -58,11 +146,14 @@ static int pick_next_hook(struct child_process *cp, void **pp_task_cb) { struct hook_cb_data *hook_cb = pp_cb; - const char *hook_path = hook_cb->hook_path; + struct string_list *hook_list = hook_cb->hook_command_list; + struct hook *h; - if (!hook_path) + if (hook_cb->hook_to_run_index >= hook_list->nr) return 0; + h = hook_list->items[hook_cb->hook_to_run_index++].util; + cp->no_stdin = 1; strvec_pushv(&cp->env, hook_cb->options->env.v); @@ -85,21 +176,20 @@ static int pick_next_hook(struct child_process *cp, cp->trace2_hook_name = hook_cb->hook_name; cp->dir = hook_cb->options->dir; - strvec_push(&cp->args, hook_path); + /* Add hook exec paths or commands */ + if (h->kind == HOOK_TRADITIONAL) + strvec_push(&cp->args, h->u.traditional.path); + + if (!cp->args.nr) + BUG("hook must have at least one command or exec path"); + strvec_pushv(&cp->args, hook_cb->options->args.v); /* * Provide per-hook internal state via task_cb for easy access, so * hook callbacks don't have to go through hook_cb->options. */ - *pp_task_cb = hook_cb->options->feed_pipe_cb_data; - - /* - * This pick_next_hook() will be called again, we're only - * running one hook, so indicate that no more work will be - * done. - */ - hook_cb->hook_path = NULL; + *pp_task_cb = h->feed_pipe_cb_data; return 1; } @@ -133,8 +223,6 @@ static int notify_hook_finished(int result, static void run_hooks_opt_clear(struct run_hooks_opt *options) { - if (options->feed_pipe_cb_data_free) - options->feed_pipe_cb_data_free(options->feed_pipe_cb_data); strvec_clear(&options->env); strvec_clear(&options->args); } @@ -142,13 +230,11 @@ static void run_hooks_opt_clear(struct run_hooks_opt *options) int run_hooks_opt(struct repository *r, const char *hook_name, struct run_hooks_opt *options) { - struct strbuf abs_path = STRBUF_INIT; struct hook_cb_data cb_data = { .rc = 0, .hook_name = hook_name, .options = options, }; - const char *const hook_path = find_hook(r, hook_name); int ret = 0; const struct run_process_parallel_opts opts = { .tr2_category = "hook", @@ -182,30 +268,21 @@ int run_hooks_opt(struct repository *r, const char *hook_name, (!options->feed_pipe_cb_data_alloc && options->feed_pipe_cb_data_free)) BUG("feed_pipe_cb_data_alloc and feed_pipe_cb_data_free must be set together"); - if (options->feed_pipe_cb_data_alloc) - options->feed_pipe_cb_data = options->feed_pipe_cb_data_alloc(options->feed_pipe_ctx); - if (options->invoked_hook) *options->invoked_hook = 0; - if (!hook_path && !options->error_if_missing) + cb_data.hook_command_list = list_hooks(r, hook_name, options); + if (!cb_data.hook_command_list->nr) { + if (options->error_if_missing) + ret = error("cannot find a hook named %s", hook_name); goto cleanup; - - if (!hook_path) { - ret = error("cannot find a hook named %s", hook_name); - goto cleanup; - } - - cb_data.hook_path = hook_path; - if (options->dir) { - strbuf_add_absolute_path(&abs_path, hook_path); - cb_data.hook_path = abs_path.buf; } run_processes_parallel(&opts); ret = cb_data.rc; cleanup: - strbuf_release(&abs_path); + hook_list_clear(cb_data.hook_command_list, options->feed_pipe_cb_data_free); + free(cb_data.hook_command_list); run_hooks_opt_clear(options); return ret; } diff --git a/hook.h b/hook.h index a6bdc6f90f..3256d2dddb 100644 --- a/hook.h +++ b/hook.h @@ -2,9 +2,41 @@ #define HOOK_H #include "strvec.h" #include "run-command.h" +#include "string-list.h" struct repository; +/** + * Represents a hook command to be run. + * Hooks can be: + * 1. "traditional" (found in the hooks directory) + * 2. "configured" (defined in Git's configuration, not yet implemented). + * The 'kind' field determines which part of the union 'u' is valid. + */ +struct hook { + enum { + HOOK_TRADITIONAL, + } kind; + union { + struct { + const char *path; + } traditional; + } u; + + /** + * Opaque data pointer used to keep internal state across callback calls. + * + * It can be accessed directly via the third hook callback arg: + * struct ... *state = pp_task_cb; + * + * The caller is responsible for managing the memory for this data by + * providing alloc/free callbacks to `run_hooks_opt`. + * + * Only useful when using `run_hooks_opt.feed_pipe`, otherwise ignore it. + */ + void *feed_pipe_cb_data; +}; + typedef void (*cb_data_free_fn)(void *data); typedef void *(*cb_data_alloc_fn)(void *init_ctx); @@ -85,19 +117,6 @@ struct run_hooks_opt */ void *feed_pipe_ctx; - /** - * Opaque data pointer used to keep internal state across callback calls. - * - * It can be accessed directly via the third callback arg 'pp_task_cb': - * struct ... *state = pp_task_cb; - * - * The caller is responsible for managing the memory for this data by - * providing alloc/free callbacks to `run_hooks_opt`. - * - * Only useful when using `run_hooks_opt.feed_pipe`, otherwise ignore it. - */ - void *feed_pipe_cb_data; - /** * Some hooks need to create a fresh `feed_pipe_cb_data` internal state, * so they can keep track of progress without affecting one another. @@ -128,7 +147,19 @@ struct hook_cb_data { /* rc reflects the cumulative failure state */ int rc; const char *hook_name; - const char *hook_path; + + /** + * A list of hook commands/paths to run for the 'hook_name' event. + * + * The 'string' member of each item holds the path (for traditional hooks) + * or the unique friendly-name for hooks specified in configs. + * The 'util' member of each item points to the corresponding struct hook. + */ + struct string_list *hook_command_list; + + /* Iterator/cursor for the above list, pointing to the next hook to run. */ + size_t hook_to_run_index; + struct run_hooks_opt *options; }; From 9fdaa6788924d4bb5ffc3a5908dae8a50e072f77 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Thu, 19 Feb 2026 00:23:47 +0200 Subject: [PATCH 3/8] hook: add "git hook list" command The previous commit introduced an ability to run multiple commands for hook events and next commit will introduce the ability to define hooks from configs, in addition to the "traditional" hooks from the hookdir. Introduce a new command "git hook list" to make inspecting hooks easier both for users and for the tests we will add. Further commits will expand on this, e.g. by adding a -z output mode. Signed-off-by: Emily Shaffer Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/git-hook.adoc | 5 ++++ builtin/hook.c | 60 +++++++++++++++++++++++++++++++++++++ hook.c | 17 ++--------- hook.h | 24 ++++++++++++++- t/t1800-hook.sh | 22 ++++++++++++++ 5 files changed, 112 insertions(+), 16 deletions(-) diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc index f6cc72d2ca..eb0ffcb8a9 100644 --- a/Documentation/git-hook.adoc +++ b/Documentation/git-hook.adoc @@ -9,6 +9,7 @@ SYNOPSIS -------- [verse] 'git hook' run [--ignore-missing] [--to-stdin=] [-- ] +'git hook' list DESCRIPTION ----------- @@ -28,6 +29,10 @@ Any positional arguments to the hook should be passed after a mandatory `--` (or `--end-of-options`, see linkgit:gitcli[7]). See linkgit:githooks[5] for arguments hooks might expect (if any). +list:: + Print a list of hooks which will be run on `` event. If no + hooks are configured for that event, print a warning and return 1. + OPTIONS ------- diff --git a/builtin/hook.c b/builtin/hook.c index 7afec380d2..51660c4941 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -6,12 +6,16 @@ #include "hook.h" #include "parse-options.h" #include "strvec.h" +#include "abspath.h" #define BUILTIN_HOOK_RUN_USAGE \ N_("git hook run [--ignore-missing] [--to-stdin=] [-- ]") +#define BUILTIN_HOOK_LIST_USAGE \ + N_("git hook list ") static const char * const builtin_hook_usage[] = { BUILTIN_HOOK_RUN_USAGE, + BUILTIN_HOOK_LIST_USAGE, NULL }; @@ -20,6 +24,61 @@ static const char * const builtin_hook_run_usage[] = { NULL }; +static int list(int argc, const char **argv, const char *prefix, + struct repository *repo) +{ + static const char *const builtin_hook_list_usage[] = { + BUILTIN_HOOK_LIST_USAGE, + NULL + }; + struct string_list *head; + struct string_list_item *item; + const char *hookname = NULL; + int ret = 0; + + struct option list_options[] = { + OPT_END(), + }; + + argc = parse_options(argc, argv, prefix, list_options, + builtin_hook_list_usage, 0); + + /* + * The only unnamed argument provided should be the hook-name; if we add + * arguments later they probably should be caught by parse_options. + */ + if (argc != 1) + usage_msg_opt(_("You must specify a hook event name to list."), + builtin_hook_list_usage, list_options); + + hookname = argv[0]; + + head = list_hooks(repo, hookname, NULL); + + if (!head->nr) { + warning(_("No hooks found for event '%s'"), hookname); + ret = 1; /* no hooks found */ + goto cleanup; + } + + for_each_string_list_item(item, head) { + struct hook *h = item->util; + + switch (h->kind) { + case HOOK_TRADITIONAL: + printf("%s\n", _("hook from hookdir")); + break; + default: + BUG("unknown hook kind"); + } + } + +cleanup: + hook_list_clear(head, NULL); + free(head); + return ret; +} + static int run(int argc, const char **argv, const char *prefix, struct repository *repo UNUSED) { @@ -77,6 +136,7 @@ int cmd_hook(int argc, parse_opt_subcommand_fn *fn = NULL; struct option builtin_hook_options[] = { OPT_SUBCOMMAND("run", &fn, run), + OPT_SUBCOMMAND("list", &fn, list), OPT_END(), }; diff --git a/hook.c b/hook.c index c008a7232d..979a97a538 100644 --- a/hook.c +++ b/hook.c @@ -61,7 +61,7 @@ static void hook_clear(struct hook *h, cb_data_free_fn cb_data_free) free(h); } -static void hook_list_clear(struct string_list *hooks, cb_data_free_fn cb_data_free) +void hook_list_clear(struct string_list *hooks, cb_data_free_fn cb_data_free) { struct string_list_item *item; @@ -101,20 +101,7 @@ static void list_hooks_add_default(struct repository *r, const char *hookname, string_list_append(hook_list, hook_path)->util = h; } -/* - * Provides a list of hook commands to run for the 'hookname' event. - * - * This function consolidates hooks from two sources: - * 1. The config-based hooks (not yet implemented). - * 2. The "traditional" hook found in the repository hooks directory - * (e.g., .git/hooks/pre-commit). - * - * The list is ordered by execution priority. - * - * The caller is responsible for freeing the memory of the returned list - * using string_list_clear() and free(). - */ -static struct string_list *list_hooks(struct repository *r, const char *hookname, +struct string_list *list_hooks(struct repository *r, const char *hookname, struct run_hooks_opt *options) { struct string_list *hook_head; diff --git a/hook.h b/hook.h index 3256d2dddb..fea221f87d 100644 --- a/hook.h +++ b/hook.h @@ -163,7 +163,29 @@ struct hook_cb_data { struct run_hooks_opt *options; }; -/* +/** + * Provides a list of hook commands to run for the 'hookname' event. + * + * This function consolidates hooks from two sources: + * 1. The config-based hooks (not yet implemented). + * 2. The "traditional" hook found in the repository hooks directory + * (e.g., .git/hooks/pre-commit). + * + * The list is ordered by execution priority. + * + * The caller is responsible for freeing the memory of the returned list + * using string_list_clear() and free(). + */ +struct string_list *list_hooks(struct repository *r, const char *hookname, + struct run_hooks_opt *options); + +/** + * Frees the memory allocated for the hook list, including the `struct hook` + * items and their internal state. + */ +void hook_list_clear(struct string_list *hooks, cb_data_free_fn cb_data_free); + +/** * Returns the path to the hook file, or NULL if the hook is missing * or disabled. Note that this points to static storage that will be * overwritten by further calls to find_hook and run_hook_*. diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index ed28a2fadb..3ec11f1249 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -10,9 +10,31 @@ test_expect_success 'git hook usage' ' test_expect_code 129 git hook run && test_expect_code 129 git hook run -h && test_expect_code 129 git hook run --unknown 2>err && + test_expect_code 129 git hook list && + test_expect_code 129 git hook list -h && grep "unknown option" err ' +test_expect_success 'git hook list: nonexistent hook' ' + cat >stderr.expect <<-\EOF && + warning: No hooks found for event '\''test-hook'\'' + EOF + test_expect_code 1 git hook list test-hook 2>stderr.actual && + test_cmp stderr.expect stderr.actual +' + +test_expect_success 'git hook list: traditional hook from hookdir' ' + test_hook test-hook <<-EOF && + echo Test hook + EOF + + cat >expect <<-\EOF && + hook from hookdir + EOF + git hook list test-hook >actual && + test_cmp expect actual +' + test_expect_success 'git hook run: nonexistent hook' ' cat >stderr.expect <<-\EOF && error: cannot find a hook named test-hook From 03b4043b9182bd3d36541371fa39f04d6d038286 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Thu, 19 Feb 2026 00:23:48 +0200 Subject: [PATCH 4/8] hook: include hooks from the config Teach the hook.[hc] library to parse configs to populate the list of hooks to run for a given event. Multiple commands can be specified for a given hook by providing "hook..command = " and "hook..event = " lines. Hooks will be started in config order of the "hook..event" lines and will be run sequentially (.jobs == 1) like before. Running the hooks in parallel will be enabled in a future patch. The "traditional" hook from the hookdir is run last, if present. A strmap cache is added to struct repository to avoid re-reading the configs on each rook run. This is useful for hooks like the ref-transaction which gets executed multiple times per process. Examples: $ git config --get-regexp "^hook\." hook.bar.command=~/bar.sh hook.bar.event=pre-commit # Will run ~/bar.sh, then .git/hooks/pre-commit $ git hook run pre-commit Signed-off-by: Emily Shaffer Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/config/hook.adoc | 15 +++ Documentation/git-hook.adoc | 128 ++++++++++++++++++++- builtin/hook.c | 3 + hook.c | 197 ++++++++++++++++++++++++++++++++- hook.h | 14 ++- repository.c | 6 + repository.h | 6 + t/t1800-hook.sh | 149 ++++++++++++++++++++++++- 8 files changed, 513 insertions(+), 5 deletions(-) create mode 100644 Documentation/config/hook.adoc diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc new file mode 100644 index 0000000000..9faafe3016 --- /dev/null +++ b/Documentation/config/hook.adoc @@ -0,0 +1,15 @@ +hook..command:: + The command to execute for `hook.`. `` is a unique + "friendly" name that identifies this hook. (The hook events that + trigger the command are configured with `hook..event`.) The + value can be an executable path or a shell oneliner. If more than + one value is specified for the same ``, only the last value + parsed is used. See linkgit:git-hook[1]. + +hook..event:: + The hook events that trigger `hook.`. The value is the name + of a hook event, like "pre-commit" or "update". (See + linkgit:githooks[5] for a complete list of hook events.) On the + specified event, the associated `hook..command` is executed. + This is a multi-valued key. To run `hook.` on multiple + events, specify the key more than once. See linkgit:git-hook[1]. diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc index eb0ffcb8a9..7e4259e4f0 100644 --- a/Documentation/git-hook.adoc +++ b/Documentation/git-hook.adoc @@ -17,12 +17,96 @@ DESCRIPTION A command interface for running git hooks (see linkgit:githooks[5]), for use by other scripted git commands. +This command parses the default configuration files for sets of configs like +so: + + [hook "linter"] + event = pre-commit + command = ~/bin/linter --cpp20 + +In this example, `[hook "linter"]` represents one script - `~/bin/linter +--cpp20` - which can be shared by many repos, and even by many hook events, if +appropriate. + +To add an unrelated hook which runs on a different event, for example a +spell-checker for your commit messages, you would write a configuration like so: + + [hook "linter"] + event = pre-commit + command = ~/bin/linter --cpp20 + [hook "spellcheck"] + event = commit-msg + command = ~/bin/spellchecker + +With this config, when you run 'git commit', first `~/bin/linter --cpp20` will +have a chance to check your files to be committed (during the `pre-commit` hook +event`), and then `~/bin/spellchecker` will have a chance to check your commit +message (during the `commit-msg` hook event). + +Commands are run in the order Git encounters their associated +`hook..event` configs during the configuration parse (see +linkgit:git-config[1]). Although multiple `hook.linter.event` configs can be +added, only one `hook.linter.command` event is valid - Git uses "last-one-wins" +to determine which command to run. + +So if you wanted your linter to run when you commit as well as when you push, +you would configure it like so: + + [hook "linter"] + event = pre-commit + event = pre-push + command = ~/bin/linter --cpp20 + +With this config, `~/bin/linter --cpp20` would be run by Git before a commit is +generated (during `pre-commit`) as well as before a push is performed (during +`pre-push`). + +And if you wanted to run your linter as well as a secret-leak detector during +only the "pre-commit" hook event, you would configure it instead like so: + + [hook "linter"] + event = pre-commit + command = ~/bin/linter --cpp20 + [hook "no-leaks"] + event = pre-commit + command = ~/bin/leak-detector + +With this config, before a commit is generated (during `pre-commit`), Git would +first start `~/bin/linter --cpp20` and second start `~/bin/leak-detector`. It +would evaluate the output of each when deciding whether to proceed with the +commit. + +For a full list of hook events which you can set your `hook..event` to, +and how hooks are invoked during those events, see linkgit:githooks[5]. + +Git will ignore any `hook..event` that specifies an event it doesn't +recognize. This is intended so that tools which wrap Git can use the hook +infrastructure to run their own hooks; see "WRAPPERS" for more guidance. + +In general, when instructions suggest adding a script to +`.git/hooks/`, you can specify it in the config instead by running: + +---- +git config set hook..command +git config set --append hook..event +---- + +This way you can share the script between multiple repos. That is, `cp +~/my-script.sh ~/project/.git/hooks/pre-commit` would become: + +---- +git config set hook.my-script.command ~/my-script.sh +git config set --append hook.my-script.event pre-commit +---- + SUBCOMMANDS ----------- run:: - Run the `` hook. See linkgit:githooks[5] for - supported hook names. + Runs hooks configured for ``, in the order they are + discovered during the config parse. The default `` from + the hookdir is run last. See linkgit:githooks[5] for supported + hook names. + Any positional arguments to the hook should be passed after a @@ -46,6 +130,46 @@ OPTIONS tools that want to do a blind one-shot run of a hook that may or may not be present. +WRAPPERS +-------- + +`git hook run` has been designed to make it easy for tools which wrap Git to +configure and execute hooks using the Git hook infrastructure. It is possible to +provide arguments and stdin via the command line, as well as specifying parallel +or series execution if the user has provided multiple hooks. + +Assuming your wrapper wants to support a hook named "mywrapper-start-tests", you +can have your users specify their hooks like so: + + [hook "setup-test-dashboard"] + event = mywrapper-start-tests + command = ~/mywrapper/setup-dashboard.py --tap + +Then, in your 'mywrapper' tool, you can invoke any users' configured hooks by +running: + +---- +git hook run mywrapper-start-tests \ + # providing something to stdin + --stdin some-tempfile-123 \ + # execute hooks in serial + # plus some arguments of your own... + -- \ + --testname bar \ + baz +---- + +Take care to name your wrapper's hook events in a way which is unlikely to +overlap with Git's native hooks (see linkgit:githooks[5]) - a hook event named +`mywrappertool-validate-commit` is much less likely to be added to native Git +than a hook event named `validate-commit`. If Git begins to use a hook event +named the same thing as your wrapper hook, it may invoke your users' hooks in +unintended and unsupported ways. + +CONFIGURATION +------------- +include::config/hook.adoc[] + SEE ALSO -------- linkgit:githooks[5] diff --git a/builtin/hook.c b/builtin/hook.c index 51660c4941..e151bb2cd1 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -68,6 +68,9 @@ static int list(int argc, const char **argv, const char *prefix, case HOOK_TRADITIONAL: printf("%s\n", _("hook from hookdir")); break; + case HOOK_CONFIGURED: + printf("%s\n", h->u.configured.friendly_name); + break; default: BUG("unknown hook kind"); } diff --git a/hook.c b/hook.c index 979a97a538..8a9b405f76 100644 --- a/hook.c +++ b/hook.c @@ -4,9 +4,11 @@ #include "gettext.h" #include "hook.h" #include "path.h" +#include "parse.h" #include "run-command.h" #include "config.h" #include "strbuf.h" +#include "strmap.h" #include "environment.h" #include "setup.h" @@ -54,6 +56,10 @@ static void hook_clear(struct hook *h, cb_data_free_fn cb_data_free) if (h->kind == HOOK_TRADITIONAL) free((void *)h->u.traditional.path); + else if (h->kind == HOOK_CONFIGURED) { + free((void *)h->u.configured.friendly_name); + free((void *)h->u.configured.command); + } if (cb_data_free) cb_data_free(h->feed_pipe_cb_data); @@ -101,6 +107,187 @@ static void list_hooks_add_default(struct repository *r, const char *hookname, string_list_append(hook_list, hook_path)->util = h; } +static void unsorted_string_list_remove(struct string_list *list, + const char *str) +{ + struct string_list_item *item = unsorted_string_list_lookup(list, str); + if (item) + unsorted_string_list_delete_item(list, item - list->items, 0); +} + +/* + * Callback struct to collect all hook.* keys in a single config pass. + * commands: friendly-name to command map. + * event_hooks: event-name to list of friendly-names map. + * disabled_hooks: set of friendly-names with hook.name.enabled = false. + */ +struct hook_all_config_cb { + struct strmap commands; + struct strmap event_hooks; + struct string_list disabled_hooks; +}; + +/* repo_config() callback that collects all hook.* configuration in one pass. */ +static int hook_config_lookup_all(const char *key, const char *value, + const struct config_context *ctx UNUSED, + void *cb_data) +{ + struct hook_all_config_cb *data = cb_data; + const char *name, *subkey; + char *hook_name; + size_t name_len = 0; + + if (parse_config_key(key, "hook", &name, &name_len, &subkey)) + return 0; + + if (!value) + return config_error_nonbool(key); + + /* Extract name, ensuring it is null-terminated. */ + hook_name = xmemdupz(name, name_len); + + if (!strcmp(subkey, "event")) { + struct string_list *hooks = + strmap_get(&data->event_hooks, value); + + if (!hooks) { + hooks = xcalloc(1, sizeof(*hooks)); + string_list_init_dup(hooks); + strmap_put(&data->event_hooks, value, hooks); + } + + /* Re-insert if necessary to preserve last-seen order. */ + unsorted_string_list_remove(hooks, hook_name); + string_list_append(hooks, hook_name); + } else if (!strcmp(subkey, "command")) { + /* Store command overwriting the old value */ + char *old = strmap_put(&data->commands, hook_name, + xstrdup(value)); + free(old); + } + + free(hook_name); + return 0; +} + +/* + * The hook config cache maps each hook event name to a string_list where + * every item's string is the hook's friendly-name and its util pointer is + * the corresponding command string. Both strings are owned by the map. + * + * Disabled hooks and hooks missing a command are already filtered out at + * parse time, so callers can iterate the list directly. + */ +void hook_cache_clear(struct strmap *cache) +{ + struct hashmap_iter iter; + struct strmap_entry *e; + + strmap_for_each_entry(cache, &iter, e) { + struct string_list *hooks = e->value; + string_list_clear(hooks, 1); /* free util (command) pointers */ + free(hooks); + } + strmap_clear(cache, 0); +} + +/* Populate `cache` with the complete hook configuration */ +static void build_hook_config_map(struct repository *r, struct strmap *cache) +{ + struct hook_all_config_cb cb_data; + struct hashmap_iter iter; + struct strmap_entry *e; + + strmap_init(&cb_data.commands); + strmap_init(&cb_data.event_hooks); + string_list_init_dup(&cb_data.disabled_hooks); + + /* Parse all configs in one run. */ + repo_config(r, hook_config_lookup_all, &cb_data); + + /* Construct the cache from parsed configs. */ + strmap_for_each_entry(&cb_data.event_hooks, &iter, e) { + struct string_list *hook_names = e->value; + struct string_list *hooks = xcalloc(1, sizeof(*hooks)); + + string_list_init_dup(hooks); + + for (size_t i = 0; i < hook_names->nr; i++) { + const char *hname = hook_names->items[i].string; + char *command; + + command = strmap_get(&cb_data.commands, hname); + if (!command) + die(_("'hook.%s.command' must be configured or " + "'hook.%s.event' must be removed;" + " aborting."), hname, hname); + + /* util stores the command; owned by the cache. */ + string_list_append(hooks, hname)->util = + xstrdup(command); + } + + strmap_put(cache, e->key, hooks); + } + + strmap_clear(&cb_data.commands, 1); + string_list_clear(&cb_data.disabled_hooks, 0); + strmap_for_each_entry(&cb_data.event_hooks, &iter, e) { + string_list_clear(e->value, 0); + free(e->value); + } + strmap_clear(&cb_data.event_hooks, 0); +} + +/* Return the hook config map for `r`, populating it first if needed. */ +static struct strmap *get_hook_config_cache(struct repository *r) +{ + struct strmap *cache = NULL; + + if (r) { + /* + * For in-repo calls, the map is stored in r->hook_config_cache, + * so repeated invocations don't parse the configs, so allocate + * it just once on the first call. + */ + if (!r->hook_config_cache) { + r->hook_config_cache = xcalloc(1, sizeof(*cache)); + strmap_init(r->hook_config_cache); + build_hook_config_map(r, r->hook_config_cache); + } + cache = r->hook_config_cache; + } + + return cache; +} + +static void list_hooks_add_configured(struct repository *r, + const char *hookname, + struct string_list *list, + struct run_hooks_opt *options) +{ + struct strmap *cache = get_hook_config_cache(r); + struct string_list *configured_hooks = strmap_get(cache, hookname); + + /* Iterate through configured hooks and initialize internal states */ + for (size_t i = 0; configured_hooks && i < configured_hooks->nr; i++) { + const char *friendly_name = configured_hooks->items[i].string; + const char *command = configured_hooks->items[i].util; + struct hook *hook = xcalloc(1, sizeof(struct hook)); + + if (options && options->feed_pipe_cb_data_alloc) + hook->feed_pipe_cb_data = + options->feed_pipe_cb_data_alloc( + options->feed_pipe_ctx); + + hook->kind = HOOK_CONFIGURED; + hook->u.configured.friendly_name = xstrdup(friendly_name); + hook->u.configured.command = xstrdup(command); + + string_list_append(list, friendly_name)->util = hook; + } +} + struct string_list *list_hooks(struct repository *r, const char *hookname, struct run_hooks_opt *options) { @@ -112,6 +299,9 @@ struct string_list *list_hooks(struct repository *r, const char *hookname, hook_head = xmalloc(sizeof(struct string_list)); string_list_init_dup(hook_head); + /* Add hooks from the config, e.g. hook.myhook.event = pre-commit */ + list_hooks_add_configured(r, hookname, hook_head, options); + /* Add the default "traditional" hooks from hookdir. */ list_hooks_add_default(r, hookname, hook_head, options); @@ -164,8 +354,13 @@ static int pick_next_hook(struct child_process *cp, cp->dir = hook_cb->options->dir; /* Add hook exec paths or commands */ - if (h->kind == HOOK_TRADITIONAL) + if (h->kind == HOOK_TRADITIONAL) { strvec_push(&cp->args, h->u.traditional.path); + } else if (h->kind == HOOK_CONFIGURED) { + /* to enable oneliners, let config-specified hooks run in shell. */ + cp->use_shell = true; + strvec_push(&cp->args, h->u.configured.command); + } if (!cp->args.nr) BUG("hook must have at least one command or exec path"); diff --git a/hook.h b/hook.h index fea221f87d..e949f5d488 100644 --- a/hook.h +++ b/hook.h @@ -3,6 +3,7 @@ #include "strvec.h" #include "run-command.h" #include "string-list.h" +#include "strmap.h" struct repository; @@ -10,17 +11,22 @@ struct repository; * Represents a hook command to be run. * Hooks can be: * 1. "traditional" (found in the hooks directory) - * 2. "configured" (defined in Git's configuration, not yet implemented). + * 2. "configured" (defined in Git's configuration via hook..event). * The 'kind' field determines which part of the union 'u' is valid. */ struct hook { enum { HOOK_TRADITIONAL, + HOOK_CONFIGURED, } kind; union { struct { const char *path; } traditional; + struct { + const char *friendly_name; + const char *command; + } configured; } u; /** @@ -185,6 +191,12 @@ struct string_list *list_hooks(struct repository *r, const char *hookname, */ void hook_list_clear(struct string_list *hooks, cb_data_free_fn cb_data_free); +/** + * Frees the hook configuration cache stored in `struct repository`. + * Called by repo_clear(). + */ +void hook_cache_clear(struct strmap *cache); + /** * Returns the path to the hook file, or NULL if the hook is missing * or disabled. Note that this points to static storage that will be diff --git a/repository.c b/repository.c index c7e75215ac..2518d66975 100644 --- a/repository.c +++ b/repository.c @@ -1,6 +1,7 @@ #include "git-compat-util.h" #include "abspath.h" #include "repository.h" +#include "hook.h" #include "odb.h" #include "config.h" #include "object.h" @@ -393,6 +394,11 @@ void repo_clear(struct repository *repo) FREE_AND_NULL(repo->index); } + if (repo->hook_config_cache) { + hook_cache_clear(repo->hook_config_cache); + FREE_AND_NULL(repo->hook_config_cache); + } + if (repo->promisor_remote_config) { promisor_remote_clear(repo->promisor_remote_config); FREE_AND_NULL(repo->promisor_remote_config); diff --git a/repository.h b/repository.h index 6063c4b846..df91338259 100644 --- a/repository.h +++ b/repository.h @@ -157,6 +157,12 @@ struct repository { /* True if commit-graph has been disabled within this process. */ int commit_graph_disabled; + /* + * Lazily-populated cache mapping hook event names to configured hooks. + * NULL until first hook use. + */ + struct strmap *hook_config_cache; + /* Configurations related to promisor remotes. */ char *repository_format_partial_clone; struct promisor_remote_config *promisor_remote_config; diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 3ec11f1249..f1048a5119 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -1,14 +1,31 @@ #!/bin/sh -test_description='git-hook command' +test_description='git-hook command and config-managed multihooks' . ./test-lib.sh . "$TEST_DIRECTORY"/lib-terminal.sh +setup_hooks () { + test_config hook.ghi.command "/path/ghi" + test_config hook.ghi.event pre-commit --add + test_config hook.ghi.event test-hook --add + test_config_global hook.def.command "/path/def" + test_config_global hook.def.event pre-commit --add +} + +setup_hookdir () { + mkdir .git/hooks + write_script .git/hooks/pre-commit <<-EOF + echo \"Legacy Hook\" + EOF + test_when_finished rm -rf .git/hooks +} + test_expect_success 'git hook usage' ' test_expect_code 129 git hook && test_expect_code 129 git hook run && test_expect_code 129 git hook run -h && + test_expect_code 129 git hook list -h && test_expect_code 129 git hook run --unknown 2>err && test_expect_code 129 git hook list && test_expect_code 129 git hook list -h && @@ -35,6 +52,15 @@ test_expect_success 'git hook list: traditional hook from hookdir' ' test_cmp expect actual ' +test_expect_success 'git hook list: configured hook' ' + test_config hook.myhook.command "echo Hello" && + test_config hook.myhook.event test-hook --add && + + echo "myhook" >expect && + git hook list test-hook >actual && + test_cmp expect actual +' + test_expect_success 'git hook run: nonexistent hook' ' cat >stderr.expect <<-\EOF && error: cannot find a hook named test-hook @@ -172,6 +198,126 @@ test_expect_success TTY 'git commit: stdout and stderr are connected to a TTY' ' test_hook_tty commit -m"B.new" ' +test_expect_success 'git hook list orders by config order' ' + setup_hooks && + + cat >expected <<-\EOF && + def + ghi + EOF + + git hook list pre-commit >actual && + test_cmp expected actual +' + +test_expect_success 'git hook list reorders on duplicate event declarations' ' + setup_hooks && + + # 'def' is usually configured globally; move it to the end by + # configuring it locally. + test_config hook.def.event "pre-commit" --add && + + cat >expected <<-\EOF && + ghi + def + EOF + + git hook list pre-commit >actual && + test_cmp expected actual +' + +test_expect_success 'hook can be configured for multiple events' ' + setup_hooks && + + # 'ghi' should be included in both 'pre-commit' and 'test-hook' + git hook list pre-commit >actual && + grep "ghi" actual && + git hook list test-hook >actual && + grep "ghi" actual +' + +test_expect_success 'git hook list shows hooks from the hookdir' ' + setup_hookdir && + + cat >expected <<-\EOF && + hook from hookdir + EOF + + git hook list pre-commit >actual && + test_cmp expected actual +' + +test_expect_success 'inline hook definitions execute oneliners' ' + test_config hook.oneliner.event "pre-commit" && + test_config hook.oneliner.command "echo \"Hello World\"" && + + echo "Hello World" >expected && + + # hooks are run with stdout_to_stderr = 1 + git hook run pre-commit 2>actual && + test_cmp expected actual +' + +test_expect_success 'inline hook definitions resolve paths' ' + write_script sample-hook.sh <<-\EOF && + echo \"Sample Hook\" + EOF + + test_when_finished "rm sample-hook.sh" && + + test_config hook.sample-hook.event pre-commit && + test_config hook.sample-hook.command "\"$(pwd)/sample-hook.sh\"" && + + echo \"Sample Hook\" >expected && + + # hooks are run with stdout_to_stderr = 1 + git hook run pre-commit 2>actual && + test_cmp expected actual +' + +test_expect_success 'hookdir hook included in git hook run' ' + setup_hookdir && + + echo \"Legacy Hook\" >expected && + + # hooks are run with stdout_to_stderr = 1 + git hook run pre-commit 2>actual && + test_cmp expected actual +' + +test_expect_success 'stdin to multiple hooks' ' + test_config hook.stdin-a.event "test-hook" && + test_config hook.stdin-a.command "xargs -P1 -I% echo a%" && + test_config hook.stdin-b.event "test-hook" && + test_config hook.stdin-b.command "xargs -P1 -I% echo b%" && + + cat >input <<-\EOF && + 1 + 2 + 3 + EOF + + cat >expected <<-\EOF && + a1 + a2 + a3 + b1 + b2 + b3 + EOF + + git hook run --to-stdin=input test-hook 2>actual && + test_cmp expected actual +' + +test_expect_success 'rejects hooks with no commands configured' ' + test_config hook.broken.event "test-hook" && + test_must_fail git hook list test-hook 2>actual && + test_grep "hook.broken.command" actual && + test_must_fail git hook run test-hook 2>actual && + test_grep "hook.broken.command" actual +' + test_expect_success 'git hook run a hook with a bad shebang' ' test_when_finished "rm -rf bad-hooks" && mkdir bad-hooks && @@ -189,6 +335,7 @@ test_expect_success 'git hook run a hook with a bad shebang' ' ' test_expect_success 'stdin to hooks' ' + mkdir -p .git/hooks && write_script .git/hooks/test-hook <<-\EOF && echo BEGIN stdin cat From 1ecce722cdb9c42dd4c69e45e02cb850cd558ef2 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Thu, 19 Feb 2026 00:23:49 +0200 Subject: [PATCH 5/8] hook: allow disabling config hooks Hooks specified via configs are always enabled, however users might want to disable them without removing from the config, like locally disabling a global hook. Add a hook..enabled config which defaults to true and can be optionally set for each configured hook. Suggested-by: Patrick Steinhardt Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/config/hook.adoc | 7 +++++++ hook.c | 20 ++++++++++++++++++++ t/t1800-hook.sh | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+) 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..event:: specified event, the associated `hook..command` is executed. This is a multi-valued key. To run `hook.` on multiple events, specify the key more than once. See linkgit:git-hook[1]. + +hook..enabled:: + Whether the hook `hook.` 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]. 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 */ + } } free(hook_name); @@ -216,6 +231,11 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache) const char *hname = hook_names->items[i].string; char *command; + /* filter out disabled hooks */ + if (unsorted_string_list_lookup(&cb_data.disabled_hooks, + hname)) + continue; + command = strmap_get(&cb_data.commands, hname); if (!command) die(_("'hook.%s.command' must be configured or " diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index f1048a5119..9797802735 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -318,6 +318,38 @@ test_expect_success 'rejects hooks with no commands configured' ' test_grep "hook.broken.command" actual ' +test_expect_success 'disabled hook is not run' ' + test_config hook.skipped.event "test-hook" && + test_config hook.skipped.command "echo \"Should not run\"" && + test_config hook.skipped.enabled false && + + git hook run --ignore-missing test-hook 2>actual && + test_must_be_empty actual +' + +test_expect_success 'disabled hook does not appear in git hook list' ' + test_config hook.active.event "pre-commit" && + test_config hook.active.command "echo active" && + test_config hook.inactive.event "pre-commit" && + test_config hook.inactive.command "echo inactive" && + test_config hook.inactive.enabled false && + + git hook list pre-commit >actual && + test_grep "active" actual && + test_grep ! "inactive" actual +' + +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\"" && + test_config_global hook.global-hook.enabled false && + test_config hook.global-hook.enabled true && + + echo "global-hook ran" >expected && + git hook run test-hook 2>actual && + test_cmp expected actual +' + test_expect_success 'git hook run a hook with a bad shebang' ' test_when_finished "rm -rf bad-hooks" && mkdir bad-hooks && From d084fa2a915784d65257fbaff43f00b3ea5c8a44 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Thu, 19 Feb 2026 00:23:50 +0200 Subject: [PATCH 6/8] hook: allow event = "" to overwrite previous values Add the ability for empty events to clear previously set multivalue variables, so the newly added "hook.*.event" behave like the other multivalued keys. Suggested-by: Patrick Steinhardt Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/config/hook.adoc | 4 +++- hook.c | 29 +++++++++++++++++++---------- t/t1800-hook.sh | 12 ++++++++++++ 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc index 0cda4745a6..64e845a260 100644 --- a/Documentation/config/hook.adoc +++ b/Documentation/config/hook.adoc @@ -12,7 +12,9 @@ hook..event:: linkgit:githooks[5] for a complete list of hook events.) On the specified event, the associated `hook..command` is executed. This is a multi-valued key. To run `hook.` on multiple - events, specify the key more than once. See linkgit:git-hook[1]. + events, specify the key more than once. An empty value resets + the list of events, clearing any previously defined events for + `hook.`. See linkgit:git-hook[1]. hook..enabled:: Whether the hook `hook.` is enabled. Defaults to `true`. diff --git a/hook.c b/hook.c index 35c24bf33d..fee0a7ab4f 100644 --- a/hook.c +++ b/hook.c @@ -147,18 +147,27 @@ static int hook_config_lookup_all(const char *key, const char *value, hook_name = xmemdupz(name, name_len); if (!strcmp(subkey, "event")) { - struct string_list *hooks = - strmap_get(&data->event_hooks, value); + if (!*value) { + /* Empty values reset previous events for this hook. */ + struct hashmap_iter iter; + struct strmap_entry *e; - if (!hooks) { - hooks = xcalloc(1, sizeof(*hooks)); - string_list_init_dup(hooks); - strmap_put(&data->event_hooks, value, hooks); + strmap_for_each_entry(&data->event_hooks, &iter, e) + unsorted_string_list_remove(e->value, hook_name); + } else { + struct string_list *hooks = + strmap_get(&data->event_hooks, value); + + if (!hooks) { + hooks = xcalloc(1, sizeof(*hooks)); + string_list_init_dup(hooks); + strmap_put(&data->event_hooks, value, hooks); + } + + /* Re-insert if necessary to preserve last-seen order. */ + unsorted_string_list_remove(hooks, hook_name); + string_list_append(hooks, hook_name); } - - /* Re-insert if necessary to preserve last-seen order. */ - unsorted_string_list_remove(hooks, hook_name); - string_list_append(hooks, hook_name); } else if (!strcmp(subkey, "command")) { /* Store command overwriting the old value */ char *old = strmap_put(&data->commands, hook_name, diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 9797802735..fb6bc554b9 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -226,6 +226,18 @@ test_expect_success 'git hook list reorders on duplicate event declarations' ' test_cmp expected actual ' +test_expect_success 'git hook list: empty event value resets events' ' + setup_hooks && + + # ghi is configured for pre-commit; reset it with an empty value + test_config hook.ghi.event "" --add && + + # only def should remain for pre-commit + echo "def" >expected && + git hook list pre-commit >actual && + test_cmp expected actual +' + test_expect_success 'hook can be configured for multiple events' ' setup_hooks && From b51e238ddf896439d2746b883d892f8f9bba649f Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Thu, 19 Feb 2026 00:23:51 +0200 Subject: [PATCH 7/8] hook: allow out-of-repo 'git hook' invocations Since hooks can now be supplied via the config, and a config can be present without a gitdir via the global and system configs, we can start to allow 'git hook run' to occur without a gitdir. This enables us to do things like run sendemail-validate hooks when running 'git send-email' from a nongit directory. It still doesn't make sense to look for hooks in the hookdir in nongit repos, though, as there is no hookdir. Signed-off-by: Emily Shaffer Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- git.c | 2 +- hook.c | 30 ++++++++++++++++++++++++++++-- t/t1800-hook.sh | 16 +++++++++++----- 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/git.c b/git.c index c5fad56813..a9e462ee32 100644 --- a/git.c +++ b/git.c @@ -586,7 +586,7 @@ static struct cmd_struct commands[] = { { "grep", cmd_grep, RUN_SETUP_GENTLY }, { "hash-object", cmd_hash_object }, { "help", cmd_help }, - { "hook", cmd_hook, RUN_SETUP }, + { "hook", cmd_hook, RUN_SETUP_GENTLY }, { "index-pack", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT }, { "init", cmd_init_db }, { "init-db", cmd_init_db }, diff --git a/hook.c b/hook.c index fee0a7ab4f..2c8252b2c4 100644 --- a/hook.c +++ b/hook.c @@ -18,6 +18,9 @@ const char *find_hook(struct repository *r, const char *name) int found_hook; + if (!r || !r->gitdir) + return NULL; + repo_git_path_replace(r, &path, "hooks/%s", name); found_hook = access(path.buf, X_OK) >= 0; #ifdef STRIP_EXTENSION @@ -268,12 +271,18 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache) strmap_clear(&cb_data.event_hooks, 0); } -/* Return the hook config map for `r`, populating it first if needed. */ +/* + * Return the hook config map for `r`, populating it first if needed. + * + * Out-of-repo calls (r->gitdir == NULL) allocate and return a temporary + * cache map; the caller is responsible for freeing it with + * hook_cache_clear() + free(). + */ static struct strmap *get_hook_config_cache(struct repository *r) { struct strmap *cache = NULL; - if (r) { + if (r && r->gitdir) { /* * For in-repo calls, the map is stored in r->hook_config_cache, * so repeated invocations don't parse the configs, so allocate @@ -285,6 +294,14 @@ static struct strmap *get_hook_config_cache(struct repository *r) build_hook_config_map(r, r->hook_config_cache); } cache = r->hook_config_cache; + } else { + /* + * Out-of-repo calls (no gitdir) allocate and return a temporary + * map cache which gets free'd immediately by the caller. + */ + cache = xcalloc(1, sizeof(*cache)); + strmap_init(cache); + build_hook_config_map(r, cache); } return cache; @@ -315,6 +332,15 @@ static void list_hooks_add_configured(struct repository *r, string_list_append(list, friendly_name)->util = hook; } + + /* + * Cleanup temporary cache for out-of-repo calls since they can't be + * stored persistently. Next out-of-repo calls will have to re-parse. + */ + if (!r || !r->gitdir) { + hook_cache_clear(cache); + free(cache); + } } struct string_list *list_hooks(struct repository *r, const char *hookname, diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index fb6bc554b9..e58151e8f8 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -131,12 +131,18 @@ test_expect_success 'git hook run -- pass arguments' ' test_cmp expect actual ' -test_expect_success 'git hook run -- out-of-repo runs excluded' ' - test_hook test-hook <<-EOF && - echo Test hook - EOF +test_expect_success 'git hook run: out-of-repo runs execute global hooks' ' + test_config_global hook.global-hook.event test-hook --add && + test_config_global hook.global-hook.command "echo no repo no problems" --add && - nongit test_must_fail git hook run test-hook + echo "global-hook" >expect && + nongit git hook list test-hook >actual && + test_cmp expect actual && + + echo "no repo no problems" >expect && + + nongit git hook run test-hook 2>actual && + test_cmp expect actual ' test_expect_success 'git -c core.hooksPath= hook run' ' From 4b12cd3ae3acbc819189758d09f9c983bde16040 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Thu, 19 Feb 2026 00:23:52 +0200 Subject: [PATCH 8/8] hook: add -z option to "git hook list" Add a NUL-terminate mode to git hook list, just in case hooks are configured with weird characters like newlines in their names. Suggested-by: Patrick Steinhardt Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/git-hook.adoc | 8 ++++++-- builtin/hook.c | 9 ++++++--- t/t1800-hook.sh | 13 +++++++++++++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc index 7e4259e4f0..12d2701b52 100644 --- a/Documentation/git-hook.adoc +++ b/Documentation/git-hook.adoc @@ -9,7 +9,7 @@ SYNOPSIS -------- [verse] 'git hook' run [--ignore-missing] [--to-stdin=] [-- ] -'git hook' list +'git hook' list [-z] DESCRIPTION ----------- @@ -113,9 +113,10 @@ Any positional arguments to the hook should be passed after a mandatory `--` (or `--end-of-options`, see linkgit:gitcli[7]). See linkgit:githooks[5] for arguments hooks might expect (if any). -list:: +list [-z]:: Print a list of hooks which will be run on `` event. If no hooks are configured for that event, print a warning and return 1. + Use `-z` to terminate output lines with NUL instead of newlines. OPTIONS ------- @@ -130,6 +131,9 @@ OPTIONS tools that want to do a blind one-shot run of a hook that may or may not be present. +-z:: + Terminate "list" output lines with NUL instead of newlines. + WRAPPERS -------- diff --git a/builtin/hook.c b/builtin/hook.c index e151bb2cd1..83020dfb4f 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -11,7 +11,7 @@ #define BUILTIN_HOOK_RUN_USAGE \ N_("git hook run [--ignore-missing] [--to-stdin=] [-- ]") #define BUILTIN_HOOK_LIST_USAGE \ - N_("git hook list ") + N_("git hook list [-z] ") static const char * const builtin_hook_usage[] = { BUILTIN_HOOK_RUN_USAGE, @@ -34,9 +34,12 @@ static int list(int argc, const char **argv, const char *prefix, struct string_list *head; struct string_list_item *item; const char *hookname = NULL; + int line_terminator = '\n'; int ret = 0; struct option list_options[] = { + OPT_SET_INT('z', NULL, &line_terminator, + N_("use NUL as line terminator"), '\0'), OPT_END(), }; @@ -66,10 +69,10 @@ static int list(int argc, const char **argv, const char *prefix, switch (h->kind) { case HOOK_TRADITIONAL: - printf("%s\n", _("hook from hookdir")); + printf("%s%c", _("hook from hookdir"), line_terminator); break; case HOOK_CONFIGURED: - printf("%s\n", h->u.configured.friendly_name); + printf("%s%c", h->u.configured.friendly_name, line_terminator); break; default: BUG("unknown hook kind"); diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index e58151e8f8..b1583e9ef9 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -61,6 +61,19 @@ test_expect_success 'git hook list: configured hook' ' test_cmp expect actual ' +test_expect_success 'git hook list: -z shows NUL-terminated output' ' + test_hook test-hook <<-EOF && + echo Test hook + EOF + test_config hook.myhook.command "echo Hello" && + test_config hook.myhook.event test-hook --add && + + printf "myhookQhook from hookdirQ" >expect && + git hook list -z test-hook >actual.raw && + nul_to_q actual && + test_cmp expect actual +' + test_expect_success 'git hook run: nonexistent hook' ' cat >stderr.expect <<-\EOF && error: cannot find a hook named test-hook