Re: [PATCH v4 3/3] config: allow overriding of global and system configuration

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1727 bytes --]

On Tue, Apr 13, 2021 at 03:33:08AM -0400, Jeff King wrote:
> On Tue, Apr 13, 2021 at 09:11:52AM +0200, Patrick Steinhardt wrote:
> 
> > +test_expect_success 'override global and system config with missing file' '
> > +	sane_unset GIT_CONFIG_NOSYSTEM &&
> > +	test_must_fail env GIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=/dev/null git config --global --list >actual &&
> > +	test_must_fail env GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=does-not-exist git config --system --list >actual &&
> > +	GIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=does-not-exist git version
> > +'
> 
> I was slightly surprised to see these still marked as test_must_fail.
> But it's because git-config, when given a _specific_ file to read, will
> complain if the file doesn't exist. And that is true independent of your
> patch.

Yeah, surprising at first but then again it does feel sensible when
thinking about it: we ask git-config(1) to explitly list the
global/system-level configuration, and when it does not exist it does
feel sane to complain.

> There is one interesting implication there. Running:
> 
>   GIT_CONFIG_SYSTEM=/dev/null git config --system --list
> 
> does _not_ complain, even though:
> 
>   GIT_CONFIG_NOSYSTEM=1 git config --system --list
> 
> does. IMHO that is quite sensible, but I wanted to point it out, as
> using /dev/null is not an exact replacement for GIT_CONFIG_NOSYSTEM (in
> my opinion it is even better ;) ).

Whereas this is the other edge case: we do ask it to explicitly list the
system-level configuration, and it _does_ exist even though it's empty.
I wouldn't expect to get an error here, and I do prefer this behaviour
to NOSYSTEM, too.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]