Re: [PATCH v2 3/4] run-command: move envvar-resetting function

2 messages, 2 authors, 2021-06-09 · open the first message on its own page

Re: [PATCH v2 3/4] run-command: move envvar-resetting function

From: Junio C Hamano <hidden>
Date: 2021-06-08 04:14:34

Jonathan Tan [off-list ref] writes:
quoted hunk
There is a function that resets environment variables, used when
invoking a sub-process in a submodule. The lazy-fetching code (used in
partial clones) will need this function in a subsequent commit, so move
it to a more central location.

Signed-off-by: Jonathan Tan <redacted>
---
 run-command.c | 10 ++++++++++
 run-command.h |  9 +++++++++
 submodule.c   | 14 ++------------
 3 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/run-command.c b/run-command.c
index be6bc128cd..a6c458119c 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1892,3 +1892,13 @@ int run_auto_maintenance(int quiet)
 
 	return run_command(&maint);
 }
+
+void prepare_other_repo_env(struct strvec *env_array)
+{
+	const char * const *var;
+
+	for (var = local_repo_env; *var; var++) {
+		if (strcmp(*var, CONFIG_DATA_ENVIRONMENT))
+			strvec_push(env_array, *var);
+	}
+}
It does make sense to move this to run-command.c from submodule.c
and the function name is already suitable for being global.  I
however cannot help wondering if we should also pay attention to the
GIT_CONFIG_KEY_$n and GIT_CONFIG_VALUE_$n pairs (which is not a new
problem in this patch).

This helper may sit better next to prep_childenv(), instead of just
saying "the location does not matter, just append randomly at the
end", though.

Otherwise looking good.

Thanks.


quoted hunk
diff --git a/run-command.h b/run-command.h
index d08414a92e..a1d9107f5b 100644
--- a/run-command.h
+++ b/run-command.h
@@ -483,4 +483,13 @@ int run_processes_parallel_tr2(int n, get_next_task_fn, start_failure_fn,
 			       task_finished_fn, void *pp_cb,
 			       const char *tr2_category, const char *tr2_label);
 
+/**
+ * Convenience function which adds all GIT_* environment variables to env_array
+ * with the exception of GIT_CONFIG_PARAMETERS. When used as the env_array of a
+ * subprocess, these entries cause the corresponding environment variables to
+ * be unset in the subprocess. See local_repo_env in cache.h for more
+ * information.
+ */
+void prepare_other_repo_env(struct strvec *env_array);
+
 #endif
diff --git a/submodule.c b/submodule.c
index 0b1d9c1dde..a30216db52 100644
--- a/submodule.c
+++ b/submodule.c
@@ -484,26 +484,16 @@ static void print_submodule_diff_summary(struct repository *r, struct rev_info *
 	strbuf_release(&sb);
 }
 
-static void prepare_submodule_repo_env_no_git_dir(struct strvec *out)
-{
-	const char * const *var;
-
-	for (var = local_repo_env; *var; var++) {
-		if (strcmp(*var, CONFIG_DATA_ENVIRONMENT))
-			strvec_push(out, *var);
-	}
-}
-
 void prepare_submodule_repo_env(struct strvec *out)
 {
-	prepare_submodule_repo_env_no_git_dir(out);
+	prepare_other_repo_env(out);
 	strvec_pushf(out, "%s=%s", GIT_DIR_ENVIRONMENT,
 		     DEFAULT_GIT_DIR_ENVIRONMENT);
 }
 
 static void prepare_submodule_repo_env_in_gitdir(struct strvec *out)
 {
-	prepare_submodule_repo_env_no_git_dir(out);
+	prepare_other_repo_env(out);
 	strvec_pushf(out, "%s=.", GIT_DIR_ENVIRONMENT);
 }

Re: [PATCH v2 3/4] run-command: move envvar-resetting function

From: Jonathan Tan <hidden>
Date: 2021-06-09 04:32:58

It does make sense to move this to run-command.c from submodule.c
and the function name is already suitable for being global.  I
however cannot help wondering if we should also pay attention to the
GIT_CONFIG_KEY_$n and GIT_CONFIG_VALUE_$n pairs (which is not a new
problem in this patch).
Note that I changed the function name (the previous one was too
submodule-specific). As for the config pairs, they are currently being
passed through - do you have a situation in mind in which they should
not be passed through?
This helper may sit better next to prep_childenv(), instead of just
saying "the location does not matter, just append randomly at the
end", though.

Otherwise looking good.

Thanks.
Thanks - I'll move it.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help