Junio C Hamano <gitster@xxxxxxxxx> writes:
> Chris Packham <judge.packham@xxxxxxxxx> writes:
>
>> As of git 2.6 this has stopped working and stdin always fails the tty
>> check.
>
> We now run that hook thru run_hook_ve(), which closes the standard
> input (as the hook is not reading anything). Perhaps you can check
> if your output is connected to the tty instead?
s|closes the standard input|opens the standard input for /dev/null|;
Having said that, here are some further thoughts:
* Hooks run via run_hook_ve() and run_hook_le() have their standard
input connected to /dev/null even before these functions were
introduced at ae98a008 (Move run_hook() from builtin-commit.c
into run-command.c (libgit), 2009-01-16). The commit
consolidated the code to run hooks in "checkout", "commit", "gc",
and "merge", all of which run their hooks with their standard
input reading from /dev/null.
* Later at dfa7a6c5 (clone: run post-checkout hook when checking
out, 2009-03-03) "git clone" learned to run post-checkout hook
the same way.
* "receive-pack" (which accepts and processes an incoming "git
push") has pre-receive and post-receive hooks, and they do get
invoked with their standard input open, but they are connected to
a pipe to be fed with the information about the push from
"receive-pack" process.
* "post-rewrite" hooks, invoked by "rebase" and "commit", does get
invoked with its standard input open, but it is fed with the
information about the original and the rewritten commit.
So in that sense, "am", primarily because it was implemented as a
script, was an oddball. It should have been connecting the standard
input to /dev/null in order to be consistent with others, but it did
not even bother to do so.
We _could_ leave the standard input connected to the original
process's standard input only for the specific hook by doing
something along the lines of the attached, but I am not sure if it
is a good change. Given that the majority of existing hooks are
spawned with their standard input connected to /dev/null (and also
after scanning the output from "git hooks --help", I did not find
any that would want to read from the standard input of the original
process that spawns it), I tend to consider that the change in 2.6
done as part of rewriting "am" in C is a bugfix, even though an
unintended one, to make things more consistent.
Besides "consistency", a hook that tried to read from "am"'s
standard input would have been incorrect in the first place, as it
is a normal mode of operation to feed one or more patch e-mails from
the standard input of "git am", i.e.
$ git am <mbox
If you want to go interactive from the hook, you'd have to open and
interact with /dev/tty yourself in your hook anyway.
builtin/am.c | 8 +++++++-
run-command.c | 30 ++++++++++++++++++++++++++----
run-command.h | 9 +++++++++
3 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 4f77e07..3d160d9 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -510,9 +510,15 @@ static void am_destroy(const struct am_state *state)
static int run_applypatch_msg_hook(struct am_state *state)
{
int ret;
+ struct child_process custom = CHILD_PROCESS_INIT;
assert(state->msg);
- ret = run_hook_le(NULL, "applypatch-msg", am_path(state, "final-commit"), NULL);
+
+ custom.env = NULL;
+ custom.no_stdin = 0;
+ custom.stdout_to_stderr = 1;
+
+ ret = run_hook_le_opt(&custom, "applypatch-msg", am_path(state, "final-commit"), NULL);
if (!ret) {
free(state->msg);
diff --git a/run-command.c b/run-command.c
index 3277cf7..dee86df 100644
--- a/run-command.c
+++ b/run-command.c
@@ -793,7 +793,7 @@ const char *find_hook(const char *name)
return path.buf;
}
-int run_hook_ve(const char *const *env, const char *name, va_list args)
+int run_hook_ve_opt(struct child_process *custom, const char *name, va_list args)
{
struct child_process hook = CHILD_PROCESS_INIT;
const char *p;
@@ -805,13 +805,35 @@ int run_hook_ve(const char *const *env, const char *name, va_list args)
argv_array_push(&hook.args, p);
while ((p = va_arg(args, const char *)))
argv_array_push(&hook.args, p);
- hook.env = env;
- hook.no_stdin = 1;
- hook.stdout_to_stderr = 1;
+ hook.env = custom->env;
+ hook.no_stdin = custom->no_stdin;
+ hook.stdout_to_stderr = custom->stdout_to_stderr;
return run_command(&hook);
}
+int run_hook_ve(const char *const *env, const char *name, va_list args)
+{
+ struct child_process custom = CHILD_PROCESS_INIT;
+
+ custom.env = env;
+ custom.no_stdin = 1;
+ custom.stdout_to_stderr = 1;
+ return run_hook_ve_opt(&custom, name, args);
+}
+
+int run_hook_le_opt(struct child_process *custom, const char *name, ...)
+{
+ va_list args;
+ int ret;
+
+ va_start(args, name);
+ ret = run_hook_ve_opt(custom, name, args);
+ va_end(args);
+
+ return ret;
+}
+
int run_hook_le(const char *const *env, const char *name, ...)
{
va_list args;
diff --git a/run-command.h b/run-command.h
index 5b4425a..33a0d72 100644
--- a/run-command.h
+++ b/run-command.h
@@ -62,6 +62,15 @@ LAST_ARG_MUST_BE_NULL
extern int run_hook_le(const char *const *env, const char *name, ...);
extern int run_hook_ve(const char *const *env, const char *name, va_list args);
+/*
+ * Same as above, but env, no_stdin and stdout_to_stderr are copied from
+ * custom to the child_process structure that spawns the hook.
+ */
+LAST_ARG_MUST_BE_NULL
+extern int run_hook_le_opt(struct child_process *custom, const char *name, ...);
+extern int run_hook_ve_opt(struct child_process *custom, const char *name, va_list args);
+
+
#define RUN_COMMAND_NO_STDIN 1
#define RUN_GIT_CMD 2 /*If this is to be git sub-command */
#define RUN_COMMAND_STDOUT_TO_STDERR 4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html