config: keep bailing on unreadable global files

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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/config.c b/config.c
index 084c62c82db56d7259e841918fe88c5add8d3fde..903291785be37f16bdf7e268088e1a973998f0b0 100644 (file)
--- a/config.c
+++ b/config.c
@@ -1512,8 +1512,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();
@@ -1546,15 +1546,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);
@@ -1615,7 +1634,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 6eaed6d62cdcf21bbf2d8e58802be75256de028c..5da1e459805a97aa47954f3068a03901aee91ce0 100755 (executable)
--- 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_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
 '
@@ -2478,7 +2478,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