On Tue, Mar 22, 2016 at 2:43 PM, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote: > 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@xxxxxxxxx> > > I should have mentioned this is > > Reported-by: Shin Fan <shinfan@xxxxxxxxxx> > >> Signed-off-by: Jeff King <peff@xxxxxxxx> >> --- >> 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@xxxxxxxxx> > Tested-by: Jonathan Nieder <jrnieder@xxxxxxxxx> > > Thanks for the quick fix. > > Sincerely, > Jonathan > > [1] http://thread.gmane.org/gmane.comp.version-control.git/287928/focus=289551 Yep, thanks Jeff. This looked fine to me. Regards, Jake -- 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