Re: git submodule should honor "-c credential.helper" command line argument

From: Jacob Keller <jacob.keller@gmail.com>
Date: Feb 19, 2016, 07:29 UTC
Message-ID: <CA+P7+xr9dLOyaVky1jvXm7MkF0JUqs5SadrTdr4o83baMp397A@mail.gmail.com>
In-Reply-To: <20160219043019.GA14764@sigill.intra.peff.net>

On Thu, Feb 18, 2016 at 8:30 PM, Jeff King <peff@peff.net> wrote:

Show 38 quoted lines

> On Thu, Feb 18, 2016 at 05:15:54PM -0800, Jacob Keller wrote:
>
>> I am looking at this more and I am stuck as to how best to provide a
>> test case.
>>
>> I think the problem as stated above is pretty straight forward, we
>> just want to stop clearing GIT_CONFIG_PARAMETERS but I can't find an
>> easy way to test that we've done the right thing. There are no current
>> tests for using a credential helper with submodule update right now.
>
> If you just want to test that GIT_CONFIG_PARAMETERS is left untouched in
> the submodule, you can tweak any config setting that would impact the
> newly-cloned repo. E.g. this:
>
>   unset GIT_COMMITTER_NAME
>   git config --global user.name='Global Name'
>   git -c user.name='Command-Line Name' clone repo-with-module foo
>   head -1 foo/.git/logs/HEAD
>   head -1 foo/.git/modules/sub/logs/HEAD
>
> shows that the parent-level clone uses the "-c" name, but the submodule
> does not.
>
> That being said, I am not sure this is the right solution. In the thread
> I linked earlier[1], Jens indicated he would prefer not to blindly share
> config with the submodules, and I think I agree. Or are you proposing to
> pick and choose the keys in GIT_CONFIG_PARAMETERS, and whitelist
> credential.*?
>
> In that case, obviously my test example would not work, though I think
> that it might be fine to put "user.name" onto the whitelist (the things
> we really would worry about is stuff like "core.worktree" that clearly
> does not make sense to carry over into the submodule).
>
> -Peff
>
> [1] http://thread.gmane.org/gmane.comp.version-control.git/264840
>

I would prefer to either.. blacklist stuff like core.worktree, or whitelist a bunch of stuff that makes sense. In this case though, I would prefer to have an explicit test of credential.helper, but I don't know if any of our tests actually have a solid test case for "credential.helper was used in a clone. There may not be test infrastructure for this though, so your test might work well enough.

As for how to whitelist config to share with the submodule I am really not 100% sure, since we just clear GIT_CONFIG_PARAMETERS, and I think we'd need a specialized variant of clear_local_git_env_vars specific to submodule then.

Thanks, Jake