hook: allow pre-push parallel execution

pre-push is the only hook that keeps stdout and stderr separate (for
backwards compatibility with git-lfs and potentially other users). This
prevents parallelizing it because run-command needs stdout_to_stderr=1
to buffer and de-interleave parallel outputs.

Since we now default to jobs=1, backwards compatibility is maintained
without needing any extension or extra config: when no parallelism is
requested, pre-push behaves exactly as before.

When the user explicitly opts into parallelism via hook.jobs > 1,
hook.<event>.jobs > 1, or -jN, they accept the changed output behavior.

Document this and let get_hook_jobs() set stdout_to_stderr=1 automatically
when jobs > 1, removing the need for any extension infrastructure.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

Documentation/config/hook.adoc

@@ -39,3 +39,7 @@ hook.jobs::
 +
 This setting has no effect unless all configured hooks for the event have
 `hook.<friendly-name>.parallel` set to `true`.
++
+For `pre-push` hooks, which normally keep stdout and stderr separate,
+setting this to a value greater than 1 (or passing `-j`) will merge stdout
+into stderr to allow correct de-interleaving of parallel output.

hook.c

@@ -555,18 +555,24 @@ static void run_hooks_opt_clear(struct run_hooks_opt *options)
 	strvec_clear(&options->args);
 }
 
+/*
+ * When running in parallel, stdout must be merged into stderr so
+ * run-command can buffer and de-interleave outputs correctly. This
+ * applies even to hooks like pre-push that normally keep stdout and
+ * stderr separate: the user has opted into parallelism, so the output
+ * stream behavior changes accordingly.
+ */
+static void merge_output_if_parallel(struct run_hooks_opt *options)
+{
+	if (options->jobs > 1)
+		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,
 				  struct string_list *hook_list)
 {
-	/*
-	 * Hooks needing separate output streams must run sequentially.
-	 * Next commit will allow parallelizing these as well.
-	 */
-	if (!options->stdout_to_stderr)
-		return 1;
-
 	/*
 	 * An explicit job count overrides everything else: this covers both
 	 * FORCE_SERIAL callers (for hooks that must never run in parallel)
@@ -575,7 +581,7 @@ static unsigned int get_hook_jobs(struct repository *r,
 	 * aggressively than the default.
 	 */
 	if (options->jobs)
-		return options->jobs;
+		goto cleanup;
 
 	/*
 	 * Use hook.jobs from the already-parsed config cache (in-repo), or
@@ -603,6 +609,8 @@ static unsigned int get_hook_jobs(struct repository *r,
 		}
 	}
 
+cleanup:
+	merge_output_if_parallel(options);
 	return options->jobs;
 }
 

hook.h

@@ -106,8 +106,10 @@ struct run_hooks_opt {
 	 * Send the hook's stdout to stderr.
 	 *
 	 * This is the default behavior for all hooks except pre-push,
-	 * which has separate stdout and stderr streams for backwards
-	 * compatibility reasons.
+	 * which keeps stdout and stderr separate for backwards compatibility.
+	 * When parallel execution is requested (jobs > 1), get_hook_jobs()
+	 * overrides this to 1 for all hooks so run-command can de-interleave
+	 * their outputs correctly.
 	 */
 	unsigned int stdout_to_stderr:1;
 

t/t1800-hook.sh

@@ -800,4 +800,36 @@ test_expect_success 'one non-parallel hook forces the whole event to run seriall
 	test_cmp expect hook.order
 '
 
+test_expect_success 'client hooks: pre-push parallel execution merges stdout to stderr' '
+	test_when_finished "rm -rf remote-par stdout.actual stderr.actual" &&
+	git init --bare remote-par &&
+	git remote add origin-par remote-par &&
+	test_commit par-commit &&
+	mkdir -p .git/hooks &&
+	setup_hooks pre-push &&
+	test_config hook.jobs 2 &&
+	git push origin-par HEAD:main >stdout.actual 2>stderr.actual &&
+	check_stdout_merged_to_stderr pre-push
+'
+
+test_expect_success 'client hooks: pre-push runs in parallel when hook.jobs > 1' '
+	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 push origin HEAD >out 2>err &&
+	echo parallel >expect &&
+	test_cmp expect repo-parallel/hook.order
+'
+
 test_done

transport.c

@@ -1391,8 +1391,10 @@ static int run_pre_push_hook(struct transport *transport,
 	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.
+	 * pre-push hooks keep stdout and stderr separate by default for
+	 * backwards compatibility. When the user opts into parallel execution
+	 * via hook.jobs > 1 or -j, get_hook_jobs() will set stdout_to_stderr=1
+	 * automatically so run-command can de-interleave the outputs.
 	 */
 	opt.stdout_to_stderr = 0;