Re: [PATCH] Use SHELL_PATH to fork commands in run_command.c:prepare_shell_cmd

On Mon, Mar 26, 2012 at 11:29:17PM -0400, Jeff King wrote:

quoted

+#ifndef SHELL_PATH
+# define SHELL_PATH "sh"
+#endif

Does this default ever kick in? The Makefile defaults SHELL_PATH to
/bin/sh, so we will always end up with at least that.

Doing so at least makes us consistent across builds, but I wonder if we
should leave it as "sh" on systems that do not set SHELL_PATH manually.
Executing "sh" via the PATH is the normal system() thing to do. I doubt
many people have an "sh" in their PATH ahead of /bin/sh, but if they do,
we are changing the behavior for them for no good reason.

The whole SHELL_PATH and SANE_TOOL_PATH mess is about helping people on
less-abled systems, and I do not mind bending the usual conventions to
make things more convenient on those systems (e.g., by not doing the
PATH lookup of the shell name). But it would be nice if that bending did
not affect people on more mainstream systems.

I think you could do that pretty easily with this in the Makefile:

diff --git a/Makefile b/Makefile
index be1957a..fcd6896 100644
--- a/Makefile
+++ b/Makefile

@@ -528,6 +528,9 @@ BINDIR_PROGRAMS_NO_X += git-cvsserver
 # Set paths to tools early so that they can be used for version tests.
 ifndef SHELL_PATH
 	SHELL_PATH = /bin/sh
+	SHELL_NAME = sh
+else
+	SHELL_NAME = $(SHELL_PATH)
 endif
 ifndef PERL_PATH
 	PERL_PATH = /usr/bin/perl

and then just pass and use $SHELL_NAME in run-command.c (I guess you'd
have to make a $SHELL_NAME_SQ, too).

One other option not discussed: what if you always just used the
basename of $SHELL_PATH? That would fix your problem (it would do "bash
-c" instead of "sh -c"), while keeping the same PATH-lookup semantics
that exist now. The only setup it wouldn't help is somebody who uses
/some/other/sh, then still has /bin in their PATH before /some/other.

-Peff