git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] Use SHELL_PATH from build system in run_command.c:prepare_shell_cmd

From
Ben Walton <bwalton@artsci.utoronto.ca>
Date
Mar 31, 2012, 01:33 UTC
Message-ID
<1333157601-6458-1-git-send-email-bwalton@artsci.utoronto.ca>
In-Reply-To
<7vvclmoit6.fsf@alter.siamese.dyndns.org>

During the testing of the 1.7.10 rc series on Solaris for OpenCSW, it was discovered that t7006-pager was failing due to finding a bad "sh" in PATH after a call to execvp("sh", ...). This call was setup by run_command.c:prepare_shell_cmd.

The PATH in use at the time saw /opt/csw/bin given precedence to traditional Solaris paths such as /usr/bin and /usr/xpg4/bin. A package named schilyutils (Joerg Schilling's utilities) was installed on the build system and it delivered a modified version of the traditional Solaris /usr/bin/sh as /opt/csw/bin/sh. This version of sh suffers from many of the same problems as /usr/bin/sh.

The command-specific pager test failed due to the broken "sh" handling ^ as a pipe character. It tried to fork two processes when it encountered "sed s/^/foo:/" as the pager command. This problem was entirely dependent on the PATH of the user at runtime.

Possible fixes for this issue are:
1. Use the standard system() or popen() which both launch a POSIX
   shell on Solaris as long as _POSIX_SOURCE is defined.
2. The git wrapper could prepend SANE_TOOL_PATH to PATH thus forcing
   all unqualified commands run to use the known good tools on the
   system.
3. The run_command.c:prepare_shell_command() could use the same
   SHELL_PATH that is in the #! line of all all scripts and not rely
   on PATH to find the sh to run.

Option 1 would preclude opening a bidirectional pipe to a filter script and would also break git for Windows as cmd.exe is spawned from system() (cf. v1.7.5-rc0~144^2, "alias: use run_command api to execute aliases, 2011-01-07).

Option 2 is not friendly to users as it would negate their ability to use tools of their choice in many cases. Alternately, injecting SANE_TOOL_PATH such that it takes precedence over /bin and /usr/bin (and anything with lower precedence than those paths) as git-sh-setup.sh does would not solve the problem either as the user environment could still allow a bad sh to be found. (Many OpenCSW users will have /opt/csw/bin leading their PATH and some subset would have schilyutils installed.)

Option 3 allows us to use a known good shell while still honouring the users' PATH for the utilities being run. Thus, it solves the problem while not negatively impacting either users or git's ability to run external commands in convenient ways. Essentially, the shell is a special case of tool that should not rely on SANE_TOOL_PATH and must be called explicitly.

With this patch applied, any code path leading to run_command.c:prepare_shell_cmd can count on using the same sane shell that all shell scripts in the git suite use. Both the build system and run_command.c will default this shell to /bin/sh unless overridden.

Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
---
 Makefile      |    2 ++
 run-command.c |    6 +++++-
 2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index be1957a..dea1f15 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_SQ)"'
+
 $(BUILT_INS): git$X
 	$(QUIET_BUILT_IN)$(RM) $@ && \
 	ln git$X $@ 2>/dev/null || \
diff --git a/run-command.c b/run-command.c
index 1db8abf..2af3e0f 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 "/bin/sh"
+#endif
+
 struct child_to_clean {
 	pid_t pid;
 	struct child_to_clean *next;
@@ -90,7 +94,7 @@ static const char **prepare_shell_cmd(const char **argv)
 		die("BUG: shell command is empty");
 
 	if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) {
-		nargv[nargc++] = "sh";
+		nargv[nargc++] = SHELL_PATH;
 		nargv[nargc++] = "-c";
 
 		if (argc < 2)
-- 
1.7.5.4
Next: Jonathan Nieder
Message 1 of 16 in “Use SHELL_PATH from build system in run_command.c:prepare_shell_cmd”
  1. Use SHELL_PATH from build system in run_command.c:prepare_shell_cmdBen Walton, Mar 31, 2012
  2. Jonathan NiederMar 31, 2012
  3. Junio C HamanoMar 31, 2012
  4. Jonathan NiederMar 31, 2012
  5. Junio C HamanoMar 31, 2012
  6. Junio C HamanoMar 31, 2012
  7. Do not use SHELL_PATH from build system in prepare_shell_cmd on WindowsJohannes Sixt, Apr 17, 2012
  8. Ben WaltonApr 17, 2012
  9. Johannes SixtApr 17, 2012
  10. Ben WaltonApr 17, 2012
  11. Jeff KingApr 17, 2012
  12. Johannes SixtApr 18, 2012
  13. Jeff KingApr 18, 2012
  14. Junio C HamanoApr 18, 2012
  15. Johannes SixtApr 19, 2012
  16. Junio C HamanoApr 19, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.