Re: [PATCH v3 4/4] trace.c: be smart about what env to print in trace_run_command()

2018-01-12 Thread Duy Nguyen
On Sat, Jan 13, 2018 at 11:54 AM, Duy Nguyen wrote: > On Sat, Jan 13, 2018 at 5:54 AM, Junio C Hamano wrote: >> Jeff King writes: >> >>> I also think this is a special case of a more general problem. FOO could >>> appear any number of times in the "env" array, as a deletion or with >>> multiple

Re: [PATCH v3 4/4] trace.c: be smart about what env to print in trace_run_command()

2018-01-12 Thread Duy Nguyen
On Sat, Jan 13, 2018 at 5:54 AM, Junio C Hamano wrote: > Jeff King writes: > >> I also think this is a special case of a more general problem. FOO could >> appear any number of times in the "env" array, as a deletion or with >> multiple values. Our prep_childenv() would treat that as "last one >>

Re: [PATCH v3 4/4] trace.c: be smart about what env to print in trace_run_command()

2018-01-12 Thread Junio C Hamano
Jeff King writes: > I also think this is a special case of a more general problem. FOO could > appear any number of times in the "env" array, as a deletion or with > multiple values. Our prep_childenv() would treat that as "last one > wins", I think. Could we just do the same here? Perhaps this

Re: [PATCH v3 4/4] trace.c: be smart about what env to print in trace_run_command()

2018-01-12 Thread Brandon Williams
On 01/12, Jeff King wrote: > On Fri, Jan 12, 2018 at 11:19:44AM -0800, Junio C Hamano wrote: > > > Jeff King 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 no

Re: [PATCH v3 4/4] trace.c: be smart about what env to print in trace_run_command()

2018-01-12 Thread Jeff King
On Fri, Jan 12, 2018 at 11:19:44AM -0800, Junio C Hamano wrote: > Jeff King 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

Re: [PATCH v3 4/4] trace.c: be smart about what env to print in trace_run_command()

2018-01-12 Thread Junio C Hamano
Jeff King 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()

Re: [PATCH v3 4/4] trace.c: be smart about what env to print in trace_run_command()

2018-01-12 Thread Jeff King
On Fri, Jan 12, 2018 at 10:24:28AM -0800, Junio C Hamano wrote: > Jeff King writes: > > >> + /* > >> + * Do we have a sequence of "unset GIT_DIR; GIT_DIR=foo"? > >> + * Then don't bother with the unset thing. > >> + */ > >> + if (i + 1 < envs.nr &&

Re: [PATCH v3 4/4] trace.c: be smart about what env to print in trace_run_command()

2018-01-12 Thread Junio C Hamano
Jeff King writes: >> +/* >> + * Do we have a sequence of "unset GIT_DIR; GIT_DIR=foo"? >> + * Then don't bother with the unset thing. >> + */ >> +if (i + 1 < envs.nr && >> +!strcmp(env, envs.items[i + 1].string)) >>

Re: [PATCH v3 4/4] trace.c: be smart about what env to print in trace_run_command()

2018-01-12 Thread Jeff King
On Fri, Jan 12, 2018 at 04:56:07PM +0700, Nguyễn Thái Ngọc Duy wrote: > The previous concatenate_env() is kinda dumb. It receives a env delta > in child_process and blindly follows it. If the run_command() user > "adds" more vars of the same value, or deletes vars that do not exist > in parent env

[PATCH v3 4/4] trace.c: be smart about what env to print in trace_run_command()

2018-01-12 Thread Nguyễn Thái Ngọc Duy
The previous concatenate_env() is kinda dumb. It receives a env delta in child_process and blindly follows it. If the run_command() user "adds" more vars of the same value, or deletes vars that do not exist in parent env in the first place (*), then why bother to print them? This patch checks chil