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

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> diff --git a/config.c b/config.c
> index 6af0244085..9dfc4a79f7 100644
> --- a/config.c
> +++ b/config.c
> @@ -1847,8 +1847,22 @@ static int git_config_from_blob_ref(config_fn_t fn,
>  const char *git_system_config(void)
>  {
>  	static const char *system_wide;
> -	if (!system_wide)
> -		system_wide = system_path(ETC_GITCONFIG);
> +
> +	if (!system_wide) {
> +		system_wide = xstrdup_or_null(getenv("GIT_CONFIG_SYSTEM"));
> +		if (system_wide) {
> +			/*
> +			 * If GIT_CONFIG_SYSTEM is set, it overrides the
> +			 * /etc/gitconfig. Furthermore, the file must exist in
> +			 * order to prevent any typos by the user.
> +			 */
> +			if (access(system_wide, R_OK))
> +				die(_("cannot access '%s'"), system_wide);
> +		} else {
> +			system_wide = system_path(ETC_GITCONFIG);
> +		}
> +	}
> +
>  	return system_wide;
>  }

I wrote the design to truly special case "/dev/null" at the pathname
string level, and we do not even try to open/read from a file when
"/dev/null" is given, so that we can even allow "/dev/null" to be
missing in a funny embedded or containerized environment, but that
does not seem to be what is going on here.

I do not think "access()" here is a good idea; the result we get
here may not match what happens when we actually try to open the
path.  Just remembering if we got the system_wide value from the
GIT_CONFIG_SYSTEM, and change what happens when the open fails when
we try to use the system-wide configuration depending on that, would
be the right approach.