Re: [PATCH v2] run-command.c: print env vars when GIT_TRACE is set

[Junio C Hamano <gitster@pobox.com>]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> 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 ...

When I read the above, I imagined that you are reducing noise from
the output by showing only modified or new variables, but it appears
that the implementation of concatenate_env() just blindly copies the
variables without checking if they are setting the same value.

I wonder how common it would be to attempt exporting a variable with
the same value, and to attempt unsetting a variable that is not
exported, which might help you reduce the clutter by hiding stuff
that truly do not matter, while keeping what matters (possibly
including the "unset" part).

> +static void concatenate_env(struct strbuf *dst, const char *const *env)
> +{
> +	int i;
> +
> +	/* Copy into destination buffer. */
> +	strbuf_grow(dst, 255);
> +	for (i = 0; env[i]; ++i) {
> +		/*
> +		 * the main interesting is setting new vars

I'll do s/interesting/& part/ while queuing.

> +		 * e.g. GIT_DIR, ignore the unsetting to reduce noise.
> +		 */
> +		if (!strchr(env[i], '='))
> +			continue;
> +		strbuf_addch(dst, ' ');
> +		sq_quote_buf(dst, env[i]);

I think you'd tweak the quoting around here in a later iteration,
based on the distinction between the two:

	$ 'GIT_DIR=.git' git foo
	$ GIT_DIR='.git' git foo

that was pointed out in a near-by message.

Thanks.