From: Jonathan Nieder <jrnieder@gmail.com>
Date: Thu, 29 Mar 2012 23:00:43 GMT
Subject: Re: [PATCH] Use SHELL_PATH from build system in run_command.c:prepare_shell_cmd
Message-ID: <20120329230043.GB27829@burratino>
In-Reply-To: <1332977191-29069-1-git-send-email-bwalton@artsci.utoronto.ca>

Ben Walton wrote:

[...]
> 2. The git wrapper could prepend SANE_TOOL_PATH to PATH for
>    consistency with builtin commands.

The phrase "builtin commands" left me confused for a moment ---
busybox-style builtins and standalone commands in C both don't get the
PATH fixup.

I think you meant scripted commands (i.e., commands that get the PATH
fixup by sourcing git-sh-setup).

> 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.

As Junio mentioned, the only insane tool that SANE_TOOL_PATH is
allowed to point to is sh.  I guess the note on insane tools was meant
as a wistful observation, but it didn't come through clearly.

Maybe a reference to v1.5.5-rc0~5^2~3 (filter-branch: use $SHELL_PATH
instead of 'sh', 2008-03-12) would make the motivation behind the
"don't trust sh" principle clearer.

> Option 3 is the best choice

Makes sense to me and the patch looks good.  Thanks for your
perseverance.

Ciao,
Jonathan