Merge branch 'nd/trace-with-env'
authorJunio C Hamano <gitster@pobox.com>
Tue, 13 Feb 2018 21:39:10 +0000 (13:39 -0800)
committerJunio C Hamano <gitster@pobox.com>
Tue, 13 Feb 2018 21:39:10 +0000 (13:39 -0800)
The tracing machinery learned to report tweaking of environment
variables as well.

* nd/trace-with-env:
run-command.c: print new cwd in trace_run_command()
run-command.c: print env vars in trace_run_command()
run-command.c: print program 'git' when tracing git_cmd mode
run-command.c: introduce trace_run_command()
trace.c: move strbuf_release() out of print_trace_line()
trace: avoid unnecessary quoting
sq_quote_argv: drop maxlen parameter

builtin/am.c
builtin/rev-parse.c
quote.c
quote.h
run-command.c
t/helper/test-run-command.c
t/t0061-run-command.sh
trace.c
index acfe9d3c8cd6dbd8362c17d55dc036ce6682ce8a..5bdd2d75781076636f93800520e9ed161295724f 100644 (file)
@@ -1061,7 +1061,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format,
        }
        write_state_text(state, "scissors", str);
 
-       sq_quote_argv(&sb, state->git_apply_opts.argv, 0);
+       sq_quote_argv(&sb, state->git_apply_opts.argv);
        write_state_text(state, "apply-opt", sb.buf);
 
        if (state->rebasing)
index 74aa644cbb37b5741c521eba53d0a040aaf1582e..96d06a5d01d6189a647baae4f8c401d8435fff37 100644 (file)
@@ -516,7 +516,7 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
                        PARSE_OPT_SHELL_EVAL);
 
        strbuf_addstr(&parsed, " --");
-       sq_quote_argv(&parsed, argv, 0);
+       sq_quote_argv(&parsed, argv);
        puts(parsed.buf);
        return 0;
 }
@@ -526,7 +526,7 @@ static int cmd_sq_quote(int argc, const char **argv)
        struct strbuf buf = STRBUF_INIT;
 
        if (argc)
-               sq_quote_argv(&buf, argv, 0);
+               sq_quote_argv(&buf, argv);
        printf("%s\n", buf.buf);
        strbuf_release(&buf);
 
diff --git a/quote.c b/quote.c
index de2922ddd63d6fc001822d116387c8ced7d7630d..37d26868656fe8c8c65de41580ab5ae9e89981fc 100644 (file)
--- a/quote.c
+++ b/quote.c
@@ -43,6 +43,22 @@ void sq_quote_buf(struct strbuf *dst, const char *src)
        free(to_free);
 }
 
+void sq_quote_buf_pretty(struct strbuf *dst, const char *src)
+{
+       static const char ok_punct[] = "+,-./:=@_^";
+       const char *p;
+
+       for (p = src; *p; p++) {
+               if (!isalpha(*p) && !isdigit(*p) && !strchr(ok_punct, *p)) {
+                       sq_quote_buf(dst, src);
+                       return;
+               }
+       }
+
+       /* if we get here, we did not need quoting */
+       strbuf_addstr(dst, src);
+}
+
 void sq_quotef(struct strbuf *dst, const char *fmt, ...)
 {
        struct strbuf src = STRBUF_INIT;
@@ -56,7 +72,7 @@ void sq_quotef(struct strbuf *dst, const char *fmt, ...)
        strbuf_release(&src);
 }
 
-void sq_quote_argv(struct strbuf *dst, const char** argv, size_t maxlen)
+void sq_quote_argv(struct strbuf *dst, const char **argv)
 {
        int i;
 
@@ -65,8 +81,16 @@ void sq_quote_argv(struct strbuf *dst, const char** argv, size_t maxlen)
        for (i = 0; argv[i]; ++i) {
                strbuf_addch(dst, ' ');
                sq_quote_buf(dst, argv[i]);
-               if (maxlen && dst->len > maxlen)
-                       die("Too many or long arguments");
+       }
+}
+
+void sq_quote_argv_pretty(struct strbuf *dst, const char **argv)
+{
+       int i;
+
+       for (i = 0; argv[i]; i++) {
+               strbuf_addch(dst, ' ');
+               sq_quote_buf_pretty(dst, argv[i]);
        }
 }
 
diff --git a/quote.h b/quote.h
index 66f5644aa29d0da4f95e693429ad6f8c0eb8cf09..ea992dcc91ef599b5d053b00ae3a036ce5e87c1a 100644 (file)
--- a/quote.h
+++ b/quote.h
@@ -30,9 +30,17 @@ struct strbuf;
  */
 
 extern void sq_quote_buf(struct strbuf *, const char *src);
