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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 26, 2012 at 11:29:17PM -0400, Jeff King 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.
> 
> 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. I doubt
> many people have an "sh" in their PATH ahead of /bin/sh, but if they do,
> we are changing the behavior for them for no good reason.
> 
> 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.

I think you could do that pretty easily with this in the Makefile:

diff --git a/Makefile b/Makefile
index be1957a..fcd6896 100644
--- a/Makefile
+++ b/Makefile
@@ -528,6 +528,9 @@ BINDIR_PROGRAMS_NO_X += git-cvsserver
 # Set paths to tools early so that they can be used for version tests.
 ifndef SHELL_PATH
 	SHELL_PATH = /bin/sh
+	SHELL_NAME = sh
+else
+	SHELL_NAME = $(SHELL_PATH)
 endif
 ifndef PERL_PATH
 	PERL_PATH = /usr/bin/perl

and then just pass and use $SHELL_NAME in run-command.c (I guess you'd
have to make a $SHELL_NAME_SQ, too).

One other option not discussed: what if you always just used the
basename of $SHELL_PATH? That would fix your problem (it would do "bash
-c" instead of "sh -c"), while keeping the same PATH-lookup semantics
that exist now. The only setup it wouldn't help is somebody who uses
/some/other/sh, then still has /bin in their PATH before /some/other.

-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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]