Re: [PATCH 02/10] hook: provide stdin via callback
- From
Emily Shaffer <nasamuffin@google.com>
- Date
- Oct 10, 2025, 19:57 UTC
- Message-ID
- <CAJoAoZm6uNtEoo_tdbqjGMSj4OnQuFesxt_iyOTgNHA1LX3iwQ@mail.gmail.com>
- In-Reply-To
- <20250925125352.1728840-3-adrian.ratiu@collabora.com>
On Thu, Sep 25, 2025 at 5:54 AM Adrian Ratiu <adrian.ratiu@collabora.com> wrote:
Show 96 quoted lines
>
> From: Emily Shaffer <emilyshaffer@google.com>
>
> This adds a callback mechanism for feeding stdin to hooks alongside
> the existing path_to_stdin (which slurps a file's content to stdin).
>
> The advantage of this new callback is that it can feed stdin without
> going through the FS layer. This helps when feeding large amount of
> data and uses the run-command parallel stdin callback introduced in
> the preceding commit.
>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
> hook.c | 8 ++++++++
> hook.h | 22 ++++++++++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/hook.c b/hook.c
> index b3de1048bf..54568d5bc0 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -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;
> @@ -140,6 +144,7 @@ int run_hooks_opt(struct repository *r, const char *hook_name,
>
> .get_next_task = pick_next_hook,
> .start_failure = notify_start_failure,
> + .feed_pipe = options->feed_pipe,
> .task_finished = notify_hook_finished,
>
> .data = &cb_data,
> @@ -148,6 +153,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");
> +
> if (options->invoked_hook)
> *options->invoked_hook = 0;
>
> diff --git a/hook.h b/hook.h
> index 11863fa734..8fdbc8c673 100644
> --- a/hook.h
> +++ b/hook.h
> @@ -1,6 +1,7 @@
> #ifndef HOOK_H
> #define HOOK_H
> #include "strvec.h"
> +#include "run-command.h"
>
> struct repository;
>
> @@ -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;
> +
> + /**
> + * Use this to keep internal state for your feed_pipe_fn callback.
> + * Only useful if you are using run_hooks_opt.feed_pipe. Otherwise, ignore it.
> + */
> + void *feed_pipe_cb_data;
> };
>
> #define RUN_HOOKS_OPT_INIT { \
> @@ -44,6 +63,9 @@ struct run_hooks_opt
> .args = STRVEC_INIT, \
> }
>
> +/**
> + * Callback data provided to feed_pipe_fn.
> + */It looks like this comment was maybe a note to yourself? (Or a note to myself, eons ago?) But hook_cb_data is used in all the parallel hook callbacks, not just feed_pipe_fn, so I don't think this is accurate.
Show 6 quoted lines
> struct hook_cb_data {
> /* rc reflects the cumulative failure state */
> int rc;
> --
> 2.49.1
>