[PATCH v2 0/5] hook API: support stdin, convert post-rewrite

[PATCH v2 0/5] hook API: support stdin, convert post-rewrite

[Ævar Arnfjörð Bjarmason]

As noted in the v1[1] this is the initial part of the greater
"config-based hooks" topic. I believe this iteration addresses all
comments on v1. Changes since then:

* Remove a couple of paragraphs in 1/4 that aren't relevant anymore,
  an already-landed topic addressed those.

* Don't needlessly change "cp->no_stdin = 1" and introduce an
  "else". This refactoring was there because that code eventually
  changes in the full "config-based hooks" topic, but going through
  those future changes I found that it wasn't for a good reason there
  either. We can just keep the "no_stdin = 1" by default, and have
  specific cases override that.

* Elaborate on why we're not converting the last "post-rewrite" hook
  here.

* Mention the future expected use for sendemail-validate in 5/5

The (passing) CI & topic branch for this is at[2].

1. https://lore.kernel.org/git/cover-0.5-00000000000-20230123T170550Z-avarab@gmail.com/
2. https://github.com/avar/git/tree/es-avar/config-based-hooks-the-beginning-2

Emily Shaffer (4):
  run-command: allow stdin for run_processes_parallel
  hook API: support passing stdin to hooks, convert am's 'post-rewrite'
  sequencer: use the new hook API for the simpler "post-rewrite" call
  hook: support a --to-stdin=<path> option

Ævar Arnfjörð Bjarmason (1):
  run-command.c: remove dead assignment in while-loop

 Documentation/git-hook.txt |  7 ++++++-
 builtin/am.c               | 20 ++++----------------
 builtin/hook.c             |  4 +++-
 hook.c                     |  5 +++++
 hook.h                     |  5 +++++
 run-command.c              | 13 +++++++++----
 sequencer.c                | 18 ++++--------------
 t/t1800-hook.sh            | 18 ++++++++++++++++++
 8 files changed, 54 insertions(+), 36 deletions(-)

