Nguyễn Thái Ngọc Duy  <[email protected]> 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.

Reply via email to