On Tue, Mar 27, 2012 at 10:46:15PM -0400, Ben Walton wrote: > > > +#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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html