public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Johannes Sixt <j.sixt@viscovery•net>
To: Jeff King <peff@peff•net>
Cc: Ben Walton <bwalton@artsci•utoronto.ca>,
	jrnieder@gmail•com, gitster@pobox•com, git@vger•kernel.org
Subject: Re: [PATCH] Do not use SHELL_PATH from build system in prepare_shell_cmd on Windows
Date: Wed, 18 Apr 2012 07:39:43 +0200	[thread overview]
Message-ID: <4F8E539F.7030902@viscovery.net> (raw)
In-Reply-To: <20120417221449.GC10797@sigill.intra.peff.net>

Am 4/18/2012 0:14, schrieb Jeff King:
> On Tue, Apr 17, 2012 at 09:03:21AM +0200, Johannes Sixt wrote:
> 
>> diff --git a/run-command.c b/run-command.c
>> index 2af3e0f..e4edede 100644
>> --- a/run-command.c
>> +++ b/run-command.c
>> @@ -94,7 +94,11 @@ static const char **prepare_shell_cmd(const char **argv)
>>  		die("BUG: shell command is empty");
>>  
>>  	if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) {
>> +#ifndef WIN32
>>  		nargv[nargc++] = SHELL_PATH;
>> +#else
>> +		nargv[nargc++] = "sh";
>> +#endif
>>  		nargv[nargc++] = "-c";
>>  
>>  		if (argc < 2)
> 
> It sounds like the real problem is not the use of a configurable shell,
> but rather the use of an absolute path. Should you maybe try to pass the
> basename of SHELL_PATH? Or maybe that is not even worth worrying about,
> as somebody on Windows is not going to ever set SHELL_PATH, since it is
> not like they are working around a non-POSIX "sh" included with the
> operating system (which is why people on Solaris typically set
> SHELL_PATH).

I thought about offering a customization point, but decided that it is not
worth the hassle: Most people download an installer, then the installer
can set up the PATH so that "sh" is not broken or something entirely
unrelated. And those who build git themselves know sufficiently well what
they are doing.

-- Hannes

  reply	other threads:[~2012-04-18  5:40 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <7vvclmoit6.fsf@alter.siamese.dyndns.org>
2012-03-31  1:33 ` [PATCH] Use SHELL_PATH from build system in run_command.c:prepare_shell_cmd Ben Walton
2012-03-31  3:48   ` Jonathan Nieder
2012-03-31  5:38     ` Junio C Hamano
2012-03-31  5:55   ` Jonathan Nieder
2012-03-31 17:49     ` Junio C Hamano
2012-03-31 18:04       ` Junio C Hamano
2012-04-17  7:03   ` [PATCH] Do not use SHELL_PATH from build system in prepare_shell_cmd on Windows Johannes Sixt
2012-04-17 13:45     ` Ben Walton
2012-04-17 14:00       ` Johannes Sixt
2012-04-17 14:04         ` Ben Walton
2012-04-17 22:14     ` Jeff King
2012-04-18  5:39       ` Johannes Sixt [this message]
2012-04-18  7:27         ` Jeff King
2012-04-18 16:30         ` Junio C Hamano
2012-04-19  5:36           ` Johannes Sixt
2012-04-19  5:49             ` Junio C Hamano

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=4F8E539F.7030902@viscovery.net \
    --to=j.sixt@viscovery$(echo .)net \
    --cc=bwalton@artsci$(echo .)utoronto.ca \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=jrnieder@gmail$(echo .)com \
    --cc=peff@peff$(echo .)net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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