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

From: Jeff King <peff@peff.net>
Date: Mar 27, 2012, 05:12 UTC
Message-ID: <20120327051200.GA20897@sigill.intra.peff.net>
In-Reply-To: <20120327050109.GC22547@burratino>

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

Show 10 quoted lines

> 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