Re: [PATCH] Use SHELL_PATH from build system in run_command.c:prepare_shell_cmd

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Mar 29, 2012, 23:00 UTC
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).

Show 10 quoted lines

> 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