Re: [PATCH v3 3/5] submodule: refrain from filtering GIT_CONFIG_COUNT

[Jonathan Tan <jonathantanmy@google.com>]

> > 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

As Peff describes, if an envvar is present in the list, it becomes
unset. (Perhaps confusingly, if an string of the form "ENVVAR=VALUE"
(note the "=") is present in the list, it becomes set to the given
value.) So in order to *not* filter out the envvar from the subprocess,
we need to filter out the envvar from env_array.

If you can think of a better way to document this, please let me know.
One way I thought of that might reduce confusion is for this function to
take the struct child_process directly. I don't like taking the whole
struct when we're just modifying env_array, but I think that this
becomes easier to document (just say that we're unsetting all these
envvars from the child process, and in the function body, say that to
unset a variable, we need to make it appear without a "=" in env_array).