On 01/10, 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 env variables in this case. Note that the code deliberately ignore
> variables unsetting because there are so many of them (to keep git
> environment clean for the next process) and really hard to read.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> A minor thing that I ignored in this patch is quoting the environment
> variables. For me it's meh. Perhaps I should just dumb the env
> without quoting at all.
A patch like this would have been very helpful to me on some previous
occasions, so thanks for writing it up.
>
> run-command.c | 3 ++-
> trace.c | 38 +++++++++++++++++++++++++++++++++++---
> trace.h | 18 +++++++++++++++---
> 3 files changed, 52 insertions(+), 7 deletions(-)
>
> diff --git a/run-command.c b/run-command.c
> index 31fc5ea86e..235367087d 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -624,7 +624,8 @@ int start_command(struct child_process *cmd)
> cmd->err = fderr[0];
> }
>
> - trace_argv_printf(cmd->argv, "trace: run_command:");
> + trace_env_argv_printf(cmd->env, cmd->argv, "trace: run_command:");
> +
> fflush(NULL);
>
> #ifndef GIT_WINDOWS_NATIVE
> diff --git a/trace.c b/trace.c
> index b7530b51a9..d8967b66e8 100644
> --- a/trace.c
> +++ b/trace.c
> @@ -146,7 +146,26 @@ static void trace_vprintf_fl(const char *file, int line, struct trace_key *key,
> print_trace_line(key, &buf);
> }
>
> +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
> + * e.g. GIT_DIR, ignore the unsetting to reduce noise.
> + */
I think you're missing a word, maybe:
'The main interesting part is setting new vars'
> + if (!strchr(env[i], '='))
> + continue;
> + strbuf_addch(dst, ' ');
> + sq_quote_buf(dst, env[i]);
> + }
At first when i read this I was under the impression that the whole
environment was going to be printed out...but i now realize that this
tracing will only print out the delta's or the additions to the
environment that the child will see. Maybe this should be called out
more clearly in the commit message?
> +}
> +
> static void trace_argv_vprintf_fl(const char *file, int line,
> + const char *const *env,
> const char **argv, const char *format,
> va_list ap)
> {
> @@ -157,6 +176,9 @@ static void trace_argv_vprintf_fl(const char *file, int line,
>
> strbuf_vaddf(&buf, format, ap);
>
> + if (env)
> + concatenate_env(&buf, env);
> +
> sq_quote_argv(&buf, argv, 0);
> print_trace_line(&trace_default_key, &buf);
> }
> @@ -214,7 +236,16 @@ void trace_argv_printf(const char **argv, const char *format, ...)
> {
> va_list ap;
> va_start(ap, format);
> - trace_argv_vprintf_fl(NULL, 0, argv, format, ap);
> + trace_argv_vprintf_fl(NULL, 0, NULL, argv, format, ap);
> + va_end(ap);
> +}
> +
> +void trace_env_argv_printf(const char *const *env, const char **argv,
> + const char *format, ...)
> +{
> + va_list ap;
> + va_start(ap, format);
> + trace_argv_vprintf_fl(NULL, 0, env, argv, format, ap);
> va_end(ap);
> }
>
> @@ -251,12 +282,13 @@ void trace_printf_key_fl(const char *file, int line, struct trace_key *key,
> va_end(ap);
> }
>
> -void trace_argv_printf_fl(const char *file, int line, const char **argv,
> +void trace_argv_printf_fl(const char *file, int line,
> + const char *const *env, const char **argv,
> const char *format, ...)
> {
> va_list ap;
> va_start(ap, format);
> - trace_argv_vprintf_fl(file, line, argv, format, ap);
> + trace_argv_vprintf_fl(file, line, env, argv, format, ap);
> va_end(ap);
> }
>
> diff --git a/trace.h b/trace.h
> index 88055abef7..830d9dcd19 100644
> --- a/trace.h
> +++ b/trace.h
> @@ -34,6 +34,10 @@ extern void trace_printf_key(struct trace_key *key, const char *format, ...);
> __attribute__((format (printf, 2, 3)))
> extern void trace_argv_printf(const char **argv, const char *format, ...);
>
> +__attribute__((format (printf, 3, 4)))
> +extern void trace_env_argv_printf(const char * const*env, const char **argv,
> + const char *format, ...);
> +
> extern void trace_strbuf(struct trace_key *key, const struct strbuf *data);
>
> /* Prints elapsed time (in nanoseconds) if GIT_TRACE_PERFORMANCE is enabled. */
> @@ -93,7 +97,14 @@ extern void trace_performance_since(uint64_t start, const char *format, ...);
> do { \
> if (trace_pass_fl(&trace_default_key)) \
> trace_argv_printf_fl(TRACE_CONTEXT, __LINE__, \
> - argv, __VA_ARGS__); \
> + NULL, argv, __VA_ARGS__); \
> + } while (0)
> +
> +#define trace_env_argv_printf(env, argv, ...) \
> + do { \
> + if (trace_pass_fl(&trace_default_key)) \
> + trace_argv_printf_fl(TRACE_CONTEXT, __LINE__, \
> + env, argv, __VA_ARGS__); \
> } while (0)
>
> #define trace_strbuf(key, data) \
> @@ -121,8 +132,9 @@ extern void trace_performance_since(uint64_t start, const char *format, ...);
> __attribute__((format (printf, 4, 5)))
> extern void trace_printf_key_fl(const char *file, int line, struct trace_key *key,
> const char *format, ...);
> -__attribute__((format (printf, 4, 5)))
> -extern void trace_argv_printf_fl(const char *file, int line, const char **argv,
> +__attribute__((format (printf, 5, 6)))
> +extern void trace_argv_printf_fl(const char *file, int line,
> + const char *const *env, const char **argv,
> const char *format, ...);
> extern void trace_strbuf_fl(const char *file, int line, struct trace_key *key,
> const struct strbuf *data);
> --
> 2.15.1.600.g899a5f85c6
>
--
Brandon Williams
