From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff King Subject: Re: [PATCH] Use SHELL_PATH to fork commands in run_command.c:prepare_shell_cmd Date: Mon, 26 Mar 2012 23:29:17 -0400 Message-ID: <20120327032917.GB17338@sigill.intra.peff.net> References: <20120326182427.GA10333@sigill.intra.peff.net> <1332816078-26829-1-git-send-email-bwalton@artsci.utoronto.ca> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Cc: jrnieder@gmail.com, gitster@pobox.com, git@vger.kernel.org To: Ben Walton X-From: git-owner@vger.kernel.org Tue Mar 27 05:29:28 2012 Return-path: Envelope-to: gcvg-git-2@plane.gmane.org Received: from vger.kernel.org ([209.132.180.67]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1SCN67-0008Sm-Kv for gcvg-git-2@plane.gmane.org; Tue, 27 Mar 2012 05:29:27 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757608Ab2C0D3W (ORCPT ); Mon, 26 Mar 2012 23:29:22 -0400 Received: from 99-108-226-0.lightspeed.iplsin.sbcglobal.net ([99.108.226.0]:60368 "EHLO peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752078Ab2C0D3W (ORCPT ); Mon, 26 Mar 2012 23:29:22 -0400 Received: (qmail 28203 invoked by uid 107); 27 Mar 2012 03:29:40 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) (smtp-auth username relayok, mechanism cram-md5) by peff.net (qpsmtpd/0.84) with ESMTPA; Mon, 26 Mar 2012 23:29:40 -0400 Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Mon, 26 Mar 2012 23:29:17 -0400 Content-Disposition: inline In-Reply-To: <1332816078-26829-1-git-send-email-bwalton@artsci.utoronto.ca> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Archived-At: 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