public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Adrian Ratiu <adrian.ratiu@collabora•com>
To: git@vger•kernel.org
Cc: Jeff King <peff@peff•net>,
	Emily Shaffer <emilyshaffer@google•com>,
	Junio C Hamano <gitster@pobox•com>,
	Patrick Steinhardt <ps@pks•im>,
	Josh Steadmon <steadmon@google•com>,
	Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail•com>,
	"brian m . carlson" <sandals@crustytoothpaste•net>,
	Adrian Ratiu <adrian.ratiu@collabora•com>
Subject: [PATCH v2 09/10] hook: introduce extensions.hookStdoutToStderr
Date: Sun, 22 Feb 2026 02:29:03 +0200	[thread overview]
Message-ID: <20260222002904.1879356-10-adrian.ratiu@collabora.com> (raw)
In-Reply-To: <20260222002904.1879356-1-adrian.ratiu@collabora.com>

All hooks already redirect stdout to stderr with the exception of
pre-push which has a known user who depends on the separate stdout
versus stderr outputs (the git-lfs project).

The pre-push behavior was a surprise which we found out about after
causing a regression for git-lfs. Notably, it might not be the only
exception (it's the one we know about). There might be more.

This presents a challenge because stdout_to_stderr is required for
hook parallelization, so run-command can buffer and de-interleave
the hook outputs using ungroup=0, when hook.jobs > 1.

Introduce an extension to enforce consistency: all hooks merge stdout
into stderr and can be safely parallelized. This provides a clean
separation and avoids breaking existing stdout vs stderr behavior.

When this extension is disabled, the `hook.jobs` config has no
effect for pre-push, to prevent garbled (interleaved) parallel
output, so it runs sequentially like before.

Alternatives I've considered to this extension include:
1. Allowing pre-push to run in parallel with interleaved output.
2. Always running pre-push sequentially (no parallel jobs for it).
3. Making users (only git-lfs? maybe more?) fix their hooks to read
   stderr not stdout.

Out of all these alternatives, I think this extension is the most
reasonable compromise, to not break existing users, allow pre-push
parallel jobs for those who need it (with correct outputs) and also
future-proofing in case there are any more exceptions to be added.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora•com>
---
 Documentation/config/extensions.adoc | 12 +++++++++
 Documentation/config/hook.adoc       |  3 +++
 repository.c                         |  1 +
 repository.h                         |  1 +
 setup.c                              |  7 ++++++
 setup.h                              |  1 +
 t/t1800-hook.sh                      | 37 ++++++++++++++++++++++++++++
 transport.c                          |  7 ++----
 8 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/Documentation/config/extensions.adoc b/Documentation/config/extensions.adoc
index 2aef3315b1..342734668d 100644
--- a/Documentation/config/extensions.adoc
+++ b/Documentation/config/extensions.adoc
@@ -102,6 +102,18 @@ The extension can be enabled automatically for new repositories by setting
 `init.defaultSubmodulePathConfig` to `true`, for example by running
 `git config --global init.defaultSubmodulePathConfig true`.
 
+hookStdoutToStderr:::
+	If enabled, the stdout of all hooks is redirected to stderr. This
+	enforces consistency, since by default most hooks already behave
+	this way, with pre-push being the only known exception.
++
+This is useful for parallel hook execution (see the `hook.jobs` config in
+linkgit:git-config[1]), as it allows the output of multiple hooks running
+in parallel to be grouped (de-interleaved) correctly.
++
+Defaults to disabled. When disabled, `hook.jobs` has no effect for pre-push
+hooks, which will always be run sequentially.
+
 worktreeConfig:::
 	If enabled, then worktrees will load config settings from the
 	`$GIT_DIR/config.worktree` file in addition to the
diff --git a/Documentation/config/hook.adoc b/Documentation/config/hook.adoc
index 6ad23ac71d..aa8a949a36 100644
--- a/Documentation/config/hook.adoc
+++ b/Documentation/config/hook.adoc
@@ -62,3 +62,6 @@ hook.jobs::
 +
 This setting has no effect unless all configured hooks for the event have
 `hook.<name>.parallel` set to `true`.
++
+This has no effect for hooks requiring separate output streams (like `pre-push`)
+unless `extensions.hookStdoutToStderr` is enabled.
diff --git a/repository.c b/repository.c
index 78aa2d31d0..d7f247b676 100644
--- a/repository.c
+++ b/repository.c
@@ -283,6 +283,7 @@ int repo_init(struct repository *repo,
 	repo->repository_format_relative_worktrees = format.relative_worktrees;
 	repo->repository_format_precious_objects = format.precious_objects;
 	repo->repository_format_submodule_path_cfg = format.submodule_path_cfg;
+	repo->repository_format_hook_stdout_to_stderr = format.hook_stdout_to_stderr;
 
 	/* take ownership of format.partial_clone */
 	repo->repository_format_partial_clone = format.partial_clone;
diff --git a/repository.h b/repository.h
index 2105768b8c..a4d7f129ad 100644
--- a/repository.h
+++ b/repository.h
@@ -173,6 +173,7 @@ struct repository {
 	int repository_format_relative_worktrees;
 	int repository_format_precious_objects;
 	int repository_format_submodule_path_cfg;
+	int repository_format_hook_stdout_to_stderr;
 
 	/* Indicate if a repository has a different 'commondir' from 'gitdir' */
 	unsigned different_commondir:1;
diff --git a/setup.c b/setup.c
index c8336eb20e..75f115faba 100644
--- a/setup.c
+++ b/setup.c
@@ -688,6 +688,9 @@ static enum extension_result handle_extension(const char *var,
 	} else if (!strcmp(ext, "submodulepathconfig")) {
 		data->submodule_path_cfg = git_config_bool(var, value);
 		return EXTENSION_OK;
+	} else if (!strcmp(ext, "hookstdouttostderr")) {
+		data->hook_stdout_to_stderr = git_config_bool(var, value);
+		return EXTENSION_OK;
 	}
 	return EXTENSION_UNKNOWN;
 }
@@ -1951,6 +1954,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
 				repo_fmt.relative_worktrees;
 			the_repository->repository_format_submodule_path_cfg =
 				repo_fmt.submodule_path_cfg;
+			the_repository->repository_format_hook_stdout_to_stderr =
+				repo_fmt.hook_stdout_to_stderr;
 			/* take ownership of repo_fmt.partial_clone */
 			the_repository->repository_format_partial_clone =
 				repo_fmt.partial_clone;
@@ -2053,6 +2058,8 @@ void check_repository_format(struct repository_format *fmt)
 		fmt->submodule_path_cfg;
 	the_repository->repository_format_relative_worktrees =
 		fmt->relative_worktrees;
+	the_repository->repository_format_hook_stdout_to_stderr =
+		fmt->hook_stdout_to_stderr;
 	the_repository->repository_format_partial_clone =
 		xstrdup_or_null(fmt->partial_clone);
 	clear_repository_format(&repo_fmt);
diff --git a/setup.h b/setup.h
index 0738dec244..9de6e9b59c 100644
--- a/setup.h
+++ b/setup.h
@@ -168,6 +168,7 @@ struct repository_format {
 	int worktree_config;
 	int relative_worktrees;
 	int submodule_path_cfg;
+	int hook_stdout_to_stderr;
 	int is_bare;
 	int hash_algo;
 	int compat_hash_algo;
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index f5c0655adb..d1a0d0a3d4 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -532,6 +532,43 @@ test_expect_success 'client hooks: pre-push expects separate stdout and stderr'
 	check_stdout_separate_from_stderr pre-push
 '
 
+test_expect_success 'client hooks: extension makes pre-push merge stdout to stderr' '
+	test_when_finished "rm -rf remote2 stdout.actual stderr.actual" &&
+	git init --bare remote2 &&
+	git remote add origin2 remote2 &&
+	test_commit B &&
+	git config set core.repositoryformatversion 1 &&
+	test_config extensions.hookStdoutToStderr true &&
+	setup_hooks pre-push &&
+	git push origin2 HEAD:main >stdout.actual 2>stderr.actual &&
+	check_stdout_merged_to_stderr pre-push
+'
+
+test_expect_success 'client hooks: pre-push defaults to serial execution' '
+	test_when_finished "rm -rf remote-serial repo-serial" &&
+	git init --bare remote-serial &&
+	git init repo-serial &&
+	git -C repo-serial remote add origin ../remote-serial &&
+	test_commit -C repo-serial A &&
+
+	# Setup 2 pre-push hooks; no parallel=true so they must run serially.
+	# Use sentinel/detector pattern: hook-1 (sentinel, configured) runs first
+	# because configured hooks precede traditional hooks in list order; hook-2
+	# (detector) runs second and checks whether hook-1 has finished.
+	git -C repo-serial config hook.hook-1.event pre-push &&
+	git -C repo-serial config hook.hook-1.command \
+	    "touch sentinel.started; sleep 2; touch sentinel.done" &&
+	git -C repo-serial config hook.hook-2.event pre-push &&
+	git -C repo-serial config hook.hook-2.command \
+	    "$(sentinel_detector sentinel hook.order)" &&
+
+	git -C repo-serial config hook.jobs 2 &&
+
+	git -C repo-serial push origin HEAD >out 2>err &&
+	echo serial >expect &&
+	test_cmp expect repo-serial/hook.order
+'
+
 test_expect_success 'client hooks: commit hooks expect stdout redirected to stderr' '
 	hooks="pre-commit prepare-commit-msg \
 		commit-msg post-commit \
diff --git a/transport.c b/transport.c
index 1581aa0886..97a9c89e16 100644
--- a/transport.c
+++ b/transport.c
@@ -1388,11 +1388,8 @@ static int run_pre_push_hook(struct transport *transport,
 	opt.feed_pipe_cb_data_alloc = pre_push_hook_data_alloc;
 	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.
-	 */
-	opt.stdout_to_stderr = 0;
+	/* merge stdout to stderr only when extensions.hookStdoutToStderr is enabled */
+	opt.stdout_to_stderr = the_repository->repository_format_hook_stdout_to_stderr;
 
 	ret = run_hooks_opt(the_repository, "pre-push", &opt);
 
-- 
2.52.0.732.gb351b5166d.dirty


  parent reply	other threads:[~2026-02-22  0:29 UTC|newest]

Thread overview: 113+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-04 17:33 [PATCH 0/4] Run hooks in parallel Adrian Ratiu
2026-02-04 17:33 ` [PATCH 1/4] config: add a repo_config_get_uint() helper Adrian Ratiu
2026-02-04 17:33 ` [PATCH 2/4] hook: allow parallel hook execution Adrian Ratiu
2026-02-11 12:41   ` Patrick Steinhardt
2026-02-12 12:25     ` Adrian Ratiu
2026-02-04 17:33 ` [PATCH 3/4] hook: introduce extensions.hookStdoutToStderr Adrian Ratiu
2026-02-04 17:33 ` [PATCH 4/4] hook: allow runtime enabling extensions.hookStdoutToStderr Adrian Ratiu
2026-02-12 10:43 ` [PATCH 0/4] Run hooks in parallel Phillip Wood
2026-02-12 14:24   ` Adrian Ratiu
2026-02-13 14:39     ` Phillip Wood
2026-02-13 17:21       ` Adrian Ratiu
2026-02-22  0:28 ` [PATCH v2 00/10] " Adrian Ratiu
2026-02-22  0:28   ` [PATCH v2 01/10] repository: fix repo_init() memleak due to missing _clear() Adrian Ratiu
2026-02-22  0:28   ` [PATCH v2 02/10] config: add a repo_config_get_uint() helper Adrian Ratiu
2026-02-22  0:28   ` [PATCH v2 03/10] hook: refactor hook_config_cache from strmap to named struct Adrian Ratiu
2026-02-22  0:28   ` [PATCH v2 04/10] hook: parse the hook.jobs config Adrian Ratiu
2026-02-22  0:28   ` [PATCH v2 05/10] hook: allow parallel hook execution Adrian Ratiu
2026-02-22  0:29   ` [PATCH v2 06/10] hook: mark non-parallelizable hooks Adrian Ratiu
2026-02-22  0:29   ` [PATCH v2 07/10] hook: add -j/--jobs option to git hook run Adrian Ratiu
2026-02-22  0:29   ` [PATCH v2 08/10] hook: add per-event jobs config Adrian Ratiu
2026-02-22  0:29   ` Adrian Ratiu [this message]
2026-02-22  0:29   ` [PATCH v2 10/10] hook: allow runtime enabling extensions.hookStdoutToStderr Adrian Ratiu
2026-03-09 13:37 ` [PATCH v3 0/9] Run hooks in parallel Adrian Ratiu
2026-03-09 13:37   ` [PATCH v3 1/9] repository: fix repo_init() memleak due to missing _clear() Adrian Ratiu
2026-03-15  4:55     ` Junio C Hamano
2026-03-15  5:05     ` Junio C Hamano
2026-03-09 13:37   ` [PATCH v3 2/9] config: add a repo_config_get_uint() helper Adrian Ratiu
2026-03-09 13:37   ` [PATCH v3 3/9] hook: parse the hook.jobs config Adrian Ratiu
2026-03-15 16:13     ` Junio C Hamano
2026-03-09 13:37   ` [PATCH v3 4/9] hook: allow parallel hook execution Adrian Ratiu
2026-03-15 20:46     ` Junio C Hamano
2026-03-18 18:02       ` Adrian Ratiu
2026-03-09 13:37   ` [PATCH v3 5/9] hook: mark non-parallelizable hooks Adrian Ratiu
2026-03-15 20:56     ` Junio C Hamano
2026-03-18 18:40       ` Adrian Ratiu
2026-03-09 13:37   ` [PATCH v3 6/9] hook: add -j/--jobs option to git hook run Adrian Ratiu
2026-03-15 21:00     ` Junio C Hamano
2026-03-18 19:00       ` Adrian Ratiu
2026-03-09 13:37   ` [PATCH v3 7/9] hook: add per-event jobs config Adrian Ratiu
2026-03-16 18:40     ` Junio C Hamano
2026-03-18 19:21       ` Adrian Ratiu
2026-03-09 13:37   ` [PATCH v3 8/9] hook: introduce extensions.hookStdoutToStderr Adrian Ratiu
2026-03-16 18:44     ` Junio C Hamano
2026-03-18 19:50       ` Adrian Ratiu
2026-03-09 13:37   ` [PATCH v3 9/9] hook: allow runtime enabling extensions.hookStdoutToStderr Adrian Ratiu
2026-03-20 13:53 ` [PATCH v4 0/9] Run hooks in parallel Adrian Ratiu
2026-03-20 13:53   ` [PATCH v4 1/9] config: add a repo_config_get_uint() helper Adrian Ratiu
2026-03-20 13:53   ` [PATCH v4 2/9] hook: parse the hook.jobs config Adrian Ratiu
2026-03-24  9:07     ` Patrick Steinhardt
2026-03-24 18:59       ` Adrian Ratiu
2026-03-20 13:53   ` [PATCH v4 3/9] hook: allow parallel hook execution Adrian Ratiu
2026-03-24  9:07     ` Patrick Steinhardt
2026-03-20 13:53   ` [PATCH v4 4/9] hook: allow pre-push parallel execution Adrian Ratiu
2026-03-20 13:53   ` [PATCH v4 5/9] hook: mark non-parallelizable hooks Adrian Ratiu
2026-03-20 13:53   ` [PATCH v4 6/9] hook: add -j/--jobs option to git hook run Adrian Ratiu
2026-03-24  9:07     ` Patrick Steinhardt
2026-03-20 13:53   ` [PATCH v4 7/9] hook: add per-event jobs config Adrian Ratiu
2026-03-24  9:08     ` Patrick Steinhardt
2026-03-20 13:53   ` [PATCH v4 8/9] hook: warn when hook.<friendly-name>.jobs is set Adrian Ratiu
2026-03-24  9:08     ` Patrick Steinhardt
2026-03-20 13:53   ` [PATCH v4 9/9] hook: add hook.<event>.enabled switch Adrian Ratiu
2026-03-24  9:08     ` Patrick Steinhardt
2026-03-25 18:43       ` Adrian Ratiu
2026-03-20 17:24   ` [PATCH v4 0/9] Run hooks in parallel Junio C Hamano
2026-03-23 15:07     ` Adrian Ratiu
2026-03-24  9:07       ` Patrick Steinhardt
2026-03-26 10:18 ` [PATCH v5 00/12] " Adrian Ratiu
2026-03-26 10:18   ` [PATCH v5 01/12] repository: fix repo_init() memleak due to missing _clear() Adrian Ratiu
2026-03-26 10:18   ` [PATCH v5 02/12] config: add a repo_config_get_uint() helper Adrian Ratiu
2026-03-26 10:18   ` [PATCH v5 03/12] hook: parse the hook.jobs config Adrian Ratiu
2026-03-26 10:18   ` [PATCH v5 04/12] hook: allow parallel hook execution Adrian Ratiu
2026-03-26 10:18   ` [PATCH v5 05/12] hook: allow pre-push parallel execution Adrian Ratiu
2026-03-26 10:18   ` [PATCH v5 06/12] hook: mark non-parallelizable hooks Adrian Ratiu
2026-03-26 10:18   ` [PATCH v5 07/12] hook: add -j/--jobs option to git hook run Adrian Ratiu
2026-03-27 14:46     ` Patrick Steinhardt
2026-03-26 10:18   ` [PATCH v5 08/12] hook: add per-event jobs config Adrian Ratiu
2026-03-26 10:18   ` [PATCH v5 09/12] hook: warn when hook.<friendly-name>.jobs is set Adrian Ratiu
2026-03-27 14:46     ` Patrick Steinhardt
2026-03-26 10:18   ` [PATCH v5 10/12] hook: move is_known_hook() to hook.c for wider use Adrian Ratiu
2026-03-27 14:46     ` Patrick Steinhardt
2026-03-27 15:59       ` Adrian Ratiu
2026-03-26 10:18   ` [PATCH v5 11/12] hook: add hook.<event>.enabled switch Adrian Ratiu
2026-03-26 10:18   ` [PATCH v5 12/12] hook: allow hook.jobs=-1 to use all available CPU cores Adrian Ratiu
2026-04-04  8:29 ` [PATCH v6 00/12] Run hooks in parallel Adrian Ratiu
2026-04-04  8:29   ` [PATCH v6 01/12] repository: fix repo_init() memleak due to missing _clear() Adrian Ratiu
2026-04-04  8:29   ` [PATCH v6 02/12] config: add a repo_config_get_uint() helper Adrian Ratiu
2026-04-04  8:29   ` [PATCH v6 03/12] hook: parse the hook.jobs config Adrian Ratiu
2026-04-04  8:29   ` [PATCH v6 04/12] hook: allow parallel hook execution Adrian Ratiu
2026-04-04  8:29   ` [PATCH v6 05/12] hook: allow pre-push parallel execution Adrian Ratiu
2026-04-04  8:29   ` [PATCH v6 06/12] hook: mark non-parallelizable hooks Adrian Ratiu
2026-04-04  8:29   ` [PATCH v6 07/12] hook: add -j/--jobs option to git hook run Adrian Ratiu
2026-04-04  8:29   ` [PATCH v6 08/12] hook: add per-event jobs config Adrian Ratiu
2026-04-04  8:29   ` [PATCH v6 09/12] hook: warn when hook.<friendly-name>.jobs is set Adrian Ratiu
2026-04-04  8:29   ` [PATCH v6 10/12] hook: move is_known_hook() to hook.c for wider use Adrian Ratiu
2026-04-04  8:29   ` [PATCH v6 11/12] hook: add hook.<event>.enabled switch Adrian Ratiu
2026-04-04  8:29   ` [PATCH v6 12/12] hook: allow hook.jobs=-1 to use all available CPU cores Adrian Ratiu
2026-04-06 16:24   ` [PATCH v6 00/12] Run hooks in parallel Junio C Hamano
2026-04-08 10:17   ` Patrick Steinhardt
2026-04-08 16:57     ` Junio C Hamano
2026-04-10  9:05 ` [PATCH v7 00/13] " Adrian Ratiu
2026-04-10  9:05   ` [PATCH v7 01/13] repository: fix repo_init() memleak due to missing _clear() Adrian Ratiu
2026-04-10  9:05   ` [PATCH v7 02/13] config: add a repo_config_get_uint() helper Adrian Ratiu
2026-04-10  9:05   ` [PATCH v7 03/13] hook: parse the hook.jobs config Adrian Ratiu
2026-04-10  9:05   ` [PATCH v7 04/13] hook: allow parallel hook execution Adrian Ratiu
2026-04-10  9:06   ` [PATCH v7 05/13] hook: allow pre-push parallel execution Adrian Ratiu
2026-04-10  9:06   ` [PATCH v7 06/13] hook: mark non-parallelizable hooks Adrian Ratiu
2026-04-10  9:06   ` [PATCH v7 07/13] hook: add -j/--jobs option to git hook run Adrian Ratiu
2026-04-10  9:06   ` [PATCH v7 08/13] hook: add per-event jobs config Adrian Ratiu
2026-04-10  9:06   ` [PATCH v7 09/13] hook: warn when hook.<friendly-name>.jobs is set Adrian Ratiu
2026-04-10  9:06   ` [PATCH v7 10/13] hook: move is_known_hook() to hook.c for wider use Adrian Ratiu
2026-04-10  9:06   ` [PATCH v7 11/13] hook: add hook.<event>.enabled switch Adrian Ratiu
2026-04-10  9:06   ` [PATCH v7 12/13] hook: allow hook.jobs=-1 to use all available CPU cores Adrian Ratiu
2026-04-10  9:06   ` [PATCH v7 13/13] t1800: test SIGPIPE with parallel hooks Adrian Ratiu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260222002904.1879356-10-adrian.ratiu@collabora.com \
    --to=adrian.ratiu@collabora$(echo .)com \
    --cc=emilyshaffer@google$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=kristofferhaugsbakk@fastmail$(echo .)com \
    --cc=peff@peff$(echo .)net \
    --cc=ps@pks$(echo .)im \
    --cc=sandals@crustytoothpaste$(echo .)net \
    --cc=steadmon@google$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox