[PATCH 2/3] submodule: store settings in worktree config when enabled

["Yoichi NAKAYAMA via GitGitGadget" <gitgitgadget@gmail.com>]

From: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>

Submodule URLs can differ between branches. For example, one branch
may use the upstream repository while another uses a fork for the same
submodule. When these branches are checked out in separate worktrees,
storing `submodule.<name>.url` in the shared repository config
prevents each worktree from using its own URL.

Store submodule settings such as `submodule.<name>.active` and
`submodule.<name>.url` in the worktree config when
`extensions.worktreeConfig` is enabled.

Settings shared between worktrees can remain in the repository
config. When such a setting exists there, 'git submodule deinit' sets
`submodule.<name>.active` to false in the worktree config to
unregister the submodule.

Unlike other settings, `submodule.<name>.gitdir` should not be shared
between worktrees, so exclude it from the config copied by 'git
worktree add'.

Signed-off-by: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>
---
 Documentation/config/submodule.adoc        |   3 +-
 builtin/submodule--helper.c                |  43 ++++---
 builtin/worktree.c                         |  23 ++++
 t/t2405-worktree-submodule.sh              | 129 +++++++++++++++++++++
 t/t7425-submodule-gitdir-path-extension.sh |  49 ++++++++
 5 files changed, 227 insertions(+), 20 deletions(-)

diff --git a/Documentation/config/submodule.adoc b/Documentation/config/submodule.adoc
index 4c424017b4..1e294147db 100644
--- a/Documentation/config/submodule.adoc
+++ b/Documentation/config/submodule.adoc
@@ -58,7 +58,8 @@ submodule.<name>.gitdir::
 	respected when `extensions.submodulePathConfig` is enabled, otherwise it
 	has no effect. When enabled, this config becomes the single source of
 	truth for submodule gitdir paths and Git will error if it is missing.
-	See linkgit:git-config[1] for details.
+	When `extensions.worktreeConfig` is enabled, this setting is stored in
+	each worktree's config. See linkgit:git-config[1] for details.
 
 submodule.active::
 	A repeated field which contains a pathspec used to match against a
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 56d7720e93..f249582ca6 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -497,7 +497,7 @@ static int validate_and_set_submodule_gitdir(struct strbuf *gitdir_path,
 		return 0;
 	}
 
