On Thu, 25 Sep 2025, Junio C Hamano [off-list ref] wrote:

Adrian Ratiu [off-list ref] writes: 

quoted

@@ -69,6 +69,10 @@ static int pick_next_hook(struct 

child_process *cp, 
 	if (hook_cb->options->path_to_stdin) { cp->no_stdin = 0; 
 cp->in = xopen(hook_cb->options->path_to_stdin, O_RDONLY); 
+	} else 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; 

OK, so when feed_pipe is defined, just like when path_to_stdin 
is specified, we stop saying there is nothing coming from the 
standard input, and intead set cp->in so that the child process 
would read from there.  Unlike path_to_stdin case it is not 
pointing at a file descriptor that is opened for a filesystem 
entity.  ".in = -1" is a standard signal to run-command.[ch] 
machinery that a pipe to that child is to be prepared. 

Yes, that is all correct.

 

quoted

@@ -37,6 +38,24 @@ struct run_hooks_opt 
 	 * Path to file which should be piped to stdin for each 
 hook.  */ const char *path_to_stdin; 
+ +	/** +	 * Callback to ask for more content to pipe to 

each hook stdin.  +	 * +	 * If a hook needs to consume 
large quantities of data (e.g. a list of all refs received in a 
+	 * client push), feeding data via in-memory strings or 
slurping to/from files via path_to_stdin +	 * will not be 
efficient, so this callback allows for piecemeal reading and 
writing.  +	 * +	 * Add initalization context to 
hook.feed_pipe_ctx.  +	 */ +	feed_pipe_fn feed_pipe; + 
void *feed_pipe_ctx; 

The comment for the member is a bit too wide.  More importantly, 
this does not seem to capture the fact that this is completely 
ignored when path_to_stdin is already in effect.  We should at 
least document it if we wanted to leave the behaviour as is, but 
I wonder if we want to detect and flag it as BUG() if both 
feed_pipe and path_to_stdin are not NULL.  There is no inherent 
reason why the data prepared in a file must take precedence over 
data coming over a pipe. 

Very good points. I agree we should flag a BUG() when both are 
provided, because the stdin feed methods are mutually exclusive.
 
This is actually something I thought about while coding/testing v1 
and just forgot to add in.

Will do in v2 and also make the comment less wide.

Many thanks for spotting this!