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>

diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc
index 21800db648dca5c02f522fa29ffc790dc02d0508..94c7a9808e29eff98210be69686c3ef3a77d0c8c 100644 (file)
--- a/Documentation/config/hook.adoc
+++ b/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.

diff --git a/hook.c b/hook.c
index 85c0de5e47b42611666de55ae49a6fd926ed471b..25762b6c8d18f948236781279e44acf2aeedece9 100644 (file)
--- a/hook.c
+++ b/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;
 }
 

diff --git a/hook.h b/hook.h
index ba7056f8723b73a7badc97da152242f430f771d2..01db4226a60306583cdb9aba831748547ebc55c2 100644 (file)
--- a/hook.h
+++ b/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;
 

diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index a3011a01ca2908b592eb8d8649c207cf2b4d92db..4a978aff5e0c1e364d54c4462c3417198d8d3758 100755 (executable)
--- a/t/t1800-hook.sh
+++ b/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

diff --git a/transport.c b/transport.c
index e53936d87b641fbb3f3fe81bdf8a44165970f5f3..9406ec4f2d682a626633938e4654cff30ec1ba44 100644 (file)
--- a/transport.c
+++ b/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;