From: Patrick Steinhardt <ps@pks.im>
Date: Tue, 21 Oct 2025 07:41:02 GMT
Subject: Re: [PATCH v2 02/10] hook: provide stdin via callback
Message-ID: <aPc5Dm_ZSpN0Lakk@pks.im>
In-Reply-To: <20251017141544.1538542-3-adrian.ratiu@collabora.com>

On Fri, Oct 17, 2025 at 05:15:36PM +0300, Adrian Ratiu wrote:
> diff --git a/hook.c b/hook.c
> index b3de1048bf..7537cf0f9e 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -65,11 +65,22 @@ static int pick_next_hook(struct child_process *cp,
>  
>  	cp->no_stdin = 1;
>  	strvec_pushv(&cp->env, hook_cb->options->env.v);
> +
> +	if (hook_cb->options->path_to_stdin && hook_cb->options->feed_pipe)
> +		BUG("options path_to_stdin and feed_pipe are mutually exclusive");
> +
>  	/* reopen the file for stdin; run_command closes it. */
>  	if (hook_cb->options->path_to_stdin) {
>  		cp->no_stdin = 0;
>  		cp->in = xopen(hook_cb->options->path_to_stdin, O_RDONLY);
>  	}
> +
> +	if (hook_cb->options->feed_pipe) {
> +		cp->no_stdin = 0;
> +		/* start_command() will allocate a pipe / stdin fd for us */
> +		cp->in = -1;
> +	}
> +
>  	cp->stdout_to_stderr = 1;
>  	cp->trace2_hook_name = hook_cb->hook_name;
>  	cp->dir = hook_cb->options->dir;
> @@ -148,6 +160,9 @@ int run_hooks_opt(struct repository *r, const char *hook_name,
>  	if (!options)
>  		BUG("a struct run_hooks_opt must be provided to run_hooks");
>  
> +	if (options->path_to_stdin && options->feed_pipe)
> +		BUG("choose only one method to populate hook stdin");

Nit: we may want to reuse the same error message here as we use above.

	BUG("options path_to_stdin and feed_pipe are mutually exclusive");

Patrick