Re: [PATCH v3 3/4] trace.c: print env vars in trace_run_command()
- Date: Fri, 12 Jan 2018 08:13:02 -0500
- From: Jeff King <peff@xxxxxxxx>
- Subject: Re: [PATCH v3 3/4] trace.c: print env vars in trace_run_command()
On Fri, Jan 12, 2018 at 04:56:06PM +0700, Nguyễn Thái Ngọc Duy wrote:
> Occasionally submodule code could execute new commands with GIT_DIR set
> to some submodule. GIT_TRACE prints just the command line which makes it
> hard to tell that it's not really executed on this repository.
> Print modified env variables (compared to parent environment) in this
> case. Actually only modified or new variables (*) are printed. Variable
> deletion is suppressed because they are often used to clean up
> repo-specific variables that git passes around between processes. When
> submodule code executes commands on another repo, it clears all these
> variables, _many_ of these, that make the output really noisy.
> (*) sort of. More on this in the next patch.
OK. I think we could probably just squash patches 3 and 4, but I'm OK
with them separate, too.
The code looks fine, though I have one nit:
> @@ -281,8 +299,17 @@ void trace_run_command(const struct child_process *cp)
> &trace_default_key, &buf))
> + strbuf_grow(&buf, 255);
IMHO these magic-numbers grows detract from the readability (because you
wonder if they're meaningful), and I doubt if they provide any real
performance gain in practice.