On Wed, Feb 24, 2016 at 10:23:28PM -0800, Jacob Keller wrote: > >> +# 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. ;) > 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html