Range-diff against v1:
1:  351c6a55a41 ! 1:  488b24e1c98 run-command.c: remove dead assignment in while-loop
    @@ Commit message
     
         Remove code that's been unused since it was added in
         c553c72eed6 (run-command: add an asynchronous parallel child
    -    processor, 2015-12-15), the next use of "i" in this function is:
    -
    -            for (i = 0; ...
    -
    -    So we'll always clobber the "i" that's set here. Presumably the "i"
    -    assignment is an artifact of WIP code that made it into our tree.
    -
    -    A subsequent commit will need to adjust the type of the "i" variable
    -    in the otherwise unrelated for-loop, which is why this is being
    -    removed now.
    +    processor, 2015-12-15).
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
2:  81eef2f60a0 = 2:  9a178577dcc run-command: allow stdin for run_processes_parallel
3:  c6b9b69c516 ! 3:  3d3dd6b900a hook API: support passing stdin to hooks, convert am's 'post-rewrite'
    @@ builtin/am.c: static int run_applypatch_msg_hook(struct am_state *state)
     
      ## hook.c ##
     @@ hook.c: static int pick_next_hook(struct child_process *cp,
    - 	if (!hook_path)
    - 		return 0;
      
    --	cp->no_stdin = 1;
    + 	cp->no_stdin = 1;
      	strvec_pushv(&cp->env, hook_cb->options->env.v);
     +	/* 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);
    -+	} else {
    -+		cp->no_stdin = 1;
     +	}
      	cp->stdout_to_stderr = 1;
      	cp->trace2_hook_name = hook_cb->hook_name;
4:  7a55c95f60f ! 4:  b96522d593f sequencer: use the new hook API for the simpler "post-rewrite" call
    @@ Commit message
     
         This leaves the more complex "post-rewrite" invocation added in
         a87a6f3c98e (commit: move post-rewrite code to libgit, 2017-11-17)
    -    here in sequencer.c unconverted. That'll be done in a subsequent
    -    commit.
    +    here in sequencer.c unconverted.
    +
    +    Here we can pass in a file's via the "in" file descriptor, in that
    +    case we don't have a file, but will need to write_in_full() to an "in"
    +    provide by the API. Support for that will be added to the hook API in
    +    the future, but we're not there yet.
     
         Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
5:  cb9ef7a89c4 ! 5:  b4e02f41194 hook: support a --to-stdin=<path> option for testing
    @@ Metadata
     Author: Emily Shaffer <emilyshaffer@google.com>
     
      ## Commit message ##
    -    hook: support a --to-stdin=<path> option for testing
    +    hook: support a --to-stdin=<path> option
     
         Expose the "path_to_stdin" API added in the preceding commit in the
    -    "git hook run" command. For now we won't be using this command
    -    interface outside of the tests, but exposing this functionality makes
    -    it easier to test the hook API.
    +    "git hook run" command.
    +
    +    For now we won't be using this command interface outside of the tests,
    +    but exposing this functionality makes it easier to test the hook
    +    API. The plan is to use this to extend the "sendemail-validate"
    +    hook[1][2].
    +
    +    1. https://lore.kernel.org/git/ad152e25-4061-9955-d3e6-a2c8b1bd24e7@amd.com
    +    2. https://lore.kernel.org/git/20230120012459.920932-1-michael.strawbridge@amd.com
     
         Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
-- 
2.39.1.1397.g8c8c074958d

[PATCH v2 1/5] run-command.c: remove dead assignment in while-loop

[Ævar Arnfjörð Bjarmason]

Remove code that's been unused since it was added in
c553c72eed6 (run-command: add an asynchronous parallel child
processor, 2015-12-15).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 run-command.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/run-command.c b/run-command.c
index 50cc011654e..b439c7974ca 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1632,9 +1632,7 @@ static void pp_buffer_stderr(struct parallel_processes *pp,
 			     const struct run_process_parallel_opts *opts,
 			     int output_timeout)
 {
-	int i;
-
-	while ((i = poll(pp->pfd, opts->processes, output_timeout) < 0)) {
+	while (poll(pp->pfd, opts->processes, output_timeout) < 0) {
 		if (errno == EINTR)
 			continue;
 		pp_cleanup(pp, opts);
-- 
2.39.1.1397.g8c8c074958d

[PATCH v2 2/5] run-command: allow stdin for run_processes_parallel

[Ævar Arnfjörð Bjarmason]

From: Emily Shaffer <emilyshaffer@google.com>

While it makes sense not to inherit stdin from the parent process to
avoid deadlocking, it's not necessary to completely ban stdin to
children. An informed user should be able to configure stdin safely. By
setting `some_child.process.no_stdin=1` before calling `get_next_task()`
we provide a reasonable default behavior but enable users to set up
stdin streaming for themselves during the callback.

`some_child.process.stdout_to_stderr`, however, remains unmodifiable by
`get_next_task()` - the rest of the run_processes_parallel() API depends
on child output in stderr.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 run-command.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/run-command.c b/run-command.c
index b439c7974ca..6bd16acb060 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1586,6 +1586,14 @@ static int pp_start_one(struct parallel_processes *pp,
 	if (i == opts->processes)
 		BUG("bookkeeping is hard");
 
+	/*
+	 * By default, do not inherit stdin from the parent process - otherwise,
+	 * all children would share stdin! Users may overwrite this to provide
+	 * something to the child's stdin by having their 'get_next_task'
+	 * callback assign 0 to .no_stdin and an appropriate integer to .in.
+	 */
+	pp->children[i].process.no_stdin = 1;
+
 	code = opts->get_next_task(&pp->children[i].process,
 				   opts->ungroup ? NULL : &pp->children[i].err,
 				   opts->data,
@@ -1601,7 +1609,6 @@ static int pp_start_one(struct parallel_processes *pp,
 		pp->children[i].process.err = -1;
 		pp->children[i].process.stdout_to_stderr = 1;
 	}
-	pp->children[i].process.no_stdin = 1;
 
 	if (start_command(&pp->children[i].process)) {
 		if (opts->start_failure)
-- 
2.39.1.1397.g8c8c074958d

[PATCH v2 3/5] hook API: support passing stdin to hooks, convert am's 'post-rewrite'

[Ævar Arnfjörð Bjarmason]

From: Emily Shaffer <emilyshaffer@google.com>

Convert the invocation of the 'post-rewrite' hook run by 'git am' to
use the hook.h library. To do this we need to add a "path_to_stdin"
member to "struct run_hooks_opt".

In our API this is supported by asking for a file path, rather
than by reading stdin. Reading directly from stdin would involve caching
the entire stdin (to memory or to disk) once the hook API is made to
support "jobs" larger than 1, along with support for executing N hooks
at a time (i.e. the upcoming config-based hooks).

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/am.c | 20 ++++----------------
 hook.c       |  5 +++++
 hook.h       |  5 +++++
 3 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 82a41cbfc4e..8be91617fef 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -495,24 +495,12 @@ static int run_applypatch_msg_hook(struct am_state *state)
  */
 static int run_post_rewrite_hook(const struct am_state *state)
 {
-	struct child_process cp = CHILD_PROCESS_INIT;
-	const char *hook = find_hook("post-rewrite");
-	int ret;
-
-	if (!hook)
-		return 0;
+	struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
 
-	strvec_push(&cp.args, hook);
-	strvec_push(&cp.args, "rebase");
+	strvec_push(&opt.args, "rebase");
+	opt.path_to_stdin = am_path(state, "rewritten");
 
-	cp.in = xopen(am_path(state, "rewritten"), O_RDONLY);
-	cp.stdout_to_stderr = 1;
-	cp.trace2_hook_name = "post-rewrite";
-
-	ret = run_command(&cp);
-
-	close(cp.in);
-	return ret;
+	return run_hooks_opt("post-rewrite", &opt);
 }
 
 /**
diff --git a/hook.c b/hook.c
index a4fa1031f28..1a848318634 100644
--- a/hook.c
+++ b/hook.c
@@ -55,6 +55,11 @@ static int pick_next_hook(struct child_process *cp,
 
 	cp->no_stdin = 1;
 	strvec_pushv(&cp->env, hook_cb->options->env.v);
+	/* 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);
+	}
 	cp->stdout_to_stderr = 1;
 	cp->trace2_hook_name = hook_cb->hook_name;
 	cp->dir = hook_cb->options->dir;
diff --git a/hook.h b/hook.h
index 4258b13da0d..19ab9a5806e 100644
--- a/hook.h
+++ b/hook.h
@@ -30,6 +30,11 @@ struct run_hooks_opt
 	 * was invoked.
 	 */
 	int *invoked_hook;
+
+	/**
+	 * Path to file which should be piped to stdin for each hook.
+	 */
+	const char *path_to_stdin;
 };
 
 #define RUN_HOOKS_OPT_INIT { \
-- 
2.39.1.1397.g8c8c074958d

[PATCH v2 4/5] sequencer: use the new hook API for the simpler "post-rewrite" call

[Ævar Arnfjörð Bjarmason]

From: Emily Shaffer <emilyshaffer@google.com>

Change the invocation of the "post-rewrite" hook added in
795160457db (sequencer (rebase -i): run the post-rewrite hook, if
needed, 2017-01-02) to use the new hook API.

This leaves the more complex "post-rewrite" invocation added in
a87a6f3c98e (commit: move post-rewrite code to libgit, 2017-11-17)
here in sequencer.c unconverted.

Here we can pass in a file's via the "in" file descriptor, in that
case we don't have a file, but will need to write_in_full() to an "in"
provide by the API. Support for that will be added to the hook API in
the future, but we're not there yet.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 sequencer.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 3e4a1972897..d8d59d05dd4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4834,8 +4834,7 @@ static int pick_commits(struct repository *r,
 		if (!stat(rebase_path_rewritten_list(), &st) &&
 				st.st_size > 0) {
 			struct child_process child = CHILD_PROCESS_INIT;
-			const char *post_rewrite_hook =
-				find_hook("post-rewrite");
+			struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT;
 
 			child.in = open(rebase_path_rewritten_list(), O_RDONLY);
 			child.git_cmd = 1;
@@ -4845,18 +4844,9 @@ static int pick_commits(struct repository *r,
 			/* we don't care if this copying failed */
 			run_command(&child);
 
-			if (post_rewrite_hook) {
-				struct child_process hook = CHILD_PROCESS_INIT;
-
-				hook.in = open(rebase_path_rewritten_list(),
-					O_RDONLY);
-				hook.stdout_to_stderr = 1;
-				hook.trace2_hook_name = "post-rewrite";
-				strvec_push(&hook.args, post_rewrite_hook);
-				strvec_push(&hook.args, "rebase");
-				/* we don't care if this hook failed */
-				run_command(&hook);
-			}
+			hook_opt.path_to_stdin = rebase_path_rewritten_list();
+			strvec_push(&hook_opt.args, "rebase");
+			run_hooks_opt("post-rewrite", &hook_opt);
 		}
 		apply_autostash(rebase_path_autostash());
 
-- 
2.39.1.1397.g8c8c074958d

[PATCH v2 5/5] hook: support a --to-stdin=<path> option

[Ævar Arnfjörð Bjarmason]

From: Emily Shaffer <emilyshaffer@google.com>

Expose the "path_to_stdin" API added in the preceding commit in the
"git hook run" command.

For now we won't be using this command interface outside of the tests,
but exposing this functionality makes it easier to test the hook
API. The plan is to use this to extend the "sendemail-validate"
hook[1][2].

1. https://lore.kernel.org/git/ad152e25-4061-9955-d3e6-a2c8b1bd24e7@amd.com
2. https://lore.kernel.org/git/20230120012459.920932-1-michael.strawbridge@amd.com

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-hook.txt |  7 ++++++-
 builtin/hook.c             |  4 +++-
 t/t1800-hook.sh            | 18 ++++++++++++++++++
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt
index 77c3a8ad909..3407f3c2c07 100644
--- a/Documentation/git-hook.txt
+++ b/Documentation/git-hook.txt
@@ -8,7 +8,7 @@ git-hook - Run git hooks
 SYNOPSIS
 --------
 [verse]
-'git hook' run [--ignore-missing] <hook-name> [-- <hook-args>]
+'git hook' run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]
 
 DESCRIPTION
 -----------
@@ -31,6 +31,11 @@ linkgit:githooks[5] for arguments hooks might expect (if any).
 OPTIONS
 -------
 
+--to-stdin::
+	For "run"; Specify a file which will be streamed into the
+	hook's stdin. The hook will receive the entire file from
+	beginning to EOF.
+
 --ignore-missing::
 	Ignore any missing hook by quietly returning zero. Used for
 	tools that want to do a blind one-shot run of a hook that may
diff --git a/builtin/hook.c b/builtin/hook.c
index b6530d189ad..f95b7965c58 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -7,7 +7,7 @@
 #include "strvec.h"
 
 #define BUILTIN_HOOK_RUN_USAGE \
-	N_("git hook run [--ignore-missing] <hook-name> [-- <hook-args>]")
+	N_("git hook run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]")
 
 static const char * const builtin_hook_usage[] = {
 	BUILTIN_HOOK_RUN_USAGE,
@@ -28,6 +28,8 @@ static int run(int argc, const char **argv, const char *prefix)
 	struct option run_options[] = {
 		OPT_BOOL(0, "ignore-missing", &ignore_missing,
 			 N_("silently ignore missing requested <hook-name>")),
+		OPT_STRING(0, "to-stdin", &opt.path_to_stdin, N_("path"),
+			   N_("file to read into hooks' stdin")),
 		OPT_END(),
 	};
 	int ret;
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 2ef3579fa7c..3506f627b6c 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -177,4 +177,22 @@ test_expect_success 'git hook run a hook with a bad shebang' '
 	test_cmp expect actual
 '
 
+test_expect_success 'stdin to hooks' '
+	write_script .git/hooks/test-hook <<-\EOF &&
+	echo BEGIN stdin
+	cat
+	echo END stdin
+	EOF
+
+	cat >expect <<-EOF &&
+	BEGIN stdin
+	hello
+	END stdin
+	EOF
+
+	echo hello >input &&
+	git hook run --to-stdin=input test-hook 2>actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.39.1.1397.g8c8c074958d

Re: [PATCH v2 5/5] hook: support a --to-stdin=<path> option

[Ævar Arnfjörð Bjarmason]

On Wed, Feb 08 2023, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> From: Emily Shaffer <emilyshaffer@google.com>
>>
>> Expose the "path_to_stdin" API added in the preceding commit in the
>> "git hook run" command.
>>
>> For now we won't be using this command interface outside of the tests,
>> but exposing this functionality makes it easier to test the hook
>> API. The plan is to use this to extend the "sendemail-validate"
>> hook[1][2].
>
> OK.
>
> What does it take to tackle the obvious leftover bits of [4/5]?  Use
> tempfile API to allocate a temporary file, slurp the input and close
> it, and then use the "path_to_stdin" feature to spawn the hook?

You did ask for it :)

The below is something I wrote for the end of the initial v2 CL, but
then decided it was way too long and dropped it.

The tl;dr is that no, that would be the shortest way forward, and
arguably what we should do now.

But those hooks currently print straight to the pipe, so having the API
force them to use a tempfile would suck.

But since this is all going for supporting N hooks in parallel the next
step requires an API that's future-proofing the feeding of the same
content to multiple hooks.

So, without further adieu, that dropped part of the v2 CL: 

The rest of this is a large digression about the future API design of
this topic, please don't read ahead unless you're very curious about
that (mainly I wanted to brain-dump this somewhere).

I considered expanding this series to include the rest of the
remaining "post-rewrite" hook in sequencer.c, but as that needs at
least a couple of prep patches to expand the API I've left it out for
now.

The main reason I didn't include (aside from the "let's start small"
of this topic) it is that I still don't like the API we'll eventually
need for the parallel hooks that take "stdin", and would like to mull
it over a bit.

What comes after this series eventually wants to convert these hooks
from (pseudocode):

	struct child_process proc = CHILD_PROCESS_INIT;
	[...]
	start_command(&proc);
	[...]
	for item in transaction:
		write_in_full(proc.in, item, strlen(item));

To e.g.:

	struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_PARALLEL;
	struct string_list to_stdin = STRING_LIST_INIT_DUP;

	[...]

        opt.feed_pipe = pipe_from_string_list;
        opt.feed_pipe_ctx = &to_stdin;

	[...]

	for item in transaction:
		string_list_append(&to_stdin, item);

	run_hooks_opt("some-hook", &opt);
	string_list_clear(&to_stdin, 0);

The reason is that if we're producing data for stdin we'll need to
give it to N hooks. For this topic we neatly side-step that with a
"to-stdin", as Junio notes in [3] and [4].

I think even that is arguably a bit ugly, but it's all internally
changable uglyness, i.e. for the "sendemail-validate" whether we feed
"git hook" with content on stdin or are forced to create a file and
feed it with "--to-stdin" is something we can change later.

But the answer to the question raised in [4] is that we'll eventually
need something like the above. But I don't like it, because:

A) The currently proposed API[5] wants to represent lines to the hooks
   are a "struct string_list", so you add "\n"-less items to it, and
   we'll always print "%s\n" for each item.

   This is an arbitrary limitation over the write_in_full() we do now,
   and will be a hassle e.g. if you have already prepared content,
   you'll first need to line-split it.

   Maybe I'm missing some reason for why the hook interface needs to
   intrinsically promise that it'll be doing write()'s to the hooks
   ending in \n's, but right now I don't see that, we could leave that
   up to each hook, and if they're spewing content larger than that
   the hook itself should just be line-buffering if they care about
   the distinction.

