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 SANE_TOOL_PATH in use at the time was lead by /opt/csw/gnu and
/opt/csw/bin as used by OpenCSW packages so that OpenCSW packaged GNU
tools are found instead of system versions. A package named
schilyutils (Joerg Schilling's utilities) was installed on the build
system and it provided a version of the traditional Solaris
/usr/bin/sh, as /opt/csw/bin/sh. This version of "sh" contains many
of the same problems as the traditional Solaris /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 for
consistency with builtin commands.
3. The run_command.c:prepare_shell_command() could use the same
SHELL_PATH that is in the #! line of all all scripts.
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 voided by the same example that turned up this issue.
SANE_TOOL_PATH might also include 'insane' tools.
Option 3 is the best choice at this time.
After this patch, $GIT_PAGER is interpreted by the same shell in
scripted commands which use the git_pager function that uses "eval"
and builtins which use the run_command machinery.
The default shell used by commands will be /bin/sh if not overridden
by the build system. (This allows for use of this code without the
build system, which was noted during the discussion as a good
quality.[1]) The build always system will pass the value of
SHELL_PATH, which default to /bin/sh as well.
[1] http://thread.gmane.org/gmane.comp.version-control.git/193866/focus=194018
Signed-off-by: Ben Walton <bwalton@xxxxxxxxxxxxxxxxxx>
---
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
--
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