git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: "Stefan Beller" <sbeller@google.com>,
	"Brandon Williams" <bmwill@google.com>,
	"Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"Johannes Sixt" <j6t@kdbg.org>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH v4 0/7] Trace env variables in run_command()
Date: Sat, 13 Jan 2018 13:49:42 +0700	[thread overview]
Message-ID: <20180113064949.6043-1-pclouds@gmail.com> (raw)
In-Reply-To: <20180112095607.18293-1-pclouds@gmail.com>

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


  parent reply	other threads:[~2018-01-13  6:50 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
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     ` Nguyễn Thái Ngọc Duy [this message]
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=20180113064949.6043-1-pclouds@gmail.com \
    --to=pclouds@gmail.com \
    --cc=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=peff@peff.net \
    --cc=sbeller@google.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).