Re: [PATCH v2 3/3] config: allow overriding of global and system configuration
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 9, 2021, 22:18 UTC
- Message-ID
- <xmqqo8enm793.fsf@gitster.g>
- In-Reply-To
- <272a3b31aa73da8d65b04e647b1b9ca860f4e783.1617975637.git.ps@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 28 quoted lines
> 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.