Re: [PATCH] var(win32): do report the GIT_SHELL_PATH that is actually used
Hi Johannes
On 11/07/2024 13:03, Johannes Schindelin wrote:
Show 36 quoted lines
> Hi Phillip,
>
> On Tue, 9 Jul 2024, Phillip Wood wrote:
>
>> On 08/07/2024 14:02, Johannes Schindelin via GitGitGadget wrote:
>>> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>>
>> Thanks for putting a patch together so quickly
>>
>>> static char *shell_path(int ident_flag UNUSED)
>>> {
>>> +#ifdef WIN32
>>> + char *p = locate_in_PATH("sh");
>>
>> If I'm reading is_busybox_applet() (which only exists in git-for-windows)
>> correctly then this will return "busybox.exe" under mingit-busybox rather than
>> ash.exe, so the calling program would have to know to set argv[0] (which is
>> likely not possible unless the calling program is written in C) or pass "sh"
>> as the first argument. As the code to support busybox does not exist upstream
>> I guess that's best handled downstream.
>
> BusyBox-w32 is unfortunately displaying strange performance patterns. It
> is partially (and expectedly) faster than the MSYS2 Bash, but in other
> scenarios it is substantially slower (which is totally unexpected).
>
> Some time ago, I tried to make this all work and investigate the
> unexpected performance issues (and hoped to fix them, too), but ran out of
> time [*1*]. That was almost two years ago, and I am unsure whether I will
> ever be able to elevate the BusyBox flavor of MinGit to a non-experimental
> state.
>
> My original plan was to eventually no longer include `busybox.exe` in
> the mingit-busybox packages, but instead a copy of that executable with
> the name `sh.exe` and thereby have it work without that hack in the Git
> code to call the `busybox.exe` with the `sh` argument inserted before the
> regular command-line arguments.Thanks for the information.
> In the context of the patch (or now: patch series) at hand, I don't think
> we need to let BusyBox play any role.
Show 5 quoted lines
> Ciao,
> Johannes
>
> Footnote *1*: Interested parties can find the latest state here:
> https://github.com/git-for-windows/git/compare/main...dscho:git:busybox