Subject: Re: [Updated PATCH 2/2] Improve transport helper exec failure reporting

From: Johannes Sixt <[email-masked]>
Date: Thu, 31 Dec 2009 16:44:37 +0100

Ilari Liusvaara schrieb:
> @@ -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)) {
> +		if (errno == ENOENT)
> +			die("Unable to find remote helper for \"%s\"",
> +				data->name);

You should set helper->silent_exec_failure = 1 when you give your own 
error message for the ENOENT case.

BTW, which error message do you see without your change in this case? You 
only say "pretty much useless", but do not give an example.

> +		else
> +			die("Unable to run helper %s: %s", helper->argv[0],
> +				strerror(errno));

You shouldn't write an error message here because start_command has 
already reported the error.

-- Hannes