On Thu, Jun 10, 2021 at 02:13:49PM -0700, Elijah Newren wrote:
> > diff --git a/submodule.c b/submodule.c
> > index 0b1d9c1dde..f09031e397 100644
> > --- a/submodule.c
> > +++ b/submodule.c
> > @@ -489,7 +489,8 @@ 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))
> > + if (strcmp(*var, CONFIG_DATA_ENVIRONMENT) &&
> > + strcmp(*var, CONFIG_COUNT_ENVIRONMENT))
> > strvec_push(out, *var);
> > }
> > }
> > --
> > 2.32.0.rc1.229.g3e70b5a671-goog
>
> I'm super confused. It appears that
> prepare_submodule_repo_env_no_git_dir() is filtering out
> "GIT_CONFIG_PARAMETERS" (CONFIG_DATA_ENVIRONMENT) and
> "GIT_CONFIG_COUNT" (CONFIG_COUNT_ENVIRONMENT), using all environment
> variables other than these ones. But the commit message talks about
> adding an extra environment variable, rather than filtering another
> out. I must be mis-reading something somewhere, but I'm struggling to
> figure it out.
I think there might be a double (triple?) negative here:
- we want to pass through the config parameters variable, but not
other local repo env variables;
- so we _don't_ want the config variable to appear in the "out"
strvec, because its presence would cause it to be cleared
from the child process environment;
- so we go through the list adding everything _except_ that variable;
- and we match using strcmp(), so a true value means "did not match",
so we should add it to the list
> Also, from looking at the other commit messages you reference, it
> appears GIT_CONFIG_PARAMETERS was just one big environment variable,
> whereas GIT_CONFIG_COUNT is closely associated with 2*N other
> environment variables...so shouldn't your loop (and perhaps also
> git-submodule.sh) also be checking GIT_CONFIG_KEY_\d+ and
> GIT_CONFIG_VALUE_\d+ ?
We definitely could clean out those GIT_CONFIG_KEY_* values. But the
COUNT serves as a master parameter. Anybody who sets COUNT would then
also set the individual key/value parameters, too (and even it only sets
it to "5", and there is a crufty GIT_CONFIG_KEY_6 in the environment,
that is not wrong).
-Peff
