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

On Tue, Mar 27, 2012 at 10:46:15PM -0400, Ben Walton wrote:

quoted

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.

Not when using the build system, but as Hannes mentioned, there is
potential for this to be used outside of the default build system, so
I think having the fallback is a good defensive option.  Should it
maybe be set to /bin/sh though to be more consistent with system()?

Yeah, I think making it "/bin/sh" is better. It's more obvious to people
reading the code as part of git (since even though it is a repetition of
the default from the Makefile, at least it is the _same_ default), and
if it does get reused, it's probably a better default.

Given the rest of the discussion that happened, I think I understand
that my patch is actually ok with the following caveats:

1. My commit message still needs work.
2. Possibly change the default setting of the SHELL_PATH macro from
   "sh" to "/bin/sh"
3. Use the _SQ variant of SHELL_PATH.

Yeah, I think so (not that I am the final word, but that at least
matches my perception of the discussion so far, too).

(The tracking of changes for SHELL_PATH is considered too heavy for
now when the other _PATH items aren't tracked the same way.  This
might make a nice separate patch series though, using the idea from
the kernel where individual commands are tracked.)

Agreed.

-Peff