B) Even if we internally line-split a "struct string_list *" is just a
   bad fit as we lose the string size, we should pass something that
   give us a "size_t len" (which we could stuff in the "util" field,
   but...)

C) We need to buffer up the stdin in full before we feed the first
   line to the hook, which seems to me to be an API design that
   creates the problem the second paragraph of [5] claims to be trying
   to avoid. I.e. "simply taking a string_list or strbuf is not as
   scalable as using a callback".

   That would be true if the result of the callback were streamed to
   the N hooks we have, but it's not the case. It's a callback
   mechanism that amounts to just handing off a big "struct
   string_list", which we need to fully populate before we start the
   hook.

D) Most importantly, the API seems to be structured around a problem
   we don't actually have.

   The more general problem *could be* that you'd want to feed N hooks
   with the same content, we ourselves get that content streamed into
   us on "stdin", and therefore need to either buffer it in full
   up-front, or as we read it re-spew it into the N hooks we're
   executing.

   But e.g. for the "reference-transaction" hook all we need to
   support N hooks is to allocate a single "size_t pos" for them, as
   when we're executing them we have a "struct ref_transaction
   *transaction" that doesn't change for the duration of the hook. The
   same goes for the "pre-push.

   Actually, the only eventual API users that really could have used
   buffering to ensure consistent results won't use the buffering
   API. E.g. for the "post-rewrite" and "rebase" hooks we consume a
   file in ".git/" to give to the hooks on stdin. Currently (and still
   with this topic) we'll only have one hook, so the file's content
   will always be the same.

   But if we were being paranoid we'd buffer it up, so that we could
   ensure that our N hooks all get the same input, but for the API
   users that could get a benefit from that we don't use it, but only
   for those that are guaranteed not to need it.