-	if (repo_config_set_gently(the_repository, key, gitdir_path->buf)) {
+	if (repo_config_set_worktree_gently(the_repository, key, gitdir_path->buf)) {
 		free(key);
 		return -1;
 	}
@@ -609,15 +609,10 @@ static void init_submodule(const char *path, const char *prefix,
 		die(_("No url found for submodule path '%s' in .gitmodules"),
 			displaypath);
 
-	/*
-	 * NEEDSWORK: In a multi-working-tree world, this needs to be
-	 * set in the per-worktree config.
-	 *
-	 * Set active flag for the submodule being initialized
-	 */
+	/* Set active flag for the submodule being initialized */
 	if (!is_submodule_active(the_repository, path)) {
 		strbuf_addf(&sb, "submodule.%s.active", sub->name);
-		repo_config_set_gently(the_repository, sb.buf, "true");
+		repo_config_set_worktree_gently(the_repository, sb.buf, "true");
 		strbuf_reset(&sb);
 	}
 
@@ -643,7 +638,7 @@ static void init_submodule(const char *path, const char *prefix,
 			free(oldurl);
 		}
 
-		if (repo_config_set_gently(the_repository, sb.buf, url))
+		if (repo_config_set_worktree_gently(the_repository, sb.buf, url))
 			die(_("Failed to register url for submodule path '%s'"),
 			    displaypath);
 		if (!(flags & OPT_QUIET))
@@ -665,7 +660,7 @@ static void init_submodule(const char *path, const char *prefix,
 			upd = submodule_update_type_to_string(sub->update_strategy.type);
 		}
 
-		if (repo_config_set_gently(the_repository, sb.buf, upd))
+		if (repo_config_set_worktree_gently(the_repository, sb.buf, upd))
 			die(_("Failed to register update mode for submodule path '%s'"), displaypath);
 	}
 
@@ -1491,7 +1486,7 @@ static void sync_submodule(const char *path, const char *prefix,
 
 	strbuf_reset(&sb);
 	strbuf_addf(&sb, "submodule.%s.url", sub->name);
-	if (repo_config_set_gently(the_repository, sb.buf, super_config_url))
+	if (repo_config_set_worktree_gently(the_repository, sb.buf, super_config_url))
 		die(_("failed to register url for submodule path '%s'"),
 		      displaypath);
 
@@ -1670,17 +1665,31 @@ static void deinit_submodule(const char *path, const char *prefix,
 
 	/* remove the .git/config entries (unless the user already did it) */
 	if (!capture_command(&cp_config, &sb_config, 0) && sb_config.len) {
+		char *file = the_repository->repository_format_worktree_config ?
+		  repo_git_path(the_repository, "config.worktree") :
+		  repo_git_path(the_repository, "config");
 		char *sub_key = xstrfmt("submodule.%s", sub->name);
 
 		/*
 		 * remove the whole section so we have a clean state when
 		 * the user later decides to init this submodule again
 		 */
-		repo_config_rename_section_in_file(the_repository, NULL, sub_key, NULL);
+		repo_config_rename_section_in_file(the_repository, file, sub_key, NULL);
 		if (!(flags & OPT_QUIET))
 			printf(_("Submodule '%s' (%s) unregistered for path '%s'\n"),
 				 sub->name, sub->url, displaypath);
+		free(file);
 		free(sub_key);
+
+		if (the_repository->repository_format_worktree_config) {
+			char *key = xstrfmt("submodule.%s.active", sub->name);
+
+			repo_config_clear(the_repository);
+			if (is_submodule_active(the_repository, sub->name))
+				repo_config_set_worktree_gently(the_repository,
+								key, "false");
+			free(key);
+		}
 	}
 
 cleanup:
@@ -3580,7 +3589,7 @@ static void configure_added_submodule(struct add_data *add_data)
 	int matched = 0;
 
 	key = xstrfmt("submodule.%s.url", add_data->sm_name);
-	repo_config_set_gently(the_repository, key, add_data->realrepo);
+	repo_config_set_worktree_gently(the_repository, key, add_data->realrepo);
 	free(key);
 
 	add_submod.git_cmd = 1;
@@ -3610,10 +3619,6 @@ static void configure_added_submodule(struct add_data *add_data)
 	if (run_command(&add_gitmodules))
 		die(_("Failed to register submodule '%s'"), add_data->sm_path);
 
-	/*
-	 * NEEDSWORK: In a multi-working-tree world this needs to be
-	 * set in the per-worktree config.
-	 */
 	/*
 	 * NEEDSWORK: In the longer run, we need to get rid of this
 	 * pattern of querying "submodule.active" before calling
@@ -3627,7 +3632,7 @@ static void configure_added_submodule(struct add_data *add_data)
 		 * current configured pathspec, set the submodule's active flag
 		 */
 		key = xstrfmt("submodule.%s.active", add_data->sm_name);
-		repo_config_set_gently(the_repository, key, "true");
+		repo_config_set_worktree_gently(the_repository, key, "true");
 		free(key);
 	} else {
 		for (size_t i = 0; i < values->nr; i++) {
@@ -3639,7 +3644,7 @@ static void configure_added_submodule(struct add_data *add_data)
 		}
 		if (!matched) { /* no pattern matched -> force-enable */
 			key = xstrfmt("submodule.%s.active", add_data->sm_name);
-			repo_config_set_gently(the_repository, key, "true");
+			repo_config_set_worktree_gently(the_repository, key, "true");
 			free(key);
 		}
 	}
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 77ecd0f71f..ef03ef979a 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -363,6 +363,9 @@ static void copy_filtered_worktree_config(const char *worktree_git_dir)
 	if (file_exists(from_file)) {
 		struct config_set cs = { { 0 } };
 		int bare;
+		struct string_list keys = STRING_LIST_INIT_DUP;
+		struct string_list_item *item;
+		int i;
 
 		if (safe_create_leading_directories(the_repository, to_file) ||
 			copy_file(the_repository, to_file, from_file, 0666)) {
@@ -386,6 +389,26 @@ static void copy_filtered_worktree_config(const char *worktree_git_dir)
 			error(_("failed to unset '%s' in '%s'"),
 				"core.worktree", to_file);
 
+		/* unset "submodule.*.gitdir" */
+		for (i = 0; i < cs.list.nr; i++) {
+			const char *var = cs.list.items[i].e->key;
+			const char *subsection, *key;
+			size_t subsection_len;
+
+			if (!parse_config_key(var, "submodule",
+					      &subsection, &subsection_len, &key) &&
+			    subsection && !strcmp(key, "gitdir"))
+				string_list_insert(&keys, var);
+		}
+		for_each_string_list_item(item, &keys) {
+			if (repo_config_set_in_file_gently(the_repository,
+							   to_file, item->string,
+							   NULL, NULL))
+				error(_("failed to unset '%s' in '%s'"),
+				      item->string, to_file);
+		}
+		string_list_clear(&keys, 0);
+
 		git_configset_clear(&cs);
 	}
 
diff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh
index f5c94d490b..d46d565ad6 100755
--- a/t/t2405-worktree-submodule.sh
+++ b/t/t2405-worktree-submodule.sh
@@ -91,4 +91,133 @@ test_expect_success 'unsetting core.worktree does not prevent running commands d
 	git -C main/.git/worktrees/checkout-recurse/modules/sub log
 '
 
+test_expect_success 'submodule config in worktree config' '
+	test_when_finished "rm -rf wt-config" &&
+	git clone "$base_path/origin/main" wt-config &&
+	(
+		cd wt-config &&
+		git config extensions.worktreeConfig true &&
+		git worktree add wt &&
+		cat .git/config >.git/expect-config &&
+
+		git submodule update --init &&
+		test_cmp .git/expect-config .git/config &&
+		echo "true" >expect &&
+		git config --worktree submodule.sub.active >actual &&
+		test_cmp expect actual &&
+		echo "$base_path/origin/sub" >expect &&
+		git config --worktree submodule.sub.url >actual &&
+		test_cmp expect actual &&
+
+		git submodule deinit sub &&
+		test_cmp .git/expect-config .git/config &&
+		test_must_be_empty .git/config.worktree &&
+
+		git submodule add ../sub sub2 &&
+		test_cmp .git/expect-config .git/config &&
+		echo "true" >expect &&
+		git config --worktree submodule.sub2.active >actual &&
+		test_cmp expect actual &&
+		echo "$base_path/origin/sub" >expect &&
+		git config --worktree submodule.sub2.url >actual &&
+		test_cmp expect actual &&
+
+		git config --file=.gitmodules submodule.sub2.url new_url &&
+		git submodule sync &&
+		test_cmp .git/expect-config .git/config &&
+		echo "new_url" >expect &&
+		git config --worktree submodule.sub2.url >actual &&
+		test_cmp expect actual &&
+
+		cd wt &&
+
+		git submodule update --init &&
+		test_cmp ../.git/expect-config ../.git/config &&
+		echo "true" >expect &&
+		git config --worktree submodule.sub.active >actual &&
+		test_cmp expect actual &&
+		echo "$base_path/origin/sub" >expect &&
+		git config --worktree submodule.sub.url >actual &&
+		test_cmp expect actual &&
+
+		git submodule deinit sub &&
+		test_cmp ../.git/expect-config ../.git/config &&
+		test_must_be_empty ../.git/worktrees/wt/config.worktree &&
+
+		git submodule add ../sub sub2 &&
+		test_cmp ../.git/expect-config ../.git/config &&
+		echo "true" >expect &&
+		git config --worktree submodule.sub2.active >actual &&
+		test_cmp expect actual &&
+		echo "$base_path/origin/sub" >expect &&
+		git config --worktree submodule.sub2.url >actual &&
+		test_cmp expect actual &&
+
+		git config --file=.gitmodules submodule.sub2.url new_url &&
+		git submodule sync &&
+		test_cmp ../.git/expect-config ../.git/config &&
+		echo "new_url" >expect &&
+		git config --worktree submodule.sub2.url >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'deinit overwrites active config' '
+	test_when_finished "rm -rf repo-wt-config" &&
+	git clone "$base_path/origin/main" repo-wt-config &&
+	(
+		cd repo-wt-config &&
+		git submodule update --init &&
+		git config extensions.worktreeConfig true &&
+		git worktree add wt &&
+		cat .git/config >.git/expect-config &&
+
+		git submodule deinit sub &&
+		test_cmp .git/expect-config .git/config &&
+		echo "false" >expect &&
+		git config --worktree submodule.sub.active >actual &&
+		test_cmp expect actual &&
+
+		cd wt &&
+
+		git submodule deinit sub &&
+		test_cmp ../.git/expect-config ../.git/config &&
+		echo "false" >expect &&
+		git config --worktree submodule.sub.active >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'do not copy gitdir in config.worktree' '
+	test_when_finished "rm -rf copy-wt-config" &&
+	git clone "$base_path/origin/main" copy-wt-config &&
+	(
+		cd copy-wt-config &&
+		git config core.repositoryformatversion 1 &&
+		git config extensions.submodulePathConfig true &&
+		git config extensions.worktreeConfig true &&
+
+		git submodule update --init &&
+		echo "true" >expect &&
+		git config --worktree submodule.sub.active >actual &&
+		test_cmp expect actual &&
+		echo "$base_path/origin/sub" >expect &&
+		git config --worktree submodule.sub.url >actual &&
+		test_cmp expect actual &&
+		echo ".git/modules/sub" >expect &&
+		git config --worktree submodule.sub.gitdir >actual &&
+		test_cmp expect actual &&
+
+		git worktree add wt &&
+		cd wt &&
+		echo "true" >expect &&
+		git config --worktree submodule.sub.active >actual &&
+		test_cmp expect actual &&
+		echo "$base_path/origin/sub" >expect &&
+		git config --worktree submodule.sub.url >actual &&
+		test_cmp expect actual &&
+		test_must_fail git config --worktree submodule.sub.gitdir
+	)
+'
+
 test_done
diff --git a/t/t7425-submodule-gitdir-path-extension.sh b/t/t7425-submodule-gitdir-path-extension.sh
index 02123b8468..3bae4c4ef1 100755
--- a/t/t7425-submodule-gitdir-path-extension.sh
+++ b/t/t7425-submodule-gitdir-path-extension.sh
@@ -78,6 +78,55 @@ test_expect_success 'gitdir config path is relative for both absolute and relati
 	)
 '
 
+test_expect_success 'gitdir for the main worktree is stored in worktree config' '
+	test_when_finished "rm -rf relative-cfg-path-test-main" &&
+	git init -b main relative-cfg-path-test-main &&
+	(
+		cd relative-cfg-path-test-main &&
+		git config core.repositoryformatversion 1 &&
+		git config extensions.submodulePathConfig true &&
+		git config extensions.worktreeConfig true &&
+
+		# Test with absolute URL
+		git submodule add "$TRASH_DIRECTORY/new-sub" sub-abs &&
+		git config --worktree submodule.sub-abs.gitdir >actual &&
+		echo ".git/modules/sub-abs" >expect &&
+		test_cmp expect actual &&
+
+		# Test with relative URL
+		git submodule add ../new-sub sub-rel &&
+		git config --worktree submodule.sub-rel.gitdir >actual &&
+		echo ".git/modules/sub-rel" >expect &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'gitdir for a linked worktree is stored in worktree config' '
+	test_when_finished "rm -rf relative-cfg-path-test-wt" &&
+	git init -b main relative-cfg-path-test-wt &&
+	(
+		cd relative-cfg-path-test-wt &&
+		git config core.repositoryformatversion 1 &&
+		git config extensions.submodulePathConfig true &&
+		git config extensions.worktreeConfig true &&
+
+		git worktree add wt &&
+		cd wt &&
+
+		# Test with absolute URL
+		git submodule add "$TRASH_DIRECTORY/new-sub" sub-abs &&
+		git config --worktree submodule.sub-abs.gitdir >actual &&
+		echo "../.git/worktrees/wt/modules/sub-abs" >expect &&
+		test_cmp expect actual &&
+
+		# Test with relative URL
+		git submodule add ../../new-sub sub-rel &&
+		git config --worktree submodule.sub-rel.gitdir >actual &&
+		echo "../.git/worktrees/wt/modules/sub-rel" >expect &&
+		test_cmp expect actual
+	)
+'
+
 test_expect_success 'clone from repo with both legacy and new-style submodules' '
 	git clone --recurse-submodules main cloned-non-extension &&
 	(
-- 
gitgitgadget