Subject: Re: [PATCH v7 15/17] hook: provide stdin by string_list or callback

From: Jonathan Tan <[email-masked]>
Date: Sun, 31 Jan 2021 23:04:48 -0800

> In cases where a hook requires only a small amount of information via
> stdin, it should be simple for users to provide a string_list alone. But
> in more complicated cases where the stdin is too large to hold in
> memory, let's provide a callback the users can populate line after line
> with instead.

[snip]

> diff --git a/hook.h b/hook.h
> index 8a7542610c..0ac83fa7ca 100644
> --- a/hook.h
> +++ b/hook.h
> @@ -2,6 +2,7 @@
>  #include "list.h"
>  #include "strbuf.h"
>  #include "strvec.h"
> +#include "run-command.h"
>  
>  struct hook
>  {
> @@ -14,6 +15,12 @@ struct hook
>  	/* The literal command to run. */
>  	struct strbuf command;
>  	int from_hookdir;
> +
> +	/*
> +	 * Use this to keep state for your feed_pipe_fn if you are using
> +	 * run_hooks_opt.feed_pipe. Otherwise, do not touch it.
> +	 */
> +	void *feed_pipe_cb_data;

When would we need per-hook state? I see in patch 14 that you give each
running process little by little (in pp_buffer_stdin()), perhaps so that
each hook can make progress at roughly the same pace, but I don't think
we can expect all hooks to work the same, so I don't think it's worth
complicating the design for all that.

>  };
>  
>  /*
> @@ -57,12 +64,24 @@ struct run_hooks_opt
>  
>  	/* Path to file which should be piped to stdin for each hook */
>  	const char *path_to_stdin;
> +	/* Pipe each string to stdin, separated by newlines */
> +	struct string_list str_stdin;
> +	/*
> +	 * Callback and state pointer to ask for more content to pipe to stdin.
> +	 * Will be called repeatedly, for each hook. See
> +	 * hook.c:pipe_from_stdin() for an example. Keep per-hook state in
> +	 * hook.feed_pipe_cb_data (per process). Keep initialization context in
> +	 * feed_pipe_ctx (shared by all processes).
> +	 */
> +	feed_pipe_fn feed_pipe;
> +	void *feed_pipe_ctx;

Instead of 3 fields, I think 2 suffice - the function and the data
(called "ctx" here). We can supply a function that treats the data as a
string_list.