submodule: introduce extensions.submodulePathConfig

index 532456644b..f4f57c9114 100644 --- a/Documentation/config/extensions.adoc +++ b/Documentation/config/extensions.adoc @@ -73,6 +73,29 @@ relativeWorktrees::: repaired with either the `--relative-paths` option or with the `worktree.useRelativePaths` config set to `true`. +submodulePathConfig::: + This extension is for the minority of users who: ++ +-- +* Encounter errors like `refusing to create ... in another submodule's git dir` + due to a number of reasons, like case-insensitive filesystem conflicts when + creating modules named `foo` and `Foo`. +* Require more flexible submodule layouts, for example due to nested names like + `foo`, `foo/bar` and `foo/baz` not supported by the default gitdir mechanism + which uses `.git/modules/<plain-name>` locations, causing further conflicts. +-- ++ +When `extensions.submodulePathConfig` is enabled, the `submodule.<name>.gitdir` +config becomes the single source of truth for all submodule gitdir paths and is +automatically set for all new submodules both during clone and init operations. ++ +Git will error out if a module does not have a corresponding +`submodule.<name>.gitdir` set. ++ +Existing (pre-extension) submodules need to be migrated by adding the missing +config entries. This is done manually for now, e.g. for each submodule: +`git config submodule.<name>.gitdir .git/modules/<name>`. + worktreeConfig::: If enabled, then worktrees will load config settings from the `$GIT_DIR/config.worktree` file in addition to the

index 0672d99117..74f1659a91 100644 --- a/Documentation/config/submodule.adoc +++ b/Documentation/config/submodule.adoc @@ -52,6 +52,13 @@ submodule.<name>.active:: submodule.active config option. See linkgit:gitsubmodules[7] for details. +submodule.<name>.gitdir:: + This sets the gitdir path for submodule <name>. This configuration is + 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. + submodule.active:: A repeated field which contains a pathspec used to match against a submodule's path to determine if the submodule is of interest to git

index 7ea82d7fa2..ef37352534 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -435,6 +435,48 @@ struct init_cb { }; #define INIT_CB_INIT { 0 } +static int validate_and_set_submodule_gitdir(struct strbuf *gitdir_path, + const char *submodule_name) +{ + const char *value; + char *key; + + if (validate_submodule_git_dir(gitdir_path->buf, submodule_name)) + return -1; + + key = xstrfmt("submodule.%s.gitdir", submodule_name); + + /* Nothing to do if the config already exists. */ + if (!repo_config_get_string_tmp(the_repository, key, &value)) { + free(key); + return 0; + } + + if (repo_config_set_gently(the_repository, key, gitdir_path->buf)) { + free(key); + return -1; + } + + free(key); + return 0; +} + +static void create_default_gitdir_config(const char *submodule_name) +{ + struct strbuf gitdir_path = STRBUF_INIT; + + repo_git_path_append(the_repository, &gitdir_path, "modules/%s", submodule_name); + if (!validate_and_set_submodule_gitdir(&gitdir_path, submodule_name)) { + strbuf_release(&gitdir_path); + return; + } + + die(_("failed to set a valid default config for 'submodule.%s.gitdir'. " + "Please ensure it is set, for example by running something like: " + "'git config submodule.%s.gitdir .git/modules/%s'"), + submodule_name, submodule_name, submodule_name); +} + static void init_submodule(const char *path, const char *prefix, const char *super_prefix, unsigned int flags) @@ -511,6 +553,10 @@ static void init_submodule(const char *path, const char *prefix, if (repo_config_set_gently(the_repository, sb.buf, upd)) die(_("Failed to register update mode for submodule path '%s'"), displaypath); } + + if (the_repository->repository_format_submodule_path_cfg) + create_default_gitdir_config(sub->name); + strbuf_release(&sb); free(displaypath); free(url); @@ -1805,8 +1851,9 @@ static int clone_submodule(const struct module_clone_data *clone_data, char *head = xstrfmt("%s/HEAD", sm_gitdir); unlink(head); free(head); - die(_("refusing to create/use '%s' in another submodule's " - "git dir"), sm_gitdir); + die(_("refusing to create/use '%s' in another submodule's git dir. " + "Enabling extensions.submodulePathConfig should fix this."), + sm_gitdir); } connect_work_tree_and_git_dir(clone_data_path, sm_gitdir, 0); @@ -3578,6 +3625,9 @@ static int module_add(int argc, const char **argv, const char *prefix, add_data.progress = !!progress; add_data.dissociate = !!dissociate; + if (the_repository->repository_format_submodule_path_cfg) + create_default_gitdir_config(add_data.sm_name); + if (add_submodule(&add_data)) goto cleanup; configure_added_submodule(&add_data);

