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

Excerpts from Jeff King's message of Mon Mar 26 23:29:17 -0400 2012:

Hi Jeff,

quoted

+run-command.o: EXTRA_CPPFLAGS = -DSHELL_PATH='"$(SHELL_PATH)"'
+

This should be $(SHELL_PATH_SQ), no?

Yes, you're right, it should be.

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()?

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.

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.

(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.)

Did I miss anything else?

Thanks
-Ben
--
Ben Walton
Systems Programmer - CHASS
University of Toronto
C:416.407.5610 | W:416.978.4302