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

On Thu, Dec 31, 2009 at 06:48:02PM +0100, Johannes Sixt wrote:

Ilari Liusvaara schrieb:

quoted

On Thu, Dec 31, 2009 at 04:44:37PM +0100, Johannes Sixt wrote:

quoted

Ilari Liusvaara schrieb:

And you change this by treating the helper command not as a git
command, but as a normal command that happens to start with 'git-'.
Whether this interpretation is suitable for the transport layer, I
do not want to decide and I will certainly not object. :-)

The transport helpers are special: they shouldn't be built-in.

An alternative solution would be to forward the silent_exec_failure
flag to exec_git_cmd() to unify the treatment of the error condition
with the non-git-command error path.

Won't work. The error in git command case would be noted in another memory
image. And passing that back would be nasty to say the least.
 

In case 3, it is expected that the child process prints a suitable
error message. Therefore, you should start with merely replacing the
unconditional

	exit(127);
by
	if (errno == ENOENT)
		exit(127);
	else
		die_errno("Cannot exec %s", cmd->argv[0]);

And then you can think about how you support the ENOENT case better.
My proposal for this was to do the PATH lookup manually before the
fork(), and then the above conditional would melt down to simply:

	die_errno("Cannot exec %s", cmd->argv[0]);

The child process can't sanely print anything. Stderr would go to
who knows where. Parent process should have much better idea what to
do with errors.


-Ilari