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 10:41:18PM -0400, Ben Walton wrote:

> diff --git a/Makefile b/Makefile
> index be1957a..344e2e0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1913,6 +1913,8 @@ builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
>  	'-DGIT_MAN_PATH="$(mandir_SQ)"' \
>  	'-DGIT_INFO_PATH="$(infodir_SQ)"'
>  
> +run-command.o: EXTRA_CPPFLAGS = -DSHELL_PATH='"$(SHELL_PATH)"'
> +

This should be $(SHELL_PATH_SQ), no?

> diff --git a/run-command.c b/run-command.c
> index 1db8abf..f005a31 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -4,6 +4,10 @@
>  #include "sigchain.h"
>  #include "argv-array.h"
>  
> +#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.

-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]