Re: [PATCH] git_config_push_parameter: handle empty GIT_CONFIG_PARAMETERS

[Jonathan Nieder <jrnieder@gmail.com>]

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