hook: add -j/--jobs option to git hook run

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 <emilyshaffer@google.com>
Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-hook.adoc b/Documentation/git-hook.adoc
index 318c637bd8eba5a57b4019e02025ddb50d17ae10..46ea52db55f268f75b6585448e71796c7b980df5 100644 (file)
--- a/Documentation/git-hook.adoc
+++ b/Documentation/git-hook.adoc
@@ -8,7 +8,8 @@ git-hook - Run git hooks
 SYNOPSIS
 --------
 [verse]
-'git hook' run [--allow-unknown-hook-name] [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]
+'git hook' run [--allow-unknown-hook-name] [--ignore-missing] [--to-stdin=<path>] [(-j|--jobs) <n>]
+       <hook-name> [-- <hook-args>]
 'git hook' list [--allow-unknown-hook-name] [-z] [--show-scope] <hook-name>
 
 DESCRIPTION
@@ -147,6 +148,23 @@ OPTIONS
        mirroring the output style of `git config --show-scope`. 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).
++
+When greater than 1, it overrides the per-hook `hook.<friendly-name>.parallel`
+setting, allowing all hooks for the event to run concurrently, even if they
+are not individually marked as parallel.
++
+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
 --------
 
@@ -169,7 +187,8 @@ running:
 git hook run --allow-unknown-hook-name 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 c0585587e5e4fafeb377d8815dedf6bd2067b55f..bea0668b47593136ed517a81e786590d8f576fa7 100644 (file)
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -8,7 +8,8 @@
 #include "parse-options.h"
 
 #define BUILTIN_HOOK_RUN_USAGE \
-       N_("git hook run [--allow-unknown-hook-name] [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]")
+       N_("git hook run [--allow-unknown-hook-name] [--ignore-missing] [--to-stdin=<path>] [(-j|--jobs) <n>]\n" \
+          "<hook-name> [-- <hook-args>]")
 #define BUILTIN_HOOK_LIST_USAGE \
        N_("git hook list [--allow-unknown-hook-name] [-z] [--show-scope] <hook-name>")
 
@@ -132,6 +133,8 @@ static int run(int argc, const char **argv, const char *prefix,
                         N_("silently ignore missing requested <hook-name>")),
                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 <n> hooks simultaneously")),
                OPT_END(),
        };
        int ret;

diff --git a/hook.c b/hook.c
index 25762b6c8d18f948236781279e44acf2aeedece9..c0b71322cf2ef6a644cf3563d1575fe2c47a743c 100644 (file)
--- a/hook.c
+++ b/hook.c
@@ -568,6 +568,22 @@ static void merge_output_if_parallel(struct run_hooks_opt *options)
                options->stdout_to_stderr = 1;
 }
 
+static void warn_non_parallel_hooks_override(unsigned int jobs,
+                                            struct string_list *hook_list)
+{
+       /* Don't warn for hooks running sequentially. */
+       if (jobs == 1)
+               return;
+
+       for (size_t i = 0; i < hook_list->nr; i++) {
+               struct hook *h = hook_list->items[i].util;
+               if (h->kind == HOOK_CONFIGURED && !h->parallel)
+                       warning(_("hook '%s' is not marked as parallel=true, "
+                                 "running in parallel anyway due to -j%u"),
+                               h->u.configured.friendly_name, jobs);
+       }
+}
+
 /* Determine how many jobs to use for hook execution. */
 static unsigned int get_hook_jobs(struct repository *r,
                                  struct run_hooks_opt *options,
@@ -611,6 +627,7 @@ static unsigned int get_hook_jobs(struct repository *r,
 
 cleanup:
        merge_output_if_parallel(options);
+       warn_non_parallel_hooks_override(options->jobs, hook_list);
        return options->jobs;
 }
 

diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 63fa25bca23c512d9c93217e55ca6aaf912bc782..aa37a5181a0e0e34c078e76f6b906607d70d68b7 100755 (executable)
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -268,10 +268,20 @@ test_expect_success 'git -c core.hooksPath=<PATH> 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 &&
@@ -289,12 +299,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' '
@@ -709,6 +728,108 @@ 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 --allow-unknown-hook-name -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 --allow-unknown-hook-name -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 --allow-unknown-hook-name -j2 test-hook >out 2>err &&
+       echo parallel >expect &&
+       test_cmp expect hook.order
+'
+
+test_expect_success 'git hook run -j2 overrides 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 overrides parallel=false; hooks run in parallel with a warning.
+       git hook run --allow-unknown-hook-name -j2 test-hook >out 2>err &&
+       echo parallel >expect &&
+       test_cmp expect hook.order
+'
+
+test_expect_success 'git hook run -j2 warns for hooks not marked parallel=true' '
+       test_config hook.hook-1.event test-hook &&
+       test_config hook.hook-1.command "true" &&
+       test_config hook.hook-2.event test-hook &&
+       test_config hook.hook-2.command "true" &&
+       # neither hook has parallel=true
+
+       git hook run --allow-unknown-hook-name -j2 test-hook >out 2>err &&
+       grep "hook .hook-1. is not marked as parallel=true" err &&
+       grep "hook .hook-2. is not marked as parallel=true" err
+'
+
 test_expect_success 'hook.jobs=1 config runs hooks in series' '
        test_when_finished "rm -f sentinel.started sentinel.done hook.order" &&