On Wed, Oct 18, 2023 at 10:28:40PM +0200, Kristoffer Haugsbakk wrote:
>
> Factor out code that retrieves the global config file so that we can use
> it in `gc.c` as well.
>
> Use the old name from the previous commit since this function acts
> functionally the same as `git_system_config` but for “global”.
>
> Signed-off-by: Kristoffer Haugsbakk <code@xxxxxxxxxxxxxxx>
> ---
> builtin/config.c | 25 ++-----------------------
> config.c | 24 ++++++++++++++++++++++++
> config.h | 1 +
> 3 files changed, 27 insertions(+), 23 deletions(-)
>
> diff --git a/builtin/config.c b/builtin/config.c
> index 6fff2655816..df06b766fad 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -708,30 +708,9 @@ int cmd_config(int argc, const char **argv, const char *prefix)
> }
>
> if (use_global_config) {
> - char *user_config, *xdg_config;
> -
> - git_global_config_paths(&user_config, &xdg_config);
> - if (!user_config)
> - /*
> - * It is unknown if HOME/.gitconfig exists, so
> - * we do not know if we should write to XDG
> - * location; error out even if XDG_CONFIG_HOME
> - * is set and points at a sane location.
> - */
> - die(_("$HOME not set"));
> -
> + given_config_source.file = git_global_config();
> given_config_source.scope = CONFIG_SCOPE_GLOBAL;
> -
> - if (access_or_warn(user_config, R_OK, 0) &&
> - xdg_config && !access_or_warn(xdg_config, R_OK, 0)) {
> - given_config_source.file = xdg_config;
> - free(user_config);
> - } else {
> - given_config_source.file = user_config;
> - free(xdg_config);
> - }
> - }
> - else if (use_system_config) {
> + } else if (use_system_config) {
> given_config_source.file = git_system_config();
> given_config_source.scope = CONFIG_SCOPE_SYSTEM;
> } else if (use_local_config) {
> diff --git a/config.c b/config.c
> index d2cdda96edd..2ff766c56ff 100644
> --- a/config.c
> +++ b/config.c
> @@ -2111,6 +2111,30 @@ char *git_system_config(void)
> return system_config;
> }
>
> +char *git_global_config(void)
> +{
> + char *user_config, *xdg_config;
> +
> + git_global_config_paths(&user_config, &xdg_config);
> + if (!user_config)
> + /*
> + * It is unknown if HOME/.gitconfig exists, so
> + * we do not know if we should write to XDG
Nit: we don't know about the intent of the caller, so they may not want
to write to the file but only read it.
> + * location; error out even if XDG_CONFIG_HOME
> + * is set and points at a sane location.
> + */
> + die(_("$HOME not set"));
Is it sensible to `die()` here in this new function that behaves more
like a library function? I imagine it would be more sensible to indicate
the error to the user and let them handle it accordingly.
Patrick
> +
> + if (access_or_warn(user_config, R_OK, 0) && xdg_config &&
> + !access_or_warn(xdg_config, R_OK, 0)) {
> + free(user_config);
> + return xdg_config;
> + } else {
> + free(xdg_config);
> + return user_config;
> + }
> +}
> +
> void git_global_config_paths(char **user_out, char **xdg_out)
> {
> char *user_config = xstrdup_or_null(getenv("GIT_CONFIG_GLOBAL"));
> diff --git a/config.h b/config.h
> index 9f04de8ee3e..5cf961b548d 100644
> --- a/config.h
> +++ b/config.h
> @@ -394,6 +394,7 @@ int config_error_nonbool(const char *);
> #endif
>
> char *git_system_config(void);
> +char *git_global_config(void);
> void git_global_config_paths(char **user, char **xdg);
>
> int git_config_parse_parameter(const char *, config_fn_t fn, void *data);
> --
> 2.42.0.2.g879ad04204
>
Attachment:
signature.asc
Description: PGP signature
