From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Glen Choo <chooglen@google.com>,
git@vger.kernel.org,
Adam Zethraeus <adam.zethraeus@includedhealth.com>
Subject: Re: bug report: pre-commit & pre-push hook output is redirected differently
Date: Thu, 07 Jul 2022 09:57:14 -0700 [thread overview]
Message-ID: <xmqq4jzs3lp1.fsf@gitster.g> (raw)
In-Reply-To: <220707.86tu7t84zh.gmgdl@evledraar.gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Thu, 07 Jul 2022 14:40:49 +0200")
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> 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.
> 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.
next prev parent reply other threads:[~2022-07-07 16:57 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-06 20:40 bug report: pre-commit & pre-push hook output is redirected differently Adam Zethraeus
2022-07-06 21:06 ` Junio C Hamano
2022-07-07 12:40 ` Ævar Arnfjörð Bjarmason
2022-07-07 16:57 ` Junio C Hamano [this message]
2022-07-07 20:55 ` Emily Shaffer
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=xmqq4jzs3lp1.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=adam.zethraeus@includedhealth.com \
--cc=avarab@gmail.com \
--cc=chooglen@google.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).