index 6faf5c7398..35a06e6719 100644 --- a/repository.c +++ b/repository.c @@ -288,6 +288,7 @@ int repo_init(struct repository *repo, repo->repository_format_worktree_config = format.worktree_config; repo->repository_format_relative_worktrees = format.relative_worktrees; repo->repository_format_precious_objects = format.precious_objects; + repo->repository_format_submodule_path_cfg = format.submodule_path_cfg; /* take ownership of format.partial_clone */ repo->repository_format_partial_clone = format.partial_clone;

index 5808a5d610..aa907bd1e4 100644 --- a/repository.h +++ b/repository.h @@ -158,6 +158,7 @@ struct repository { int repository_format_worktree_config; int repository_format_relative_worktrees; int repository_format_precious_objects; + int repository_format_submodule_path_cfg; /* Indicate if a repository has a different 'commondir' from 'gitdir' */ unsigned different_commondir:1;

index 7086741e6c..207fa36e10 100644 --- a/setup.c +++ b/setup.c @@ -687,6 +687,9 @@ static enum extension_result handle_extension(const char *var, } else if (!strcmp(ext, "relativeworktrees")) { data->relative_worktrees = git_config_bool(var, value); return EXTENSION_OK; + } else if (!strcmp(ext, "submodulepathconfig")) { + data->submodule_path_cfg = git_config_bool(var, value); + return EXTENSION_OK; } return EXTENSION_UNKNOWN; } @@ -1865,6 +1868,8 @@ const char *setup_git_directory_gently(int *nongit_ok) repo_fmt.worktree_config; the_repository->repository_format_relative_worktrees = repo_fmt.relative_worktrees; + the_repository->repository_format_submodule_path_cfg = + repo_fmt.submodule_path_cfg; /* take ownership of repo_fmt.partial_clone */ the_repository->repository_format_partial_clone = repo_fmt.partial_clone; @@ -1963,6 +1968,8 @@ void check_repository_format(struct repository_format *fmt) fmt->ref_storage_format); the_repository->repository_format_worktree_config = fmt->worktree_config; + the_repository->repository_format_submodule_path_cfg = + fmt->submodule_path_cfg; the_repository->repository_format_relative_worktrees = fmt->relative_worktrees; the_repository->repository_format_partial_clone =