-extern void sq_quote_argv(struct strbuf *, const char **argv, size_t maxlen);
+extern void sq_quote_argv(struct strbuf *, const char **argv);
 extern void sq_quotef(struct strbuf *, const char *fmt, ...);
 
+/*
+ * These match their non-pretty variants, except that they avoid
+ * quoting when there are no exotic characters. These should only be used for
+ * human-readable output, as sq_dequote() is not smart enough to dequote it.
+ */
+void sq_quote_buf_pretty(struct strbuf *, const char *src);
+void sq_quote_argv_pretty(struct strbuf *, const char **argv);
+
 /* This unwraps what sq_quote() produces in place, but returns
  * NULL if the input does not look like what sq_quote would have
  * produced.
index 31fc5ea86eb6a3fe3650b8ac50f19f8621d90b9d..a483d5904a3ec1acae8908dd2e699fa00bcaaa9d 100644 (file)
@@ -6,6 +6,7 @@
 #include "thread-utils.h"
 #include "strbuf.h"
 #include "string-list.h"
+#include "quote.h"
 
 void child_process_init(struct child_process *child)
 {
@@ -556,6 +557,90 @@ static int wait_or_whine(pid_t pid, const char *argv0, int in_signal)
        return code;
 }
 
+static void trace_add_env(struct strbuf *dst, const char *const *deltaenv)
+{
+       struct string_list envs = STRING_LIST_INIT_DUP;
+       const char *const *e;
+       int i;
+       int printed_unset = 0;
+
+       /* Last one wins, see run-command.c:prep_childenv() for context */
+       for (e = deltaenv; e && *e; e++) {
+               struct strbuf key = STRBUF_INIT;
+               char *equals = strchr(*e, '=');
+
+               if (equals) {
+                       strbuf_add(&key, *e, equals - *e);
+                       string_list_insert(&envs, key.buf)->util = equals + 1;
+               } else {
+                       string_list_insert(&envs, *e)->util = NULL;
+               }
+               strbuf_release(&key);
+       }
+
+       /* "unset X Y...;" */
+       for (i = 0; i < envs.nr; i++) {
+               const char *var = envs.items[i].string;
+               const char *val = envs.items[i].util;
+
+               if (val || !getenv(var))
+                       continue;
+
+               if (!printed_unset) {
+                       strbuf_addstr(dst, " unset");
+                       printed_unset = 1;
+               }
+               strbuf_addf(dst, " %s", var);
+       }
+       if (printed_unset)
+               strbuf_addch(dst, ';');
+
+       /* ... followed by "A=B C=D ..." */
+       for (i = 0; i < envs.nr; i++) {
+               const char *var = envs.items[i].string;
+               const char *val = envs.items[i].util;
+               const char *oldval;
+
+               if (!val)
+                       continue;
+
+               oldval = getenv(var);
+               if (oldval && !strcmp(val, oldval))
+                       continue;
+
+               strbuf_addf(dst, " %s=", var);
+               sq_quote_buf_pretty(dst, val);
+       }
+       string_list_clear(&envs, 0);
+}
+
+static void trace_run_command(const struct child_process *cp)
+{
+       struct strbuf buf = STRBUF_INIT;
+
+       if (!trace_want(&trace_default_key))
+               return;
+
+       strbuf_addf(&buf, "trace: run_command:");
+       if (cp->dir) {
+               strbuf_addstr(&buf, " cd ");
+               sq_quote_buf_pretty(&buf, cp->dir);
+               strbuf_addch(&buf, ';');
+       }
+       /*
+        * The caller is responsible for initializing cp->env from
+        * cp->env_array if needed. We only check one place.
+        */
+       if (cp->env)
+               trace_add_env(&buf, cp->env);
+       if (cp->git_cmd)
+               strbuf_addstr(&buf, " git");
+       sq_quote_argv_pretty(&buf, cp->argv);
+
+       trace_printf("%s", buf.buf);
+       strbuf_release(&buf);
+}
+
 int start_command(struct child_process *cmd)
 {
        int need_in, need_out, need_err;
@@ -624,7 +709,8 @@ int start_command(struct child_process *cmd)
                cmd->err = fderr[0];
        }
 
-       trace_argv_printf(cmd->argv, "trace: run_command:");
+       trace_run_command(cmd);
+
        fflush(NULL);
 
 #ifndef GIT_WINDOWS_NATIVE
