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