Re: [PATCH v3 4/4] trace.c: be smart about what env to print in trace_run_command()
- From
- Brandon Williams <bmwill@google.com>
- Date
- Jan 12, 2018, 20:28 UTC
- Message-ID
- <20180112202835.GA108169@google.com>
- In-Reply-To
- <20180112192314.GA19004@sigill.intra.peff.net>
On 01/12, Jeff King wrote:
Show 31 quoted lines
> On Fri, Jan 12, 2018 at 11:19:44AM -0800, Junio C Hamano wrote:
>
> > Jeff King <peff@peff.net> 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