From: Delilah Ashley Wu Date: Fri, 10 Oct 2025 01:14:08 +0000 (+0000) Subject: config: read global scope via config_sequence X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=eb3a952ca15b23953a709cfb0575cc82179c3960;p=thirdparty%2Fgit.git config: read global scope via config_sequence The output of `git config list --global` should include both the home (`$HOME/.gitconfig`) and XDG (`$XDG_CONFIG_HOME/git/config`) configs, but it only reads from the former. We assumed each config scope corresponds to a single config file. Under this assumption, `git config list --global` reads the global config by calling `git_config_from_file_with_options(...,"~/.gitconfig", ...)`. This function usage restricts us to a single config file. Because the global scope includes two files, we should read the configs via another method. The output of `git config list --show-scope --show-origin` (without `--global`) correctly includes both the home and XDG config files. So there's existing code that respects both locations, namely the `do_git_config_sequence()` function which reads from all scopes. Introduce flags to make it possible to ignore all but the global scope (i.e. ignore system, local, worktree, and cmdline). Then, reuse the function to read only the global scope when `--global` is specified. This was the suggested solution in the bug report: https://lore.kernel.org/git/kl6ly1oze7wb.fsf@chooglen-macbookpro.roam.corp.google.com. Then, modify the tests to check that `git config list --global` includes both home and XDG configs. This patch introduces a regression. If both global config files are unreadable, then `git config list --global` should exit non-zero. This is no longer the case, so mark the corresponding test as a "TODO known breakage" and address the issue in the next patch, config: keep bailing on unreadable global files. Implementation notes: 1. The `ignore_global` flag is not set anywhere, so the `if (!opts->ignore_global)` condition is always met. We can remove this flag if desired. 2. I've assumed that `config_source->scope == CONFIG_SCOPE_GLOBAL` iff `--global` is specified. This comparison determines whether to call `do_git_config_sequence()` for the global scope, or to keep calling `git_config_from_file_with_options()` for other scopes. 3. Keep populating `opts->source.file` in `builtin/config.c` because it is used as the destination config file for write operations. The proposed changes could convolute the code because there is no single source of truth for the config file locations in the global scope. Add a comment to help clarify this. Please let me know if it's unclear. Reported-by: Jade Lovelace Suggested-by: Glen Choo Helped-by: Derrick Stolee Signed-off-by: Delilah Ashley Wu Reviewed-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- diff --git a/builtin/config.c b/builtin/config.c index 75852bd79d..166568420b 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -776,6 +776,18 @@ static void location_options_init(struct config_location_options *opts, } if (opts->use_global_config) { + /* + * Since global config is sourced from more than one location, + * use `config.c#do_git_config_sequence()` with `opts->options` + * to read it. However, writing global config should point to a + * single destination, set in `opts->source.file`. + */ + opts->options.ignore_repo = 1; + opts->options.ignore_cmdline= 1; + opts->options.ignore_worktree = 1; + opts->options.ignore_system = 1; + opts->source.scope = CONFIG_SCOPE_GLOBAL; + opts->source.file = opts->file_to_free = git_global_config(); if (!opts->source.file) /* diff --git a/config.c b/config.c index f1def0dcfb..084c62c82d 100644 --- a/config.c +++ b/config.c @@ -1538,22 +1538,27 @@ static int do_git_config_sequence(const struct config_options *opts, worktree_config = NULL; } - if (git_config_system() && system_config && + if (!opts->ignore_system && git_config_system() && system_config && !access_or_die(system_config, R_OK, opts->system_gently ? ACCESS_EACCES_OK : 0)) ret += git_config_from_file_with_options(fn, system_config, data, CONFIG_SCOPE_SYSTEM, NULL); - git_global_config_paths(&user_config, &xdg_config); + if (!opts->ignore_global) { + git_global_config_paths(&user_config, &xdg_config); + + if (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK)) + ret += git_config_from_file_with_options(fn, xdg_config, data, + CONFIG_SCOPE_GLOBAL, NULL); - if (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK)) - ret += git_config_from_file_with_options(fn, xdg_config, data, - CONFIG_SCOPE_GLOBAL, NULL); + if (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK)) + ret += git_config_from_file_with_options(fn, user_config, data, + CONFIG_SCOPE_GLOBAL, NULL); - if (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK)) - ret += git_config_from_file_with_options(fn, user_config, data, - CONFIG_SCOPE_GLOBAL, NULL); + free(xdg_config); + free(user_config); + } if (!opts->ignore_repo && repo_config && !access_or_die(repo_config, R_OK, 0)) @@ -1572,8 +1577,6 @@ static int do_git_config_sequence(const struct config_options *opts, die(_("unable to parse command-line config")); free(system_config); - free(xdg_config); - free(user_config); free(repo_config); free(worktree_config); return ret; @@ -1603,7 +1606,8 @@ int config_with_options(config_fn_t fn, void *data, */ if (config_source && config_source->use_stdin) { ret = git_config_from_stdin(fn, data, config_source->scope); - } else if (config_source && config_source->file) { + } else if (config_source && config_source->file && + config_source->scope != CONFIG_SCOPE_GLOBAL) { ret = git_config_from_file_with_options(fn, config_source->file, data, config_source->scope, NULL); diff --git a/config.h b/config.h index 19c87fc0bc..9425fe115d 100644 --- a/config.h +++ b/config.h @@ -87,6 +87,8 @@ typedef int (*config_parser_event_fn_t)(enum config_event_t type, struct config_options { unsigned int respect_includes : 1; + unsigned int ignore_system : 1; + unsigned int ignore_global : 1; unsigned int ignore_repo : 1; unsigned int ignore_worktree : 1; unsigned int ignore_cmdline : 1; diff --git a/t/t1300-config.sh b/t/t1300-config.sh index 61e44027bc..6eaed6d62c 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -2367,7 +2367,7 @@ test_expect_success 'list with nonexistent global config' ' git config ${mode_prefix}list --show-scope ' -test_expect_success 'list --global with nonexistent global config' ' +test_expect_failure 'list --global with nonexistent global config' ' rm -rf "$HOME"/.gitconfig "$HOME"/.config/git/config && test_must_fail git config ${mode_prefix}list --global --show-scope ' @@ -2424,7 +2424,7 @@ test_expect_success 'list --global with both home and xdg' ' global file:$HOME/.gitconfig home.config=true EOF git config ${mode_prefix}list --global --show-scope --show-origin >output && - ! test_cmp expect output + test_cmp expect output ' test_expect_success 'override global and system config' ' @@ -2478,7 +2478,7 @@ test_expect_success 'override global and system config' ' test_cmp expect output ' -test_expect_success 'override global and system config with missing file' ' +test_expect_failure 'override global and system config with missing file' ' test_must_fail env GIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=/dev/null git config ${mode_prefix}list --global && test_must_fail env GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_SYSTEM=does-not-exist git config ${mode_prefix}list --system && GIT_CONFIG_GLOBAL=does-not-exist GIT_CONFIG_SYSTEM=does-not-exist git version diff --git a/t/t1306-xdg-files.sh b/t/t1306-xdg-files.sh index 0318755799..475bd26aba 100755 --- a/t/t1306-xdg-files.sh +++ b/t/t1306-xdg-files.sh @@ -71,7 +71,7 @@ test_expect_success 'read with --list: xdg file exists and ~/.gitconfig exists' echo user.name=read_config >expected && echo user.name=read_gitconfig >>expected && git config --global --list >actual && - ! test_cmp expected actual + test_cmp expected actual '