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 <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Author: 10ne1

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

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`.

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;
 

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 */
 };
 
 /**

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);
 

t/t1800-hook.sh

@@ -989,4 +989,73 @@ test_expect_success 'hook.<event>.jobs still requires hook.<name>.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