index 8522fa8575..568bb9f1d1 100644 --- a/setup.h +++ b/setup.h @@ -130,6 +130,7 @@ struct repository_format { char *partial_clone; /* value of extensions.partialclone */ int worktree_config; int relative_worktrees; + int submodule_path_cfg; int is_bare; int hash_algo; int compat_hash_algo;

index d937911fbc..f7af389c79 100644 --- a/submodule.c +++ b/submodule.c @@ -31,6 +31,7 @@ #include "commit-reach.h" #include "read-cache-ll.h" #include "setup.h" +#include "advice.h" static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF; static int initialized_fetch_ref_tips; @@ -2164,8 +2165,9 @@ int submodule_move_head(const char *path, const char *super_prefix, if (validate_submodule_git_dir(git_dir, sub->name) < 0) die(_("refusing to create/use '%s' in " - "another submodule's git dir"), - git_dir); + "another submodule's git dir. " + "Enabling extensions.submodulePathConfig " + "should fix this."), git_dir); free(git_dir); } } else { @@ -2576,30 +2578,37 @@ cleanup: void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r, const char *submodule_name) { - /* - * NEEDSWORK: The current way of mapping a submodule's name to - * its location in .git/modules/ has problems with some naming - * schemes. For example, if a submodule is named "foo" and - * another is named "foo/bar" (whether present in the same - * superproject commit or not - the problem will arise if both - * superproject commits have been checked out at any point in - * time), or if two submodule names only have different cases in - * a case-insensitive filesystem. - * - * There are several solutions, including encoding the path in - * some way, introducing a submodule.<name>.gitdir config in - * .git/config (not .gitmodules) that allows overriding what the - * gitdir of a submodule would be (and teach Git, upon noticing - * a clash, to automatically determine a non-clashing name and - * to write such a config), or introducing a - * submodule.<name>.gitdir config in .gitmodules that repo - * administrators can explicitly set. Nothing has been decided, - * so for now, just append the name at the end of the path. - */ - repo_git_path_append(r, buf, "modules/"); - strbuf_addstr(buf, submodule_name); + if (!r->repository_format_submodule_path_cfg) { + /* + * If extensions.submodulePathConfig is disabled, + * continue to use the plain path. + */ + repo_git_path_append(r, buf, "modules/%s", submodule_name); + } else { + const char *gitdir; + char *key; + int ret; - if (validate_submodule_git_dir(buf->buf, submodule_name) < 0) + /* Otherwise the extension is enabled, so use the gitdir config. */ + key = xstrfmt("submodule.%s.gitdir", submodule_name); + ret = repo_config_get_string_tmp(r, key, &gitdir); + FREE_AND_NULL(key); + + if (ret) + die(_("the 'submodule.%s.gitdir' config does not exist for module '%s'. " + "Please ensure it is set, for example by running something like: " + "'git config submodule.%s.gitdir .git/modules/%s'. For details " + "see the extensions.submodulePathConfig documentation."), + submodule_name, submodule_name, submodule_name, submodule_name); + + strbuf_addstr(buf, gitdir); + } + + /* validate because users might have modified the config */ + if (validate_submodule_git_dir(buf->buf, submodule_name)) { + advise(_("enabling extensions.submodulePathConfig might fix the " + "following error, if it's not already enabled.")); die(_("refusing to create/use '%s' in another submodule's " - "git dir"), buf->buf); + " git dir."), buf->buf); + } }

new file mode 100644 index 0000000000..4e0cfdc605 --- /dev/null +++ b/t/lib-verify-submodule-gitdir-path.sh @@ -0,0 +1,24 @@ +# Helper to verify if repo $1 contains a submodule named $2 with gitdir path $3 + +# This does not check filesystem existence. That is done in submodule.c via the +# submodule_name_to_gitdir() API which this helper ends up calling. The gitdirs +# might or might not exist (e.g. when adding a new submodule), so this only +# checks the expected configuration path, which might be overridden by the user. + +verify_submodule_gitdir_path () { + repo="$1" && + name="$2" && + path="$3" && + ( + cd "$repo" && + # Compute expected absolute path + expected="$(git rev-parse --git-common-dir)/$path" && + expected="$(test-tool path-utils real_path "$expected")" && + # Compute actual absolute path + actual="$(git submodule--helper gitdir "$name")" && + actual="$(test-tool path-utils real_path "$actual")" && + echo "$expected" >expect && + echo "$actual" >actual && + test_cmp expect actual + ) +}

index a5531df415..2c565beb8d 100644 --- a/t/meson.build +++ b/t/meson.build @@ -884,6 +884,7 @@ integration_tests = [ 't7422-submodule-output.sh', 't7423-submodule-symlinks.sh', 't7424-submodule-mixed-ref-formats.sh', + 't7425-submodule-gitdir-path-extension.sh', 't7450-bad-git-dotfiles.sh', 't7500-commit-template-squash-signoff.sh', 't7501-commit-basic-functionality.sh',

new file mode 100755 index 0000000000..453183e27c --- /dev/null +++ b/t/t7425-submodule-gitdir-path-extension.sh @@ -0,0 +1,160 @@ +#!/bin/sh + +test_description='submodulePathConfig extension works as expected' + +. ./test-lib.sh +. "$TEST_DIRECTORY"/lib-verify-submodule-gitdir-path.sh + +test_expect_success 'setup: allow file protocol' ' + git config --global protocol.file.allow always +' + +test_expect_success 'create repo with mixed extension submodules' ' + git init -b main legacy-sub && + test_commit -C legacy-sub legacy-initial && + legacy_rev=$(git -C legacy-sub rev-parse HEAD) && + + git init -b main new-sub && + test_commit -C new-sub new-initial && + new_rev=$(git -C new-sub rev-parse HEAD) && + + git init -b main main && + ( + cd main && + git submodule add ../legacy-sub legacy && + test_commit legacy-sub && + + # trigger the "die_path_inside_submodule" check + test_must_fail git submodule add ../new-sub "legacy/nested" && + + git config core.repositoryformatversion 1 && + git config extensions.submodulePathConfig true && + + git submodule add ../new-sub "New Sub" && + test_commit new && + + # retrigger the "die_path_inside_submodule" check with encoding + test_must_fail git submodule add ../new-sub "New Sub/nested2" + ) +' + +test_expect_success 'verify new submodule gitdir config' ' + git -C main config submodule."New Sub".gitdir >actual && + echo ".git/modules/New Sub" >expect && + test_cmp expect actual && + verify_submodule_gitdir_path main "New Sub" "modules/New Sub" +' + +test_expect_success 'manual add and verify legacy submodule gitdir config' ' + # the legacy module should not contain a gitdir config, because it + # was added before the extension was enabled. Add and test it. + test_must_fail git -C main config submodule.legacy.gitdir && + git -C main config submodule.legacy.gitdir .git/modules/legacy && + git -C main config submodule.legacy.gitdir >actual && + echo ".git/modules/legacy" >expect && + test_cmp expect actual && + verify_submodule_gitdir_path main "legacy" "modules/legacy" +' + +test_expect_success 'gitdir config path is relative for both absolute and relative urls' ' + test_when_finished "rm -rf relative-cfg-path-test" && + git init -b main relative-cfg-path-test && + ( + cd relative-cfg-path-test && + git config core.repositoryformatversion 1 && + git config extensions.submodulePathConfig true && + + # Test with absolute URL + git submodule add "$TRASH_DIRECTORY/new-sub" sub-abs && + git config 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 submodule.sub-rel.gitdir >actual && + echo ".git/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 && + ( + cd cloned-non-extension && + + test_path_is_dir .git/modules/legacy && + test_path_is_dir .git/modules/"New Sub" && + + test_must_fail git config submodule.legacy.gitdir && + test_must_fail git config submodule."New Sub".gitdir && + + git submodule status >list && + test_grep "$legacy_rev legacy" list && + test_grep "$new_rev New Sub" list + ) && + + git clone -c extensions.submodulePathConfig=true --recurse-submodules main cloned-extension && + ( + cd cloned-extension && + + test_path_is_dir .git/modules/legacy && + test_path_is_dir ".git/modules/New Sub" && + + git config submodule.legacy.gitdir && + git config submodule."New Sub".gitdir && + + git submodule status >list && + test_grep "$legacy_rev legacy" list && + test_grep "$new_rev New Sub" list + ) +' + +test_expect_success 'commit and push changes to encoded submodules' ' + git -C legacy-sub config receive.denyCurrentBranch updateInstead && + git -C new-sub config receive.denyCurrentBranch updateInstead && + git -C main config receive.denyCurrentBranch updateInstead && + ( + cd cloned-extension && + + git -C legacy switch --track -C main origin/main && + test_commit -C legacy second-commit && + git -C legacy push && + + git -C "New Sub" switch --track -C main origin/main && + test_commit -C "New Sub" second-commit && + git -C "New Sub" push && + + # Stage and commit submodule changes in superproject + git switch --track -C main origin/main && + git add legacy "New Sub" && + git commit -m "update submodules" && + + # push superproject commit to main repo + git push + ) && + + # update expected legacy & new submodule checksums + legacy_rev=$(git -C legacy-sub rev-parse HEAD) && + new_rev=$(git -C new-sub rev-parse HEAD) +' + +test_expect_success 'fetch mixed submodule changes and verify updates' ' + ( + cd main && + + # only update submodules because superproject was + # pushed into at the end of last test + git submodule update --init --recursive && + + test_path_is_dir .git/modules/legacy && + test_path_is_dir ".git/modules/New Sub" && + + # Verify both submodules are at the expected commits + git submodule status >list && + test_grep "$legacy_rev legacy" list && + test_grep "$new_rev New Sub" list + ) +' + +test_done

index 964e1f1569..ffb9c8b522 100755 --- a/t/t9902-completion.sh +++ b/t/t9902-completion.sh @@ -3053,6 +3053,7 @@ test_expect_success 'git config set - variable name - __git_compute_second_level submodule.sub.fetchRecurseSubmodules Z submodule.sub.ignore Z submodule.sub.active Z + submodule.sub.gitdir Z EOF '