Re: [PATCH v2] git: submodule honor -c credential.* from command line
- From
Jeff King <peff@peff.net>
- Date
- Feb 25, 2016, 07:00 UTC
- Message-ID
- <20160225070036.GA5654@sigill.intra.peff.net>
- In-Reply-To
- <CA+P7+xpfuPkwcdeXVTiTdqRCX16J4pus-wmxe7Sipu_GXCNSoA@mail.gmail.com>
On Wed, Feb 24, 2016 at 10:23:28PM -0800, Jacob Keller wrote:
Show 15 quoted lines
> >> +# Sanitize the local git environment for use within a submodule. We
> >> +# can't simply use clear_local_git_env since we want to preserve some
> >> +# of the settings from GIT_CONFIG_PARAMETERS.
> >> +sanitize_local_git_env()
> >> +{
> >> + local sanitized_config = $(git submodule--helper sanitize-config)
> >> + clear_local_git_env
> >> + GIT_CONFIG_PARAMETERS=$sanitized_config
> >> +}
> >
> > Do we need to export GIT_CONFIG_PARAMETERS? I guess not; if it is
> > already exported, we don't need, and if it isn't, then by definition
> > $sanitized_config will be empty.
> >
> How does modifying an exported variable work?Generally, variables which came to the shell from the environment are marked for export, and modifying a marked-for-export variable will not change its export flag.
I have a nagging feeling that there was some shell deep in the past where that was not the case, but I can't find any mention of it. So either I dreamed it, or it is so old and broken that even the autoconf portability page does not bother with it. ;)
Show 8 quoted lines
> I 100% agree. I think the test file is useful for now, and there are > (currently) no other tests for submodule--helper, so I'd like to get > them all confined to this test. I think we need a real way to test the > change here, but I think figuring out how to test the > credential.helper is a bit outside the scope of what i had time for > today. I can try to find some cycles to check out tomorrow. You > mentioned we'd need a test in the same idea as one of the http clone > tests? I don't know where to start with something like this though.
I think something like this would work:
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh index 6414635..c5ce8ff 100755 --- a/t/t5550-http-fetch-dumb.sh +++ b/t/t5550-http-fetch-dumb.sh @@ -91,6 +91,20 @@ test_expect_success 'configured username does not override URL' ' expect_askpass pass user@host ' +test_expect_success 'cmdline credential config passes to submodules' ' + git init super && + set_askpass user@host pass@host && + ( + cd super && + git submodule add "$HTTPD_URL/auth/dumb/repo.git" sub && + git commit -m "add submodule" + ) && + set_askpass wrong pass@host && + git -c "credential.$HTTPD_URL.username=user@host" \ + clone --recursive super super-clone && + expect_askpass pass user@host +' + test_expect_success 'fetch changes via http' ' echo content >>file && git commit -a -m two && but it does not seem to pass with your patch (even after I fixed up the weird "local" thing). I think the problem is that we ask submodule--helper to do the clone, and it uses local_repo_env. So in addition to your patch, you probably need a C version of the same thing which outputs to an argv_array. -Peff