config: read from config sequence for global scope

The output of `git config list --global` should include both
`$HOME/.gitconfig` and `$XDG_CONFIG_HOME/git/config`, but it only reads
from the former.

We've assumed each config scope corresponds to a single config file
location. Because of 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. Since the global
scope includes more than one file, we should use another method to read
the global config.

The output of `git config list --show-scope --show-origin` 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 that make it
possible to ignore all but the global scope (i.e. ignore system, local,
worktree, and cmdline). Then, reuse this function for reading only the
global scope. This was the suggested solution in the original bug report
thread [1].

However, this change introduces a regression. If the global config file
does not exist or is unreadable, `git config list --global` should fail.
Since `do_git_config_sequence()` reads from all scopes, it will still
succeed when the global config file is missing. This regression will be
addressed in the next patch.

Modify the tests as follows:
    A. Assert that the output of `git config list --global` now includes
       both the home and XDG config.
    B. Expect success even when the global config file is missing or
       unreadable, marking them as a "TODO known breakage".

Note 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.

Note 2: It's 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.

Note 3: Keep populating `opts->source.file` in `builtin/config.c`
because it is used as the destination 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.

[1]: https://lore.kernel.org/git/[email protected].

Reported-by: Jade Lovelace <[email protected]>
Helped-by: Derrick Stolee <[email protected]>
Suggested-by: Glen Choo <[email protected]>
Signed-off-by: Delilah Ashley Wu <[email protected]>

Author: delilahw

builtin/config.c

@@ -768,6 +768,18 @@ static void location_options_init(struct config_location_options *opts,
 	}
 
 	if (opts->use_global_config) {
+		/*
+		 * Reading config is handled by `config.c#do_git_config_sequence()`
+		 * because the global config can be sourced from multiple files.
+		 * Writing config should point to a single destination as set in
+		 * `opts->source.file` below.
+		 */
+		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)
 			/*

config.c

@@ -1524,22 +1524,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))
@@ -1558,8 +1563,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;
@@ -1589,7 +1592,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);

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;

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_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
 '
@@ -2429,7 +2429,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' '
@@ -2483,7 +2483,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

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
 '