Re: bug report: pre-commit & pre-push hook output is redirected differently
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Jul 7, 2022, 12:40 UTC
- Message-ID
- <220707.86tu7t84zh.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <xmqqa69mgdde.fsf@gitster.g>
On Wed, Jul 06 2022, Junio C Hamano wrote:
Show 28 quoted lines
> Adam Zethraeus <adam.zethraeus@includedhealth.com> writes: > >> What did you do before the bug happened? (Steps to reproduce your issue) >> >> Installed identical pre-commit and pre-push hooks: >> >> ``` >> #!/bin/bash >> >>>&1 echo "stdout" >>>&2 echo "stderr" >> exit 1 >> ``` >> >> What did you expect to happen? (Expected behavior) >> >> `git push` and `git commit` should have the same hook behavior. >> >> What happened instead? (Actual behavior) >> >> The pre-commit hook was run with stdout redirected to stderr but the >> pre-push hook's output was unaltered. > > Without looking into it very much, the output of hooks is an area > with known regression at 2.36, so let me redirect it to those who > are likely to know it ;-) > > Thanks for a report.
I may be missing something, but I think this report has nothing to do with any recent changes or regressions, but is merely noting a behavior change between pre-push and some other hooks that we've had since 1.8.2, or since the "pre-push" hook was added in ec55559f937 (push: Add support for pre-push hooks, 2013-01-13).
I tested this with a local v2.30.0, and the behavior was the same.
This will "fix" it:
diff --git a/transport.c b/transport.c
index 52db7a3cb09..0cc7d05e0da 100644
--- a/transport.c
+++ b/transport.c
@@ -1225,6 +1225,7 @@ static int run_pre_push_hook(struct transport *transport,
strvec_push(&proc.args, transport->url);
proc.in = -1;
+ proc.stdout_to_stderr = 1;
proc.trace2_hook_name = "pre-push";
if (start_command(&proc)) {But whether that's a fix or not depends on whether we think we should make this behavior consistent. I tend to think so, but it would be a behavior change to long-established behavior in pre-push.
It *is* something we need to be careful of when converting the rest of the hooks to the hooks API, i.e. we need tests for how stderr/stdout is handled for each one.
But this being different is just because some hook use the hook.c API (and before that the helper in run-command.c), and others use "struct child_process" or whatever explicitly (such as "pre-push").
Since it's up to each callsite to set up the "proc" (or equivalent) some supply "stdout_to_stderr", some don't.
From some quick grepping it seems the odd ones out are pre-push and proc-receive, but I only skimmed a "git grep" to find the second one, and may have missed others.