v4: - incorporates Jeff patches and moves them on top - removes strbuf release from print_trace_line - prints 'unset a b c' instead of 'unset a; unset b; unset c' - squashes v3 3/4 and 4/4 and Junio's patch into 6/7 - adds tests - cwd is separated in 7/7 interdiff with what's on 'pu': diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c index d24d157379..0ab71f14fb 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -56,6 +56,10 @@ int cmd_main(int argc, const char **argv) if (argc < 3) return 1; + while (!strcmp(argv[1], "env")) { + argv_array_push(&proc.env_array, argv[2]); + argv += 2; + } proc.argv = (const char **)argv + 2; if (!strcmp(argv[1], "start-command-ENOENT")) { diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index e4739170aa..e6208316c3 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -141,4 +141,26 @@ test_expect_success 'run_command outputs ' ' test_cmp expect actual ' +test_trace() { + local expected="$1" + shift + GIT_TRACE=1 test-run-command "$@" run-command true 2>&1 >/dev/null | \ + sed 's/.* run_command: //' >actual && + echo "$expected true" >expected && + test_cmp expected 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 test_trace "def=1" env abc=1 env def=1 && + abc=1 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 test_trace "unset abc def;" env abc env def && + abc=1 def=2 test_trace "unset def; abc=3" env abc env def env abc=3 && + abc=1 test_trace "unset abc;" env abc=2 env abc +' + test_done diff --git a/trace.c b/trace.c index 9f49bcdd03..4bfd3fce10 100644 --- a/trace.c +++ b/trace.c @@ -132,7 +132,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, @@ -145,6 +144,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, @@ -160,6 +160,7 @@ static void trace_argv_vprintf_fl(const char *file, int line, 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, @@ -172,6 +173,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, @@ -191,6 +193,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 @@ -272,17 +275,17 @@ void trace_performance_fl(const char *file, int line, uint64_t nanos, #endif /* HAVE_VARIADIC_MACROS */ -static void concatenate_env(struct strbuf *dst, const char *const *deltaenv) +static void 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... */ + /* 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_reset(&key); strbuf_add(&key, *e, equals - *e); @@ -290,30 +293,39 @@ static void concatenate_env(struct strbuf *dst, const char *const *deltaenv) } else { string_list_insert(&envs, *e)->util = NULL; } + strbuf_release(&key); } - /* series of "unset X; unset Y;..." */ + /* "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) + if (val || !getenv(var)) continue; - if (getenv(var)) - strbuf_addf(dst, " unset %s;", var); + + 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 *old_value; + const char *oldval; if (!val) continue; - old_value = getenv(var); - if (old_value && !strcmp(old_value, val)) + + oldval = getenv(var); + if (oldval && !strcmp(val, oldval)) continue; + strbuf_addf(dst, " %s=", var); sq_quote_buf_pretty(dst, val); } @@ -328,8 +340,6 @@ void trace_run_command(const struct child_process *cp) &trace_default_key, &buf)) return; - strbuf_grow(&buf, 255); - strbuf_addf(&buf, "trace: run_command:"); if (cp->dir) { @@ -343,7 +353,7 @@ void trace_run_command(const struct child_process *cp) * cp->env_array if needed. We only check one place. */ if (cp->env) - concatenate_env(&buf, cp->env); + add_env(&buf, cp->env); if (cp->git_cmd) strbuf_addstr(&buf, " git"); -- 2.15.1.600.g899a5f85c6