From 85b90f1166ad85c8b9f07b91bd8a54f626edcc04 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Mon, 9 Mar 2026 15:37:31 +0200 Subject: [PATCH 1/9] repository: fix repo_init() memleak due to missing _clear() There is an old pre-existing memory leak in repo_init() due to failing to call clear_repository_format() in the error case. It went undetected because a specific bug is required to trigger it: enable a v1 extension in a repository with format v0. Obviously this can only happen in a development environment, so it does not trigger in normal usage, however the memleak is real and needs fixing. Fix it by also calling clear_repository_format() in the error case. Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- repository.c | 1 + 1 file changed, 1 insertion(+) diff --git a/repository.c b/repository.c index 8717a1693a..f0bc2917d2 100644 --- a/repository.c +++ b/repository.c @@ -316,6 +316,7 @@ int repo_init(struct repository *repo, return 0; error: + clear_repository_format(&format); repo_clear(repo); return -1; } From a9710a3aeacd50f7835a5c32052e0455ade20a6e Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Mon, 9 Mar 2026 15:37:32 +0200 Subject: [PATCH 2/9] config: add a repo_config_get_uint() helper Next commits add a 'hook.jobs' config option of type 'unsigned int', so add a helper to parse it since the API only supports int and ulong. An alternative is to make 'hook.jobs' an 'int' or parse it as an 'int' then cast it to unsigned, however it's better to use proper helpers for the type. Using 'ulong' is another option which already has helpers, but it's a bit excessive in size for just the jobs number. Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- config.c | 28 ++++++++++++++++++++++++++++ config.h | 13 +++++++++++++ parse.c | 9 +++++++++ parse.h | 1 + 4 files changed, 51 insertions(+) diff --git a/config.c b/config.c index 156f2a24fa..a1b92fe083 100644 --- a/config.c +++ b/config.c @@ -1212,6 +1212,15 @@ int git_config_int(const char *name, const char *value, return ret; } +unsigned int git_config_uint(const char *name, const char *value, + const struct key_value_info *kvi) +{ + unsigned int ret; + if (!git_parse_uint(value, &ret)) + die_bad_number(name, value, kvi); + return ret; +} + int64_t git_config_int64(const char *name, const char *value, const struct key_value_info *kvi) { @@ -1907,6 +1916,18 @@ int git_configset_get_int(struct config_set *set, const char *key, int *dest) return 1; } +int git_configset_get_uint(struct config_set *set, const char *key, unsigned int *dest) +{ + const char *value; + struct key_value_info kvi; + + if (!git_configset_get_value(set, key, &value, &kvi)) { + *dest = git_config_uint(key, value, &kvi); + return 0; + } else + return 1; +} + int git_configset_get_ulong(struct config_set *set, const char *key, unsigned long *dest) { const char *value; @@ -2356,6 +2377,13 @@ int repo_config_get_int(struct repository *repo, return git_configset_get_int(repo->config, key, dest); } +int repo_config_get_uint(struct repository *repo, + const char *key, unsigned int *dest) +{ + git_config_check_init(repo); + return git_configset_get_uint(repo->config, key, dest); +} + int repo_config_get_ulong(struct repository *repo, const char *key, unsigned long *dest) { diff --git a/config.h b/config.h index ba426a960a..bf47fb3afc 100644 --- a/config.h +++ b/config.h @@ -267,6 +267,12 @@ int git_config_int(const char *, const char *, const struct key_value_info *); int64_t git_config_int64(const char *, const char *, const struct key_value_info *); +/** + * Identical to `git_config_int`, but for unsigned ints. + */ +unsigned int git_config_uint(const char *, const char *, + const struct key_value_info *); + /** * Identical to `git_config_int`, but for unsigned longs. */ @@ -560,6 +566,7 @@ int git_configset_get_value(struct config_set *cs, const char *key, int git_configset_get_string(struct config_set *cs, const char *key, char **dest); int git_configset_get_int(struct config_set *cs, const char *key, int *dest); +int git_configset_get_uint(struct config_set *cs, const char *key, unsigned int *dest); int git_configset_get_ulong(struct config_set *cs, const char *key, unsigned long *dest); int git_configset_get_bool(struct config_set *cs, const char *key, int *dest); int git_configset_get_bool_or_int(struct config_set *cs, const char *key, int *is_bool, int *dest); @@ -650,6 +657,12 @@ int repo_config_get_string_tmp(struct repository *r, */ int repo_config_get_int(struct repository *r, const char *key, int *dest); +/** + * Similar to `repo_config_get_int` but for unsigned ints. + */ +int repo_config_get_uint(struct repository *r, + const char *key, unsigned int *dest); + /** * Similar to `repo_config_get_int` but for unsigned longs. */ diff --git a/parse.c b/parse.c index 48313571aa..d77f28046a 100644 --- a/parse.c +++ b/parse.c @@ -107,6 +107,15 @@ int git_parse_int64(const char *value, int64_t *ret) return 1; } +int git_parse_uint(const char *value, unsigned int *ret) +{ + uintmax_t tmp; + if (!git_parse_unsigned(value, &tmp, maximum_unsigned_value_of_type(unsigned int))) + return 0; + *ret = tmp; + return 1; +} + int git_parse_ulong(const char *value, unsigned long *ret) { uintmax_t tmp; diff --git a/parse.h b/parse.h index ea32de9a91..a6dd37c4cb 100644 --- a/parse.h +++ b/parse.h @@ -5,6 +5,7 @@ int git_parse_signed(const char *value, intmax_t *ret, intmax_t max); int git_parse_unsigned(const char *value, uintmax_t *ret, uintmax_t max); int git_parse_ssize_t(const char *, ssize_t *); int git_parse_ulong(const char *, unsigned long *); +int git_parse_uint(const char *value, unsigned int *ret); int git_parse_int(const char *value, int *ret); int git_parse_int64(const char *value, int64_t *ret); int git_parse_double(const char *value, double *ret); From 06dcc4d2f61d21c56aec9836a0467a05b7dffb2e Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Mon, 9 Mar 2026 15:37:33 +0200 Subject: [PATCH 3/9] hook: parse the hook.jobs config The hook.jobs config is a global way to set hook parallelization for all hooks, in the sense that it is not per-event nor per-hook. Finer-grained configs will be added in later commits which can override it, for e.g. via a per-event type job options. Next commits will also add to this item's documentation. Parse hook.jobs config key in hook_config_lookup_all() and store its value in hook_all_config_cb.jobs, then transfer it into hook_config_cache.jobs after the config pass completes. This is mostly plumbing and the cached value is not yet used. Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/config/hook.adoc | 4 ++++ hook.c | 22 ++++++++++++++++++++-- hook.h | 1 + 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc index 9e78f26439..b7847f9338 100644 --- a/Documentation/config/hook.adoc +++ b/Documentation/config/hook.adoc @@ -22,3 +22,7 @@ hook..enabled:: 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]. + +hook.jobs:: + Specifies how many hooks can be run simultaneously during parallelized + hook execution. If unspecified, defaults to 1 (serial execution). diff --git a/hook.c b/hook.c index 4f4f060156..e6e44a5fcb 100644 --- a/hook.c +++ b/hook.c @@ -127,11 +127,13 @@ struct hook_config_cache_entry { * commands: friendly-name to command map. * event_hooks: event-name to list of friendly-names map. * disabled_hooks: set of friendly-names with hook..enabled = false. + * jobs: value of the global hook.jobs key. Defaults to 0 if unset. */ struct hook_all_config_cb { struct strmap commands; struct strmap event_hooks; struct string_list disabled_hooks; + unsigned int jobs; }; /* repo_config() callback that collects all hook.* configuration in one pass. */ @@ -147,6 +149,20 @@ static int hook_config_lookup_all(const char *key, const char *value, if (parse_config_key(key, "hook", &name, &name_len, &subkey)) return 0; + /* Handle plain hook. entries that have no hook name component. */ + if (!name) { + if (!strcmp(subkey, "jobs") && value) { + unsigned int v; + if (!git_parse_uint(value, &v)) + warning(_("hook.jobs must be a positive integer, ignoring: '%s'"), value); + else if (!v) + warning(_("hook.jobs must be positive, ignoring: 0")); + else + data->jobs = v; + } + return 0; + } + if (!value) return config_error_nonbool(key); @@ -245,7 +261,7 @@ void hook_cache_clear(struct hook_config_cache *cache) static void build_hook_config_map(struct repository *r, struct hook_config_cache *cache) { - struct hook_all_config_cb cb_data; + struct hook_all_config_cb cb_data = { 0 }; struct hashmap_iter iter; struct strmap_entry *e; @@ -253,7 +269,7 @@ static void build_hook_config_map(struct repository *r, strmap_init(&cb_data.event_hooks); string_list_init_dup(&cb_data.disabled_hooks); - /* Parse all configs in one run. */ + /* Parse all configs in one run, capturing hook.* including hook.jobs. */ repo_config(r, hook_config_lookup_all, &cb_data); /* Construct the cache from parsed configs. */ @@ -297,6 +313,8 @@ static void build_hook_config_map(struct repository *r, strmap_put(&cache->hooks, e->key, hooks); } + cache->jobs = cb_data.jobs; + strmap_clear(&cb_data.commands, 1); string_list_clear(&cb_data.disabled_hooks, 0); strmap_for_each_entry(&cb_data.event_hooks, &iter, e) { diff --git a/hook.h b/hook.h index 0432df963f..a7eab00480 100644 --- a/hook.h +++ b/hook.h @@ -208,6 +208,7 @@ void hook_free(void *p, const char *str UNUSED); */ struct hook_config_cache { struct strmap hooks; /* maps event name -> string_list of hooks */ + unsigned int jobs; /* hook.jobs config value; 0 if unset (defaults to serial) */ }; /** From 31c9a1b13f2976b28bb85d8d17d837210a038690 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Mon, 9 Mar 2026 15:37:34 +0200 Subject: [PATCH 4/9] hook: allow parallel hook execution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hooks always run in sequential order due to the hardcoded jobs == 1 passed to run_process_parallel(). Remove that hardcoding to allow users to run hooks in parallel (opt-in). Users need to decide which hooks to run in parallel, by specifying "parallel = true" in the config, because git cannot know if their specific hooks are safe to run or not in parallel (for e.g. two hooks might write to the same file or call the same program). Some hooks are unsafe to run in parallel by design: these will marked in the next commit using RUN_HOOKS_OPT_INIT_FORCE_SERIAL. The hook.jobs config specifies the default number of jobs applied to all hooks which have parallelism enabled. Signed-off-by: Emily Shaffer Helped-by: Ævar Arnfjörð Bjarmason Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/config/hook.adoc | 13 +++ hook.c | 66 +++++++++++++-- hook.h | 25 ++++++ t/t1800-hook.sh | 142 +++++++++++++++++++++++++++++++++ 4 files changed, 240 insertions(+), 6 deletions(-) diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc index b7847f9338..45811d1032 100644 --- a/Documentation/config/hook.adoc +++ b/Documentation/config/hook.adoc @@ -23,6 +23,19 @@ hook..enabled:: in a system or global config file and needs to be disabled for a specific repository. See linkgit:git-hook[1]. +hook..parallel:: + Whether the hook `hook.` may run in parallel with other hooks + for the same event. Defaults to `false`. Set to `true` only when the + hook script is safe to run concurrently with other hooks for the same + event. If any hook for an event does not have this set to `true`, + all hooks for that event run sequentially regardless of `hook.jobs`. + Only configured (named) hooks need to declare this. Traditional hooks + found in the hooks directory do not need to, and run in parallel when + the effective job count is greater than 1. See linkgit:git-hook[1]. + hook.jobs:: Specifies how many hooks can be run simultaneously during parallelized hook execution. If unspecified, defaults to 1 (serial execution). ++ +This setting has no effect unless all configured hooks for the event have +`hook..parallel` set to `true`. diff --git a/hook.c b/hook.c index e6e44a5fcb..815b299bf8 100644 --- a/hook.c +++ b/hook.c @@ -120,6 +120,7 @@ struct hook_config_cache_entry { char *command; enum config_scope scope; int disabled; + unsigned int parallel:1; }; /* @@ -127,12 +128,14 @@ struct hook_config_cache_entry { * commands: friendly-name to command map. * event_hooks: event-name to list of friendly-names map. * disabled_hooks: set of friendly-names with hook..enabled = false. + * parallel_hooks: friendly-name to parallel flag. * jobs: value of the global hook.jobs key. Defaults to 0 if unset. */ struct hook_all_config_cb { struct strmap commands; struct strmap event_hooks; struct string_list disabled_hooks; + struct strmap parallel_hooks; unsigned int jobs; }; @@ -223,6 +226,10 @@ static int hook_config_lookup_all(const char *key, const char *value, default: break; /* ignore unrecognised values */ } + } else if (!strcmp(subkey, "parallel")) { + int v = git_parse_maybe_bool(value); + if (v >= 0) + strmap_put(&data->parallel_hooks, hook_name, (void *)(uintptr_t)v); } free(hook_name); @@ -268,6 +275,7 @@ static void build_hook_config_map(struct repository *r, strmap_init(&cb_data.commands); strmap_init(&cb_data.event_hooks); string_list_init_dup(&cb_data.disabled_hooks); + strmap_init(&cb_data.parallel_hooks); /* Parse all configs in one run, capturing hook.* including hook.jobs. */ repo_config(r, hook_config_lookup_all, &cb_data); @@ -285,6 +293,7 @@ static void build_hook_config_map(struct repository *r, enum config_scope scope = (enum config_scope)(uintptr_t)hook_names->items[i].util; struct hook_config_cache_entry *entry; + void *par = strmap_get(&cb_data.parallel_hooks, hname); char *command; int is_disabled = @@ -307,6 +316,7 @@ static void build_hook_config_map(struct repository *r, entry->command = command ? xstrdup(command) : NULL; entry->scope = scope; entry->disabled = is_disabled; + entry->parallel = (int)(uintptr_t)par; string_list_append(hooks, hname)->util = entry; } @@ -316,6 +326,7 @@ static void build_hook_config_map(struct repository *r, cache->jobs = cb_data.jobs; strmap_clear(&cb_data.commands, 1); + strmap_clear(&cb_data.parallel_hooks, 0); /* values are uintptr_t, not heap ptrs */ string_list_clear(&cb_data.disabled_hooks, 0); strmap_for_each_entry(&cb_data.event_hooks, &iter, e) { string_list_clear(e->value, 0); @@ -392,6 +403,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->parallel = entry->parallel; string_list_append(list, friendly_name)->util = hook; } @@ -541,21 +553,67 @@ static void run_hooks_opt_clear(struct run_hooks_opt *options) strvec_clear(&options->args); } +/* Determine how many jobs to use for hook execution. */ +static unsigned int get_hook_jobs(struct repository *r, + struct run_hooks_opt *options, + struct string_list *hook_list) +{ + unsigned int jobs; + + /* + * Hooks needing separate output streams must run sequentially. Next + * commits will add an extension to allow parallelizing these as well. + */ + if (!options->stdout_to_stderr) + return 1; + + /* An explicit job count (FORCE_SERIAL jobs=1, or -j from CLI). */ + if (options->jobs) + return options->jobs; + + /* + * Use hook.jobs from the already-parsed config cache (in-repo), or + * fall back to a direct config lookup (out-of-repo). Default to 1. + */ + if (r && r->gitdir && r->hook_config_cache) + /* Use the already-parsed cache (in-repo) */ + jobs = r->hook_config_cache->jobs ? r->hook_config_cache->jobs : 1; + else + /* No cache present (out-of-repo call), use direct cfg lookup */ + jobs = repo_config_get_uint(r, "hook.jobs", &jobs) ? 1 : jobs; + + /* + * Cap to serial any configured hook not marked as parallel = true. + * This enforces the parallel = false default, even for "traditional" + * hooks from the hookdir which cannot be marked parallel = true. + */ + for (size_t i = 0; jobs > 1 && i < hook_list->nr; i++) { + struct hook *h = hook_list->items[i].util; + if (h->kind == HOOK_CONFIGURED && !h->parallel) + jobs = 1; + } + + return jobs; +} + int run_hooks_opt(struct repository *r, const char *hook_name, struct run_hooks_opt *options) { + struct string_list *hook_list = list_hooks(r, hook_name, options); struct hook_cb_data cb_data = { .rc = 0, .hook_name = hook_name, + .hook_command_list = hook_list, .options = options, }; int ret = 0; + unsigned int jobs = get_hook_jobs(r, options, hook_list); const struct run_process_parallel_opts opts = { .tr2_category = "hook", .tr2_label = hook_name, - .processes = options->jobs, - .ungroup = options->jobs == 1, + .processes = jobs, + .ungroup = jobs == 1, .get_next_task = pick_next_hook, .start_failure = notify_start_failure, @@ -571,9 +629,6 @@ int run_hooks_opt(struct repository *r, const char *hook_name, if (options->path_to_stdin && options->feed_pipe) BUG("options path_to_stdin and feed_pipe are mutually exclusive"); - 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. @@ -584,7 +639,6 @@ int run_hooks_opt(struct repository *r, const char *hook_name, if (options->invoked_hook) *options->invoked_hook = 0; - 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); diff --git a/hook.h b/hook.h index a7eab00480..a0f6a9db47 100644 --- a/hook.h +++ b/hook.h @@ -35,6 +35,13 @@ struct hook { } configured; } u; + /** + * Whether this hook may run in parallel with other hooks for the same + * event. Only useful for configured (named) hooks. Traditional hooks + * always default to 0 (serial). Set via `hook..parallel = true`. + */ + unsigned int parallel:1; + /** * Opaque data pointer used to keep internal state across callback calls. * @@ -73,6 +80,8 @@ struct run_hooks_opt * * If > 1, output will be buffered and de-interleaved (ungroup=0). * If == 1, output will be real-time (ungroup=1). + * If == 0, the 'hook.jobs' config is used or, if the config is unset, + * defaults to 1 (serial execution). */ unsigned int jobs; @@ -153,7 +162,23 @@ struct run_hooks_opt hook_data_free_fn feed_pipe_cb_data_free; }; +/** + * Default initializer for hooks. Parallelism is opt-in: .jobs = 0 defers to + * the 'hook.jobs' config, falling back to serial (1) if unset. + */ #define RUN_HOOKS_OPT_INIT { \ + .env = STRVEC_INIT, \ + .args = STRVEC_INIT, \ + .stdout_to_stderr = 1, \ + .jobs = 0, \ +} + +/** + * Initializer for hooks that must always run sequentially regardless of + * 'hook.jobs'. Use this when git knows the hook cannot safely be parallelized + * .jobs = 1 is non-overridable. + */ +#define RUN_HOOKS_OPT_INIT_FORCE_SERIAL { \ .env = STRVEC_INIT, \ .args = STRVEC_INIT, \ .stdout_to_stderr = 1, \ diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index cbf7d2bf80..57733e8a73 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -21,6 +21,57 @@ setup_hookdir () { test_when_finished rm -rf .git/hooks } +# write_sentinel_hook [sentinel] +# +# Writes a hook that marks itself as started, sleeps for a few seconds, then +# marks itself done. The sleep must be long enough that sentinel_detector can +# observe .started before .done appears when both hooks +# run concurrently in parallel mode. +write_sentinel_hook () { + sentinel="${2:-sentinel}" + write_script "$1" <<-EOF + touch ${sentinel}.started && + sleep 2 && + touch ${sentinel}.done + EOF +} + +# sentinel_detector +# +# Returns a shell command string suitable for use as hook..command. +# The detector must be registered after the sentinel: +# 1. In serial mode, the sentinel has completed (and .done exists) +# before the detector starts. +# 2. In parallel mode, both run concurrently so .done has not appeared +# yet and the detector just sees .started. +# +# At start, poll until .started exists to absorb startup jitter, then +# write to : +# 1. 'serial' if .done exists (sentinel finished before we started), +# 2. 'parallel' if only .started exists (sentinel still running), +# 3. 'timeout' if .started never appeared. +# +# The command ends with ':' so when git appends "$@" for hooks that receive +# positional arguments (e.g. pre-push), the result ': "$@"' is valid shell +# rather than a syntax error 'fi "$@"'. +sentinel_detector () { + cat <<-EOF + i=0 + while ! test -f ${1}.started && test \$i -lt 10; do + sleep 1 + i=\$((i+1)) + done + if test -f ${1}.done; then + echo serial >${2} + elif test -f ${1}.started; then + echo parallel >${2} + else + echo timeout >${2} + fi + : + EOF +} + test_expect_success 'git hook usage' ' test_expect_code 129 git hook && test_expect_code 129 git hook run && @@ -626,4 +677,95 @@ test_expect_success 'server push-to-checkout hook expects stdout redirected to s check_stdout_merged_to_stderr push-to-checkout ' +test_expect_success 'hook.jobs=1 config runs hooks in series' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + + # Use two configured hooks so the execution order is deterministic: + # hook-1 (sentinel) is listed before hook-2 (detector), so hook-1 + # always runs first even in serial mode. + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + + test_config hook.jobs 1 && + + git hook run test-hook >out 2>err && + echo serial >expect && + test_cmp expect hook.order +' + +test_expect_success 'hook.jobs=2 config runs hooks in parallel' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_when_finished "rm -rf .git/hooks" && + + mkdir -p .git/hooks && + write_sentinel_hook .git/hooks/test-hook && + + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + test_config hook.hook-2.parallel true && + + test_config hook.jobs 2 && + + git hook run test-hook >out 2>err && + echo parallel >expect && + test_cmp expect hook.order +' + +test_expect_success 'hook..parallel=true enables parallel execution' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + test_config hook.hook-1.parallel true && + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + test_config hook.hook-2.parallel true && + + test_config hook.jobs 2 && + + git hook run test-hook >out 2>err && + echo parallel >expect && + test_cmp expect hook.order +' + +test_expect_success 'hook..parallel=false (default) forces serial execution' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + + test_config hook.jobs 2 && + + git hook run test-hook >out 2>err && + echo serial >expect && + test_cmp expect hook.order +' + +test_expect_success 'one non-parallel hook forces the whole event to run serially' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + test_config hook.hook-1.parallel true && + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + # hook-2 has no parallel=true: should force serial for all + + test_config hook.jobs 2 && + + git hook run test-hook >out 2>err && + echo serial >expect && + test_cmp expect hook.order +' + test_done From 0f993d3e986e3b5fe041c1f38e59a97eff039ece Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Mon, 9 Mar 2026 15:37:35 +0200 Subject: [PATCH 5/9] hook: mark non-parallelizable hooks Several hooks are known to be inherently non-parallelizable, so initialize them with RUN_HOOKS_OPT_INIT_FORCE_SERIAL. This pins jobs=1 and overrides any hook.jobs or runtime -j flags. These hooks are: applypatch-msg, pre-commit, prepare-commit-msg, commit-msg, post-commit, post-checkout, and push-to-checkout. Signed-off-by: Emily Shaffer Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/config/hook.adoc | 4 ++++ builtin/am.c | 8 +++++--- builtin/checkout.c | 19 +++++++++++++------ builtin/clone.c | 6 ++++-- builtin/receive-pack.c | 3 ++- builtin/worktree.c | 2 +- commit.c | 2 +- t/t1800-hook.sh | 16 ++++++++++++++++ 8 files changed, 46 insertions(+), 14 deletions(-) diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc index 45811d1032..d2e4b33240 100644 --- a/Documentation/config/hook.adoc +++ b/Documentation/config/hook.adoc @@ -36,6 +36,10 @@ hook..parallel:: hook.jobs:: Specifies how many hooks can be run simultaneously during parallelized hook execution. If unspecified, defaults to 1 (serial execution). + Some hooks always run sequentially regardless of this setting because + git knows they cannot safely be parallelized: `applypatch-msg`, + `pre-commit`, `prepare-commit-msg`, `commit-msg`, `post-commit`, + `post-checkout`, and `push-to-checkout`. + This setting has no effect unless all configured hooks for the event have `hook..parallel` set to `true`. diff --git a/builtin/am.c b/builtin/am.c index e0c767e223..45a8e78d0b 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -490,9 +490,11 @@ static int run_applypatch_msg_hook(struct am_state *state) assert(state->msg); - if (!state->no_verify) - ret = run_hooks_l(the_repository, "applypatch-msg", - am_path(state, "final-commit"), NULL); + if (!state->no_verify) { + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_FORCE_SERIAL; + strvec_push(&opt.args, am_path(state, "final-commit")); + ret = run_hooks_opt(the_repository, "applypatch-msg", &opt); + } if (!ret) { FREE_AND_NULL(state->msg); diff --git a/builtin/checkout.c b/builtin/checkout.c index 164a56480a..f351daadc9 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -31,6 +31,7 @@ #include "resolve-undo.h" #include "revision.h" #include "setup.h" +#include "strvec.h" #include "submodule.h" #include "symlinks.h" #include "trace2.h" @@ -123,13 +124,19 @@ static void branch_info_release(struct branch_info *info) static int post_checkout_hook(struct commit *old_commit, struct commit *new_commit, int changed) { - return run_hooks_l(the_repository, "post-checkout", - oid_to_hex(old_commit ? &old_commit->object.oid : null_oid(the_hash_algo)), - oid_to_hex(new_commit ? &new_commit->object.oid : null_oid(the_hash_algo)), - changed ? "1" : "0", NULL); - /* "new_commit" can be NULL when checking out from the index before - a commit exists. */ + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_FORCE_SERIAL; + /* + * "new_commit" can be NULL when checking out from the index before + * a commit exists. + */ + strvec_pushl(&opt.args, + oid_to_hex(old_commit ? &old_commit->object.oid : null_oid(the_hash_algo)), + oid_to_hex(new_commit ? &new_commit->object.oid : null_oid(the_hash_algo)), + changed ? "1" : "0", + NULL); + + return run_hooks_opt(the_repository, "post-checkout", &opt); } static int update_some(const struct object_id *oid, struct strbuf *base, diff --git a/builtin/clone.c b/builtin/clone.c index fba3c9c508..d23b0cafcf 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -647,6 +647,7 @@ static int checkout(int submodule_progress, struct tree *tree; struct tree_desc t; int err = 0; + struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT_FORCE_SERIAL; if (option_no_checkout) return 0; @@ -697,8 +698,9 @@ static int checkout(int submodule_progress, if (write_locked_index(the_repository->index, &lock_file, COMMIT_LOCK)) die(_("unable to write new index file")); - err |= run_hooks_l(the_repository, "post-checkout", oid_to_hex(null_oid(the_hash_algo)), - oid_to_hex(&oid), "1", NULL); + strvec_pushl(&hook_opt.args, oid_to_hex(null_oid(the_hash_algo)), + oid_to_hex(&oid), "1", NULL); + err |= run_hooks_opt(the_repository, "post-checkout", &hook_opt); if (!err && (option_recurse_submodules.nr > 0)) { struct child_process cmd = CHILD_PROCESS_INIT; diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 6066cbae8e..91e4f7e068 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1463,7 +1463,8 @@ static const char *push_to_checkout(unsigned char *hash, struct strvec *env, const char *work_tree) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_FORCE_SERIAL; + opt.invoked_hook = invoked_hook; strvec_pushf(env, "GIT_WORK_TREE=%s", absolute_path(work_tree)); diff --git a/builtin/worktree.c b/builtin/worktree.c index bc2d0d645b..d4e7c33205 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -609,7 +609,7 @@ done: * is_junk is cleared, but do return appropriate code when hook fails. */ if (!ret && opts->checkout && !opts->orphan) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_FORCE_SERIAL; strvec_pushl(&opt.env, "GIT_DIR", "GIT_WORK_TREE", NULL); strvec_pushl(&opt.args, diff --git a/commit.c b/commit.c index 0ffdd6679e..a2c4ffaac5 100644 --- a/commit.c +++ b/commit.c @@ -1979,7 +1979,7 @@ size_t ignored_log_message_bytes(const char *buf, size_t len) int run_commit_hook(int editor_is_used, const char *index_file, int *invoked_hook, const char *name, ...) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_FORCE_SERIAL; va_list args; const char *arg; diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 57733e8a73..dad7583f3a 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -768,4 +768,20 @@ test_expect_success 'one non-parallel hook forces the whole event to run seriall test_cmp expect hook.order ' +test_expect_success 'hook.jobs=2 is ignored for force-serial hooks (pre-commit)' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_config hook.hook-1.event pre-commit && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + test_config hook.hook-1.parallel true && + test_config hook.hook-2.event pre-commit && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + test_config hook.hook-2.parallel true && + test_config hook.jobs 2 && + git commit --allow-empty -m "test: verify force-serial on pre-commit" && + echo serial >expect && + test_cmp expect hook.order +' + test_done From 8d792d3ff5ff3b65a7a8456cff85e451eb741a03 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Mon, 9 Mar 2026 15:37:36 +0200 Subject: [PATCH 6/9] hook: add -j/--jobs option to git hook run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Expose the parallel job count as a command-line flag so callers can request parallelism without relying only on the hook.jobs config. Add tests covering serial/parallel execution and TTY behaviour under -j1 vs -jN. Signed-off-by: Emily Shaffer Helped-by: Ævar Arnfjörð Bjarmason Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/git-hook.adoc | 18 +++++- builtin/hook.c | 5 +- hook.c | 14 ++-- t/t1800-hook.sh | 123 ++++++++++++++++++++++++++++++++++-- 4 files changed, 144 insertions(+), 16 deletions(-) diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc index 4d4e728327..17105cc729 100644 --- a/Documentation/git-hook.adoc +++ b/Documentation/git-hook.adoc @@ -8,7 +8,8 @@ git-hook - Run git hooks SYNOPSIS -------- [verse] -'git hook' run [--ignore-missing] [--to-stdin=] [-- ] +'git hook' run [--ignore-missing] [--to-stdin=] [(-j|--jobs) ] + [-- ] 'git hook' list [-z] [--show-scope] DESCRIPTION @@ -139,6 +140,18 @@ OPTIONS in parentheses after the friendly name of each configured hook, to show where it was defined. Traditional hooks from the hookdir are unaffected. +-j:: +--jobs:: + Only valid for `run`. ++ +Specify how many hooks to run simultaneously. If this flag is not specified, +the value of the `hook.jobs` config is used, see linkgit:git-config[1]. If +neither is specified, defaults to 1 (serial execution). Some hooks always run +sequentially regardless of this flag or the `hook.jobs` config, because git +knows they cannot safely run in parallel: `applypatch-msg`, `pre-commit`, +`prepare-commit-msg`, `commit-msg`, `post-commit`, `post-checkout`, and +`push-to-checkout`. + WRAPPERS -------- @@ -161,7 +174,8 @@ running: git hook run mywrapper-start-tests \ # providing something to stdin --stdin some-tempfile-123 \ - # execute hooks in serial + # execute multiple hooks in parallel + --jobs 3 \ # plus some arguments of your own... -- \ --testname bar \ diff --git a/builtin/hook.c b/builtin/hook.c index ff446948fa..6ec0319cc5 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -7,7 +7,8 @@ #include "parse-options.h" #define BUILTIN_HOOK_RUN_USAGE \ - N_("git hook run [--ignore-missing] [--to-stdin=] [-- ]") + N_("git hook run [--ignore-missing] [--to-stdin=] [(-j|--jobs) ]\n" \ + " [-- ]") #define BUILTIN_HOOK_LIST_USAGE \ N_("git hook list [-z] [--show-scope] ") @@ -109,6 +110,8 @@ static int run(int argc, const char **argv, const char *prefix, N_("silently ignore missing requested ")), OPT_STRING(0, "to-stdin", &opt.path_to_stdin, N_("path"), N_("file to read into hooks' stdin")), + OPT_UNSIGNED('j', "jobs", &opt.jobs, + N_("run up to hooks simultaneously")), OPT_END(), }; int ret; diff --git a/hook.c b/hook.c index 815b299bf8..299cbf9e97 100644 --- a/hook.c +++ b/hook.c @@ -567,15 +567,17 @@ static unsigned int get_hook_jobs(struct repository *r, if (!options->stdout_to_stderr) return 1; - /* An explicit job count (FORCE_SERIAL jobs=1, or -j from CLI). */ - if (options->jobs) - return options->jobs; + /* Pinned serial: FORCE_SERIAL (internal) or explicit -j1 from CLI. */ + if (options->jobs == 1) + return 1; /* - * Use hook.jobs from the already-parsed config cache (in-repo), or - * fall back to a direct config lookup (out-of-repo). Default to 1. + * Resolve effective job count: -jN (when given) overrides config. + * Default to 1 when both config an -jN are missing. */ - if (r && r->gitdir && r->hook_config_cache) + if (options->jobs > 1) + jobs = options->jobs; + else if (r && r->gitdir && r->hook_config_cache) /* Use the already-parsed cache (in-repo) */ jobs = r->hook_config_cache->jobs ? r->hook_config_cache->jobs : 1; else diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index dad7583f3a..fbe8be25c8 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -238,10 +238,20 @@ test_expect_success 'git -c core.hooksPath= hook run' ' ' test_hook_tty () { - cat >expect <<-\EOF - STDOUT TTY - STDERR TTY - EOF + expect_tty=$1 + shift + + if test "$expect_tty" != "no_tty"; then + cat >expect <<-\EOF + STDOUT TTY + STDERR TTY + EOF + else + cat >expect <<-\EOF + STDOUT NO TTY + STDERR NO TTY + EOF + fi test_when_finished "rm -rf repo" && git init repo && @@ -259,12 +269,21 @@ test_hook_tty () { test_cmp expect repo/actual } -test_expect_success TTY 'git hook run: stdout and stderr are connected to a TTY' ' - test_hook_tty hook run pre-commit +test_expect_success TTY 'git hook run -j1: stdout and stderr are connected to a TTY' ' + # hooks running sequentially (-j1) are always connected to the tty for + # optimum real-time performance. + test_hook_tty tty hook run -j1 pre-commit +' + +test_expect_success TTY 'git hook run -jN: stdout and stderr are not connected to a TTY' ' + # Hooks are not connected to the tty when run in parallel, instead they + # output to a pipe through which run-command collects and de-interlaces + # their outputs, which then gets passed either to the tty or a sideband. + test_hook_tty no_tty hook run -j2 pre-commit ' test_expect_success TTY 'git commit: stdout and stderr are connected to a TTY' ' - test_hook_tty commit -m"B.new" + test_hook_tty tty commit -m"B.new" ' test_expect_success 'git hook list orders by config order' ' @@ -677,6 +696,96 @@ test_expect_success 'server push-to-checkout hook expects stdout redirected to s check_stdout_merged_to_stderr push-to-checkout ' +test_expect_success 'parallel hook output is not interleaved' ' + test_when_finished "rm -rf .git/hooks" && + + write_script .git/hooks/test-hook <<-EOF && + echo "Hook 1 Start" + sleep 1 + echo "Hook 1 End" + EOF + + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "echo \"Hook 2 Start\"; sleep 2; echo \"Hook 2 End\"" && + test_config hook.hook-2.parallel true && + test_config hook.hook-3.event test-hook && + test_config hook.hook-3.command \ + "echo \"Hook 3 Start\"; sleep 3; echo \"Hook 3 End\"" && + test_config hook.hook-3.parallel true && + + git hook run -j3 test-hook >out 2>err.parallel && + + # Verify Hook 1 output is grouped + sed -n "/Hook 1 Start/,/Hook 1 End/p" err.parallel >hook1_out && + test_line_count = 2 hook1_out && + + # Verify Hook 2 output is grouped + sed -n "/Hook 2 Start/,/Hook 2 End/p" err.parallel >hook2_out && + test_line_count = 2 hook2_out && + + # Verify Hook 3 output is grouped + sed -n "/Hook 3 Start/,/Hook 3 End/p" err.parallel >hook3_out && + test_line_count = 2 hook3_out +' + +test_expect_success 'git hook run -j1 runs hooks in series' ' + test_when_finished "rm -rf .git/hooks" && + + test_config hook.series-1.event "test-hook" && + test_config hook.series-1.command "echo 1" --add && + test_config hook.series-2.event "test-hook" && + test_config hook.series-2.command "echo 2" --add && + + mkdir -p .git/hooks && + write_script .git/hooks/test-hook <<-EOF && + echo 3 + EOF + + cat >expected <<-\EOF && + 1 + 2 + 3 + EOF + + git hook run -j1 test-hook 2>actual && + test_cmp expected actual +' + +test_expect_success 'git hook run -j2 runs hooks in parallel' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_when_finished "rm -rf .git/hooks" && + + mkdir -p .git/hooks && + write_sentinel_hook .git/hooks/test-hook && + + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + test_config hook.hook-2.parallel true && + + git hook run -j2 test-hook >out 2>err && + echo parallel >expect && + test_cmp expect hook.order +' + +test_expect_success 'git hook run -j2 is blocked by parallel=false' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + # hook-1 intentionally has no parallel=true + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + # hook-2 also has no parallel=true + + # -j2 must not override parallel=false on configured hooks. + git hook run -j2 test-hook >out 2>err && + echo serial >expect && + test_cmp expect hook.order +' + test_expect_success 'hook.jobs=1 config runs hooks in series' ' test_when_finished "rm -f sentinel.started sentinel.done hook.order" && From 24c7af4d22a9378532db7f9a4b5ef3107576431d Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Mon, 9 Mar 2026 15:37:37 +0200 Subject: [PATCH 7/9] hook: add per-event jobs config Add a hook..jobs count config that allows users to override the global hook.jobs setting for specific hook events. This allows finer-grained control over parallelism on a per-event basis. For example, to run `post-receive` hooks with up to 4 parallel jobs while keeping other events at their global default: [hook] post-receive.jobs = 4 Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/config/hook.adoc | 19 +++++++++++ hook.c | 47 +++++++++++++++++++++++---- hook.h | 1 + t/t1800-hook.sh | 59 ++++++++++++++++++++++++++++++++++ 4 files changed, 120 insertions(+), 6 deletions(-) diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc index d2e4b33240..ca97ea6f1e 100644 --- a/Documentation/config/hook.adoc +++ b/Documentation/config/hook.adoc @@ -33,9 +33,28 @@ hook..parallel:: found in the hooks directory do not need to, and run in parallel when the effective job count is greater than 1. See linkgit:git-hook[1]. +hook..jobs:: + Specifies how many hooks can be run simultaneously for the `` + hook event (e.g. `hook.post-receive.jobs = 4`). Overrides `hook.jobs` + for this specific event. The same parallelism restrictions apply: this + setting has no effect unless all configured hooks for the event have + `hook..parallel` set to `true`. Must be a positive int, + zero is rejected with a warning. See linkgit:git-hook[1]. ++ +Note on naming: although this key resembles `hook..*` +(a per-hook setting), `` must be the event name, not a hook +friendly name. The key component is stored literally and looked up by +event name at runtime with no translation between the two namespaces. +A key like `hook.my-hook.jobs` is stored under `"my-hook"` but the +lookup at runtime uses the event name (e.g. `"post-receive"`), so +`hook.my-hook.jobs` is silently ignored even when `my-hook` is +registered for that event. Use `hook.post-receive.jobs` or any other +valid event name when setting `hook..jobs`. + hook.jobs:: Specifies how many hooks can be run simultaneously during parallelized hook execution. If unspecified, defaults to 1 (serial execution). + Can be overridden on a per-event basis with `hook..jobs`. Some hooks always run sequentially regardless of this setting because git knows they cannot safely be parallelized: `applypatch-msg`, `pre-commit`, `prepare-commit-msg`, `commit-msg`, `post-commit`, diff --git a/hook.c b/hook.c index 299cbf9e97..b70c4c15ec 100644 --- a/hook.c +++ b/hook.c @@ -129,6 +129,8 @@ struct hook_config_cache_entry { * event_hooks: event-name to list of friendly-names map. * disabled_hooks: set of friendly-names with hook..enabled = false. * parallel_hooks: friendly-name to parallel flag. + * event_jobs: event-name to per-event jobs count (heap-allocated unsigned int *, + * where NULL == unset). * jobs: value of the global hook.jobs key. Defaults to 0 if unset. */ struct hook_all_config_cb { @@ -136,6 +138,7 @@ struct hook_all_config_cb { struct strmap event_hooks; struct string_list disabled_hooks; struct strmap parallel_hooks; + struct strmap event_jobs; unsigned int jobs; }; @@ -230,6 +233,20 @@ static int hook_config_lookup_all(const char *key, const char *value, int v = git_parse_maybe_bool(value); if (v >= 0) strmap_put(&data->parallel_hooks, hook_name, (void *)(uintptr_t)v); + } else if (!strcmp(subkey, "jobs")) { + unsigned int v; + if (!git_parse_uint(value, &v)) + warning(_("hook.%s.jobs must be a positive integer, ignoring: '%s'"), + hook_name, value); + else if (!v) + warning(_("hook.%s.jobs must be positive, ignoring: 0"), hook_name); + else { + unsigned int *old; + unsigned int *p = xmalloc(sizeof(*p)); + *p = v; + old = strmap_put(&data->event_jobs, hook_name, p); + free(old); + } } free(hook_name); @@ -262,6 +279,7 @@ void hook_cache_clear(struct hook_config_cache *cache) free(hooks); } strmap_clear(&cache->hooks, 0); + strmap_clear(&cache->event_jobs, 1); /* free heap-allocated unsigned int * values */ } /* Populate `cache` with the complete hook configuration */ @@ -276,6 +294,7 @@ static void build_hook_config_map(struct repository *r, strmap_init(&cb_data.event_hooks); string_list_init_dup(&cb_data.disabled_hooks); strmap_init(&cb_data.parallel_hooks); + strmap_init(&cb_data.event_jobs); /* Parse all configs in one run, capturing hook.* including hook.jobs. */ repo_config(r, hook_config_lookup_all, &cb_data); @@ -324,6 +343,7 @@ static void build_hook_config_map(struct repository *r, } cache->jobs = cb_data.jobs; + cache->event_jobs = cb_data.event_jobs; strmap_clear(&cb_data.commands, 1); strmap_clear(&cb_data.parallel_hooks, 0); /* values are uintptr_t, not heap ptrs */ @@ -556,6 +576,7 @@ static void run_hooks_opt_clear(struct run_hooks_opt *options) /* Determine how many jobs to use for hook execution. */ static unsigned int get_hook_jobs(struct repository *r, struct run_hooks_opt *options, + const char *hook_name, struct string_list *hook_list) { unsigned int jobs; @@ -572,22 +593,36 @@ static unsigned int get_hook_jobs(struct repository *r, return 1; /* - * Resolve effective job count: -jN (when given) overrides config. - * Default to 1 when both config an -jN are missing. + * Resolve effective job count: -j N (when given) overrides config. + * hook..jobs overrides hook.jobs. + * Unset configs and -jN default to 1. */ - if (options->jobs > 1) + if (options->jobs > 1) { jobs = options->jobs; - else if (r && r->gitdir && r->hook_config_cache) + } else if (r && r->gitdir && r->hook_config_cache) { /* Use the already-parsed cache (in-repo) */ + unsigned int *event_jobs = strmap_get(&r->hook_config_cache->event_jobs, + hook_name); jobs = r->hook_config_cache->jobs ? r->hook_config_cache->jobs : 1; - else + if (event_jobs) + jobs = *event_jobs; + } else { /* No cache present (out-of-repo call), use direct cfg lookup */ + unsigned int event_jobs; + char *key; jobs = repo_config_get_uint(r, "hook.jobs", &jobs) ? 1 : jobs; + key = xstrfmt("hook.%s.jobs", hook_name); + if (!repo_config_get_uint(r, key, &event_jobs) && event_jobs) + jobs = event_jobs; + free(key); + } /* * Cap to serial any configured hook not marked as parallel = true. * This enforces the parallel = false default, even for "traditional" * hooks from the hookdir which cannot be marked parallel = true. + * The same restriction applies whether jobs came from hook.jobs or + * hook..jobs. */ for (size_t i = 0; jobs > 1 && i < hook_list->nr; i++) { struct hook *h = hook_list->items[i].util; @@ -609,7 +644,7 @@ int run_hooks_opt(struct repository *r, const char *hook_name, .options = options, }; int ret = 0; - unsigned int jobs = get_hook_jobs(r, options, hook_list); + unsigned int jobs = get_hook_jobs(r, options, hook_name, hook_list); const struct run_process_parallel_opts opts = { .tr2_category = "hook", .tr2_label = hook_name, diff --git a/hook.h b/hook.h index a0f6a9db47..e8603c4370 100644 --- a/hook.h +++ b/hook.h @@ -233,6 +233,7 @@ void hook_free(void *p, const char *str UNUSED); */ struct hook_config_cache { struct strmap hooks; /* maps event name -> string_list of hooks */ + struct strmap event_jobs; /* maps event name -> heap-allocated unsigned int * */ unsigned int jobs; /* hook.jobs config value; 0 if unset (defaults to serial) */ }; diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index fbe8be25c8..195cc5333e 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -893,4 +893,63 @@ test_expect_success 'hook.jobs=2 is ignored for force-serial hooks (pre-commit)' test_cmp expect hook.order ' +test_expect_success 'hook..jobs overrides hook.jobs for that event' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + test_config hook.hook-1.parallel true && + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + test_config hook.hook-2.parallel true && + + # Global hook.jobs=1 (serial), but per-event override allows parallel. + test_config hook.jobs 1 && + test_config hook.test-hook.jobs 2 && + + git hook run test-hook >out 2>err && + echo parallel >expect && + test_cmp expect hook.order +' + +test_expect_success 'hook..jobs=1 forces serial even when hook.jobs>1' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + test_config hook.hook-1.parallel true && + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + test_config hook.hook-2.parallel true && + + # Global hook.jobs=4 allows parallel, but per-event override forces serial. + test_config hook.jobs 4 && + test_config hook.test-hook.jobs 1 && + + git hook run test-hook >out 2>err && + echo serial >expect && + test_cmp expect hook.order +' + +test_expect_success 'hook..jobs still requires hook..parallel=true' ' + test_when_finished "rm -f sentinel.started sentinel.done hook.order" && + test_config hook.hook-1.event test-hook && + test_config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + # hook-1 intentionally has no parallel=true + test_config hook.hook-2.event test-hook && + test_config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + # hook-2 also has no parallel=true + + # Per-event jobs=2 but no hook has parallel=true: must still run serially. + test_config hook.test-hook.jobs 2 && + + git hook run test-hook >out 2>err && + echo serial >expect && + test_cmp expect hook.order +' + test_done From 8a101e3d2c2dba36637f538fc3bb0c501fdd2c59 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Mon, 9 Mar 2026 15:37:38 +0200 Subject: [PATCH 8/9] hook: introduce extensions.hookStdoutToStderr All hooks already redirect stdout to stderr with the exception of pre-push which has a known user who depends on the separate stdout versus stderr outputs (the git-lfs project). The pre-push behavior was a surprise which we found out about after causing a regression for git-lfs. Notably, it might not be the only exception (it's the one we know about). There might be more. This presents a challenge because stdout_to_stderr is required for hook parallelization, so run-command can buffer and de-interleave the hook outputs using ungroup=0, when hook.jobs > 1. Introduce an extension to enforce consistency: all hooks merge stdout into stderr and can be safely parallelized. This provides a clean separation and avoids breaking existing stdout vs stderr behavior. When this extension is disabled, the `hook.jobs` config has no effect for pre-push, to prevent garbled (interleaved) parallel output, so it runs sequentially like before. Alternatives I've considered to this extension include: 1. Allowing pre-push to run in parallel with interleaved output. 2. Always running pre-push sequentially (no parallel jobs for it). 3. Making users (only git-lfs? maybe more?) fix their hooks to read stderr not stdout. Out of all these alternatives, I think this extension is the most reasonable compromise, to not break existing users, allow pre-push parallel jobs for those who need it (with correct outputs) and also future-proofing in case there are any more exceptions to be added. Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/config/extensions.adoc | 12 +++++++++ Documentation/config/hook.adoc | 3 +++ repository.c | 1 + repository.h | 1 + setup.c | 7 ++++++ setup.h | 1 + t/t1800-hook.sh | 37 ++++++++++++++++++++++++++++ transport.c | 7 ++---- 8 files changed, 64 insertions(+), 5 deletions(-) diff --git a/Documentation/config/extensions.adoc b/Documentation/config/extensions.adoc index be6678bb5b..1a189ff045 100644 --- a/Documentation/config/extensions.adoc +++ b/Documentation/config/extensions.adoc @@ -116,6 +116,18 @@ The extension can be enabled automatically for new repositories by setting `init.defaultSubmodulePathConfig` to `true`, for example by running `git config --global init.defaultSubmodulePathConfig true`. +hookStdoutToStderr::: + If enabled, the stdout of all hooks is redirected to stderr. This + enforces consistency, since by default most hooks already behave + this way, with pre-push being the only known exception. ++ +This is useful for parallel hook execution (see the `hook.jobs` config in +linkgit:git-config[1]), as it allows the output of multiple hooks running +in parallel to be grouped (de-interleaved) correctly. ++ +Defaults to disabled. When disabled, `hook.jobs` has no effect for pre-push +hooks, which will always be run sequentially. + worktreeConfig::: If enabled, then worktrees will load config settings from the `$GIT_DIR/config.worktree` file in addition to the diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc index ca97ea6f1e..5e74fdce65 100644 --- a/Documentation/config/hook.adoc +++ b/Documentation/config/hook.adoc @@ -62,3 +62,6 @@ hook.jobs:: + This setting has no effect unless all configured hooks for the event have `hook..parallel` set to `true`. ++ +This has no effect for hooks requiring separate output streams (like `pre-push`) +unless `extensions.hookStdoutToStderr` is enabled. diff --git a/repository.c b/repository.c index f0bc2917d2..7901f80f00 100644 --- a/repository.c +++ b/repository.c @@ -301,6 +301,7 @@ int repo_init(struct repository *repo, repo->repository_format_relative_worktrees = format.relative_worktrees; repo->repository_format_precious_objects = format.precious_objects; repo->repository_format_submodule_path_cfg = format.submodule_path_cfg; + repo->repository_format_hook_stdout_to_stderr = format.hook_stdout_to_stderr; /* take ownership of format.partial_clone */ repo->repository_format_partial_clone = format.partial_clone; diff --git a/repository.h b/repository.h index 8d6e0b3dd2..ddb62fc436 100644 --- a/repository.h +++ b/repository.h @@ -182,6 +182,7 @@ struct repository { int repository_format_relative_worktrees; int repository_format_precious_objects; int repository_format_submodule_path_cfg; + int repository_format_hook_stdout_to_stderr; /* Indicate if a repository has a different 'commondir' from 'gitdir' */ unsigned different_commondir:1; diff --git a/setup.c b/setup.c index 393b970ae4..b12c5ca313 100644 --- a/setup.c +++ b/setup.c @@ -710,6 +710,9 @@ static enum extension_result handle_extension(const char *var, } else if (!strcmp(ext, "submodulepathconfig")) { data->submodule_path_cfg = git_config_bool(var, value); return EXTENSION_OK; + } else if (!strcmp(ext, "hookstdouttostderr")) { + data->hook_stdout_to_stderr = git_config_bool(var, value); + return EXTENSION_OK; } return EXTENSION_UNKNOWN; } @@ -1976,6 +1979,8 @@ const char *setup_git_directory_gently(int *nongit_ok) repo_fmt.relative_worktrees; the_repository->repository_format_submodule_path_cfg = repo_fmt.submodule_path_cfg; + the_repository->repository_format_hook_stdout_to_stderr = + repo_fmt.hook_stdout_to_stderr; /* take ownership of repo_fmt.partial_clone */ the_repository->repository_format_partial_clone = repo_fmt.partial_clone; @@ -2098,6 +2103,8 @@ void check_repository_format(struct repository_format *fmt) fmt->submodule_path_cfg; the_repository->repository_format_relative_worktrees = fmt->relative_worktrees; + the_repository->repository_format_hook_stdout_to_stderr = + fmt->hook_stdout_to_stderr; the_repository->repository_format_partial_clone = xstrdup_or_null(fmt->partial_clone); clear_repository_format(&repo_fmt); diff --git a/setup.h b/setup.h index bcb16b0b4a..6d7ddb3d02 100644 --- a/setup.h +++ b/setup.h @@ -168,6 +168,7 @@ struct repository_format { int worktree_config; int relative_worktrees; int submodule_path_cfg; + int hook_stdout_to_stderr; int is_bare; int hash_algo; int compat_hash_algo; diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 195cc5333e..8f2f5e40bc 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -605,6 +605,43 @@ test_expect_success 'client hooks: pre-push expects separate stdout and stderr' check_stdout_separate_from_stderr pre-push ' +test_expect_success 'client hooks: extension makes pre-push merge stdout to stderr' ' + test_when_finished "rm -rf remote2 stdout.actual stderr.actual" && + git init --bare remote2 && + git remote add origin2 remote2 && + test_commit B && + git config set core.repositoryformatversion 1 && + test_config extensions.hookStdoutToStderr true && + setup_hooks pre-push && + git push origin2 HEAD:main >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr pre-push +' + +test_expect_success 'client hooks: pre-push defaults to serial execution' ' + test_when_finished "rm -rf remote-serial repo-serial" && + git init --bare remote-serial && + git init repo-serial && + git -C repo-serial remote add origin ../remote-serial && + test_commit -C repo-serial A && + + # Setup 2 pre-push hooks; no parallel=true so they must run serially. + # Use sentinel/detector pattern: hook-1 (sentinel, configured) runs first + # because configured hooks precede traditional hooks in list order; hook-2 + # (detector) runs second and checks whether hook-1 has finished. + git -C repo-serial config hook.hook-1.event pre-push && + git -C repo-serial config hook.hook-1.command \ + "touch sentinel.started; sleep 2; touch sentinel.done" && + git -C repo-serial config hook.hook-2.event pre-push && + git -C repo-serial config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + + git -C repo-serial config hook.jobs 2 && + + git -C repo-serial push origin HEAD >out 2>err && + echo serial >expect && + test_cmp expect repo-serial/hook.order +' + test_expect_success 'client hooks: commit hooks expect stdout redirected to stderr' ' hooks="pre-commit prepare-commit-msg \ commit-msg post-commit \ diff --git a/transport.c b/transport.c index 56a4015389..e25a94dc92 100644 --- a/transport.c +++ b/transport.c @@ -1390,11 +1390,8 @@ static int run_pre_push_hook(struct transport *transport, 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 - * them to keep backwards compatibility with existing hooks. - */ - opt.stdout_to_stderr = 0; + /* merge stdout to stderr only when extensions.hookStdoutToStderr is enabled */ + opt.stdout_to_stderr = the_repository->repository_format_hook_stdout_to_stderr; ret = run_hooks_opt(the_repository, "pre-push", &opt); From 863135ca3ded32c8fd97a97d881afb375cd6daf0 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Mon, 9 Mar 2026 15:37:39 +0200 Subject: [PATCH 9/9] hook: allow runtime enabling extensions.hookStdoutToStderr Add a new config `hook.forceStdoutToStderr` which allows enabling extensions.hookStdoutToStderr by default at runtime, both for new and existing repositories. This makes it easier for users to enable hook parallelization for hooks like pre-push by enforcing output consistency. See previous commit for a more in-depth explanation & alternatives considered. Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- Documentation/config/extensions.adoc | 3 ++ Documentation/config/hook.adoc | 6 +++ hook.c | 28 ++++++++++- hook.h | 1 + setup.c | 10 ++++ t/t1800-hook.sh | 69 ++++++++++++++++++++++++++++ 6 files changed, 115 insertions(+), 2 deletions(-) diff --git a/Documentation/config/extensions.adoc b/Documentation/config/extensions.adoc index 1a189ff045..dd0a59de4d 100644 --- a/Documentation/config/extensions.adoc +++ b/Documentation/config/extensions.adoc @@ -127,6 +127,9 @@ in parallel to be grouped (de-interleaved) correctly. + Defaults to disabled. When disabled, `hook.jobs` has no effect for pre-push hooks, which will always be run sequentially. ++ +The extension can also be enabled by setting `hook.forceStdoutToStderr` +to `true` in the global configuration. worktreeConfig::: If enabled, then worktrees will load config settings from the diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc index 5e74fdce65..1e88c0fb25 100644 --- a/Documentation/config/hook.adoc +++ b/Documentation/config/hook.adoc @@ -65,3 +65,9 @@ This setting has no effect unless all configured hooks for the event have + This has no effect for hooks requiring separate output streams (like `pre-push`) unless `extensions.hookStdoutToStderr` is enabled. + +hook.forceStdoutToStderr:: + A boolean that enables the `extensions.hookStdoutToStderr` behavior + (merging stdout to stderr for all hooks) globally. This effectively + forces all hooks to behave as if the extension was enabled, allowing + parallel execution for hooks like `pre-push`. diff --git a/hook.c b/hook.c index b70c4c15ec..7bf7831645 100644 --- a/hook.c +++ b/hook.c @@ -132,6 +132,7 @@ struct hook_config_cache_entry { * event_jobs: event-name to per-event jobs count (heap-allocated unsigned int *, * where NULL == unset). * jobs: value of the global hook.jobs key. Defaults to 0 if unset. + * force_stdout_to_stderr: value of hook.forceStdoutToStderr. Defaults to 0. */ struct hook_all_config_cb { struct strmap commands; @@ -140,6 +141,7 @@ struct hook_all_config_cb { struct strmap parallel_hooks; struct strmap event_jobs; unsigned int jobs; + int force_stdout_to_stderr; }; /* repo_config() callback that collects all hook.* configuration in one pass. */ @@ -165,6 +167,10 @@ static int hook_config_lookup_all(const char *key, const char *value, warning(_("hook.jobs must be positive, ignoring: 0")); else data->jobs = v; + } else if (!strcmp(subkey, "forcestdouttostderr") && value) { + int v = git_parse_maybe_bool(value); + if (v >= 0) + data->force_stdout_to_stderr = v; } return 0; } @@ -344,6 +350,7 @@ static void build_hook_config_map(struct repository *r, cache->jobs = cb_data.jobs; cache->event_jobs = cb_data.event_jobs; + cache->force_stdout_to_stderr = cb_data.force_stdout_to_stderr; strmap_clear(&cb_data.commands, 1); strmap_clear(&cb_data.parallel_hooks, 0); /* values are uintptr_t, not heap ptrs */ @@ -573,6 +580,20 @@ static void run_hooks_opt_clear(struct run_hooks_opt *options) strvec_clear(&options->args); } +static void hook_force_apply_stdout_to_stderr(struct repository *r, + struct run_hooks_opt *options) +{ + int force = 0; + + if (r && r->gitdir && r->hook_config_cache) + force = r->hook_config_cache->force_stdout_to_stderr; + else + repo_config_get_bool(r, "hook.forceStdoutToStderr", &force); + + if (force) + options->stdout_to_stderr = 1; +} + /* Determine how many jobs to use for hook execution. */ static unsigned int get_hook_jobs(struct repository *r, struct run_hooks_opt *options, @@ -582,9 +603,12 @@ static unsigned int get_hook_jobs(struct repository *r, unsigned int jobs; /* - * Hooks needing separate output streams must run sequentially. Next - * commits will add an extension to allow parallelizing these as well. + * Apply hook.forceStdoutToStderr before anything else: it affects + * whether we can run in parallel or not. */ + hook_force_apply_stdout_to_stderr(r, options); + + /* Hooks needing separate output streams must run sequentially. */ if (!options->stdout_to_stderr) return 1; diff --git a/hook.h b/hook.h index e8603c4370..d4da78f277 100644 --- a/hook.h +++ b/hook.h @@ -235,6 +235,7 @@ struct hook_config_cache { struct strmap hooks; /* maps event name -> string_list of hooks */ struct strmap event_jobs; /* maps event name -> heap-allocated unsigned int * */ unsigned int jobs; /* hook.jobs config value; 0 if unset (defaults to serial) */ + int force_stdout_to_stderr; /* hook.forceStdoutToStderr config value */ }; /** diff --git a/setup.c b/setup.c index b12c5ca313..bc4e95b522 100644 --- a/setup.c +++ b/setup.c @@ -2362,6 +2362,7 @@ void initialize_repository_version(int hash_algo, struct strbuf repo_version = STRBUF_INIT; int target_version = GIT_REPO_VERSION; int default_submodule_path_config = 0; + int default_hook_stdout_to_stderr = 0; /* * Note that we initialize the repository version to 1 when the ref @@ -2419,6 +2420,15 @@ void initialize_repository_version(int hash_algo, repo_config_set(the_repository, "extensions.submodulepathconfig", "true"); } + repo_config_get_bool(the_repository, "hook.forceStdoutToStderr", + &default_hook_stdout_to_stderr); + if (default_hook_stdout_to_stderr) { + /* extensions.hookstdouttostderr requires at least version 1 */ + if (target_version == 0) + target_version = 1; + repo_config_set(the_repository, "extensions.hookstdouttostderr", "true"); + } + strbuf_addf(&repo_version, "%d", target_version); repo_config_set(the_repository, "core.repositoryformatversion", repo_version.buf); diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 8f2f5e40bc..4fdd06072d 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -989,4 +989,73 @@ test_expect_success 'hook..jobs still requires hook..parallel=true' test_cmp expect hook.order ' +test_expect_success '`git init` respects hook.forceStdoutToStderr' ' + test_when_finished "rm -rf repo-init" && + test_config_global hook.forceStdoutToStderr true && + git init repo-init && + git -C repo-init config extensions.hookStdoutToStderr >actual && + echo true >expect && + test_cmp expect actual +' + +test_expect_success '`git init` does not set extensions.hookStdoutToStderr by default' ' + test_when_finished "rm -rf upstream" && + git init upstream && + test_must_fail git -C upstream config extensions.hookStdoutToStderr +' + +test_expect_success '`git clone` does not set extensions.hookStdoutToStderr by default' ' + test_when_finished "rm -rf upstream repo-clone-no-ext" && + git init upstream && + git clone upstream repo-clone-no-ext && + test_must_fail git -C repo-clone-no-ext config extensions.hookStdoutToStderr +' + +test_expect_success '`git clone` respects hook.forceStdoutToStderr' ' + test_when_finished "rm -rf upstream repo-clone" && + git init upstream && + test_config_global hook.forceStdoutToStderr true && + git clone upstream repo-clone && + git -C repo-clone config extensions.hookStdoutToStderr >actual && + echo true >expect && + test_cmp expect actual +' + +test_expect_success 'hook.forceStdoutToStderr enables extension for existing repos' ' + test_when_finished "rm -rf remote-repo existing-repo" && + git init --bare remote-repo && + git init -b main existing-repo && + # No local extensions.hookStdoutToStderr config set here + # so global config should apply + test_config_global hook.forceStdoutToStderr true && + cd existing-repo && + test_commit A && + git remote add origin ../remote-repo && + setup_hooks pre-push && + git push origin HEAD >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr pre-push && + cd .. +' + +test_expect_success 'hook.forceStdoutToStderr enables pre-push parallel runs' ' + test_when_finished "rm -rf repo-parallel remote-parallel" && + git init --bare remote-parallel && + git init repo-parallel && + git -C repo-parallel remote add origin ../remote-parallel && + test_commit -C repo-parallel A && + + write_sentinel_hook repo-parallel/.git/hooks/pre-push && + git -C repo-parallel config hook.hook-2.event pre-push && + git -C repo-parallel config hook.hook-2.command \ + "$(sentinel_detector sentinel hook.order)" && + git -C repo-parallel config hook.hook-2.parallel true && + + git -C repo-parallel config hook.jobs 2 && + git -C repo-parallel config hook.forceStdoutToStderr true && + + git -C repo-parallel push origin HEAD >out 2>err && + echo parallel >expect && + test_cmp expect repo-parallel/hook.order +' + test_done