From: Phillip Wood Date: Mon, 08 Sep 2025 14:23:53 GMT Subject: Re: [PATCH 3/9] submodule: add gitdir path config override Message-ID: In-Reply-To: <20250816213642.3517822-4-adrian.ratiu@collabora.com> Hi Adrian On 16/08/2025 22:36, Adrian Ratiu wrote: > This adds an ability to override gitdir paths via config files > (not .gitmodules), such that any encoding scheme can be changed > and JGit & co don't need to exactly match the default encoding. Reading the old email thread you linked to in the cover letter, my understanding is that this was suggested as an alternative to changing the gitdir for submodules from "modules" to "submodules". Do the later patches set this key when encoding the gitdir so that JGit can find the gitdir by reading the config? Thanks Phillip > A new test and a helper are added. The helper will be used by > further tests exercising gitdir paths & encodings. > > Based-on-patch-by: Brandon Williams > Signed-off-by: Adrian Ratiu > --- > builtin/submodule--helper.c | 17 +++++++++++++++++ > submodule.c | 11 +++++++++++ > t/lib-verify-submodule-gitdir-path.sh | 15 +++++++++++++++ > t/t7400-submodule-basic.sh | 15 +++++++++++++++ > 4 files changed, 58 insertions(+) > create mode 100644 t/lib-verify-submodule-gitdir-path.sh > > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c > index 7243429c6f..30e40d6c79 100644 > --- a/builtin/submodule--helper.c > +++ b/builtin/submodule--helper.c > @@ -1214,6 +1214,22 @@ static int module_summary(int argc, const char **argv, const char *prefix, > return ret; > } > > +static int module_gitdir(int argc, const char **argv, const char *prefix UNUSED, > + struct repository *repo UNUSED) > +{ > + struct strbuf gitdir = STRBUF_INIT; > + > + if (argc != 2) > + usage(_("git submodule--helper gitdir ")); > + > + submodule_name_to_gitdir(&gitdir, the_repository, argv[1]); > + > + printf("%s\n", gitdir.buf); > + > + strbuf_release(&gitdir); > + return 0; > +} > + > struct sync_cb { > const char *prefix; > const char *super_prefix; > @@ -3597,6 +3613,7 @@ int cmd_submodule__helper(int argc, > NULL > }; > struct option options[] = { > + OPT_SUBCOMMAND("gitdir", &fn, module_gitdir), > OPT_SUBCOMMAND("clone", &fn, module_clone), > OPT_SUBCOMMAND("add", &fn, module_add), > OPT_SUBCOMMAND("update", &fn, module_update), > diff --git a/submodule.c b/submodule.c > index dbf2244e60..bf78636195 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -2611,6 +2611,17 @@ void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r, > * administrators can explicitly set. Nothing has been decided, > * so for now, just append the name at the end of the path. > */ > + char *gitdir_path, *key; > + > + /* Allow config override. */ > + key = xstrfmt("submodule.%s.gitdirpath", submodule_name); > + if (!repo_config_get_string(r, key, &gitdir_path)) { > + strbuf_addstr(buf, gitdir_path); > + free(key); > + free(gitdir_path); > + return; > + } > + free(key); > > /* Legacy behavior: allow existing paths under modules/. */ > repo_git_path_append(r, buf, "modules/"); > diff --git a/t/lib-verify-submodule-gitdir-path.sh b/t/lib-verify-submodule-gitdir-path.sh > new file mode 100644 > index 0000000000..fb5cb8eea4 > --- /dev/null > +++ b/t/lib-verify-submodule-gitdir-path.sh > @@ -0,0 +1,15 @@ > +# Helper to verify if repo $1 contains a submodule named $2 with gitdir in path $3 > + > +verify_submodule_gitdir_path() { > + repo="$1" && > + name="$2" && > + path="$3" && > + ( > + cd "$repo" && > + cat >expect <<-EOF && > + $(git rev-parse --git-common-dir)/$path > + EOF > + git submodule--helper gitdir "$name" >actual && > + test_cmp expect actual > + ) > +} > diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh > index 178c386212..f4d4fb8397 100755 > --- a/t/t7400-submodule-basic.sh > +++ b/t/t7400-submodule-basic.sh > @@ -13,6 +13,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main > export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME > > . ./test-lib.sh > +. "$TEST_DIRECTORY"/lib-verify-submodule-gitdir-path.sh > > test_expect_success 'setup - enable local submodules' ' > git config --global protocol.file.allow always > @@ -1505,4 +1506,18 @@ test_expect_success 'submodule add fails when name is reused' ' > ) > ' > > +test_expect_success 'submodule helper gitdir config overrides' ' > + verify_submodule_gitdir_path test-submodule child submodules/child && > + ( > + cd test-submodule && > + git config submodule.child.gitdirpath ".git/submodules/custom-child" > + ) && > + verify_submodule_gitdir_path test-submodule child submodules/custom-child && > + ( > + cd test-submodule && > + git config --unset submodule.child.gitdirpath > + ) && > + verify_submodule_gitdir_path test-submodule child submodules/child > +' > + > test_done