Re: [PATCH] Use SHELL_PATH from build system in run_command.c:prepare_shell_cmd

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Mar 31, 2012, 03:48 UTC
Message-ID: <20120331034802.GB2475@burratino>
In-Reply-To: <1333157601-6458-1-git-send-email-bwalton@artsci.utoronto.ca>

Ben Walton wrote:

> During the testing of the 1.7.10 rc series on Solaris for OpenCSW, it
> was discovered that t7006-pager was failing

[...]

Show 19 quoted lines

> --- a/run-command.c
> +++ b/run-command.c
> @@ -4,6 +4,10 @@
>  #include "sigchain.h"
>  #include "argv-array.h"
>  
> +#ifndef SHELL_PATH
> +# define SHELL_PATH "/bin/sh"
> +#endif
> +
>  struct child_to_clean {
>  	pid_t pid;
>  	struct child_to_clean *next;
> @@ -90,7 +94,7 @@ static const char **prepare_shell_cmd(const char **argv)
>  		die("BUG: shell command is empty");
>  
>  	if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) {
> -		nargv[nargc++] = "sh";
> +		nargv[nargc++] = SHELL_PATH;

The underlying problem is an old one.  Thanks for fixing it.

For what it's worth,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

[...]

> Essentially, the shell is a
> special case of tool that should not rely on SANE_TOOL_PATH and must
> be called explicitly.

It is probably annoying to hear me say this, but:

The above doesn't tell me _why_ it is a special case and that on Solaris users have been burned by "sh" being the original Bourne shell or a temperamental version of ksh so SHELL_PATH usually points to bash or ksh93 instead.

I trust the reader enough to fill in the blank, though, so I think the patch is ok as is.