Re: [PATCH v2] git: submodule honor -c credential.* from command line

On Wed, Feb 24, 2016 at 10:23:28PM -0800, Jacob Keller wrote:

quoted

quoted

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