git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 2/2] hook: make ungroup opt-out instead of opt-in

From
Adrian Ratiu <adrian.ratiu@collabora.com>
Date
Jan 14, 2026, 18:57 UTC
Message-ID
<20260114185731.2381550-3-adrian.ratiu@collabora.com>
In-Reply-To
<20260114185731.2381550-1-adrian.ratiu@collabora.com>

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>
---
 builtin/hook.c         |  6 ------
 builtin/receive-pack.c | 12 ++++++++---
 commit.c               |  3 ---
 hook.c                 |  3 ---
 hook.h                 |  1 +
 t/t1800-hook.sh        | 49 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 59 insertions(+), 15 deletions(-)
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 <hook-args> 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 ebd9d9e26e..4e7631132a 100644
--- a/hook.c
+++ b/hook.c
@@ -199,9 +199,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

-- 
2.52.0.732.gb351b5166d.dirty
Previous: Adrian RatiuNext: Jeff King
Message 20 of 30 in “hook: make stdout_to_stderr optional”
  1. hook: make stdout_to_stderr optionalAdrian Ratiu, Jan 13, 2026
  2. Patrick SteinhardtJan 13, 2026
  3. Adrian RatiuJan 13, 2026
  4. Junio C HamanoJan 13, 2026
  5. Junio C HamanoJan 13, 2026
  6. Adrian RatiuJan 13, 2026
  7. Junio C HamanoJan 13, 2026
  8. Adrian RatiuJan 13, 2026
  9. Adrian RatiuJan 13, 2026
  10. hook: allow hooks to disable stdout_to_stderrAdrian Ratiu, Jan 13, 2026
  11. Jeff KingJan 14, 2026
  12. Adrian RatiuJan 14, 2026
  13. Adrian RatiuJan 14, 2026
  14. Kristoffer HaugsbakkJan 14, 2026
  15. Jeff KingJan 14, 2026
  16. Jeff KingJan 14, 2026
  17. Adrian RatiuJan 14, 2026
  18. Kristoffer HaugsbakkJan 14, 2026
  19. 0/2 Fix two hook conversion regressionsAdrian Ratiu, Jan 14, 2026
  20. 2/2 hook: make ungroup opt-out instead of opt-inAdrian Ratiu, Jan 14, 2026
  21. Jeff KingJan 14, 2026
  22. Adrian RatiuJan 14, 2026
  23. Kristoffer HaugsbakkJan 18, 2026
  24. 1/2 hook: allow hooks to disable stdout_to_stderrAdrian Ratiu, Jan 14, 2026
  25. Junio C HamanoJan 15, 2026
  26. Adrian RatiuJan 15, 2026
  27. Junio C HamanoJan 15, 2026
  28. Adrian RatiuJan 15, 2026
  29. Junio C HamanoJan 15, 2026
  30. Adrian RatiuJan 15, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.