So before the next iteration I'll try to find some time to play with
that. I.e. I think we can rip out the whole "struct string_list" feeder,
and just have an eventual mechanism for each of the N hooks to
init/release their "feed stdin" state with callbacks, and to save away
their state in their own "void *".

Then e.g. for the reference-transaction we'd just allocate a "size_t
pos" per hook, and then just spew content at them on the fly from the
transaction struct, no pre-generation necessary.

Such an interface will nicely support streaming without pre-buffering
delay, and could even run lock-less while multi-threaded (the source
data being const, and (almost) all state per-thread.

The interface would then be general enough to support
pre-slurping/buffering content at the start, and then streaming to N
hooks, e.g. for the "read a file, but guarantee that everyone gets the
same version". We won't need a string list for that, at most a strbuf,
but maybe we can just mmap() those...

3. https://lore.kernel.org/git/xmqqy1pskfo6.fsf@gitster.g/
4. https://lore.kernel.org/git/xmqqtu0gkaye.fsf@gitster.g/
5. https://lore.kernel.org/git/patch-v5-24.36-bb119fa7cc0-20210902T125110Z-avarab@gmail.com/

Re: [PATCH v2 5/5] hook: support a --to-stdin=<path> option

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> From: Emily Shaffer <emilyshaffer@google.com>
>
> Expose the "path_to_stdin" API added in the preceding commit in the
> "git hook run" command.
>
> For now we won't be using this command interface outside of the tests,
> but exposing this functionality makes it easier to test the hook
> API. The plan is to use this to extend the "sendemail-validate"
> hook[1][2].

OK.

What does it take to tackle the obvious leftover bits of [4/5]?  Use
tempfile API to allocate a temporary file, slurp the input and close
it, and then use the "path_to_stdin" feature to spawn the hook?

[PATCH v2 5/5] hook: support a --to-stdin=<path> option

[Ævar Arnfjörð Bjarmason]

From: Emily Shaffer <emilyshaffer@google.com>

Expose the "path_to_stdin" API added in the preceding commit in the
"git hook run" command.

For now we won't be using this command interface outside of the tests,
but exposing this functionality makes it easier to test the hook
API. The plan is to use this to extend the "sendemail-validate"
hook[1][2].

1. https://lore.kernel.org/git/ad152e25-4061-9955-d3e6-a2c8b1bd24e7@amd.com
2. https://lore.kernel.org/git/20230120012459.920932-1-michael.strawbridge@amd.com

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-hook.txt |  7 ++++++-
 builtin/hook.c             |  4 +++-
 t/t1800-hook.sh            | 18 ++++++++++++++++++
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt
index 77c3a8ad909..3407f3c2c07 100644
--- a/Documentation/git-hook.txt
+++ b/Documentation/git-hook.txt
@@ -8,7 +8,7 @@ git-hook - Run git hooks
 SYNOPSIS
 --------
 [verse]
-'git hook' run [--ignore-missing] <hook-name> [-- <hook-args>]
+'git hook' run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]
 
 DESCRIPTION
 -----------
