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

From: Elijah Newren <newren@gmail.com>
Date: Jun 10, 2021, 21:13 UTC
Message-ID: <CABPp-BFpW7KLQGbe1kz5FNWABrTNaHOCZw3poXBMWQ9MP91GuQ@mail.gmail.com>
In-Reply-To: <e1a40108f42addf8a589c1d0ac4ed76fb525be9b.1623345496.git.jonathantanmy@google.com>

Adding Peff to cc as per comments about 89044baa8b ("submodule: stop sanitizing config options", 2016-05-04) below.

On Thu, Jun 10, 2021 at 10:35 AM Jonathan Tan <jonathantanmy@google.com> wrote:

Show 35 quoted lines

>
> 14111fc492 ("git: submodule honor -c credential.* from command line",
> 2016-03-01) taught Git to pass through the GIT_CONFIG_PARAMETERS
> environment variable when invoking a subprocess on behalf of a
> submodule. But when d8d77153ea ("config: allow specifying config entries
> via envvar pairs", 2021-01-15) introduced support for GIT_CONFIG_COUNT
> (and its associated GIT_CONFIG_KEY_? and GIT_CONFIG_VALUE_?), the
> subprocess mechanism wasn't updated to also pass through these
> variables.
>
> Since they are conceptually the same (d8d77153ea was written to address
> a shortcoming of GIT_CONFIG_PARAMETERS), update the submodule subprocess
> mechanism to also pass through GIT_CONFIG_COUNT.
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
>  submodule.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> 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.

Digging around for a while led me to commit 89044baa8b ("submodule: stop sanitizing config options", 2016-05-04), which suggests that the passing of GIT_CONFIG_PARAMETERS is not done here but in git-submodule.sh. It still didn't make it clear to me why it's stripped out here, but something makes me thing that git-submodule.sh should be affected by your change as well.

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

I've been looking at this patch longer than I care to admit and I still feel like I don't have a clue what's going on.