Re: bug report: pre-commit & pre-push hook output is redirected differently

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 7, 2022, 16:57 UTC
Message-ID: <xmqq4jzs3lp1.fsf@gitster.g>
In-Reply-To: <220707.86tu7t84zh.gmgdl@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 5 quoted lines

> 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).

"behavior change" meaning a regression of a particular hook, or "behavior difference" between hooks, each of which never changed the behavior?

> I tested this with a local v2.30.0, and the behavior was the same.

I guess you meant the latter. If so, the inconsistency may be unfortunate, but I agree that it is not cut-and-dried that it is a good idea to change pre-push to spew its output to standard error stream.

> 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.

Absolutely.

Show 10 quoted lines

> 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.

We'd probably need an inventory of them all anyway before we can push the hook.c API forward. If the oddball ones are very small minority, it may be worth having a transition period and make backward incompatible change to unify where the output goes. If they are random mixture, on the other hand, the hook.c API may have to gain a bit for the caller to tell where the output of the hook should go.

Thanks.