Ilari Liusvaara <ilari.liusvaara@xxxxxxxxxxx> writes:
> Previously transport-helper exec failure error reporting was pretty
> much useless as it didn't report errors from execve, only from pipe
> and fork. Now that run-command passes errno from exec, use the
> improved support to actually print useful errors if execution fails.
>
> Signed-off-by: Ilari Liusvaara <ilari.liusvaara@xxxxxxxxxxx>
Looks pretty straightforward, except that I have this nagging feeling that
we should *not* be married to the idea of "'proc->git_cmd = 1' is merely a
way to save you from typing 'git-' prefix in start_command(proc)".
> diff --git a/transport-helper.c b/transport-helper.c
> index 5078c71..0965c9b 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -31,13 +31,19 @@ static struct child_process *get_helper(struct transport *transport)
> helper->out = -1;
> helper->err = 0;
> helper->argv = xcalloc(4, sizeof(*helper->argv));
> - strbuf_addf(&buf, "remote-%s", data->name);
> + strbuf_addf(&buf, "git-remote-%s", data->name);
> helper->argv[0] = strbuf_detach(&buf, NULL);
> helper->argv[1] = transport->remote->name;
> helper->argv[2] = transport->url;
> - helper->git_cmd = 1;
> - if (start_command(helper))
> - die("Unable to run helper: git %s", helper->argv[0]);
> + helper->git_cmd = 0;
> + if (start_command(helper)) {
For example, we might later want to use different $PATH only when running
a git subcommand, and telling run_command() explicitly that we are running
a git thing would help if you don't add "git-" to the command line and
drop "proc->git_cmd = 1" in the caller like your patch does.
> + if (errno == ENOENT)
> + die("Unable to find remote helper for \"%s\"",
> + data->name);
> + else
> + die("Unable to run helper %s: %s", helper->argv[0],
> + strerror(errno));
> + }
> data->helper = helper;
>
> write_str_in_full(helper->in, "capabilities\n");
> --
> 1.6.6.3.gaa2e1
--
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