On 01/12, Jeff King wrote: > On Fri, Jan 12, 2018 at 11:19:44AM -0800, Junio C Hamano wrote: > > > Jeff King <peff@xxxxxxxx> writes: > > > > > The actual prep_childenv() code looks like it would always do > > > last-one-wins, so we should treat FOO as unset in that final case. But > > > that only kicks in on non-Windows. > > > > > > On Windows we feed cmd->env straight to mingw_spawnvpe(). That ends up > > > in make_environment_block(), which looks like it does something similar. > > > > > > It's too bad the prep code is not shared, since then we could probably > > > just ask _it_ which deltas it applied. > > > > Yeah, but that function does a lot more than computing delta. > > > > It's primary point, which comes from ae25394b ("run-command: prepare > > child environment before forking", 2017-04-19), is to create a full > > copy of the environment, not just a series of putenv/unsetenv that > > describes what gets changed, and that is done to avoid any > > allocation after fork before exec in the child process. > > > > I guess prep_childenv() could take an extra and optional string-list > > to report what "deltas" it applied to the tracing machinery. I am > > not sure if that is worth it, though. > > Yes, that's exactly what I meant. I think if it covered all platforms it > might be worth it (not so much for code de-duping, but because then we > would know our display logic always matched what we exec'd). But unless > somebody wants to refactor all of the Windows spawn code, it's > definitely not a good idea. And not having access to a windows box I didn't want to try and refactor the windows-only code when I introduced prep_childenv() :) -- Brandon Williams