Re: [PATCH 3/3] trace: add GIT_TRACE_STDIN

[Jeff King <peff@peff.net>]

On Tue, Jun 16, 2015 at 03:49:07PM -0400, Jeff King wrote:

> Another option would be to stop trying to intercept stdin in git.c, and
> instead make this a feature of run-command.c. That is, right before we
> exec a process, tee its stdin there. That means that you cannot do:
> 
>   GIT_TRACE_STDIN=/tmp/foo.out git foo
> 
> to collect the stdin of foo. But that is not really an interesting case
> anyway. You can run "tee" yourself if you want. The interesting cases
> are the ones where git is spawning a sub-process, and you want to
> intercept the data moving between the git processes.

Hmm. I guess we do not actually have to move the stdin interception
there. We can just move the config-checking there, like the patch below.

It basically just converts trace.foo.bar into GIT_TRACE_BAR when we are
running "foo" as a git command.  This does work, but is perhaps
potentially confusing to the user, because it only kicks in when _git_
runs "foo". IOW, this works:

  git config trace.upload-pack.foo /path/to/foo.out
  git daemon

and will trace as you expect. But then running:

  git upload-pack

yourself will do nothing.

I dunno.

---
 run-command.c | 17 +++++++++++++++++
 trace.c       | 44 ++++++++++++++++++++++++++++++++++++++++++++
 trace.h       |  9 +++++++++
 3 files changed, 70 insertions(+)

diff --git a/run-command.c b/run-command.c
index 4d73e90..2febbb5 100644
--- a/run-command.c
+++ b/run-command.c
@@ -284,6 +284,23 @@ int start_command(struct child_process *cmd)
 
 	if (!cmd->argv)
 		cmd->argv = cmd->args.argv;
+	if (cmd->git_cmd) {
+		/*
+		 * Load any extra variables into env_array. But
+		 * if we weren't going to use it (in favor of "env"),
+		 * then consolidate the two. Make sure the original "env"
+		 * goes after what we add, so that it can override.
+		 *
+		 * We cannot just keep two lists, because we may hand off the
+		 * single list to a spawn() implementation.
+		 */
+		trace_config_for(cmd->argv[0], &cmd->env_array);
+		if (cmd->env_array.argc && cmd->env) {
+			for (; *cmd->env; cmd->env++)
+				argv_array_push(&cmd->env_array, *cmd->env);
+			cmd->env = NULL;
+		}
+	}
 	if (!cmd->env)
 		cmd->env = cmd->env_array.argv;
 
diff --git a/trace.c b/trace.c
index a7ec484..86c988e 100644
--- a/trace.c
+++ b/trace.c
@@ -24,6 +24,7 @@
 
 #include "cache.h"
 #include "quote.h"
+#include "argv-array.h"
 
 static size_t expand_trace_name(struct strbuf *out, const char *fmt,
 				void *data)
@@ -449,3 +450,46 @@ void trace_command_performance(const char **argv)
 	sq_quote_argv(&command_line, argv, 0);
 	command_start_time = getnanotime();
 }
+
+struct trace_config_data {
+	const char *want_cmd;
+	struct argv_array *out;
+};
+
+static int trace_config_cb(const char *var, const char *value, void *vdata)
+{
+	struct trace_config_data *data = vdata;
+	const char *have_cmd, *key;
+	int have_len;
+
+	if (!parse_config_key(var, "trace", &have_cmd, &have_len, &key) &&
+	    have_cmd &&
+	    !strncmp(data->want_cmd, have_cmd, have_len) &&
+	    data->want_cmd[have_len] == '\0') {
+		struct strbuf buf = STRBUF_INIT;
+
+		strbuf_addstr(&buf, "GIT_TRACE_");
+		while (*key)
+			strbuf_addch(&buf, toupper(*key++));
+
+		/*
+		 * Environment always takes precedence over config, so do not
+		 * override existing variables. We cannot rely on setenv()'s
+		 * overwrite flag here, because we may pass the list off to
+		 * a spawn() implementation, which always overwrites.
+		 */
+		if (!getenv(buf.buf))
+			argv_array_pushf(data->out, "%s=%s", buf.buf, value);
+
+		strbuf_release(&buf);
+	}
+	return 0;
+}
+
+void trace_config_for(const char *cmd, struct argv_array *out)
+{
+	struct trace_config_data data;
+	data.want_cmd = cmd;
+	data.out = out;
+	git_config(trace_config_cb, &data);
+}
diff --git a/trace.h b/trace.h
index 179b249..83618e9 100644
--- a/trace.h
+++ b/trace.h
@@ -4,6 +4,8 @@
 #include "git-compat-util.h"
 #include "strbuf.h"
 
+struct argv_array;
+
 struct trace_key {
 	const char * const key;
 	int fd;
@@ -20,6 +22,13 @@ extern uint64_t getnanotime(void);
 extern void trace_command_performance(const char **argv);
 extern void trace_verbatim(struct trace_key *key, const void *buf, unsigned len);
 
+/**
+ * Load any trace-related config for git command "cmd", and insert the matching
+ * environment variables into "out", which is suitable for use by run-command
+ * and friends.
+ */
+void trace_config_for(const char *cmd, struct argv_array *out);
+
 #ifndef HAVE_VARIADIC_MACROS
 
 __attribute__((format (printf, 1, 2)))
-- 
2.4.3.699.g84b4da7