git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Brandon Williams <bmwill@google.com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] run-command.c: print env vars when GIT_TRACE is set
Date: Wed, 10 Jan 2018 10:09:45 -0800	[thread overview]
Message-ID: <20180110180945.GA53941@google.com> (raw)
In-Reply-To: <20180110104835.22905-1-pclouds@gmail.com>

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@gmail.com>
> ---
>  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

  reply	other threads:[~2018-01-10 18:09 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 [this message]
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
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=20180110180945.GA53941@google.com \
    --to=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.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).