Jeff King <peff@xxxxxxxx> writes:
> I also think this is a special case of a more general problem. FOO could
> appear any number of times in the "env" array, as a deletion or with
> multiple values. Our prep_childenv() would treat that as "last one
> wins", I think. Could we just do the same here?
Perhaps this should be squashed into the original 4/4 instead of
being a separate patch. We'd probably want some sort of test, I
wonder? Not tested at all beyond compiling...
-- >8 --
Subject: [PATCH 7/4] run-command.c: don't be too cute in concatenate_env()
Instead of relying on "sort" being stable to sort "unset VAR"
immediately before "VAR=VAL" to remove the former, just pick the
last manipulation for each VAR from the list of environment tweaks
and show them in the output.
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
trace.c | 68 ++++++++++++++++++++---------------------------------------------
1 file changed, 21 insertions(+), 47 deletions(-)
diff --git a/trace.c b/trace.c
index aba2825044..9f49bcdd03 100644
--- a/trace.c
+++ b/trace.c
@@ -272,76 +272,50 @@ void trace_performance_fl(const char *file, int line, uint64_t nanos,
#endif /* HAVE_VARIADIC_MACROS */
-static void sort_deltaenv(struct string_list *envs,
- const char *const *deltaenv)
+static void concatenate_env(struct strbuf *dst, const char *const *deltaenv)
{
- struct strbuf key = STRBUF_INIT;
+ struct string_list envs = STRING_LIST_INIT_DUP;
const char *const *e;
+ int i;
+ /* Last one wins... */
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);
- string_list_append(envs, key.buf)->util = equals + 1;
+ string_list_insert(&envs, key.buf)->util = equals + 1;
} else {
- string_list_append(envs, *e)->util = NULL;
+ string_list_insert(&envs, *e)->util = NULL;
}
}
- string_list_sort(envs);
- strbuf_release(&key);
-}
-
-
-static void concatenate_env(struct strbuf *dst, const char *const *deltaenv)
-{
- struct string_list envs = STRING_LIST_INIT_DUP;
- int i;
-
- /*
- * Construct a sorted string list consisting of the delta
- * env. We need this to detect the case when the same var is
- * deleted first, then added again.
- */
- sort_deltaenv(&envs, deltaenv);
- /*
- * variable deletion first because it's printed like separate
- * shell commands
- */
+ /* series of "unset X; unset Y;..." */
for (i = 0; i < envs.nr; i++) {
- const char *env = envs.items[i].string;
- const char *p = envs.items[i].util;
+ const char *var = envs.items[i].string;
+ const char *val = envs.items[i].util;
- if (p || !getenv(env))
+ if (val)
continue;
-
- /*
- * Do we have a sequence of "unset GIT_DIR; GIT_DIR=foo"?
- * Then don't bother with the unset thing.
- */
- if (i + 1 < envs.nr &&
- !strcmp(env, envs.items[i + 1].string))
- continue;
-
- strbuf_addf(dst, " unset %s;", env);
+ if (getenv(var))
+ strbuf_addf(dst, " unset %s;", var);
}
+ /* ... followed by "A=B C=D ..." */
for (i = 0; i < envs.nr; i++) {
- const char *env = envs.items[i].string;
- const char *p = envs.items[i].util;
+ const char *var = envs.items[i].string;
+ const char *val = envs.items[i].util;
const char *old_value;
- if (!p)
+ if (!val)
continue;
-
- old_value = getenv(env);
- if (old_value && !strcmp(old_value, p))
+ old_value = getenv(var);
+ if (old_value && !strcmp(old_value, val))
continue;
-
- strbuf_addf(dst, " %s=", env);
- sq_quote_buf_pretty(dst, p);
+ strbuf_addf(dst, " %s=", var);
+ sq_quote_buf_pretty(dst, val);
}
string_list_clear(&envs, 0);
}
