From: Duy Nguyen <pclouds@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>, Git Mailing List <git@vger.kernel.org>,
Stefan Beller <sbeller@google.com>,
Brandon Williams <bmwill@google.com>,
Johannes Sixt <j6t@kdbg.org>
Subject: Re: [PATCH v3 4/4] trace.c: be smart about what env to print in trace_run_command()
Date: Sat, 13 Jan 2018 11:54:26 +0700 [thread overview]
Message-ID: <CACsJy8CeYY34G=JjwHWqM+QbBG41pkW7VP+Aowak26rdM1qpPg@mail.gmail.com> (raw)
In-Reply-To: <xmqq7esmwuwn.fsf@gitster.mtv.corp.google.com>
On Sat, Jan 13, 2018 at 5:54 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> 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 should be squashed into the original 4/4 instead of
> being a separate patch. We'd probably want some sort of test, I
> wonder? Not tested at all beyond compiling...
>
> -- >8 --
> Subject: [PATCH 7/4] run-command.c: don't be too cute in concatenate_env()
>
> Instead of relying on "sort" being stable to sort "unset VAR"
> immediately before "VAR=VAL" to remove the former, just pick the
> last manipulation for each VAR from the list of environment tweaks
> and show them in the output.
This is not enough. Imagine we have GIT_DIR=foo in parent env, then a
sequence of "GIT_DIR", "GIT_DIR=foo" in deltaenv. Because we process
set/unset in two separate loops, the "last one wins" rule does not see
that "GIT_DIR=foo" wins over "unset GIT_DIR;". So we might print
"unset GIT_DIR; GIT_DIR=foo", which is fine even if it's redundant.
Except that we don't print that.
The problem comes from comparing with parent env. The new var has the
same value as parent env so we won't print "GIT_DIR=foo", just "unset
GIT_DIR;". This is wrong.
I'm tempted to just get the final child env from prep_childenv() then
compare with parent env and print the differences. It will not work
with Windows though, so Windows gets the old trace line without env
delta. I hope some Windows contributor will jump in at some point if
they want env tracing works for them too.
--
Duy
next prev parent reply other threads:[~2018-01-13 4:55 UTC|newest]
Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-10 10:48 [PATCH] run-command.c: print env vars when GIT_TRACE is set Nguyễn Thái Ngọc Duy
2018-01-10 18:09 ` Brandon Williams
2018-01-10 19:14 ` Stefan Beller
2018-01-10 19:26 ` Brandon Williams
2018-01-10 19:35 ` Stefan Beller
2018-01-11 9:47 ` [PATCH v2] " Nguyễn Thái Ngọc Duy
2018-01-11 11:25 ` Duy Nguyen
2018-01-11 17:53 ` Brandon Williams
2018-01-11 18:20 ` Stefan Beller
2018-01-11 19:27 ` Junio C Hamano
2018-01-12 9:23 ` Duy Nguyen
2018-01-12 9:56 ` [PATCH v3 0/4] " Nguyễn Thái Ngọc Duy
2018-01-12 9:56 ` [PATCH v3 1/4] trace.c: introduce trace_run_command() Nguyễn Thái Ngọc Duy
2018-01-12 13:05 ` Jeff King
2018-01-12 13:11 ` Jeff King
2018-01-12 9:56 ` [PATCH v3 2/4] trace.c: print program 'git' when child_process.git_cmd is set Nguyễn Thái Ngọc Duy
2018-01-12 13:05 ` Jeff King
2018-01-12 9:56 ` [PATCH v3 3/4] trace.c: print env vars in trace_run_command() Nguyễn Thái Ngọc Duy
2018-01-12 13:13 ` Jeff King
2018-01-12 9:56 ` [PATCH v3 4/4] trace.c: be smart about what env to print " Nguyễn Thái Ngọc Duy
2018-01-12 13:33 ` Jeff King
2018-01-12 18:24 ` Junio C Hamano
2018-01-12 18:45 ` Jeff King
2018-01-12 19:19 ` Junio C Hamano
2018-01-12 19:23 ` Jeff King
2018-01-12 20:28 ` Brandon Williams
2018-01-12 22:54 ` Junio C Hamano
2018-01-13 4:54 ` Duy Nguyen [this message]
2018-01-13 6:47 ` Duy Nguyen
2018-01-12 13:36 ` [PATCH v3 0/4] run-command.c: print env vars when GIT_TRACE is set Jeff King
2018-01-12 13:38 ` [PATCH 5/4] sq_quote_argv: drop maxlen parameter Jeff King
2018-01-12 23:20 ` Junio C Hamano
2018-01-12 13:39 ` [PATCH 6/4] trace: avoid unnecessary quoting Jeff King
2018-01-12 18:06 ` [PATCH v3 0/4] run-command.c: print env vars when GIT_TRACE is set Stefan Beller
2018-01-12 17:19 ` Stefan Beller
2018-01-13 6:54 ` Duy Nguyen
2018-01-13 6:49 ` [PATCH v4 0/7] Trace env variables in run_command() Nguyễn Thái Ngọc Duy
2018-01-13 6:49 ` [PATCH v4 1/7] sq_quote_argv: drop maxlen parameter Nguyễn Thái Ngọc Duy
2018-01-13 6:49 ` [PATCH v4 2/7] trace: avoid unnecessary quoting Nguyễn Thái Ngọc Duy
2018-01-13 6:49 ` [PATCH v4 3/7] trace.c: move strbuf_release() out of print_trace_line() Nguyễn Thái Ngọc Duy
2018-01-13 7:14 ` Jeff King
2018-01-13 6:49 ` [PATCH v4 4/7] trace.c: introduce trace_run_command() Nguyễn Thái Ngọc Duy
2018-01-13 6:49 ` [PATCH v4 5/7] trace.c: print program 'git' when child_process.git_cmd is set Nguyễn Thái Ngọc Duy
2018-01-13 6:49 ` [PATCH v4 6/7] trace.c: print env vars in trace_run_command() Nguyễn Thái Ngọc Duy
2018-01-13 7:25 ` Jeff King
2018-01-16 22:13 ` Junio C Hamano
2018-01-16 22:20 ` Stefan Beller
2018-01-17 1:32 ` Junio C Hamano
2018-01-13 6:49 ` [PATCH v4 7/7] trace.c: print new cwd " Nguyễn Thái Ngọc Duy
2018-01-13 7:26 ` [PATCH v4 0/7] Trace env variables in run_command() Jeff King
2018-01-15 10:59 ` [PATCH v5 " Nguyễn Thái Ngọc Duy
2018-01-15 10:59 ` [PATCH v5 1/7] sq_quote_argv: drop maxlen parameter Nguyễn Thái Ngọc Duy
2018-01-15 10:59 ` [PATCH v5 2/7] trace: avoid unnecessary quoting Nguyễn Thái Ngọc Duy
2018-01-15 10:59 ` [PATCH v5 3/7] trace.c: move strbuf_release() out of print_trace_line() Nguyễn Thái Ngọc Duy
2018-01-16 18:24 ` Brandon Williams
2018-01-15 10:59 ` [PATCH v5 4/7] trace.c: introduce trace_run_command() Nguyễn Thái Ngọc Duy
2018-01-17 22:21 ` Jeff King
2018-01-17 22:41 ` Junio C Hamano
2018-01-15 10:59 ` [PATCH v5 5/7] trace.c: print program 'git' when child_process.git_cmd is set Nguyễn Thái Ngọc Duy
2018-01-15 10:59 ` [PATCH v5 6/7] trace.c: print env vars in trace_run_command() Nguyễn Thái Ngọc Duy
2018-01-16 18:32 ` Brandon Williams
2018-01-15 10:59 ` [PATCH v5 7/7] trace.c: print new cwd " Nguyễn Thái Ngọc Duy
2018-01-18 9:45 ` [PATCH v6 0/7] Trace env variables in run_command() Nguyễn Thái Ngọc Duy
2018-01-18 9:45 ` [PATCH v6 1/7] sq_quote_argv: drop maxlen parameter Nguyễn Thái Ngọc Duy
2018-01-18 9:45 ` [PATCH v6 2/7] trace: avoid unnecessary quoting Nguyễn Thái Ngọc Duy
2018-01-18 9:45 ` [PATCH v6 3/7] trace.c: move strbuf_release() out of print_trace_line() Nguyễn Thái Ngọc Duy
2018-01-18 9:45 ` [PATCH v6 4/7] run-command.c: introduce trace_run_command() Nguyễn Thái Ngọc Duy
2018-01-18 9:45 ` [PATCH v6 5/7] run-command.c: print program 'git' when tracing git_cmd mode Nguyễn Thái Ngọc Duy
2018-01-18 9:45 ` [PATCH v6 6/7] run-command.c: print env vars in trace_run_command() Nguyễn Thái Ngọc Duy
2018-01-18 9:45 ` [PATCH v6 7/7] run-command.c: print new cwd " Nguyễn Thái Ngọc Duy
2018-01-19 21:39 ` [PATCH v6 0/7] Trace env variables in run_command() Jeff King
2018-01-11 10:07 ` [PATCH] run-command.c: print env vars when GIT_TRACE is set Jeff King
2018-01-11 11:13 ` Duy Nguyen
2018-01-11 18:21 ` Johannes Sixt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CACsJy8CeYY34G=JjwHWqM+QbBG41pkW7VP+Aowak26rdM1qpPg@mail.gmail.com' \
--to=pclouds@gmail.com \
--cc=bmwill@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=peff@peff.net \
--cc=sbeller@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).