Subject: Pass or not to pass config environment down...

From: Junio C Hamano <[email-masked]>
Date: Tue, 23 Mar 2021 11:57:57 -0700

I was grepping around and found this piece of code today:

        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);
                }
        }

which tries to "unsetenv" the environment variables that pertain to
the current repository listed in loocal_repo_env[], but makes
exception for GIT_CONFIG_PARAMETERS.

It originally came from 14111fc4 (git: submodule honor -c
credential.* from command line, 2016-02-29) and later simplified by
89044baa (submodule: stop sanitizing config options, 2016-05-04).

Now after d8d77153 (config: allow specifying config entries via
envvar pairs, 2021-01-12), we have yet another way to pass a set of
custom one-shot configuration via the environment variable, using
GIT_CONFIG_COUNT (which is in local_repo_env[] and will be removed
from the environment by this helper function), GIT_CONFIG_KEY_$n and
GIT_CONFIG_VALUE_$n (which are unbound set and naturally not in
local_repo_env[]).  Leaving the latter two exported will not hurt if
we do intend to hide the custom configuration from the subprocess by
unsetting GIT_CONFIG_COUNT, but should we be doing so?

There are many run_command() users that just pass local_repo_env[]
to the child.env when running a subprocess.  Given that the code
that works in a submodule, which presumably is THE primary target
of the "we do not want to pass environment variables that pertain to
the current repository but not to the repository the child process
works in" consideration that the local_repo_env[] is about, does *not*
want the GIT_CONFIG_PARAMETERS cleansed, I have to wonder if the
environment variables (the original GIT_CONFIG_PARAMETERS as well as
Patrick's GIT_CONFIG_{COUNT,KEY_$n,VALUE_$n}) should be in that
local_repo_env[] array in the first place.  If we remove them, the
above helper function can just go away and be replaced with the
usual child.env = local_repo_env assignment like everybody else.

Comments?

 environment.c | 2 --
 1 file changed, 2 deletions(-)

diff --git c/environment.c w/environment.c
index 2f27008424..6dcee6a9c5 100644
--- c/environment.c
+++ w/environment.c
@@ -116,8 +116,6 @@ static char *super_prefix;
 const char * const local_repo_env[] = {
 	ALTERNATE_DB_ENVIRONMENT,
 	CONFIG_ENVIRONMENT,
-	CONFIG_DATA_ENVIRONMENT,
-	CONFIG_COUNT_ENVIRONMENT,
 	DB_ENVIRONMENT,
 	GIT_DIR_ENVIRONMENT,
 	GIT_WORK_TREE_ENVIRONMENT,