git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Anthony Sottile" <asottile@umich.edu>,
	"Emily Shaffer" <emilyshaffer@google.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 5/6] hook API: don't redundantly re-set "no_stdin" and "stdout_to_stderr"
Date: Thu, 21 Apr 2022 14:25:30 +0200	[thread overview]
Message-ID: <patch-5.6-98c26c9917b-20220421T122108Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-0.6-00000000000-20220421T122108Z-avarab@gmail.com>

Amend code added in 96e7225b310 (hook: add 'run' subcommand,
2021-12-22) top stop setting these two flags. We use the
run_process_parallel() API added in c553c72eed6 (run-command: add an
asynchronous parallel child processor, 2015-12-15), which always sets
these in pp_start_one() (in addition to setting .err = -1).

Note that an assert() to check that these values are already what
we're setting them to here would fail. That's because in
pp_start_one() we'll set these after calling this "get_next_task"
callback (which we call pick_next_hook()). But the only case where we
weren't setting these just after returning from this function was if
we took the "return 0" path here, in which case we wouldn't have set
these.

So while this code wasn't wrong, it was entirely redundant. The
run_process_parallel() also can't work with a generic "struct
child_process", it needs one that's behaving in a way that it expects
when it comes to stderr/stdout. So we shouldn't be changing these
values, or in this case keeping around code that gives the impression
that doing in the general case is OK.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 hook.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hook.c b/hook.c
index eadb2d58a7b..68ee4030551 100644
--- a/hook.c
+++ b/hook.c
@@ -53,9 +53,7 @@ static int pick_next_hook(struct child_process *cp,
 	if (!hook_path)
 		return 0;
 
-	cp->no_stdin = 1;
 	strvec_pushv(&cp->env_array, hook_cb->options->env.v);
-	cp->stdout_to_stderr = 1;
 	cp->trace2_hook_name = hook_cb->hook_name;
 	cp->dir = hook_cb->options->dir;
 
-- 
2.36.0.893.g80a51c675f6


  parent reply	other threads:[~2022-04-21 12:26 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-19 18:59 git 2.36.0 regression: pre-commit hooks no longer have stdout/stderr as tty Anthony Sottile
2022-04-19 23:37 ` Emily Shaffer
2022-04-19 23:52   ` Anthony Sottile
2022-04-20  9:00   ` Phillip Wood
2022-04-20 12:25     ` Ævar Arnfjörð Bjarmason
2022-04-20 16:22       ` Emily Shaffer
2022-04-20 16:42     ` Junio C Hamano
2022-04-20 17:09       ` Emily Shaffer
2022-04-20 17:25         ` Junio C Hamano
2022-04-20 17:41           ` Emily Shaffer
2022-04-21 12:03             ` Ævar Arnfjörð Bjarmason
2022-04-21 17:24               ` Junio C Hamano
2022-04-21 18:40                 ` Junio C Hamano
2022-04-20  4:23 ` Junio C Hamano
2022-04-21 12:25 ` [PATCH 0/6] hook API: connect hooks to the TTY again, fixes a v2.36.0 regression Ævar Arnfjörð Bjarmason
2022-04-21 12:25   ` [PATCH 1/6] run-command API: replace run_processes_parallel_tr2() with opts struct Ævar Arnfjörð Bjarmason
2022-04-23  4:24     ` Junio C Hamano
2022-04-28 23:16     ` Emily Shaffer
2022-04-29 16:44       ` Junio C Hamano
2022-04-21 12:25   ` [PATCH 2/6] run-command tests: test stdout of run_command_parallel() Ævar Arnfjörð Bjarmason
2022-04-23  4:24     ` Junio C Hamano
2022-04-21 12:25   ` [PATCH 3/6] run-command: add an "ungroup" option to run_process_parallel() Ævar Arnfjörð Bjarmason
2022-04-23  3:54     ` Junio C Hamano
2022-04-28 23:26     ` Emily Shaffer
2022-04-21 12:25   ` [PATCH 4/6] hook tests: fix redirection logic error in 96e7225b310 Ævar Arnfjörð Bjarmason
2022-04-23  3:54     ` Junio C Hamano
2022-04-21 12:25   ` Ævar Arnfjörð Bjarmason [this message]
2022-04-29 22:54     ` [PATCH 5/6] hook API: don't redundantly re-set "no_stdin" and "stdout_to_stderr" Junio C Hamano
2022-04-21 12:25   ` [PATCH 6/6] hook API: fix v2.36.0 regression: hooks should be connected to a TTY Ævar Arnfjörð Bjarmason
2022-04-28 23:31     ` Emily Shaffer
2022-04-29 23:09     ` Junio C Hamano
2022-04-21 17:35   ` [PATCH 0/6] hook API: connect hooks to the TTY again, fixes a v2.36.0 regression Junio C Hamano
2022-04-21 18:50     ` Ævar Arnfjörð Bjarmason
2022-05-18 20:05   ` [PATCH v2 0/8] " Ævar Arnfjörð Bjarmason
2022-05-18 20:05     ` [PATCH v2 1/8] run-command tests: change if/if/... to if/else if/else Ævar Arnfjörð Bjarmason
2022-05-18 20:05     ` [PATCH v2 2/8] run-command API: use "opts" struct for run_processes_parallel{,_tr2}() Ævar Arnfjörð Bjarmason
2022-05-18 21:45       ` Junio C Hamano
2022-05-25 13:18       ` Emily Shaffer
2022-05-18 20:05     ` [PATCH v2 3/8] run-command tests: test stdout of run_command_parallel() Ævar Arnfjörð Bjarmason
2022-05-18 20:05     ` [PATCH v2 4/8] run-command.c: add an initializer for "struct parallel_processes" Ævar Arnfjörð Bjarmason
2022-05-18 20:05     ` [PATCH v2 5/8] run-command: add an "ungroup" option to run_process_parallel() Ævar Arnfjörð Bjarmason
2022-05-18 21:51       ` Junio C Hamano
2022-05-26 17:18       ` Emily Shaffer
2022-05-27 16:08         ` Junio C Hamano
2022-05-18 20:05     ` [PATCH v2 6/8] hook tests: fix redirection logic error in 96e7225b310 Ævar Arnfjörð Bjarmason
2022-05-18 20:05     ` [PATCH v2 7/8] hook API: don't redundantly re-set "no_stdin" and "stdout_to_stderr" Ævar Arnfjörð Bjarmason
2022-05-18 20:05     ` [PATCH v2 8/8] hook API: fix v2.36.0 regression: hooks should be connected to a TTY Ævar Arnfjörð Bjarmason
2022-05-18 21:53       ` Junio C Hamano
2022-05-26 17:23       ` Emily Shaffer
2022-05-26 18:23         ` Ævar Arnfjörð Bjarmason
2022-05-26 18:54           ` Emily Shaffer
2022-05-25 11:30     ` [PATCH v2 0/8] hook API: connect hooks to the TTY again, fixes a v2.36.0 regression Johannes Schindelin
2022-05-25 13:00       ` Ævar Arnfjörð Bjarmason
2022-05-25 16:57       ` Junio C Hamano
2022-05-26  1:10         ` Junio C Hamano
2022-05-26 10:16           ` Ævar Arnfjörð Bjarmason
2022-05-26 16:36             ` Junio C Hamano
2022-05-26 19:13               ` Ævar Arnfjörð Bjarmason
2022-05-26 19:56                 ` Junio C Hamano
2022-05-27  9:14     ` [PATCH v3 0/2] " Ævar Arnfjörð Bjarmason
2022-05-27  9:14       ` [PATCH v3 1/2] run-command: add an "ungroup" option to run_process_parallel() Ævar Arnfjörð Bjarmason
2022-05-27 16:58         ` Junio C Hamano
2022-05-27  9:14       ` [PATCH v3 2/2] hook API: fix v2.36.0 regression: hooks should be connected to a TTY Ævar Arnfjörð Bjarmason
2022-05-27 17:17       ` [PATCH v3 0/2] hook API: connect hooks to the TTY again, fixes a v2.36.0 regression Junio C Hamano
2022-05-31 17:32       ` [PATCH v4 " Ævar Arnfjörð Bjarmason
2022-05-31 17:32         ` [PATCH v4 1/2] run-command: add an "ungroup" option to run_process_parallel() Ævar Arnfjörð Bjarmason
2022-06-01 16:49           ` Johannes Schindelin
2022-06-01 17:09             ` Junio C Hamano
2022-05-31 17:32         ` [PATCH v4 2/2] hook API: fix v2.36.0 regression: hooks should be connected to a TTY Ævar Arnfjörð Bjarmason
2022-06-01 16:50           ` Johannes Schindelin
2022-06-01 16:53         ` [PATCH v4 0/2] hook API: connect hooks to the TTY again, fixes a v2.36.0 regression Johannes Schindelin
2022-06-02 14:07         ` [PATCH v5 " Ævar Arnfjörð Bjarmason
2022-06-02 14:07           ` [PATCH v5 1/2] run-command: add an "ungroup" option to run_process_parallel() Ævar Arnfjörð Bjarmason
2022-06-02 14:07           ` [PATCH v5 2/2] hook API: fix v2.36.0 regression: hooks should be connected to a TTY Ævar Arnfjörð Bjarmason
2022-06-02 20:05           ` [PATCH v5 0/2] hook API: connect hooks to the TTY again, fixes a v2.36.0 regression Junio C Hamano
2022-06-03  8:51           ` Phillip Wood
2022-06-03  9:20             ` Ævar Arnfjörð Bjarmason
2022-06-03 13:21               ` Phillip Wood
2022-06-07  8:49                 ` Ævar Arnfjörð Bjarmason
2022-06-07  8:48           ` [PATCH v6 " Ævar Arnfjörð Bjarmason
2022-06-07  8:48             ` [PATCH v6 1/2] run-command: add an "ungroup" option to run_process_parallel() Ævar Arnfjörð Bjarmason
2022-06-17  0:07               ` Emily Shaffer
2022-06-07  8:48             ` [PATCH v6 2/2] hook API: fix v2.36.0 regression: hooks should be connected to a TTY Ævar Arnfjörð Bjarmason
2022-06-07 17:08               ` Junio C Hamano
2022-06-07 17:02             ` [PATCH v6 0/2] hook API: connect hooks to the TTY again, fixes a v2.36.0 regression Junio C Hamano

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=patch-5.6-98c26c9917b-20220421T122108Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=asottile@umich.edu \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood123@gmail.com \
    /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).