index d24d157379f30cafdeeb772e82bf5ee4c862272c..153342e44dd11ae357cc299a9214f4c365614a5e 100644 (file)
@@ -54,6 +54,15 @@ int cmd_main(int argc, const char **argv)
        struct child_process proc = CHILD_PROCESS_INIT;
        int jobs;
 
+       if (argc < 3)
+               return 1;
+       while (!strcmp(argv[1], "env")) {
+               if (!argv[2])
+                       die("env specifier without a value");
+               argv_array_push(&proc.env_array, argv[2]);
+               argv += 2;
+               argc -= 2;
+       }
        if (argc < 3)
                return 1;
        proc.argv = (const char **)argv + 2;
index e4739170aa2b7c833cd51a06a7ac6764c8bf0494..24c92b6cd7b1c54eb6541a81abd7e5812b3b99b0 100755 (executable)
@@ -141,4 +141,41 @@ test_expect_success 'run_command outputs ' '
        test_cmp expect actual
 '
 
+test_trace () {
+       expect="$1"
+       shift
+       GIT_TRACE=1 test-run-command "$@" run-command true 2>&1 >/dev/null | \
+               sed 's/.* run_command: //' >actual &&
+       echo "$expect true" >expect &&
+       test_cmp expect actual
+}
+
+test_expect_success 'GIT_TRACE with environment variables' '
+       test_trace "abc=1 def=2" env abc=1 env def=2 &&
+       test_trace "abc=2" env abc env abc=1 env abc=2 &&
+       test_trace "abc=2" env abc env abc=2 &&
+       (
+               abc=1 && export abc &&
+               test_trace "def=1" env abc=1 env def=1
+       ) &&
+       (
+               abc=1 && export abc &&
+               test_trace "def=1" env abc env abc=1 env def=1
+       ) &&
+       test_trace "def=1" env non-exist env def=1 &&
+       test_trace "abc=2" env abc=1 env abc env abc=2 &&
+       (
+               abc=1 def=2 && export abc def &&
+               test_trace "unset abc def;" env abc env def
+       ) &&
+       (
+               abc=1 def=2 && export abc def &&
+               test_trace "unset def; abc=3" env abc env def env abc=3
+       ) &&
+       (
+               abc=1 && export abc &&
+               test_trace "unset abc;" env abc=2 env abc
+       )
+'
+
 test_done
diff --git a/trace.c b/trace.c
index b7530b51a9e4d20825d42fcfd2a5dd389ad3b44e..7f3b08e148044c6c94cbef03ae265e134391357a 100644 (file)
--- a/trace.c
+++ b/trace.c
@@ -131,7 +131,6 @@ static void print_trace_line(struct trace_key *key, struct strbuf *buf)
 {
        strbuf_complete_line(buf);
        trace_write(key, buf->buf, buf->len);
-       strbuf_release(buf);
 }
 
 static void trace_vprintf_fl(const char *file, int line, struct trace_key *key,
@@ -144,6 +143,7 @@ static void trace_vprintf_fl(const char *file, int line, struct trace_key *key,
 
        strbuf_vaddf(&buf, format, ap);
        print_trace_line(key, &buf);
+       strbuf_release(&buf);
 }
 
 static void trace_argv_vprintf_fl(const char *file, int line,
@@ -157,8 +157,9 @@ static void trace_argv_vprintf_fl(const char *file, int line,
 
        strbuf_vaddf(&buf, format, ap);
 
-       sq_quote_argv(&buf, argv, 0);
+       sq_quote_argv_pretty(&buf, argv);
        print_trace_line(&trace_default_key, &buf);
+       strbuf_release(&buf);
 }
 
 void trace_strbuf_fl(const char *file, int line, struct trace_key *key,
@@ -171,6 +172,7 @@ void trace_strbuf_fl(const char *file, int line, struct trace_key *key,
 
        strbuf_addbuf(&buf, data);
        print_trace_line(key, &buf);
+       strbuf_release(&buf);
 }
 
 static void trace_performance_vprintf_fl(const char *file, int line,
@@ -190,6 +192,7 @@ static void trace_performance_vprintf_fl(const char *file, int line,
        }
 
        print_trace_line(&trace_perf_key, &buf);
+       strbuf_release(&buf);
 }
 
 #ifndef HAVE_VARIADIC_MACROS
@@ -426,6 +429,6 @@ void trace_command_performance(const char **argv)
                atexit(print_command_performance_atexit);
 
        strbuf_reset(&command_line);
-       sq_quote_argv(&command_line, argv, 0);
+       sq_quote_argv_pretty(&command_line, argv);
        command_start_time = getnanotime();
 }