git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ben Walton <bwalton@artsci.utoronto.ca>
To: peff@peff.net, jrnieder@gmail.com, gitster@pobox.com
Cc: git@vger.kernel.org, Ben Walton <bwalton@artsci.utoronto.ca>
Subject: [PATCH] Use SHELL_PATH to fork commands in run_command.c:prepare_shell_cmd
Date: Mon, 26 Mar 2012 22:41:18 -0400	[thread overview]
Message-ID: <1332816078-26829-1-git-send-email-bwalton@artsci.utoronto.ca> (raw)
In-Reply-To: <20120326182427.GA10333@sigill.intra.peff.net>

The shell spawned in run-command.c:prepare_shell_cmd was hard coded to
"sh."  This was breaking "t7006-pager:command-specific pager" when the
"sh" found in the PATH happened to be a broken one such as Solaris'
/usr/bin/sh.  (The breakage in this case was due to ^ being
interpreted the same as | which was seeing two processes forked
instead of a single sed process.)

To allow the build system to supply a value that is deemed sane, thus
removing variation injected by the local user environment, define a
macro named SHELL_PATH (to match the variable of the same name in the
build system).  Use this macro instead of the literal "sh" when
preparing a set of arguments to be forked.  Set "sh" as the default
value of this macro if none is set through other means to preserve the
original behaviour.

The Makefile now sets EXTRA_CPPFLAGS to a value that includes a
definition of SHELL_PATH when building run_command.c.  This sees the
sane shell passed to run_command.c and used when forking external
commands.

Leveraging SANE_TOOL_PATH was considered here but had downsides that
may have made it an incomplete solution.  For example, some builds may
use bash as the shell and not want sh used at all.

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..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)"'
+
 $(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..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
+
 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

  reply	other threads:[~2012-03-27  2:41 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-25 12:31 [PATCH 0/2] Make run-command.c honour SHELL_PATH Ben Walton
2012-03-25 12:31 ` [PATCH 1/2] run-command.c: Define SHELL_PATH macro for use in prepare_shell_cmd Ben Walton
2012-03-25 12:31 ` [PATCH 2/2] Makefile: Set EXTRA_CPPFLAGS during the compilation of run-command Ben Walton
2012-03-26  1:11 ` [PATCH 0/2] Make run-command.c honour SHELL_PATH Jonathan Nieder
2012-03-26 13:38   ` Ben Walton
2012-03-26 18:12     ` Jeff King
2012-03-26 18:19       ` Ben Walton
2012-03-26 18:24         ` Jeff King
2012-03-27  2:41           ` Ben Walton [this message]
2012-03-27  3:29             ` [PATCH] Use SHELL_PATH to fork commands in run_command.c:prepare_shell_cmd Jeff King
2012-03-27  3:34               ` Jeff King
2012-03-27  5:01               ` Jonathan Nieder
2012-03-27  5:12                 ` Jeff King
2012-03-27  5:53                   ` Jonathan Nieder
2012-03-27  6:23               ` Johannes Sixt
2012-03-28  2:46               ` Ben Walton
2012-03-28  4:22                 ` Jeff King
2012-03-28 23:26                   ` [PATCH] Use SHELL_PATH from build system " Ben Walton
2012-03-29  4:02                     ` Junio C Hamano
2012-03-29  6:09                       ` Jonathan Nieder
     [not found]                       ` <1333073831-sup-5734@pinkfloyd.chass.utoronto.ca>
2012-03-30  6:32                         ` Jonathan Nieder
2012-03-29 23:00                     ` Jonathan Nieder
2012-03-28 23:28                   ` [PATCH] Use SHELL_PATH to fork commands " Ben Walton
2012-03-27  4:26             ` Jonathan Nieder
2012-03-27  4:49               ` Jonathan Nieder
2012-03-27  2:45           ` [PATCH 0/2] Make run-command.c honour SHELL_PATH Ben Walton
2012-03-26 18:17     ` Jonathan Nieder
2012-03-26 18:08   ` Jeff King
2012-03-26 17:58 ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1332816078-26829-1-git-send-email-bwalton@artsci.utoronto.ca \
    --to=bwalton@artsci.utoronto.ca \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).