[PATCH/RFC 4/4] config: keep bailing on unreadable global files
- From
- Delilah Ashley Wu via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Oct 10, 2025, 01:14 UTC
- Message-ID
- <6119cee0c6557e67f3eb4e2f9d488e8684a63c99.1760058849.git.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.1938.git.1760058849.gitgitgadget@gmail.com>
From: Delilah Ashley Wu <delilahwu@microsoft.com>
The expected behaviour for `git config list` is:
A. Without `--global`, it should not bail on unreadable/non-existent
global config files. B. With `--global`, it should bail when both `$HOME/.gitconfig` and
`$XDG_CONFIG_HOME/git/config` are unreadable. It should not bail
when one or more of them is readable.The previous patch, config: read global scope via config_sequence, introduced a regression in scenario B. When both global config files are unreadable, running `git config list --global` would not fail. For example, `GIT_CONFIG_GLOBAL=does-not-exist git config list --global` exits with status code 0.
Assuming that `config_source->scope == CONFIG_SCOPE_GLOBAL` iff the `--global` argument is specified, use this to determine whether to bail. When reading only the global scope and both config files are unreadable, then adjust the return code to be non-zero.
Note: When bailing, the exit code is not determined by sum of the return codes of the underlying operations. Instead, the exit code is modified via a single decrement. If this is undesirable, we can change it to sum the return codes of the underlying operations instead.
Lastly, modify the tests to remove the known breakage/regression. The tests for scenario B will now pass.
Helped-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Delilah Ashley Wu <delilahwu@microsoft.com> Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- config.c | 40 +++++++++++++++++++++++++++++++--------- t/t1300-config.sh | 4 ++-- 2 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/config.c b/config.c index 4b9f3831b1..3057c16f59 100644 --- a/config.c +++ b/config.c @@ -1500,8 +1500,8 @@ int git_config_system(void) } static int do_git_config_sequence(const struct config_options *opts, - const struct repository *repo, - config_fn_t fn, void *data) + const struct repository *repo, config_fn_t fn, + void *data, enum config_scope scope) { int ret = 0; char *system_config = git_system_config(); @@ -1534,15 +1534,34 @@ static int do_git_config_sequence(const struct config_options *opts, NULL); if (!opts->ignore_global) { + int global_config_success_count = 0; + int nonzero_ret_on_global_config_error = scope == CONFIG_SCOPE_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 (!ret) + global_config_success_count++; + } + + 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 (!ret) + global_config_success_count++; + } - 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 (nonzero_ret_on_global_config_error && + !global_config_success_count) + --ret; free(xdg_config); free(user_config); @@ -1603,7 +1622,10 @@ int config_with_options(config_fn_t fn, void *data, ret = git_config_from_blob_ref(fn, repo, config_source->blob, data, config_source->scope); } else { - ret = do_git_config_sequence(opts, repo, fn, data); + ret = do_git_config_sequence(opts, repo, fn, data, + config_source ? + config_source->scope : + CONFIG_SCOPE_UNKNOWN); } if (inc.remote_urls) { diff --git a/t/t1300-config.sh b/t/t1300-config.sh index 42f256e122..0c3911183c 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -2372,7 +2372,7 @@ test_expect_success 'list with nonexistent global config' ' git config ${mode_prefix}list --show-scope ' -test_expect_failure 'list --global with nonexistent global config' ' +test_expect_success '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 ' @@ -2483,7 +2483,7 @@ test_expect_success 'override global and system config' ' test_cmp expect output ' -test_expect_failure 'override global and system config with missing file' ' +test_expect_success '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
-- gitgitgadget