Merge branch 'kh/maintenance-use-xdg-when-it-should' into jch

When $HOME/.gitignore is missing but XDG config file available, we
should write into the latter, not former.  "git gc" and "git
maintenance" wrote into a wrong "global config" file, which have
been corrected.

* kh/maintenance-use-xdg-when-it-should:
  maintenance: use XDG config if it exists
  config: factor out global config file retrieval
  config: rename global config function
  config: format newlines

6 files changed, 86 insertions, 43 deletions

diff --git a/builtin/config.c b/builtin/config.c
index 11a4d4ef14..08fe36d499 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -708,30 +708,11 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 	}
 
 	if (use_global_config) {
-		char *user_config, *xdg_config;
-
-		git_global_config(&user_config, &xdg_config);
-		if (!user_config)
-			/*
-			 * It is unknown if HOME/.gitconfig exists, so
-			 * we do not know if we should write to XDG
-			 * location; error out even if XDG_CONFIG_HOME
-			 * is set and points at a sane location.
-			 */
+		given_config_source.file = git_global_config();
+		if (!given_config_source.file)
 			die(_("$HOME not set"));
-
 		given_config_source.scope = CONFIG_SCOPE_GLOBAL;
-
-		if (access_or_warn(user_config, R_OK, 0) &&
-		    xdg_config && !access_or_warn(xdg_config, R_OK, 0)) {
-			given_config_source.file = xdg_config;
-			free(user_config);
-		} else {
-			given_config_source.file = user_config;
-			free(xdg_config);
-		}
-	}
-	else if (use_system_config) {
+	} else if (use_system_config) {
 		given_config_source.file = git_system_config();
 		given_config_source.scope = CONFIG_SCOPE_SYSTEM;
 	} else if (use_local_config) {
@@ -760,7 +741,6 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 		given_config_source.scope = CONFIG_SCOPE_COMMAND;
 	}
 
-
 	if (respect_includes_opt == -1)
 		config_options.respect_includes = !given_config_source.file;
 	else
diff --git a/builtin/gc.c b/builtin/gc.c
index 7c11d5ebef..cb80ced6cb 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1543,19 +1543,18 @@ static int maintenance_register(int argc, const char **argv, const char *prefix)
 
 	if (!found) {
 		int rc;
-		char *user_config = NULL, *xdg_config = NULL;
+		char *global_config_file = NULL;
 
 		if (!config_file) {
-			git_global_config(&user_config, &xdg_config);
-			config_file = user_config;
-			if (!user_config)
-				die(_("$HOME not set"));
+			global_config_file = git_global_config();
+			config_file = global_config_file;
 		}
+		if (!config_file)
+			die(_("$HOME not set"));
 		rc = git_config_set_multivar_in_file_gently(
 			config_file, "maintenance.repo", maintpath,
 			CONFIG_REGEX_NONE, 0);
-		free(user_config);
-		free(xdg_config);
+		free(global_config_file);
 
 		if (rc)
 			die(_("unable to add '%s' value of '%s'"),
@@ -1612,18 +1611,18 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi
 
 	if (found) {
 		int rc;
-		char *user_config = NULL, *xdg_config = NULL;
+		char *global_config_file = NULL;
+
 		if (!config_file) {
-			git_global_config(&user_config, &xdg_config);
-			config_file = user_config;
-			if (!user_config)
-				die(_("$HOME not set"));
+			global_config_file = git_global_config();
+			config_file = global_config_file;
 		}
+		if (!config_file)
+			die(_("$HOME not set"));
 		rc = git_config_set_multivar_in_file_gently(
 			config_file, key, NULL, maintpath,
 			CONFIG_FLAGS_MULTI_REPLACE | CONFIG_FLAGS_FIXED_VALUE);
-		free(user_config);
-		free(xdg_config);
+		free(global_config_file);
 
 		if (rc &&
 		    (!force || rc == CONFIG_NOTHING_SET))
diff --git a/builtin/var.c b/builtin/var.c
index 8cf7dd9e2e..cf5567208a 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -90,7 +90,7 @@ static char *git_config_val_global(int ident_flag UNUSED)
 	char *user, *xdg;
 	size_t unused;
 
-	git_global_config(&user, &xdg);
+	git_global_config_paths(&user, &xdg);
 	if (xdg && *xdg) {
 		normalize_path_copy(xdg, xdg);
 		strbuf_addf(&buf, "%s\n", xdg);
diff --git a/config.c b/config.c
index 9ff6ae1cb9..3cfeb3d8bd 100644
--- a/config.c
+++ b/config.c
@@ -95,7 +95,6 @@ static long config_file_ftell(struct config_source *conf)
 	return ftell(conf->u.file);
 }
 
-
 static int config_buf_fgetc(struct config_source *conf)
 {
 	if (conf->u.buf.pos < conf->u.buf.len)
@@ -1988,7 +1987,27 @@ char *git_system_config(void)
 	return system_config;
 }
 
-void git_global_config(char **user_out, char **xdg_out)
+char *git_global_config(void)
+{
+	char *user_config, *xdg_config;
+
+	git_global_config_paths(&user_config, &xdg_config);
+	if (!user_config) {
+		free(xdg_config);
+		return NULL;
+	}
+
+	if (access_or_warn(user_config, R_OK, 0) && xdg_config &&
+	    !access_or_warn(xdg_config, R_OK, 0)) {
+		free(user_config);
+		return xdg_config;
+	} else {
+		free(xdg_config);
+		return user_config;
+	}
+}
+
+void git_global_config_paths(char **user_out, char **xdg_out)
 {
 	char *user_config = xstrdup_or_null(getenv("GIT_CONFIG_GLOBAL"));
 	char *xdg_config = NULL;
@@ -2041,7 +2060,7 @@ static int do_git_config_sequence(const struct config_options *opts,
 							 data, CONFIG_SCOPE_SYSTEM,
 							 NULL);
 
-	git_global_config(&user_config, &xdg_config);
+	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,
@@ -3418,7 +3437,6 @@ out_free:
 write_err_out:
 	ret = write_error(get_lock_file_path(&lock));
 	goto out_free;
-
 }
 
 void git_config_set_multivar_in_file(const char *config_filename,
diff --git a/config.h b/config.h
index 14f881ecfa..5dba984f77 100644
--- a/config.h
+++ b/config.h
@@ -382,7 +382,8 @@ int config_error_nonbool(const char *);
 #endif
 
 char *git_system_config(void);
-void git_global_config(char **user, char **xdg);
+char *git_global_config(void);
+void git_global_config_paths(char **user, char **xdg);
 
 int git_config_parse_parameter(const char *, config_fn_t fn, void *data);
 
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 00d29871e6..0943dfa18a 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -67,6 +67,51 @@ test_expect_success 'maintenance.auto config option' '
 	test_subcommand ! git maintenance run --auto --quiet  <false
 '
 
+test_expect_success 'register uses XDG_CONFIG_HOME config if it exists' '
+	test_when_finished rm -r .config/git/config &&
+	(
+		XDG_CONFIG_HOME=.config &&
+		export XDG_CONFIG_HOME &&
+		mkdir -p $XDG_CONFIG_HOME/git &&
+		>$XDG_CONFIG_HOME/git/config &&
+		git maintenance register &&
+		git config --file=$XDG_CONFIG_HOME/git/config --get maintenance.repo >actual &&
+		pwd >expect &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'register does not need XDG_CONFIG_HOME config to exist' '
+	test_when_finished git maintenance unregister &&
+	test_path_is_missing $XDG_CONFIG_HOME/git/config &&
+	git maintenance register &&
+	git config --global --get maintenance.repo >actual &&
+	pwd >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'unregister uses XDG_CONFIG_HOME config if it exists' '
+	test_when_finished rm -r .config/git/config &&
+	(
+		XDG_CONFIG_HOME=.config &&
+		export XDG_CONFIG_HOME &&
+		mkdir -p $XDG_CONFIG_HOME/git &&
+		>$XDG_CONFIG_HOME/git/config &&
+		git maintenance register &&
+		git maintenance unregister &&
+		test_must_fail git config --file=$XDG_CONFIG_HOME/git/config --get maintenance.repo >actual &&
+		test_must_be_empty actual
+	)
+'
+
+test_expect_success 'unregister does not need XDG_CONFIG_HOME config to exist' '
+	test_path_is_missing $XDG_CONFIG_HOME/git/config &&
+	git maintenance register &&
+	git maintenance unregister &&
+	test_must_fail git config --global --get maintenance.repo >actual &&
+	test_must_be_empty actual
+'
+
 test_expect_success 'maintenance.<task>.enabled' '
 	git config maintenance.gc.enabled false &&
 	git config maintenance.commit-graph.enabled true &&