diff mbox series

[v2,7/8] hook API: don't redundantly re-set "no_stdin" and "stdout_to_stderr"

Message ID patch-v2-7.8-bf7d871565f-20220518T195858Z-avarab@gmail.com (mailing list archive)
State Superseded
Headers show
Series hook API: connect hooks to the TTY again, fixes a v2.36.0 regression | expand

Commit Message

Ævar Arnfjörð Bjarmason May 18, 2022, 8:05 p.m. UTC
Amend code added in 96e7225b310 (hook: add 'run' subcommand,
2021-12-22) to 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

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 mbox series


diff --git a/hook.c b/hook.c
index 9aefccfc34a..dc498ef5c39 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;