Re: [RFC PATCH 2/2] Improve transport helper exec failure reporting
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 25, 2009, 07:44 UTC
- Message-ID
- <7vljgrebv2.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1261676971-3285-3-git-send-email-ilari.liusvaara@elisanet.fi>
Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
Show 6 quoted lines
> 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@elisanet.fi>
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)".
Show 18 quoted lines
> 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.
Show 12 quoted lines
> + 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