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

[Jeff King <peff@peff.net>]

On Tue, Mar 27, 2012 at 12:01:10AM -0500, Jonathan Nieder wrote:

> Jeff King wrote:
> 
> > 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.
> 
> It's more common for system() to default to /bin/sh.

Hmm, you're right. I think I was fooled by the fact that it puts "sh"
into argv[0]. It doesn't seem to actually do a PATH lookup, though (at
least not on my system).

In that case, Ben's patch is actually making us _more_ normal and
like system(), which is probably a slight benefit (although it's still a
behavior change from what we've been doing).

> I noticed the Makefile already doesn't do this sort of thing for
> mandir and htmldir, but do you think changes to SHELL_PATH should be
> tracked to force a rebuild when it changes?

Yeah, that would be nice.

In general, I wish adding these sorts of dependencies wasn't so manual
and painful. I'm not sure of a good solution short of totally retooling
our build system, though.

-Peff