From: Adrian Ratiu Date: Wed, 14 Jan 2026 18:57:31 +0000 (+0200) Subject: hook: make ungroup opt-out instead of opt-in X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d075ed42205843917913ab6d1c9b02657eb01191;p=thirdparty%2Fgit.git 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 Suggested-by: Jeff King Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- diff --git a/builtin/hook.c b/builtin/hook.c index 73e7b8c2e8..7afec380d2 100644 --- 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 is * mandatory. diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index ef1f77be8c..2d1a94f3a9 100644 --- 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 efd0c02683..28bb5ce029 100644 --- 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 47feff9cd7..bbf7f0df21 100644 --- 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 2488db7133..b2b4db2b3d 100644 --- 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 0e4f93fb31..0555a1fd42 100755 --- 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