On Sat, Oct 3, 2015 at 6:43 AM, Junio C Hamano <gits...@pobox.com> wrote:
> Junio C Hamano <gits...@pobox.com> writes:
>
>> Chris Packham <judge.pack...@gmail.com> 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?
Possibly but I still need to be able to push a prompt out to the user
and receive a response.
>
> 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
And my current hook handles that by assuming that the user input is "N"
>
> If you want to go interactive from the hook, you'd have to open and
> interact with /dev/tty yourself in your hook anyway.
That may be what I have to do, although I have absolutely no idea how.
>
> 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
