diff --git a/Documentation/config/extensions.adoc b/Documentation/config/extensions.adoc index a13b287d4b..934fc080ab 100644 --- a/Documentation/config/extensions.adoc +++ b/Documentation/config/extensions.adoc @@ -116,6 +116,21 @@ 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. ++ +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 `$GIT_DIR/config.worktree` file in addition to the diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc index 64e845a260..7b81d2b615 100644 --- a/Documentation/config/hook.adoc +++ b/Documentation/config/hook.adoc @@ -22,3 +22,52 @@ 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..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 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`, + `post-checkout`, and `push-to-checkout`. ++ +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. + +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/Documentation/git-hook.adoc b/Documentation/git-hook.adoc index 12d2701b52..c9492f6f8e 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] DESCRIPTION @@ -134,6 +135,18 @@ OPTIONS -z:: Terminate "list" output lines with NUL instead of newlines. +-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 -------- @@ -156,7 +169,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/am.c b/builtin/am.c index c439f868dc..4d4c8ae9fd 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 1d1667fa4c..ddbe8474d2 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/hook.c b/builtin/hook.c index 83020dfb4f..1fdde80113 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -9,7 +9,8 @@ #include "abspath.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] ") @@ -97,6 +98,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/builtin/receive-pack.c b/builtin/receive-pack.c index 519f6597b0..b5300f77ac 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1453,7 +1453,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/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/hook.c b/hook.c index 2c8252b2c4..d5675df238 100644 --- a/hook.c +++ b/hook.c @@ -118,16 +118,34 @@ static void unsorted_string_list_remove(struct string_list *list, unsorted_string_list_delete_item(list, item - list->items, 0); } +/* + * Cache entry stored as the .util pointer of string_list items inside the + * hook config cache. Carries both the resolved command and the parallel flag. + */ +struct hook_config_cache_entry { + char *command; + unsigned int parallel:1; +}; + /* * 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. + * 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. + * force_stdout_to_stderr: value of hook.forceStdoutToStderr. Defaults to 0. */ struct hook_all_config_cb { struct strmap commands; struct strmap event_hooks; struct string_list disabled_hooks; + 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. */ @@ -143,6 +161,24 @@ 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; + } else if (!strcmp(subkey, "forcestdouttostderr") && value) { + int v = git_parse_maybe_bool(value); + if (v >= 0) + data->force_stdout_to_stderr = v; + } + return 0; + } + if (!value) return config_error_nonbool(key); @@ -191,6 +227,24 @@ 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); + } 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); @@ -205,31 +259,40 @@ static int hook_config_lookup_all(const char *key, const char *value, * 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) +void hook_cache_clear(struct hook_config_cache *cache) { struct hashmap_iter iter; struct strmap_entry *e; - strmap_for_each_entry(cache, &iter, e) { + strmap_for_each_entry(&cache->hooks, &iter, e) { struct string_list *hooks = e->value; - string_list_clear(hooks, 1); /* free util (command) pointers */ + for (size_t i = 0; i < hooks->nr; i++) { + struct hook_config_cache_entry *entry = hooks->items[i].util; + free(entry->command); + free(entry); + } + string_list_clear(hooks, 0); free(hooks); } - strmap_clear(cache, 0); + strmap_clear(&cache->hooks, 0); + strmap_clear(&cache->event_jobs, 1); /* free heap-allocated unsigned int * values */ } /* Populate `cache` with the complete hook configuration */ -static void build_hook_config_map(struct repository *r, struct strmap *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; 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); + strmap_init(&cb_data.event_jobs); - /* 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. */ @@ -241,6 +304,8 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache) for (size_t i = 0; i < hook_names->nr; i++) { const char *hname = hook_names->items[i].string; + struct hook_config_cache_entry *entry; + void *par = strmap_get(&cb_data.parallel_hooks, hname); char *command; /* filter out disabled hooks */ @@ -254,15 +319,22 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache) "'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); + /* util stores a cache entry; owned by the cache. */ + CALLOC_ARRAY(entry, 1); + entry->command = xstrdup(command); + entry->parallel = par ? (int)(uintptr_t)par : 0; + string_list_append(hooks, hname)->util = entry; } - strmap_put(cache, e->key, hooks); + strmap_put(&cache->hooks, e->key, hooks); } + 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 */ string_list_clear(&cb_data.disabled_hooks, 0); strmap_for_each_entry(&cb_data.event_hooks, &iter, e) { string_list_clear(e->value, 0); @@ -272,35 +344,35 @@ static void build_hook_config_map(struct repository *r, struct strmap *cache) } /* - * Return the hook config map for `r`, populating it first if needed. + * Return the hook config cache 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 + * cache; the caller is responsible for freeing it with * hook_cache_clear() + free(). */ -static struct strmap *get_hook_config_cache(struct repository *r) +static struct hook_config_cache *get_hook_config_cache(struct repository *r) { - struct strmap *cache = NULL; + struct hook_config_cache *cache = NULL; 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 + * For in-repo calls, the cache is stored in r->hook_config_cache, + * so repeated invocations don't parse the configs; 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); + CALLOC_ARRAY(r->hook_config_cache, 1); + strmap_init(&r->hook_config_cache->hooks); 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 which gets freed immediately by the caller. */ - cache = xcalloc(1, sizeof(*cache)); - strmap_init(cache); + CALLOC_ARRAY(cache, 1); + strmap_init(&cache->hooks); build_hook_config_map(r, cache); } @@ -312,13 +384,13 @@ static void list_hooks_add_configured(struct repository *r, 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); + struct hook_config_cache *cache = get_hook_config_cache(r); + struct string_list *configured_hooks = strmap_get(&cache->hooks, 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_config_cache_entry *entry = configured_hooks->items[i].util; struct hook *hook = xcalloc(1, sizeof(struct hook)); if (options && options->feed_pipe_cb_data_alloc) @@ -328,7 +400,8 @@ static void list_hooks_add_configured(struct repository *r, hook->kind = HOOK_CONFIGURED; hook->u.configured.friendly_name = xstrdup(friendly_name); - hook->u.configured.command = xstrdup(command); + hook->u.configured.command = xstrdup(entry->command); + hook->parallel = entry->parallel; string_list_append(list, friendly_name)->util = hook; } @@ -464,21 +537,101 @@ 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, + const char *hook_name, + struct string_list *hook_list) +{ + unsigned int jobs; + + /* + * 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; + + /* Pinned serial: FORCE_SERIAL (internal) or explicit -j1 from CLI. */ + if (options->jobs == 1) + return 1; + + /* + * 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) { + jobs = options->jobs; + } 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; + 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; + 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_name, 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, @@ -494,9 +647,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. @@ -508,7 +658,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 e949f5d488..6ea6dbca40 100644 --- a/hook.h +++ b/hook.h @@ -29,6 +29,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. * @@ -62,6 +69,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; @@ -142,7 +151,23 @@ struct run_hooks_opt cb_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, \ @@ -191,11 +216,22 @@ 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); +/** + * Persistent cache for hook configuration, stored on `struct repository`. + * Populated lazily on first hook use and freed by repo_clear(). + */ +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 */ +}; + /** * Frees the hook configuration cache stored in `struct repository`. * Called by repo_clear(). */ -void hook_cache_clear(struct strmap *cache); +void hook_cache_clear(struct hook_config_cache *cache); /** * Returns the path to the hook file, or NULL if the hook is missing 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); diff --git a/repository.c b/repository.c index 0b8f7ec200..aecd1cbb0b 100644 --- a/repository.c +++ b/repository.c @@ -307,6 +307,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; @@ -322,6 +323,7 @@ int repo_init(struct repository *repo, return 0; error: + clear_repository_format(&format); repo_clear(repo); return -1; } diff --git a/repository.h b/repository.h index 078059a6e0..daa7ae7d16 100644 --- a/repository.h +++ b/repository.h @@ -12,6 +12,7 @@ struct lock_file; struct pathspec; struct object_database; struct submodule_cache; +struct hook_config_cache; struct promisor_remote_config; struct remote_state; @@ -170,7 +171,7 @@ struct repository { * Lazily-populated cache mapping hook event names to configured hooks. * NULL until first hook use. */ - struct strmap *hook_config_cache; + struct hook_config_cache *hook_config_cache; /* Configurations related to promisor remotes. */ char *repository_format_partial_clone; @@ -181,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 02e5a71779..cce48a4bcf 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; } @@ -1997,6 +2000,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; @@ -2119,6 +2124,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); @@ -2376,6 +2383,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 @@ -2433,6 +2441,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/setup.h b/setup.h index 80bc6e5f07..b2a4ee3260 100644 --- a/setup.h +++ b/setup.h @@ -170,6 +170,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 b1583e9ef9..d8f60f2ced 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 && @@ -188,10 +239,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 && @@ -209,12 +270,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' ' @@ -462,6 +532,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 \ @@ -553,4 +660,329 @@ 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" && + + # 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_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_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_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 diff --git a/transport.c b/transport.c index 259e9d7b70..02d514e567 100644 --- a/transport.c +++ b/transport.c @@ -1392,11 +1392,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);