@@ -31,6 +31,11 @@ linkgit:githooks[5] for arguments hooks might expect (if any).
 OPTIONS
 -------
 
+--to-stdin::
+	For "run"; Specify a file which will be streamed into the
+	hook's stdin. The hook will receive the entire file from
+	beginning to EOF.
+
 --ignore-missing::
 	Ignore any missing hook by quietly returning zero. Used for
 	tools that want to do a blind one-shot run of a hook that may
diff --git a/builtin/hook.c b/builtin/hook.c
index b6530d189ad..f95b7965c58 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -7,7 +7,7 @@
 #include "strvec.h"
 
 #define BUILTIN_HOOK_RUN_USAGE \
-	N_("git hook run [--ignore-missing] <hook-name> [-- <hook-args>]")
+	N_("git hook run [--ignore-missing] [--to-stdin=<path>] <hook-name> [-- <hook-args>]")
 
 static const char * const builtin_hook_usage[] = {
 	BUILTIN_HOOK_RUN_USAGE,
@@ -28,6 +28,8 @@ static int run(int argc, const char **argv, const char *prefix)
 	struct option run_options[] = {
 		OPT_BOOL(0, "ignore-missing", &ignore_missing,
 			 N_("silently ignore missing requested <hook-name>")),
+		OPT_STRING(0, "to-stdin", &opt.path_to_stdin, N_("path"),
+			   N_("file to read into hooks' stdin")),
 		OPT_END(),
 	};
 	int ret;
diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
index 2ef3579fa7c..3506f627b6c 100755
--- a/t/t1800-hook.sh
+++ b/t/t1800-hook.sh
@@ -177,4 +177,22 @@ test_expect_success 'git hook run a hook with a bad shebang' '
 	test_cmp expect actual
 '
 
+test_expect_success 'stdin to hooks' '
+	write_script .git/hooks/test-hook <<-\EOF &&
+	echo BEGIN stdin
+	cat
+	echo END stdin
+	EOF
+
+	cat >expect <<-EOF &&
+	BEGIN stdin
+	hello
+	END stdin
+	EOF
+
+	echo hello >input &&
+	git hook run --to-stdin=input test-hook 2>actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.39.1.1475.gc2542cdc5ef