Re: [PATCH] git_config_push_parameter: handle empty GIT_CONFIG_PARAMETERS

> Jeff King wrote[1]:
>
>> Subject: 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: Jonathan Nieder <jrnieder@gmail.com>
>
> I should have mentioned this is
>
> Reported-by: Shin Fan <shinfan@google.com>
>
>> Signed-off-by: Jeff King <peff@peff.net>
>> ---
>> I just did this on master, and it is standalone. But for the reasons
>> above I think it would also be fine to stick on the tip of the
>> jk/submodule-c-credential topic.
>>
>>  config.c               |  2 +-
>>  t/t1300-repo-config.sh | 14 ++++++++++++++
>>  2 files changed, 15 insertions(+), 1 deletion(-)
>
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
> Tested-by: Jonathan Nieder <jrnieder@gmail.com>
>
> Thanks for the quick fix.
>
> Sincerely,
> Jonathan
>
> [1] http://thread.gmane.org/gmane.comp.version-control.git/287928/focus=289551