public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: "Delilah Ashley Wu via GitGitGadget" <gitgitgadget@gmail•com>
To: git@vger•kernel.org
Cc: Delilah Ashley Wu <delilahwu@microsoft•com>,
	Derrick Stolee <stolee@gmail•com>,
	Johannes Schindelin <johannes.schindelin@gmx•de>,
	Patrick Steinhardt <ps@pks•im>,
	Delilah Ashley Wu <delilahwu@linux•microsoft.com>,
	Delilah Ashley Wu <delilahwu@microsoft•com>
Subject: [PATCH/RFC 4/4] config: keep bailing on unreadable global files
Date: Fri, 10 Oct 2025 01:14:09 +0000	[thread overview]
Message-ID: <6119cee0c6557e67f3eb4e2f9d488e8684a63c99.1760058849.git.gitgitgadget@gmail.com> (raw)
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

  parent reply	other threads:[~2025-10-10  1:14 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-10  1:14 [PATCH/RFC 0/4] config: read both home and xdg files for --global Delilah Ashley Wu via GitGitGadget
2025-10-10  1:14 ` [PATCH/RFC 1/4] cleanup_path: force forward slashes on Windows Delilah Ashley Wu via GitGitGadget
2025-11-19 17:47   ` Junio C Hamano
2025-10-10  1:14 ` [PATCH/RFC 2/4] config: test home and xdg files in `list --global` Delilah Ashley Wu via GitGitGadget
2025-11-19 18:29   ` Junio C Hamano
2025-10-10  1:14 ` [PATCH/RFC 3/4] config: read global scope via config_sequence Delilah Ashley Wu via GitGitGadget
2025-11-19 18:39   ` Junio C Hamano
2025-10-10  1:14 ` Delilah Ashley Wu via GitGitGadget [this message]
2025-10-10  1:27 ` [PATCH/RFC 0/4] config: read both home and xdg files for --global Kristoffer Haugsbakk
2025-11-22  1:36   ` Delilah Ashley Wu
2026-01-20 20:41     ` Junio C Hamano
2025-11-17 13:29 ` Johannes Schindelin
2025-11-18  0:28   ` Junio C Hamano
2025-11-19 14:44 ` Junio C Hamano
2025-11-22  2:00   ` Delilah Ashley Wu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6119cee0c6557e67f3eb4e2f9d488e8684a63c99.1760058849.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail$(echo .)com \
    --cc=delilahwu@linux$(echo .)microsoft.com \
    --cc=delilahwu@microsoft$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=johannes.schindelin@gmx$(echo .)de \
    --cc=ps@pks$(echo .)im \
    --cc=stolee@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox