config: correctly read worktree configs in submodules

One of the steps in do_git_config_sequence() is to load the
worktree-specific config file. Although the function receives a git_dir
string, it relies on git_pathdup(), which uses the_repository->git_dir,
to make the path to the file. Furthermore, it also checks that
extensions.worktreeConfig is set through the
repository_format_worktree_config variable, which refers to
the_repository only. Thus, when a submodule has worktree-specific
settings, a command executed in the superproject that recurses into the
submodule won't find the said settings.

This will be especially important in the next patch: git-grep will learn
to honor sparse checkouts and, when running with --recurse-submodules,
the submodule's sparse checkout settings must be loaded. As these
settings are stored in the config.worktree file, they would be ignored
without this patch. So let's fix this by reading the right
config.worktree file and extensions.worktreeConfig setting, based on the
git_dir and commondir paths given to do_git_config_sequence(). Also
add a test to avoid any regressions.

Signed-off-by: Matheus Tavares <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>

Author: matheustavares

config.c

@@ -1747,11 +1747,22 @@ static int do_git_config_sequence(const struct config_options *opts,
 		ret += git_config_from_file(fn, repo_config, data);
 
 	current_parsing_scope = CONFIG_SCOPE_WORKTREE;
-	if (!opts->ignore_worktree && repository_format_worktree_config) {
-		char *path = git_pathdup("config.worktree");
-		if (!access_or_die(path, R_OK, 0))
-			ret += git_config_from_file(fn, path, data);
-		free(path);
+	if (!opts->ignore_worktree && repo_config && opts->git_dir) {
+		struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
+		struct strbuf buf = STRBUF_INIT;
+
+		read_repository_format(&repo_fmt, repo_config);
+
+		if (!verify_repository_format(&repo_fmt, &buf) &&
+		    repo_fmt.worktree_config) {
+			char *path = mkpathdup("%s/config.worktree", opts->git_dir);
+			if (!access_or_die(path, R_OK, 0))
+				ret += git_config_from_file(fn, path, data);
+			free(path);
+		}
+
+		strbuf_release(&buf);
+		clear_repository_format(&repo_fmt);
 	}
 
 	current_parsing_scope = CONFIG_SCOPE_COMMAND;

t/helper/test-config.c

@@ -2,12 +2,19 @@
 #include "cache.h"
 #include "config.h"
 #include "string-list.h"
+#include "submodule-config.h"
 
 /*
  * This program exposes the C API of the configuration mechanism
  * as a set of simple commands in order to facilitate testing.
  *
- * Reads stdin and prints result of command to stdout:
+ * Usage: test-tool config [--submodule=<path>] <cmd> [<args>]
+ *
+ * If --submodule=<path> is given, <cmd> will operate on the submodule at the
+ * given <path>. This option is not valid for the commands: read_early_config,
+ * configset_get_value and configset_get_value_multi.
+ *
+ * Possible cmds are:
  *
  * get_value -> prints the value with highest priority for the entered key
  *
@@ -86,23 +93,51 @@ int cmd__config(int argc, const char **argv)
 	const struct string_list *strptr;
 	struct config_set cs = { .hash_initialized = 0 };
 	enum test_config_exit_code ret = TC_SUCCESS;
+	struct repository *repo = the_repository;
+	const char *subrepo_path = NULL;
 
 	argc--; /* skip over "config" */
 	argv++;
 
 	if (argc == 0)
 		goto print_usage_error;
 
+	if (skip_prefix(*argv, "--submodule=", &subrepo_path)) {
+		argc--;
+		argv++;
+		if (argc == 0)
+			goto print_usage_error;
+	}
+
 	if (argc == 2 && !strcmp(argv[0], "read_early_config")) {
+		if (subrepo_path) {
+			fprintf(stderr, "Cannot use --submodule with read_early_config\n");
+			return TC_USAGE_ERROR;
+		}
 		read_early_config(early_config_cb, (void *)argv[1]);
 		return TC_SUCCESS;
 	}
 
 	setup_git_directory();
 	git_configset_init(&cs);
 
+	if (subrepo_path) {
+		const struct submodule *sub;
+		struct repository *subrepo = xcalloc(1, sizeof(*repo));
+
+		sub = submodule_from_path(the_repository, &null_oid, subrepo_path);
+		if (!sub || repo_submodule_init(subrepo, the_repository, sub)) {
+			fprintf(stderr, "Invalid argument to --submodule: '%s'\n",
+				subrepo_path);
+			free(subrepo);
+			ret = TC_USAGE_ERROR;
+			goto out;
+		}
+		repo = subrepo;
+	}
+
 	if (argc == 2 && !strcmp(argv[0], "get_value")) {
-		if (!git_config_get_value(argv[1], &v)) {
+		if (!repo_config_get_value(repo, argv[1], &v)) {
 			if (!v)
 				printf("(NULL)\n");
 			else
@@ -112,7 +147,7 @@ int cmd__config(int argc, const char **argv)
 			ret = TC_VALUE_NOT_FOUND;
 		}
 	} else if (argc == 2 && !strcmp(argv[0], "get_value_multi")) {
-		strptr = git_config_get_value_multi(argv[1]);
+		strptr = repo_config_get_value_multi(repo, argv[1]);
 		if (strptr) {
 			for (i = 0; i < strptr->nr; i++) {
 				v = strptr->items[i].string;
@@ -126,27 +161,33 @@ int cmd__config(int argc, const char **argv)
 			ret = TC_VALUE_NOT_FOUND;
 		}
 	} else if (argc == 2 && !strcmp(argv[0], "get_int")) {
-		if (!git_config_get_int(argv[1], &val)) {
+		if (!repo_config_get_int(repo, argv[1], &val)) {
 			printf("%d\n", val);
 		} else {
 			printf("Value not found for \"%s\"\n", argv[1]);
 			ret = TC_VALUE_NOT_FOUND;
 		}
 	} else if (argc == 2 && !strcmp(argv[0], "get_bool")) {
-		if (!git_config_get_bool(argv[1], &val)) {
+		if (!repo_config_get_bool(repo, argv[1], &val)) {
 			printf("%d\n", val);
 		} else {
+
 			printf("Value not found for \"%s\"\n", argv[1]);
 			ret = TC_VALUE_NOT_FOUND;
 		}
 	} else if (argc == 2 && !strcmp(argv[0], "get_string")) {
-		if (!git_config_get_string_const(argv[1], &v)) {
+		if (!repo_config_get_string_const(repo, argv[1], &v)) {
 			printf("%s\n", v);
 		} else {
 			printf("Value not found for \"%s\"\n", argv[1]);
 			ret = TC_VALUE_NOT_FOUND;
 		}
 	} else if (argc >= 2 && !strcmp(argv[0], "configset_get_value")) {
+		if (subrepo_path) {
+			fprintf(stderr, "Cannot use --submodule with configset_get_value\n");
+			ret = TC_USAGE_ERROR;
+			goto out;
+		}
 		for (i = 2; i < argc; i++) {
 			int err;
 			if ((err = git_configset_add_file(&cs, argv[i]))) {
@@ -165,6 +206,11 @@ int cmd__config(int argc, const char **argv)
 			ret = TC_VALUE_NOT_FOUND;
 		}
 	} else if (argc >= 2 && !strcmp(argv[0], "configset_get_value_multi")) {
+		if (subrepo_path) {
+			fprintf(stderr, "Cannot use --submodule with configset_get_value_multi\n");
+			ret = TC_USAGE_ERROR;
+			goto out;
+		}
 		for (i = 2; i < argc; i++) {
 			int err;
 			if ((err = git_configset_add_file(&cs, argv[i]))) {
@@ -187,14 +233,19 @@ int cmd__config(int argc, const char **argv)
 			ret = TC_VALUE_NOT_FOUND;
 		}
 	} else if (!strcmp(argv[0], "iterate")) {
-		git_config(iterate_cb, NULL);
+		repo_config(repo, iterate_cb, NULL);
 	} else {
 print_usage_error:
-		fprintf(stderr, "Invalid syntax. Usage: test-tool config <cmd> [args]\n");
+		fprintf(stderr, "Invalid syntax. Usage: test-tool config"
+				" [--submodule=<path>] <cmd> [args]\n");
 		ret = TC_USAGE_ERROR;
 	}
 
 out:
 	git_configset_clear(&cs);
+	if (repo != the_repository) {
+		repo_clear(repo);
+		free(repo);
+	}
 	return ret;
 }

t/t2404-worktree-config.sh

@@ -76,4 +76,20 @@ test_expect_success 'config.worktree no longer read without extension' '
 	test_cmp_config -C wt2 shared this.is
 '
 
+test_expect_success 'correctly read config.worktree from submodules' '
+	test_unconfig extensions.worktreeConfig &&
+	git init sub &&
+	(
+		cd sub &&
+		test_commit A &&
+		git config extensions.worktreeConfig true &&
+		git config --worktree wtconfig.sub test-value
+	) &&
+	git submodule add ./sub &&
+	git commit -m "add sub" &&
+	echo test-value >expect &&
+	test-tool config --submodule=sub get_value wtconfig.sub >actual &&
+	test_cmp expect actual
+'
+
 test_done