From: phillip.wood123@gmail.com
Date: Wed, 17 Jul 2024 14:55:46 GMT
Subject: Re: [PATCH] var(win32): do report the GIT_SHELL_PATH that is actually used
Message-ID: <fdb26d61-d755-4daf-ab5d-6e1a36b7b871@gmail.com>
In-Reply-To: <66345b2d-6662-710a-5513-6cc70507b4d0@gmx.de>

Hi Johannes

On 11/07/2024 13:03, Johannes Schindelin wrote:
> 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.

Agreed

Best Wishes

Phillip

> Ciao,
> Johannes
> 
> Footnote *1*: Interested parties can find the latest state here:
> https://github.com/git-for-windows/git/compare/main...dscho:git:busybox