diff mbox series

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

Message ID patch-v2-3.5-3d3dd6b900a-20230208T191924Z-avarab@gmail.com (mailing list archive)
State Accepted
Commit 917e0802493a39d77c4bdbdf9aaa5d8d69b7a7b0
Headers show
Series [v2,1/5] run-command.c: remove dead assignment in while-loop | expand

Commit Message

Ævar Arnfjörð Bjarmason Feb. 8, 2023, 7:21 p.m. UTC 
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(-)

Comments

Junio C Hamano Feb. 8, 2023, 9:12 p.m. UTC | #1 
Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> 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).

OK, that is a sensible plan to spool and dup/tee the input to
children.  It may not be necessary yet at this step, but it is very
good to be thinking ahead.

Looking good.
diff mbox series

Patch

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 { \