git_config_push_parameter: handle empty GIT_CONFIG_PARAMETERS

The "git -c var=value" option stuffs the config value into
$GIT_CONFIG_PARAMETERS, so that sub-processes can see it.
When the config is later read via git_config() or similar,
we parse it back out of that variable.  The parsing end is a
little bit picky; it assumes that each entry was generated
with sq_quote_buf(), and that there is no extraneous
whitespace.

On the generating end, we are careful to append to an
existing $GIT_CONFIG_PARAMETERS variable if it exists.
However, our test for "should we add a space separator" is
too liberal: it will add one even if the environment
variable exists but is empty. As a result, you might end up
with:

   GIT_CONFIG_PARAMETERS=" 'core.foo=bar'"

which the parser will choke on.

This was hard to trigger in older versions of git, since we
only set the variable when we had something to put into it
(though you could certainly trigger it manually). But since
14111fc (git: submodule honor -c credential.* from command
line, 2016-02-29), the submodule code will unconditionally
put the $GIT_CONFIG_PARAMETERS variable into the environment
of any operation in the submodule, whether it is empty or
not. So any of those operations which themselves use "git
-c" will generate the unparseable value and fail.

We can easily fix it by catching this case on the generating
side. While we're adding a test, let's also check that
multiple layers of "git -c" work, which was previously not
tested at all.

Reported-by: Shin Fan <shinfan@google.com>
Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Tested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

2 files changed, 15 insertions, 1 deletions

diff --git a/config.c b/config.c
index b95ac3a9cd..8570eec51d 100644
--- a/config.c
+++ b/config.c
@@ -161,7 +161,7 @@ void git_config_push_parameter(const char *text)
 {
 	struct strbuf env = STRBUF_INIT;
 	const char *old = getenv(CONFIG_DATA_ENVIRONMENT);
-	if (old) {
+	if (old && *old) {
 		strbuf_addstr(&env, old);
 		strbuf_addch(&env, ' ');
 	}
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 52678e7d0a..c967740304 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1083,6 +1083,20 @@ test_expect_success 'git -c complains about empty key and value' '
 	test_must_fail git -c "" rev-parse
 '
 
+test_expect_success 'multiple git -c appends config' '
+	test_config alias.x "!git -c x.two=2 config --get-regexp ^x\.*" &&
+	cat >expect <<-\EOF &&
+	x.one 1
+	x.two 2
+	EOF
+	git -c x.one=1 x >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'git -c is not confused by empty environment' '
+	GIT_CONFIG_PARAMETERS="" git -c x.one=1 config --list
+'
+
 test_expect_success 'git config --edit works' '
 	git config -f tmp test.value no &&
 	echo test.value=yes >expect &&