hook: make ungroup opt-out instead of opt-in

In 857f047e40 (hook: allow overriding the ungroup option, 2025-12-26),
I accidentally made the ungroup option opt-in instead of opt-out and
despite my best efforts to set it for all API users, I missed a case
which requires it to be set: the pre-push hook which regressed.

The only thing I needed in that commit was a way to change the default,
to convert the remaining receive-pack hooks which require ungroup == 0
for sideband output, so it doesn't matter if it's on or off by default.

Bring back the original behavior by setting it for all hooks in the
struct run_hooks_opt initializer, which nicely allows changing the
default value only where needed, in receive-pack.c.

While at it add a few hook tests which exercise receive-pack sideband
output since they are the only ungroup=0 exceptions and there are no
other tests exercising this functionality.

Fixes: 857f047e40f7 ("hook: allow overriding the ungroup option")
Reported-by: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/hook.c b/builtin/hook.c
index 73e7b8c2e878eb134006cd34fbdb15053b465eb3..7afec380d2e579d0b704434144aaf21bdfcb030c 100644 (file)
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -43,12 +43,6 @@ static int run(int argc, const char **argv, const char *prefix,
        if (!argc)
                goto usage;
 
-       /*
-        * All current "hook run" use-cases require ungrouped child output.
-        * If this changes, a hook run argument can be added to toggle it.
-        */
-       opt.ungroup = 1;
-
        /*
         * Having a -- for "run" when providing <hook-args> is
         * mandatory.

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index ef1f77be8c9db68b92ea504f0831d4451fff3f5b..2d1a94f3a97690f8c4010423433e141fed3a5161 100644 (file)
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -905,8 +905,10 @@ static int run_receive_hook(struct command *commands,
        prepare_push_cert_sha1(&opt);
 
        /* set up sideband printer */
-       if (use_sideband)
+       if (use_sideband) {
                opt.consume_output = hook_output_to_sideband;
+               opt.ungroup = 0; /* mandatory for sideband output */
+       }
 
        /* set up stdin callback */
        feed_state.cmd = commands;
@@ -933,8 +935,10 @@ static int run_update_hook(struct command *cmd)
                     oid_to_hex(&cmd->new_oid),
                     NULL);
 
-       if (use_sideband)
+       if (use_sideband) {
                opt.consume_output = hook_output_to_sideband;
+               opt.ungroup = 0; /* mandatory for sideband output */
+       }
 
        return run_hooks_opt(the_repository, "update", &opt);
 }
@@ -1623,8 +1627,10 @@ static void run_update_post_hook(struct command *commands)
        if (!opt.args.nr)
                return;
 
-       if (use_sideband)
+       if (use_sideband) {
                opt.consume_output = hook_output_to_sideband;
+               opt.ungroup = 0; /* mandatory for sideband output */
+       }
 
        run_hooks_opt(the_repository, "post-update", &opt);
 }

diff --git a/commit.c b/commit.c
index efd0c026831e6b41ea428151ed7a1a6a3c8223e3..28bb5ce029f3c521f0e2cc670a9667989a84f580 100644 (file)
--- a/commit.c
+++ b/commit.c
@@ -1978,9 +1978,6 @@ int run_commit_hook(int editor_is_used, const char *index_file,
                strvec_push(&opt.args, arg);
        va_end(args);
 
-       /* All commit hook use-cases require ungrouping child output. */
-       opt.ungroup = 1;
-
        opt.invoked_hook = invoked_hook;
        return run_hooks_opt(the_repository, name, &opt);
 }

diff --git a/hook.c b/hook.c
index 47feff9cd72bb43efc9626eb79084ec4266b8b79..bbf7f0df218d4a97516ed5009eea03087138e920 100644 (file)
--- a/hook.c
+++ b/hook.c
@@ -200,9 +200,6 @@ int run_hooks(struct repository *r, const char *hook_name)
 {
        struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
 
-       /* All use-cases of this API require ungrouping. */
-       opt.ungroup = 1;
-
        return run_hooks_opt(r, hook_name, &opt);
 }
 

diff --git a/hook.h b/hook.h
index 2488db71339d834c7e57a52188118a5f665fafd2..b2b4db2b3d5627e6b16d64f8efecbd24e6175218 100644 (file)
--- a/hook.h
+++ b/hook.h
@@ -99,6 +99,7 @@ struct run_hooks_opt
        .env = STRVEC_INIT, \
        .args = STRVEC_INIT, \
        .stdout_to_stderr = 1, \
+       .ungroup = 1, \
 }
 
 struct hook_cb_data {

diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 0e4f93fb3147cade6af23410a091b2af7fa3a60f..0555a1fd42e39a6721104c1694be8135c5b3fc41 100755 (executable)
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -311,4 +311,53 @@ test_expect_success 'server push-to-checkout hook expects stdout redirected to s
        check_stdout_merged_to_stderr push-to-checkout
 '
 
+test_expect_success 'receive-pack hooks sideband output works' '
+       git init --bare target-sideband.git &&
+       test_commit sideband-a &&
+       git remote add origin-sideband ./target-sideband.git &&
+
+       # pre-receive hook
+       test_hook -C target-sideband.git pre-receive <<-\EOF &&
+       echo "stdout pre-receive"
+       echo "stderr pre-receive" >&2
+       EOF
+
+       git push origin-sideband HEAD:refs/heads/pre-receive 2>actual &&
+       test_grep "remote: stdout pre-receive" actual &&
+       test_grep "remote: stderr pre-receive" actual &&
+
+       # update hook
+       test_hook -C target-sideband.git update <<-\EOF &&
+       echo "stdout update"
+       echo "stderr update" >&2
+       EOF
+
+       test_commit sideband-b &&
+       git push origin-sideband HEAD:refs/heads/update 2>actual &&
+       test_grep "remote: stdout update" actual &&
+       test_grep "remote: stderr update" actual &&
+
+       # post-receive hook
+       test_hook -C target-sideband.git post-receive <<-\EOF &&
+       echo >&1 "stdout post-receive"
+       echo >&2 "stderr post-receive"
+       EOF
+
+       test_commit sideband-c &&
+       git push origin-sideband HEAD:refs/heads/post-receive 2>actual &&
+       test_grep "remote: stdout post-receive" actual &&
+       test_grep "remote: stderr post-receive" actual &&
+
+       # post-update hook
+       test_hook -C target-sideband.git post-update <<-\EOF &&
+       echo >&1 "stdout post-update"
+       echo >&2 "stderr post-update"
+       EOF
+
+       test_commit sideband-d &&
+       git push origin-sideband HEAD:refs/heads/post-update 2>actual &&
+       test_grep "remote: stdout post-update" actual &&
+       test_grep "remote: stderr post-update" actual
+'
+
 test_done