Re: [PATCH 3/4] run-command: prep_childenv on all platforms

From: Phillip Wood <phillip.wood123@gmail.com>
Date: May 21, 2025, 13:07 UTC
Message-ID: <e5efc450-dfe1-48fa-9dcd-7e0634957e1f@crinan.ddns.net>
In-Reply-To: <aC2HJxvrorJdMkL9@pks.im>

On 21/05/2025 08:56, Patrick Steinhardt wrote:

Show 11 quoted lines

> On Tue, May 20, 2025 at 03:34:57PM -0400, D. Ben Knoble wrote:
>> We only prepare the child environment on non-Windows platforms, but
>> prep_childenv is the natural interposition point for our subprocess
>> system to adjust the environment as needed. Use it for Windows
>> platforms, also. In subsequent commits we'll use this interposition
>> point to modify the environment on all platforms.
> 
> What is the consequence though of calling `prep_childenv()` on Windows
> now? Why didn't we call it before this change? Details like this should
> definitely go into the commit message to explain why it's safe to add
> the call now.

The environment prep for windows is currently implemented in compat/mingw.c:make_environment_block(). That function is careful to handle unicode characters correctly, it is hard to see how that is compatible this change.

Best Wishes

Phillip