Merge branch 'dw/config-read-both-global' into jch

The git config --global read operations have been updated to respect
both $HOME/.gitconfig and $XDG_CONFIG_HOME/git/config, fixing an
inconsistency where only the former was read when both configuration
files are present.

* dw/config-read-both-global:
  config: read global scope via config_sequence
  config: let sequence require a successful file
  path: use forward slashes in XDG config on Windows

Author: gitster

builtin/config.c

@@ -957,6 +957,17 @@ 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,
+		 * read it using `do_git_config_sequence()` with other scopes
+		 * ignored. 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.file = opts->file_to_free = git_global_config();
 		if (!opts->source.file)
 			/*

config.c

@@ -1553,11 +1553,27 @@ int git_config_system(void)
 	return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0);
 }
 
+static void attempt_git_config_from_file_with_options(config_fn_t fn,
+						      const char *filename,
+						      void *data,
+						      enum config_scope scope,
+						      const struct config_options *opts,
+						      int *success_count,
+						      int *cumulative_ret)
+{
+	int ret = git_config_from_file_with_options(fn, filename, data,
+						    scope, opts);
+	if (!ret)
+		(*success_count)++;
+	*cumulative_ret += ret;
+}
+
 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, int require_successful_config)
 {
 	int ret = 0;
+	int success_count = 0;
 	char *system_config = git_system_config();
 	char *xdg_config = NULL;
 	char *user_config = NULL;
@@ -1580,44 +1596,54 @@ 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);
+		attempt_git_config_from_file_with_options(fn, system_config, data,
+							  CONFIG_SCOPE_SYSTEM, NULL,
+							  &success_count, &ret);
 
-	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))
+			attempt_git_config_from_file_with_options(fn, xdg_config,
+								  data,
+								  CONFIG_SCOPE_GLOBAL,
+								  NULL, &success_count, &ret);
 
-	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))
+			attempt_git_config_from_file_with_options(fn, user_config,
+								  data,
+								  CONFIG_SCOPE_GLOBAL,
+								  NULL, &success_count, &ret);
+
+		free(xdg_config);
+		free(user_config);
+	}
 
 	if (!opts->ignore_repo && repo_config &&
 	    !access_or_die(repo_config, R_OK, 0))
-		ret += git_config_from_file_with_options(fn, repo_config, data,
-							 CONFIG_SCOPE_LOCAL, NULL);
+		attempt_git_config_from_file_with_options(fn, repo_config, data,
+							  CONFIG_SCOPE_LOCAL, NULL, &success_count, &ret);
 
 	if (!opts->ignore_worktree && worktree_config &&
 	    repo && repo->repository_format_worktree_config &&
-	    !access_or_die(worktree_config, R_OK, 0)) {
-			ret += git_config_from_file_with_options(fn, worktree_config, data,
-								 CONFIG_SCOPE_WORKTREE,
-								 NULL);
-	}
+	    !access_or_die(worktree_config, R_OK, 0))
+		attempt_git_config_from_file_with_options(fn, worktree_config, data,
+							  CONFIG_SCOPE_WORKTREE,
+							  NULL, &success_count, &ret);
 
 	if (!opts->ignore_cmdline && git_config_from_parameters(fn, data) < 0)
 		die(_("unable to parse command-line config"));
 
 	free(system_config);
-	free(xdg_config);
-	free(user_config);
 	free(repo_config);
 	free(worktree_config);
+
+	if (require_successful_config && !success_count && !ret)
+		ret = -1;
+
 	return ret;
 }
 
@@ -1645,15 +1671,17 @@ 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);
 	} else if (config_source && config_source->blob) {
 		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_GLOBAL);
 	}
 
 	if (inc.remote_urls) {

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;

path.c

@@ -1544,19 +1544,23 @@ int looks_like_command_line_option(const char *str)
 
 char *xdg_config_home_for(const char *subdir, const char *filename)
 {
+	char *ret;
 	const char *home, *config_home;
 
 	assert(subdir);
 	assert(filename);
 	config_home = getenv("XDG_CONFIG_HOME");
 	if (config_home && *config_home)
-		return mkpathdup("%s/%s/%s", config_home, subdir, filename);
-
-	home = getenv("HOME");
-	if (home)
-		return mkpathdup("%s/.config/%s/%s", home, subdir, filename);
+		ret = mkpathdup("%s/%s/%s", config_home, subdir, filename);
+	else if ((home = getenv("HOME")))
+		ret = mkpathdup("%s/.config/%s/%s", home, subdir, filename);
+	else
+		return NULL;
 
-	return NULL;
+#ifdef GIT_WINDOWS_NATIVE
+	convert_slashes(ret);
+#endif
+	return ret;
 }
 
 char *xdg_config_home(const char *filename)

t/t1300-config.sh

@@ -2350,6 +2350,38 @@ test_expect_success '--show-origin with --default' '
 	test_cmp expect actual
 '
 
+test_expect_success 'set up xdg config --show-origin tests' '
+	mkdir -p "$HOME"/.config/git &&
+	cat >"$HOME"/.config/git/config <<-EOF
+	[xdg]
+		config = true
+	EOF
+'
+
+test_expect_success MINGW '--show-origin converts backslashes in xdg path to forward slashes on Windows' '
+	backslash_home="$(echo "$HOME" | tr / \\\\)" &&
+	echo "file:$HOME/.config/git/config	true" >expect &&
+
+	(
+		sane_unset XDG_CONFIG_HOME &&
+		HOME="$backslash_home" git config ${mode_get} --show-origin xdg.config >actual
+	) &&
+	test_cmp expect actual &&
+
+	XDG_CONFIG_HOME="$backslash_home\\.config" git config ${mode_get} --show-origin xdg.config >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '--show-origin with default xdg path' '
+	echo "file:$HOME/.config/git/config	true" >expect &&
+	git config ${mode_get} --show-origin xdg.config >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'clean up xdg config --show-origin tests' '
+	rm -rf "$HOME"/.config/git
+'
+
 test_expect_success '--show-scope with --list' '
 	cat >expect <<-EOF &&
 	global	user.global=true
@@ -2425,6 +2457,89 @@ test_expect_success '--show-scope with --default' '
 	test_cmp expect actual
 '
 
+test_expect_success 'list with nonexistent global config gracefully exits' '
+	rm -f "$HOME"/.gitconfig "$HOME"/.config/git/config &&
+	git config ${mode_prefix}list &&
+	git config ${mode_prefix}list --show-scope
+'
+
+test_expect_success 'list --global with nonexistent global config fails' '
+	rm -f "$HOME"/.gitconfig "$HOME"/.config/git/config &&
+	test_must_fail git config ${mode_prefix}list --global &&
+	test_must_fail git config ${mode_prefix}list --global --show-scope
+'
+
+test_expect_success 'list and get --global with only home' '
+	rm -f "$HOME"/.config/git/config &&
+
+	test_when_finished rm -f \"\$HOME\"/.gitconfig &&
+	cat >"$HOME"/.gitconfig <<-EOF &&
+	[home]
+		config = true
+	EOF
+
+	cat >expect <<-EOF &&
+	global	home.config=true
+	EOF
+	git config ${mode_prefix}list --global --show-scope >actual &&
+	test_cmp expect actual &&
+
+	echo true >expect &&
+	git config ${mode_get} --global home.config >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'list and get --global with only xdg' '
+	rm -f "$HOME"/.gitconfig &&
+
+	test_when_finished rm -rf \"\$HOME\"/.config/git &&
+	mkdir -p "$HOME"/.config/git &&
+	cat >"$HOME"/.config/git/config <<-EOF &&
+	[xdg]
+		config = true
+	EOF
+
+	cat >expect <<-EOF &&
+	global	xdg.config=true
+	EOF
+	git config ${mode_prefix}list --global --show-scope >actual &&
+	test_cmp expect actual &&
+
+	echo true >expect &&
+	git config ${mode_get} --global xdg.config >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'list and get --global with both home and xdg' '
+	test_when_finished rm -f \"\$HOME\"/.gitconfig &&
+	cat >"$HOME"/.gitconfig <<-EOF &&
+	[home]
+		config = home
+	EOF
+
+	test_when_finished rm -rf \"\$HOME\"/.config/git &&
+	mkdir -p "$HOME"/.config/git &&
+	cat >"$HOME"/.config/git/config <<-EOF &&
+	[xdg]
+		config = xdg
+	EOF
+
+	cat >expect <<-EOF &&
+	global	file:$HOME/.config/git/config	xdg.config=xdg
+	global	file:$HOME/.gitconfig	home.config=home
+	EOF
+	git config ${mode_prefix}list --global --show-scope --show-origin >actual &&
+	test_cmp expect actual &&
+
+	echo xdg >expect &&
+	git config ${mode_get} --global xdg.config >actual &&
+	test_cmp expect actual &&
+
+	echo home >expect &&
+	git config ${mode_get} --global home.config >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'override global and system config' '
 	test_when_finished rm -f \"\$HOME\"/.gitconfig &&
 	cat >"$HOME"/.gitconfig <<-EOF &&

t/t1306-xdg-files.sh

@@ -52,6 +52,8 @@ test_expect_success 'read with --get: xdg file exists and ~/.gitconfig exists' '
 	echo "	name = read_gitconfig" >>.gitconfig &&
 	echo read_gitconfig >expected &&
 	git config --get user.name >actual &&
+	test_cmp expected actual &&
+	git config --global --get user.name >actual &&
 	test_cmp expected actual
 '
 
@@ -68,7 +70,8 @@ test_expect_success 'read with --list: xdg file exists and ~/.gitconfig exists'
 	>.gitconfig &&
 	echo "[user]" >.gitconfig &&
 	echo "	name = read_gitconfig" >>.gitconfig &&
-	echo user.name=read_gitconfig >expected &&
+	echo user.name=read_config >expected &&
+	echo user.name=read_gitconfig >>expected &&
 	git config --global --list >actual &&
 	test_cmp expected actual
 '