* [PATCH 0/9] Encode submodule gitdir names to avoid conflicts
@ 2025-08-16 21:36 Adrian Ratiu
2025-08-16 21:36 ` [PATCH 1/9] submodule--helper: use submodule_name_to_gitdir in add_submodule Adrian Ratiu
` (9 more replies)
0 siblings, 10 replies; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-16 21:36 UTC (permalink / raw)
To: git
Cc: Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Stefan Beller,
Patrick Steinhardt, Adrian Ratiu
Hello,
This is a continuation of work done back in 2018 [1], so a big thank you to
everyone who participated in the initial thread, especially Brandon on whose
code this is partially based upon. Hope you are still around and doing well. :)
It's mostly a rewrite from scratch addressig open feedback. I decided to
iterate upen Brandon's url-encoding design instead of pursuing alternatives
like a custom encoding, name hashing or round-trip encoding/decoding using
an in-memory git mapping (we'd still have to encode/hash the paths to avoid
colflicts so IIUC this last one is more complicated for little gain).
I tried to organize and explain the commits in a logical way which is also
easy to review, keeping the encoding parts, new tests, code moving around
and path update churn as clearly separated as possible.
This is based on master and I've merged and succesfully run all tests in
both the next and seen branches.
P.S. I plan to give a short talk at the mini-summit in 2 weesks based on this
series and some other patches I wish to propose on the ML, so if any of you
are attending and wish to connect in person, see you there!
Link: https://lore.kernel.org/git/20180807230637.247200-1-bmwill@google.com/ [1]
Adrian Ratiu (9):
submodule--helper: use submodule_name_to_gitdir in add_submodule
submodule: create new gitdirs under submodules path
submodule: add gitdir path config override
t: submodules: add basic mixed gitdir path tests
strbuf: bring back is_rfc3986_unreserved
submodule: encode gitdir paths to avoid conflicts
submodule: remove validate_submodule_git_dir()
t: move nested gitdir tests to proper location
t: add gitdir encoding tests
Documentation/fetch-options.adoc | 2 +-
Documentation/git-fetch.adoc | 2 +-
Documentation/git-submodule.adoc | 2 +-
Documentation/gitsubmodules.adoc | 8 +-
builtin/credential-store.c | 6 -
builtin/submodule--helper.c | 49 +++--
setup.c | 2 +-
strbuf.c | 6 +
strbuf.h | 2 +
submodule.c | 158 +++++++---------
submodule.h | 5 -
t/lib-submodule-update.sh | 50 ++---
t/lib-verify-submodule-gitdir-path.sh | 15 ++
t/meson.build | 1 +
t/t0035-safe-bare-repository.sh | 4 +-
t/t1600-index.sh | 4 +-
t/t2405-worktree-submodule.sh | 8 +-
t/t2501-cwd-empty.sh | 2 +-
t/t3600-rm.sh | 8 +-
t/t5526-fetch-submodules.sh | 2 +-
t/t5619-clone-local-ambiguous-transport.sh | 4 +-
t/t6120-describe.sh | 4 +-
t/t7001-mv.sh | 4 +-
t/t7400-submodule-basic.sh | 33 +++-
t/t7406-submodule-update.sh | 14 +-
t/t7407-submodule-foreach.sh | 6 +-
t/t7408-submodule-reference.sh | 22 +--
t/t7412-submodule-absorbgitdirs.sh | 22 +--
t/t7423-submodule-symlinks.sh | 8 +-
t/t7425-submodule-mixed-gitdir-paths.sh | 207 +++++++++++++++++++++
t/t7450-bad-git-dotfiles.sh | 73 +-------
t/t7527-builtin-fsmonitor.sh | 4 +-
32 files changed, 451 insertions(+), 286 deletions(-)
create mode 100644 t/lib-verify-submodule-gitdir-path.sh
create mode 100755 t/t7425-submodule-mixed-gitdir-paths.sh
--
2.50.1.679.gbf363a8fbb.dirty
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/9] submodule--helper: use submodule_name_to_gitdir in add_submodule
2025-08-16 21:36 [PATCH 0/9] Encode submodule gitdir names to avoid conflicts Adrian Ratiu
@ 2025-08-16 21:36 ` Adrian Ratiu
2025-08-20 19:04 ` Josh Steadmon
2025-08-16 21:36 ` [PATCH 2/9] submodule: create new gitdirs under submodules path Adrian Ratiu
` (8 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-16 21:36 UTC (permalink / raw)
To: git
Cc: Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Stefan Beller,
Patrick Steinhardt, Adrian Ratiu
While testing submodule gitdir path encoding, I noticed submodule--helper
is still using a hardcoded name-based path leading to test failures, so
convert it to the common helper function introduced by commit ce125d431a
("submodule: extract path to submodule gitdir func") and used in other
locations accross the source tree.
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
builtin/submodule--helper.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 07a1935cbe..7243429c6f 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -3213,10 +3213,11 @@ static int add_submodule(const struct add_data *add_data)
free(submod_gitdir_path);
} else {
struct child_process cp = CHILD_PROCESS_INIT;
+ struct strbuf submod_gitdir = STRBUF_INIT;
- submod_gitdir_path = xstrfmt(".git/modules/%s", add_data->sm_name);
+ submodule_name_to_gitdir(&submod_gitdir, the_repository, add_data->sm_name);
- if (is_directory(submod_gitdir_path)) {
+ if (is_directory(submod_gitdir.buf)) {
if (!add_data->force) {
struct strbuf msg = STRBUF_INIT;
char *die_msg;
@@ -3225,8 +3226,8 @@ static int add_submodule(const struct add_data *add_data)
"locally with remote(s):\n"),
add_data->sm_name);
- append_fetch_remotes(&msg, submod_gitdir_path);
- free(submod_gitdir_path);
+ append_fetch_remotes(&msg, submod_gitdir.buf);
+ strbuf_release(&submod_gitdir);
strbuf_addf(&msg, _("If you want to reuse this local git "
"directory instead of cloning again from\n"
@@ -3244,7 +3245,7 @@ static int add_submodule(const struct add_data *add_data)
"submodule '%s'\n"), add_data->sm_name);
}
}
- free(submod_gitdir_path);
+ strbuf_release(&submod_gitdir);
clone_data.prefix = add_data->prefix;
clone_data.path = add_data->sm_path;
--
2.50.1.679.gbf363a8fbb.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/9] submodule: create new gitdirs under submodules path
2025-08-16 21:36 [PATCH 0/9] Encode submodule gitdir names to avoid conflicts Adrian Ratiu
2025-08-16 21:36 ` [PATCH 1/9] submodule--helper: use submodule_name_to_gitdir in add_submodule Adrian Ratiu
@ 2025-08-16 21:36 ` Adrian Ratiu
2025-08-16 21:36 ` [PATCH 3/9] submodule: add gitdir path config override Adrian Ratiu
` (7 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-16 21:36 UTC (permalink / raw)
To: git
Cc: Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Stefan Beller,
Patrick Steinhardt, Adrian Ratiu
This is in preparation for encoding the submodule names to avoid conflicts
like submodules named foo and foo/bar together with case-insensitive file-
system handling and other corner cases like reserved filenames on Windows.
Backward compatibility is kept with plain-name modules already existing at
paths like .git/modules/<name>, however a clear separation between legacy
(plain) and new (encoded) namespaces is desirable, to avoid situations like
an existing plain-name module containing the encoding escape character/
Thus we split the new-style (encoded) gitdir name paths to .git/submodules,
while legacy-style paths remain under .git/modules.
This is just a default directory change with the accompanying test updates,
in preparation for the actual encoding additions in future commits.
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
Documentation/fetch-options.adoc | 2 +-
Documentation/git-fetch.adoc | 2 +-
Documentation/git-submodule.adoc | 2 +-
Documentation/gitsubmodules.adoc | 8 ++--
setup.c | 2 +-
submodule.c | 28 +++++++++---
t/lib-submodule-update.sh | 50 +++++++++++-----------
t/t0035-safe-bare-repository.sh | 4 +-
t/t1600-index.sh | 4 +-
t/t2405-worktree-submodule.sh | 8 ++--
t/t2501-cwd-empty.sh | 2 +-
t/t3600-rm.sh | 8 ++--
t/t5526-fetch-submodules.sh | 2 +-
t/t5619-clone-local-ambiguous-transport.sh | 4 +-
t/t6120-describe.sh | 4 +-
t/t7001-mv.sh | 4 +-
t/t7400-submodule-basic.sh | 18 ++++----
t/t7406-submodule-update.sh | 10 ++---
t/t7407-submodule-foreach.sh | 6 +--
t/t7408-submodule-reference.sh | 22 +++++-----
t/t7412-submodule-absorbgitdirs.sh | 22 +++++-----
t/t7423-submodule-symlinks.sh | 8 ++--
t/t7450-bad-git-dotfiles.sh | 32 +++++++-------
t/t7527-builtin-fsmonitor.sh | 4 +-
24 files changed, 136 insertions(+), 120 deletions(-)
diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
index b01372e4b3..f8d3f65009 100644
--- a/Documentation/fetch-options.adoc
+++ b/Documentation/fetch-options.adoc
@@ -210,7 +210,7 @@ ifndef::git-pull[]
submodule that has commits that are referenced by a newly fetched
superproject commit but are missing in the local submodule clone. A
changed submodule can be fetched as long as it is present locally e.g.
- in `$GIT_DIR/modules/` (see linkgit:gitsubmodules[7]); if the upstream
+ in `$GIT_DIR/submodules/` (see linkgit:gitsubmodules[7]); if the upstream
adds a new submodule, that submodule cannot be fetched until it is
cloned e.g. by `git submodule update`.
+
diff --git a/Documentation/git-fetch.adoc b/Documentation/git-fetch.adoc
index 16f5d9d69a..2923a29bef 100644
--- a/Documentation/git-fetch.adoc
+++ b/Documentation/git-fetch.adoc
@@ -304,7 +304,7 @@ include::config/fetch.adoc[]
BUGS
----
Using --recurse-submodules can only fetch new commits in submodules that are
-present locally e.g. in `$GIT_DIR/modules/`. If the upstream adds a new
+present locally e.g. in `$GIT_DIR/submodules/`. If the upstream adds a new
submodule, that submodule cannot be fetched until it is cloned e.g. by `git
submodule update`. This is expected to be fixed in a future Git version.
diff --git a/Documentation/git-submodule.adoc b/Documentation/git-submodule.adoc
index 503c84a200..9389862208 100644
--- a/Documentation/git-submodule.adoc
+++ b/Documentation/git-submodule.adoc
@@ -266,7 +266,7 @@ registered submodules, and sync any nested submodules within.
absorbgitdirs::
If a git directory of a submodule is inside the submodule,
move the git directory of the submodule into its superproject's
- `$GIT_DIR/modules` path and then connect the git directory and
+ `$GIT_DIR/submodules` path and then connect the git directory and
its working directory by setting the `core.worktree` and adding
a .git file pointing to the git directory embedded in the
superprojects git directory.
diff --git a/Documentation/gitsubmodules.adoc b/Documentation/gitsubmodules.adoc
index f7b5a25a0c..061e24f316 100644
--- a/Documentation/gitsubmodules.adoc
+++ b/Documentation/gitsubmodules.adoc
@@ -21,12 +21,12 @@ The submodule has its own history; the repository it is embedded
in is called a superproject.
On the filesystem, a submodule usually (but not always - see FORMS below)
-consists of (i) a Git directory located under the `$GIT_DIR/modules/`
+consists of (i) a Git directory located under the `$GIT_DIR/submodules/`
directory of its superproject, (ii) a working directory inside the
superproject's working directory, and a `.git` file at the root of
the submodule's working directory pointing to (i).
-Assuming the submodule has a Git directory at `$GIT_DIR/modules/foo/`
+Assuming the submodule has a Git directory at `$GIT_DIR/submodules/foo/`
and a working directory at `path/to/bar/`, the superproject tracks the
submodule via a `gitlink` entry in the tree at `path/to/bar` and an entry
in its `.gitmodules` file (see linkgit:gitmodules[5]) of the form
@@ -137,7 +137,7 @@ using older versions of Git.
It is possible to construct these old form repositories manually.
+
When deinitialized or deleted (see below), the submodule's Git
-directory is automatically moved to `$GIT_DIR/modules/<name>/`
+directory is automatically moved to `$GIT_DIR/submodules/<name>/`
of the superproject.
* Deinitialized submodule: A `gitlink`, and a `.gitmodules` entry,
@@ -162,7 +162,7 @@ possible to checkout past commits without requiring fetching
from another repository.
+
To completely remove a submodule, manually delete
-`$GIT_DIR/modules/<name>/`.
+`$GIT_DIR/submodules/<name>/`.
ACTIVE SUBMODULES
-----------------
diff --git a/setup.c b/setup.c
index 98ddbf377f..d054dafa6a 100644
--- a/setup.c
+++ b/setup.c
@@ -1416,7 +1416,7 @@ static int is_implicit_bare_repo(const char *path)
* we are inside $GIT_DIR of a worktree of a non-embedded
* submodule, whose superproject is not a bare repository.
*/
- if (strstr(path, "/.git/modules/"))
+ if (strstr(path, "/.git/modules/") || strstr(path, "/.git/submodules/"))
return 1;
return 0;
diff --git a/submodule.c b/submodule.c
index fff3c75570..dbf2244e60 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1278,22 +1278,29 @@ void check_for_new_submodule_commits(struct object_id *oid)
/*
* Returns 1 if there is at least one submodule gitdir in
- * $GIT_DIR/modules and 0 otherwise. This follows
+ * $GIT_DIR/(sub)modules and 0 otherwise. This follows
* submodule_name_to_gitdir(), which looks for submodules in
- * $GIT_DIR/modules, not $GIT_COMMON_DIR.
+ * $GIT_DIR/(sub)modules, not $GIT_COMMON_DIR.
*
- * A submodule can be moved to $GIT_DIR/modules manually by running "git
- * submodule absorbgitdirs", or it may be initialized there by "git
- * submodule update".
+ * A submodule can be moved to $GIT_DIR/(sub)modules manually by running
+ * "git submodule absorbgitdirs", or it may be initialized there by
+ * "git submodule update".
*/
static int repo_has_absorbed_submodules(struct repository *r)
{
int ret;
struct strbuf buf = STRBUF_INIT;
+ /* check legacy path */
repo_git_path_append(r, &buf, "modules/");
ret = file_exists(buf.buf) && !is_empty_dir(buf.buf);
+ strbuf_reset(&buf);
+
+ /* new (encoded name) path */
+ repo_git_path_append(r, &buf, "submodules/");
+ ret |= file_exists(buf.buf) && !is_empty_dir(buf.buf);
strbuf_release(&buf);
+
return ret;
}
@@ -2273,7 +2280,7 @@ int validate_submodule_git_dir(char *git_dir, const char *submodule_name)
*
* Example: having a submodule named `hippo` and another one named
* `hippo/hooks` would result in the git directories
- * `.git/modules/hippo/` and `.git/modules/hippo/hooks/`, respectively,
+ * `.git/submodules/hippo/` and `.git/submodules/hippo/hooks/`, respectively,
* but the latter directory is already designated to contain the hooks
* of the former.
*/
@@ -2604,6 +2611,15 @@ 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.
*/
+
+ /* Legacy behavior: allow existing paths under modules/<name>. */
repo_git_path_append(r, buf, "modules/");
strbuf_addstr(buf, submodule_name);
+ if (!access(buf->buf, F_OK))
+ return;
+
+ /* New style (encoded) paths go under submodules/<encoded>. */
+ strbuf_reset(buf);
+ repo_git_path_append(r, buf, "submodules/");
+ strbuf_addstr(buf, submodule_name);
}
diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 36f767cb74..b6b2be1df5 100644
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -161,7 +161,7 @@ replace_gitfile_with_git_dir () {
}
# Test that the .git directory in the submodule is unchanged (except for the
-# core.worktree setting, which appears only in $GIT_DIR/modules/$1/config).
+# core.worktree setting, which appears only in $GIT_DIR/submodules/$1/config).
# Call this function before test_submodule_content as the latter might
# write the index file leading to false positive index differences.
#
@@ -170,23 +170,23 @@ replace_gitfile_with_git_dir () {
test_git_directory_is_unchanged () {
# does core.worktree point at the right place?
echo "../../../$1" >expect &&
- git -C ".git/modules/$1" config core.worktree >actual &&
+ git -C ".git/submodules/$1" config core.worktree >actual &&
test_cmp expect actual &&
# remove it temporarily before comparing, as
# "$1/.git/config" lacks it...
- git -C ".git/modules/$1" config --unset core.worktree &&
- diff -r ".git/modules/$1" "$1/.git" &&
+ git -C ".git/submodules/$1" config --unset core.worktree &&
+ diff -r ".git/submodules/$1" "$1/.git" &&
# ... and then restore.
- git -C ".git/modules/$1" config core.worktree "../../../$1"
+ git -C ".git/submodules/$1" config core.worktree "../../../$1"
}
test_git_directory_exists () {
- test -e ".git/modules/$1" &&
+ test -e ".git/submodules/$1" &&
if test -f sub1/.git
then
# does core.worktree point at the right place?
echo "../../../$1" >expect &&
- git -C ".git/modules/$1" config core.worktree >actual &&
+ git -C ".git/submodules/$1" config core.worktree >actual &&
test_cmp expect actual
fi
}
@@ -225,22 +225,22 @@ reset_work_tree_to () {
reset_work_tree_to_interested () {
reset_work_tree_to $1 &&
# make the submodule git dirs available
- if ! test -d submodule_update/.git/modules/sub1
+ if ! test -d submodule_update/.git/submodules/sub1
then
- mkdir -p submodule_update/.git/modules &&
- cp -r submodule_update_repo/.git/modules/sub1 submodule_update/.git/modules/sub1
- GIT_WORK_TREE=. git -C submodule_update/.git/modules/sub1 config --unset core.worktree
+ mkdir -p submodule_update/.git/submodules &&
+ cp -r submodule_update_repo/.git/submodules/sub1 submodule_update/.git/submodules/sub1
+ GIT_WORK_TREE=. git -C submodule_update/.git/submodules/sub1 config --unset core.worktree
fi &&
- if ! test -d submodule_update/.git/modules/sub1/modules/sub2
+ if ! test -d submodule_update/.git/submodules/sub1/submodules/sub2
then
- mkdir -p submodule_update/.git/modules/sub1/modules &&
- cp -r submodule_update_repo/.git/modules/sub1/modules/sub2 submodule_update/.git/modules/sub1/modules/sub2
+ mkdir -p submodule_update/.git/submodules/sub1/submodules &&
+ cp -r submodule_update_repo/.git/submodules/sub1/submodules/sub2 submodule_update/.git/submodules/sub1/submodules/sub2
# core.worktree is unset for sub2 as it is not checked out
fi &&
# indicate we are interested in the submodule:
git -C submodule_update config submodule.sub1.url "bogus" &&
# sub1 might not be checked out, so use the git dir
- git -C submodule_update/.git/modules/sub1 config submodule.sub2.url "bogus"
+ git -C submodule_update/.git/submodules/sub1 config submodule.sub2.url "bogus"
}
# Test that the superproject contains the content according to commit "$1"
@@ -742,7 +742,7 @@ test_submodule_recursing_with_args_common () {
$command remove_sub1 &&
test_superproject_content origin/remove_sub1 &&
! test -e sub1 &&
- test_must_fail git config -f .git/modules/sub1/config core.worktree
+ test_must_fail git config -f .git/submodules/sub1/config core.worktree
)
'
# ... absorbing a .git directory along the way.
@@ -753,7 +753,7 @@ test_submodule_recursing_with_args_common () {
cd submodule_update &&
git branch -t remove_sub1 origin/remove_sub1 &&
replace_gitfile_with_git_dir sub1 &&
- rm -rf .git/modules &&
+ rm -rf .git/submodules &&
$command remove_sub1 &&
test_superproject_content origin/remove_sub1 &&
! test -e sub1 &&
@@ -803,8 +803,8 @@ test_submodule_recursing_with_args_common () {
$command no_submodule &&
test_superproject_content origin/no_submodule &&
test_path_is_missing sub1 &&
- test_must_fail git config -f .git/modules/sub1/config core.worktree &&
- test_must_fail git config -f .git/modules/sub1/modules/sub2/config core.worktree
+ test_must_fail git config -f .git/submodules/sub1/config core.worktree &&
+ test_must_fail git config -f .git/submodules/sub1/submodules/sub2/config core.worktree
)
'
@@ -937,7 +937,7 @@ test_submodule_switch_recursing_with_args () {
cd submodule_update &&
git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
replace_gitfile_with_git_dir sub1 &&
- rm -rf .git/modules &&
+ rm -rf .git/submodules &&
$command replace_sub1_with_directory &&
test_superproject_content origin/replace_sub1_with_directory &&
test_git_directory_exists sub1
@@ -946,15 +946,15 @@ test_submodule_switch_recursing_with_args () {
# ... and ignored files are ignored
test_expect_success "$command: replace submodule with a file works ignores ignored files in submodule" '
- test_when_finished "rm submodule_update/.git/modules/sub1/info/exclude" &&
+ test_when_finished "rm submodule_update/.git/submodules/sub1/info/exclude" &&
prolog &&
reset_work_tree_to_interested add_sub1 &&
(
cd submodule_update &&
- rm -rf .git/modules/sub1/info &&
+ rm -rf .git/submodules/sub1/info &&
git branch -t replace_sub1_with_file origin/replace_sub1_with_file &&
- mkdir .git/modules/sub1/info &&
- echo ignored >.git/modules/sub1/info/exclude &&
+ mkdir .git/submodules/sub1/info &&
+ echo ignored >.git/submodules/sub1/info/exclude &&
: >sub1/ignored &&
$command replace_sub1_with_file &&
test_superproject_content origin/replace_sub1_with_file &&
@@ -1034,7 +1034,7 @@ test_submodule_forced_switch_recursing_with_args () {
cd submodule_update &&
git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
replace_gitfile_with_git_dir sub1 &&
- rm -rf .git/modules/sub1 &&
+ rm -rf .git/submodules/sub1 &&
$command replace_sub1_with_directory &&
test_superproject_content origin/replace_sub1_with_directory &&
test_git_directory_exists sub1
diff --git a/t/t0035-safe-bare-repository.sh b/t/t0035-safe-bare-repository.sh
index ae7ef092ab..a480ddf8d6 100755
--- a/t/t0035-safe-bare-repository.sh
+++ b/t/t0035-safe-bare-repository.sh
@@ -41,7 +41,7 @@ test_expect_success 'setup an embedded bare repo, secondary worktree and submodu
submodule add --name subn -- ./bare-repo subd
) &&
test_path_is_dir outer-repo/.git/worktrees/outer-secondary &&
- test_path_is_dir outer-repo/.git/modules/subn
+ test_path_is_dir outer-repo/.git/submodules/subn
'
test_expect_success 'safe.bareRepository unset' '
@@ -100,7 +100,7 @@ test_expect_success 'no trace in $GIT_DIR of secondary worktree' '
'
test_expect_success 'no trace in $GIT_DIR of a submodule' '
- expect_accepted_implicit -C outer-repo/.git/modules/subn
+ expect_accepted_implicit -C outer-repo/.git/submodules/subn
'
test_done
diff --git a/t/t1600-index.sh b/t/t1600-index.sh
index 03239e9faa..0e5e8efb20 100755
--- a/t/t1600-index.sh
+++ b/t/t1600-index.sh
@@ -87,10 +87,10 @@ test_expect_success 'index.skipHash config option' '
git -c protocol.file.allow=always submodule add ./ sub &&
git config index.skipHash false &&
git -C sub config index.skipHash true &&
- rm -f .git/modules/sub/index &&
+ rm -f .git/submodules/sub/index &&
>sub/file &&
git -C sub add a &&
- test_trailing_hash .git/modules/sub/index >hash &&
+ test_trailing_hash .git/submodules/sub/index >hash &&
test_cmp expect hash &&
git -C sub fsck
'
diff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh
index 11018f37c7..c18c2efca5 100755
--- a/t/t2405-worktree-submodule.sh
+++ b/t/t2405-worktree-submodule.sh
@@ -62,7 +62,7 @@ test_expect_success 'submodule is checked out after manually adding submodule wo
test_expect_success 'checkout --recurse-submodules uses $GIT_DIR for submodules in a linked worktree' '
git -C main worktree add "$base_path/checkout-recurse" --detach &&
git -C checkout-recurse submodule update --init &&
- echo "gitdir: ../../main/.git/worktrees/checkout-recurse/modules/sub" >expect-gitfile &&
+ echo "gitdir: ../../main/.git/worktrees/checkout-recurse/submodules/sub" >expect-gitfile &&
cat checkout-recurse/sub/.git >actual-gitfile &&
test_cmp expect-gitfile actual-gitfile &&
git -C main/sub rev-parse HEAD >expect-head-main &&
@@ -73,7 +73,7 @@ test_expect_success 'checkout --recurse-submodules uses $GIT_DIR for submodules
test_cmp expect-head-main actual-head-main
'
-test_expect_success 'core.worktree is removed in $GIT_DIR/modules/<name>/config, not in $GIT_COMMON_DIR/modules/<name>/config' '
+test_expect_success 'core.worktree is removed in $GIT_DIR/submodules/<name>/config, not in $GIT_COMMON_DIR/submodules/<name>/config' '
echo "../../../sub" >expect-main &&
git -C main/sub config --get core.worktree >actual-main &&
test_cmp expect-main actual-main &&
@@ -81,14 +81,14 @@ test_expect_success 'core.worktree is removed in $GIT_DIR/modules/<name>/config,
git -C checkout-recurse/sub config --get core.worktree >actual-linked &&
test_cmp expect-linked actual-linked &&
git -C checkout-recurse checkout --recurse-submodules first &&
- test_expect_code 1 git -C main/.git/worktrees/checkout-recurse/modules/sub config --get core.worktree >linked-config &&
+ test_expect_code 1 git -C main/.git/worktrees/checkout-recurse/submodules/sub config --get core.worktree >linked-config &&
test_must_be_empty linked-config &&
git -C main/sub config --get core.worktree >actual-main &&
test_cmp expect-main actual-main
'
test_expect_success 'unsetting core.worktree does not prevent running commands directly against the submodule repository' '
- git -C main/.git/worktrees/checkout-recurse/modules/sub log
+ git -C main/.git/worktrees/checkout-recurse/submodules/sub log
'
test_done
diff --git a/t/t2501-cwd-empty.sh b/t/t2501-cwd-empty.sh
index be9140bbaa..bb8751433f 100755
--- a/t/t2501-cwd-empty.sh
+++ b/t/t2501-cwd-empty.sh
@@ -239,7 +239,7 @@ test_submodule_removal () {
test "$path_status" = dir && test_status=test_must_fail
test_when_finished "git reset --hard HEAD~1" &&
- test_when_finished "rm -rf .git/modules/my_submodule" &&
+ test_when_finished "rm -rf .git/submodules/my_submodule" &&
git checkout foo/bar/baz &&
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 1f16e6b522..5b8ed57538 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -582,7 +582,7 @@ test_expect_success 'rm of a conflicted populated submodule with a .git director
(
cd submod &&
rm .git &&
- cp -R ../.git/modules/sub .git &&
+ cp -R ../.git/submodules/sub .git &&
GIT_WORK_TREE=. git config --unset core.worktree
) &&
test_must_fail git merge conflict2 &&
@@ -617,9 +617,9 @@ test_expect_success 'rm of a populated submodule with a .git directory migrates
(
cd submod &&
rm .git &&
- cp -R ../.git/modules/sub .git &&
+ cp -R ../.git/submodules/sub .git &&
GIT_WORK_TREE=. git config --unset core.worktree &&
- rm -r ../.git/modules/sub
+ rm -r ../.git/submodules/sub
) &&
git rm submod 2>output.err &&
test_path_is_missing submod &&
@@ -709,7 +709,7 @@ test_expect_success "rm absorbs submodule's nested .git directory" '
(
cd submod/subsubmod &&
rm .git &&
- mv ../../.git/modules/sub/modules/sub .git &&
+ mv ../../.git/submodules/sub/submodules/sub .git &&
GIT_WORK_TREE=. git config --unset core.worktree
) &&
git rm submod 2>output.err &&
diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 5e566205ba..b7385bc088 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -1143,7 +1143,7 @@ test_expect_success 'fetch --recurse-submodules updates name-conflicted, unpopul
head1=$(git -C same-name-1/submodule rev-parse HEAD) &&
head2=$(git -C same-name-2/submodule rev-parse HEAD) &&
(
- cd same-name-downstream/.git/modules/submodule &&
+ cd same-name-downstream/.git/submodules/submodule &&
# The submodule has core.worktree pointing to the "git
# rm"-ed directory, overwrite the invalid value. See
# comment in get_fetch_task_from_changed() for more
diff --git a/t/t5619-clone-local-ambiguous-transport.sh b/t/t5619-clone-local-ambiguous-transport.sh
index cce62bf78d..cf2d5e7bfb 100755
--- a/t/t5619-clone-local-ambiguous-transport.sh
+++ b/t/t5619-clone-local-ambiguous-transport.sh
@@ -38,7 +38,7 @@ test_expect_success 'setup' '
ln -s "$(cd .. && pwd)/sensitive" repo/objects &&
mkdir -p "$HTTPD_URL/dumb" &&
- ln -s "../../../.git/modules/sub/../../../repo/" "$URI" &&
+ ln -s "../../../.git/submodules/sub/../../../repo/" "$URI" &&
git add . &&
git commit -m "initial commit"
@@ -57,7 +57,7 @@ test_expect_success 'ambiguous transport does not lead to arbitrary file-inclusi
git clone malicious clone &&
test_must_fail git -C clone submodule update --init 2>err &&
- test_path_is_missing clone/.git/modules/sub/objects/secret &&
+ test_path_is_missing clone/.git/submodules/sub/objects/secret &&
# We would actually expect "transport .file. not allowed" here,
# but due to quirks of the URL detection in Git, we mis-parse
# the absolute path as a bogus URL and die before that step.
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 256ccaefb7..56460ae8b5 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -357,7 +357,7 @@ test_expect_success 'setup and absorb a submodule' '
'
test_expect_success 'describe chokes on severely broken submodules' '
- mv .git/modules/sub1/ .git/modules/sub_moved &&
+ mv .git/submodules/sub1/ .git/submodules/sub_moved &&
test_must_fail git describe --dirty
'
@@ -371,7 +371,7 @@ test_expect_success 'describe with --work-tree ignoring a broken submodule' '
cd "$TEST_DIRECTORY" &&
git --git-dir "$TRASH_DIRECTORY/.git" --work-tree "$TRASH_DIRECTORY" describe --broken >"$TRASH_DIRECTORY/out"
) &&
- test_when_finished "mv .git/modules/sub_moved .git/modules/sub1" &&
+ test_when_finished "mv .git/submodules/sub_moved .git/submodules/sub1" &&
grep broken out
'
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 920479e925..89b06ae3c1 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -360,7 +360,7 @@ test_expect_success 'git mv moves a submodule with a .git directory and no .gitm
(
cd sub &&
rm -f .git &&
- cp -R -P -p ../.git/modules/sub .git &&
+ cp -R -P -p ../.git/submodules/sub .git &&
GIT_WORK_TREE=. git config --unset core.worktree
) &&
mkdir mod &&
@@ -380,7 +380,7 @@ test_expect_success 'git mv moves a submodule with a .git directory and .gitmodu
(
cd sub &&
rm -f .git &&
- cp -R -P -p ../.git/modules/sub .git &&
+ cp -R -P -p ../.git/submodules/sub .git &&
GIT_WORK_TREE=. git config --unset core.worktree
) &&
mkdir mod &&
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index fd3e7e355e..178c386212 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -163,7 +163,7 @@ test_expect_success 'submodule add' '
cd addtest &&
git submodule add -q "$submodurl" submod >actual &&
test_must_be_empty actual &&
- echo "gitdir: ../.git/modules/submod" >expect &&
+ echo "gitdir: ../.git/submodules/submod" >expect &&
test_cmp expect submod/.git &&
(
cd submod &&
@@ -976,21 +976,21 @@ test_expect_success 'submodule add --name allows to replace a submodule with ano
echo "$submodurl/repo" >expect &&
git config remote.origin.url >actual &&
test_cmp expect actual &&
- echo "gitdir: ../.git/modules/repo" >expect &&
+ echo "gitdir: ../.git/submodules/repo" >expect &&
test_cmp expect .git
) &&
rm -rf repo &&
git rm repo &&
git submodule add -q --name repo_new "$submodurl/bare.git" repo >actual &&
test_must_be_empty actual &&
- echo "gitdir: ../.git/modules/submod" >expect &&
+ echo "gitdir: ../.git/submodules/submod" >expect &&
test_cmp expect submod/.git &&
(
cd repo &&
echo "$submodurl/bare.git" >expect &&
git config remote.origin.url >actual &&
test_cmp expect actual &&
- echo "gitdir: ../.git/modules/repo_new" >expect &&
+ echo "gitdir: ../.git/submodules/repo_new" >expect &&
test_cmp expect .git
) &&
echo "repo" >expect &&
@@ -1045,8 +1045,8 @@ test_expect_success 'recursive relative submodules stay relative' '
(
cd clone2 &&
git submodule update --init --recursive &&
- echo "gitdir: ../.git/modules/sub3" >./sub3/.git_expect &&
- echo "gitdir: ../../../.git/modules/sub3/modules/dirdir/subsub" >./sub3/dirdir/subsub/.git_expect
+ echo "gitdir: ../.git/submodules/sub3" >./sub3/.git_expect &&
+ echo "gitdir: ../../../.git/submodules/sub3/submodules/dirdir/subsub" >./sub3/dirdir/subsub/.git_expect
) &&
test_cmp clone2/sub3/.git_expect clone2/sub3/.git &&
test_cmp clone2/sub3/dirdir/subsub/.git_expect clone2/sub3/dirdir/subsub/.git
@@ -1108,8 +1108,8 @@ test_expect_success 'submodule deinit should remove the whole submodule section
'
test_expect_success 'submodule deinit should unset core.worktree' '
- test_path_is_file .git/modules/example/config &&
- test_must_fail git config -f .git/modules/example/config core.worktree
+ test_path_is_file .git/submodules/example/config &&
+ test_must_fail git config -f .git/submodules/example/config core.worktree
'
test_expect_success 'submodule deinit from subdirectory' '
@@ -1231,7 +1231,7 @@ test_expect_success 'submodule deinit absorbs .git directory if .git is a direct
(
cd init &&
rm .git &&
- mv ../.git/modules/example .git &&
+ mv ../.git/submodules/example .git &&
GIT_WORK_TREE=. git config --unset core.worktree
) &&
git submodule deinit init &&
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 3adab12091..f0c4da1ffa 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -864,7 +864,7 @@ test_expect_success 'submodule add places git-dir in superprojects git-dir' '
(cd deeper/submodule &&
git log > ../../expected
) &&
- (cd .git/modules/deeper/submodule &&
+ (cd .git/submodules/deeper/submodule &&
git log > ../../../../actual
) &&
test_cmp expected actual
@@ -882,7 +882,7 @@ test_expect_success 'submodule update places git-dir in superprojects git-dir' '
(cd deeper/submodule &&
git log > ../../expected
) &&
- (cd .git/modules/deeper/submodule &&
+ (cd .git/submodules/deeper/submodule &&
git log > ../../../../actual
) &&
test_cmp expected actual
@@ -899,7 +899,7 @@ test_expect_success 'submodule add places git-dir in superprojects git-dir recur
git commit -m "added subsubmodule" &&
git push origin :
) &&
- (cd .git/modules/deeper/submodule/modules/subsubmodule &&
+ (cd .git/submodules/deeper/submodule/submodules/subsubmodule &&
git log > ../../../../../actual
) &&
git add deeper/submodule &&
@@ -949,7 +949,7 @@ test_expect_success 'submodule update places git-dir in superprojects git-dir re
(cd submodule/subsubmodule &&
git log > ../../expected
) &&
- (cd .git/modules/submodule/modules/subsubmodule &&
+ (cd .git/submodules/submodule/submodules/subsubmodule &&
git log > ../../../../../actual
) &&
test_cmp expected actual
@@ -1298,7 +1298,7 @@ test_expect_success CASE_INSENSITIVE_FS,SYMLINKS \
git init captain &&
(
cd captain &&
- git submodule add --name x/y "$hook_repo_path" A/modules/x &&
+ git submodule add --name x/y "$hook_repo_path" A/submodules/x &&
test_tick &&
git commit -m add-submodule &&
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 77b6d0040e..75ba826968 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -368,9 +368,9 @@ test_expect_success 'test "update --recursive" with a flag with spaces' '
git rev-parse --resolve-git-dir nested1/.git &&
git rev-parse --resolve-git-dir nested1/nested2/.git &&
git rev-parse --resolve-git-dir nested1/nested2/nested3/.git &&
- test -f .git/modules/nested1/objects/info/alternates &&
- test -f .git/modules/nested1/modules/nested2/objects/info/alternates &&
- test -f .git/modules/nested1/modules/nested2/modules/nested3/objects/info/alternates
+ test -f .git/submodules/nested1/objects/info/alternates &&
+ test -f .git/submodules/nested1/submodules/nested2/objects/info/alternates &&
+ test -f .git/submodules/nested1/submodules/nested2/submodules/nested3/objects/info/alternates
)
'
diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
index f860e7bbf4..25f4aec57e 100755
--- a/t/t7408-submodule-reference.sh
+++ b/t/t7408-submodule-reference.sh
@@ -61,7 +61,7 @@ test_expect_success 'submodule add --reference uses alternates' '
git commit -m B-super-added &&
git repack -ad
) &&
- test_alternate_is_used super/.git/modules/sub/objects/info/alternates super/sub
+ test_alternate_is_used super/.git/submodules/sub/objects/info/alternates super/sub
'
test_expect_success 'submodule add --reference with --dissociate does not use alternates' '
@@ -71,7 +71,7 @@ test_expect_success 'submodule add --reference with --dissociate does not use al
git commit -m B-super-added &&
git repack -ad
) &&
- test_path_is_missing super/.git/modules/sub-dissociate/objects/info/alternates
+ test_path_is_missing super/.git/submodules/sub-dissociate/objects/info/alternates
'
test_expect_success 'that reference gets used with add' '
@@ -94,14 +94,14 @@ test_expect_success 'updating superproject keeps alternates' '
test_when_finished "rm -rf super-clone" &&
git clone super super-clone &&
git -C super-clone submodule update --init --reference ../B &&
- test_alternate_is_used super-clone/.git/modules/sub/objects/info/alternates super-clone/sub
+ test_alternate_is_used super-clone/.git/submodules/sub/objects/info/alternates super-clone/sub
'
test_expect_success 'updating superproject with --dissociate does not keep alternates' '
test_when_finished "rm -rf super-clone" &&
git clone super super-clone &&
git -C super-clone submodule update --init --reference ../B --dissociate &&
- test_path_is_missing super-clone/.git/modules/sub/objects/info/alternates
+ test_path_is_missing super-clone/.git/submodules/sub/objects/info/alternates
'
test_expect_success 'submodules use alternates when cloning a superproject' '
@@ -112,7 +112,7 @@ test_expect_success 'submodules use alternates when cloning a superproject' '
# test superproject has alternates setup correctly
test_alternate_is_used .git/objects/info/alternates . &&
# test submodule has correct setup
- test_alternate_is_used .git/modules/sub/objects/info/alternates sub
+ test_alternate_is_used .git/submodules/sub/objects/info/alternates sub
)
'
@@ -127,7 +127,7 @@ test_expect_success 'missing submodule alternate fails clone and submodule updat
# update of the submodule succeeds
test_must_fail git submodule update --init &&
# and we have no alternates:
- test_path_is_missing .git/modules/sub/objects/info/alternates &&
+ test_path_is_missing .git/submodules/sub/objects/info/alternates &&
test_path_is_missing sub/file1
)
'
@@ -142,7 +142,7 @@ test_expect_success 'ignoring missing submodule alternates passes clone and subm
# update of the submodule succeeds
git submodule update --init &&
# and we have no alternates:
- test_path_is_missing .git/modules/sub/objects/info/alternates &&
+ test_path_is_missing .git/submodules/sub/objects/info/alternates &&
test_path_is_file sub/file1
)
'
@@ -176,18 +176,18 @@ test_expect_success 'nested submodule alternate in works and is actually used' '
# test superproject has alternates setup correctly
test_alternate_is_used .git/objects/info/alternates . &&
# immediate submodule has alternate:
- test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub &&
+ test_alternate_is_used .git/submodules/subwithsub/objects/info/alternates subwithsub &&
# nested submodule also has alternate:
- test_alternate_is_used .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
+ test_alternate_is_used .git/submodules/subwithsub/submodules/sub/objects/info/alternates subwithsub/sub
)
'
check_that_two_of_three_alternates_are_used() {
test_alternate_is_used .git/objects/info/alternates . &&
# immediate submodule has alternate:
- test_alternate_is_used .git/modules/subwithsub/objects/info/alternates subwithsub &&
+ test_alternate_is_used .git/submodules/subwithsub/objects/info/alternates subwithsub &&
# but nested submodule has no alternate:
- test_path_is_missing .git/modules/subwithsub/modules/sub/objects/info/alternates
+ test_path_is_missing .git/submodules/subwithsub/submodules/sub/objects/info/alternates
}
diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
index 0490499573..dbaca9c69f 100755
--- a/t/t7412-submodule-absorbgitdirs.sh
+++ b/t/t7412-submodule-absorbgitdirs.sh
@@ -29,13 +29,13 @@ test_expect_success 'absorb the git dir' '
cat >expect <<-EOF &&
Migrating git directory of '\''sub1'\'' from
'\''$cwd/sub1/.git'\'' to
- '\''$cwd/.git/modules/sub1'\''
+ '\''$cwd/.git/submodules/sub1'\''
EOF
git submodule absorbgitdirs 2>actual &&
test_cmp expect actual &&
git fsck &&
test -f sub1/.git &&
- test -d .git/modules/sub1 &&
+ test -d .git/submodules/sub1 &&
git status >actual.1 &&
git -C sub1 rev-parse HEAD >actual.2 &&
test_cmp expect.1 actual.1 &&
@@ -47,7 +47,7 @@ test_expect_success 'absorbing does not fail for deinitialized submodules' '
git submodule deinit --all &&
git submodule absorbgitdirs 2>err &&
test_must_be_empty err &&
- test -d .git/modules/sub1 &&
+ test -d .git/submodules/sub1 &&
test -d sub1 &&
! test -e sub1/.git
'
@@ -68,12 +68,12 @@ test_expect_success 'absorb the git dir in a nested submodule' '
cat >expect <<-EOF &&
Migrating git directory of '\''sub1/nested'\'' from
'\''$cwd/sub1/nested/.git'\'' to
- '\''$cwd/.git/modules/sub1/modules/nested'\''
+ '\''$cwd/.git/submodules/sub1/submodules/nested'\''
EOF
git submodule absorbgitdirs 2>actual &&
test_cmp expect actual &&
test -f sub1/nested/.git &&
- test -d .git/modules/sub1/modules/nested &&
+ test -d .git/submodules/sub1/submodules/nested &&
git status >actual.1 &&
git -C sub1/nested rev-parse HEAD >actual.2 &&
test_cmp expect.1 actual.1 &&
@@ -84,11 +84,11 @@ test_expect_success 're-setup nested submodule' '
# un-absorb the direct submodule, to test if the nested submodule
# is still correct (needs a rewrite of the gitfile only)
rm -rf sub1/.git &&
- mv .git/modules/sub1 sub1/.git &&
+ mv .git/submodules/sub1 sub1/.git &&
GIT_WORK_TREE=. git -C sub1 config --unset core.worktree &&
# fixup the nested submodule
- echo "gitdir: ../.git/modules/nested" >sub1/nested/.git &&
- GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \
+ echo "gitdir: ../.git/submodules/nested" >sub1/nested/.git &&
+ GIT_WORK_TREE=../../../nested git -C sub1/.git/submodules/nested config \
core.worktree "../../../nested" &&
# make sure this re-setup is correct
git status --ignore-submodules=none &&
@@ -105,13 +105,13 @@ test_expect_success 'absorb the git dir in a nested submodule' '
cat >expect <<-EOF &&
Migrating git directory of '\''sub1'\'' from
'\''$cwd/sub1/.git'\'' to
- '\''$cwd/.git/modules/sub1'\''
+ '\''$cwd/.git/submodules/sub1'\''
EOF
git submodule absorbgitdirs 2>actual &&
test_cmp expect actual &&
test -f sub1/.git &&
test -f sub1/nested/.git &&
- test -d .git/modules/sub1/modules/nested &&
+ test -d .git/submodules/sub1/submodules/nested &&
git status >actual.1 &&
git -C sub1/nested rev-parse HEAD >actual.2 &&
test_cmp expect.1 actual.1 &&
@@ -133,7 +133,7 @@ test_expect_success 'absorb the git dir outside of primary worktree' '
cat >expect <<-EOF &&
Migrating git directory of '\''sub2'\'' from
'\''$cwd/repo-wt/sub2/.git'\'' to
- '\''$cwd/repo-bare.git/worktrees/repo-wt/modules/sub2'\''
+ '\''$cwd/repo-bare.git/worktrees/repo-wt/submodules/sub2'\''
EOF
git -C repo-wt submodule absorbgitdirs 2>actual &&
test_cmp expect actual
diff --git a/t/t7423-submodule-symlinks.sh b/t/t7423-submodule-symlinks.sh
index 3d3c7af3ce..a51235136d 100755
--- a/t/t7423-submodule-symlinks.sh
+++ b/t/t7423-submodule-symlinks.sh
@@ -49,19 +49,19 @@ test_expect_success SYMLINKS 'git restore --recurse-submodules must not be confu
test_expect_success SYMLINKS 'git restore --recurse-submodules must not migrate git dir of symlinked repo' '
prepare_symlink_to_repo &&
- rm -rf .git/modules &&
+ rm -rf .git/submodules &&
test_must_fail git restore --recurse-submodules a/sm &&
test_path_is_dir a/target/.git &&
- test_path_is_missing .git/modules/a/sm &&
+ test_path_is_missing .git/submodules/a/sm &&
test_path_is_missing a/target/submodule_file
'
test_expect_success SYMLINKS 'git checkout -f --recurse-submodules must not migrate git dir of symlinked repo when removing submodule' '
prepare_symlink_to_repo &&
- rm -rf .git/modules &&
+ rm -rf .git/submodules &&
test_must_fail git checkout -f --recurse-submodules initial &&
test_path_is_dir a/target/.git &&
- test_path_is_missing .git/modules/a/sm
+ test_path_is_missing .git/submodules/a/sm
'
test_done
diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh
index f512eed278..4e2ced3636 100755
--- a/t/t7450-bad-git-dotfiles.sh
+++ b/t/t7450-bad-git-dotfiles.sh
@@ -77,28 +77,28 @@ test_expect_success 'create innocent subrepo' '
test_expect_success 'submodule add refuses invalid names' '
test_must_fail \
- git submodule add --name ../../modules/evil "$PWD/innocent" evil
+ git submodule add --name ../../submodules/evil "$PWD/innocent" evil
'
test_expect_success 'add evil submodule' '
git submodule add "$PWD/innocent" evil &&
- mkdir modules &&
- cp -r .git/modules/evil modules &&
- write_script modules/evil/hooks/post-checkout <<-\EOF &&
+ mkdir submodules &&
+ cp -r .git/submodules/evil submodules &&
+ write_script submodules/evil/hooks/post-checkout <<-\EOF &&
echo >&2 "RUNNING POST CHECKOUT"
EOF
git config -f .gitmodules submodule.evil.update checkout &&
git config -f .gitmodules --rename-section \
- submodule.evil submodule.../../modules/evil &&
- git add modules &&
+ submodule.evil submodule.../../submodules/evil &&
+ git add submodules &&
git commit -am evil
'
# This step seems like it shouldn't be necessary, since the payload is
# contained entirely in the evil submodule. But due to the vagaries of the
-# submodule code, checking out the evil module will fail unless ".git/modules"
+# submodule code, checking out the evil module will fail unless ".git/submodules"
# exists. Adding another submodule (with a name that sorts before "evil") is an
# easy way to make sure this is the case in the victim clone.
test_expect_success 'add other submodule' '
@@ -350,8 +350,8 @@ test_expect_success 'submodule git dir nesting detection must work with parallel
cat err &&
grep -E "(already exists|is inside git dir|not a git repository)" err &&
{
- test_path_is_missing .git/modules/hippo/HEAD ||
- test_path_is_missing .git/modules/hippo/hooks/HEAD
+ test_path_is_missing .git/submodules/hippo/HEAD ||
+ test_path_is_missing .git/submodules/hippo/hooks/HEAD
}
'
@@ -361,10 +361,10 @@ test_expect_success 'checkout -f --recurse-submodules must not use a nested gitd
cd nested_checkout &&
git submodule init &&
git submodule update thing1 &&
- mkdir -p .git/modules/hippo/hooks/refs &&
- mkdir -p .git/modules/hippo/hooks/objects/info &&
- echo "../../../../objects" >.git/modules/hippo/hooks/objects/info/alternates &&
- echo "ref: refs/heads/master" >.git/modules/hippo/hooks/HEAD
+ mkdir -p .git/submodules/hippo/hooks/refs &&
+ mkdir -p .git/submodules/hippo/hooks/objects/info &&
+ echo "../../../../objects" >.git/submodules/hippo/hooks/objects/info/alternates &&
+ echo "ref: refs/heads/master" >.git/submodules/hippo/hooks/HEAD
) &&
test_must_fail git -C nested_checkout checkout -f --recurse-submodules HEAD 2>err &&
cat err &&
@@ -390,13 +390,13 @@ test_expect_success SYMLINKS,!WINDOWS,!MINGW 'submodule must not checkout into d
git config unset -f repo/.gitmodules submodule.sub.path &&
printf "\tpath = \"sub\r\"\n" >>repo/.gitmodules &&
- git config unset -f repo/.git/modules/sub/config core.worktree &&
+ git config unset -f repo/.git/submodules/sub/config core.worktree &&
{
printf "[core]\n" &&
printf "\tworktree = \"../../../sub\r\"\n"
- } >>repo/.git/modules/sub/config &&
+ } >>repo/.git/submodules/sub/config &&
- ln -s .git/modules/sub/hooks repo/sub &&
+ ln -s .git/submodules/sub/hooks repo/sub &&
git -C repo add -A &&
git -C repo commit -m submodule &&
diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
index 409cd0cd12..ded482fdf2 100755
--- a/t/t7527-builtin-fsmonitor.sh
+++ b/t/t7527-builtin-fsmonitor.sh
@@ -866,7 +866,7 @@ test_expect_success 'submodule always visited' '
'
# If a submodule has a `sub/.git/` directory (rather than a file
-# pointing to the super's `.git/modules/sub`) and `core.fsmonitor`
+# pointing to the super's `.git/submodules/sub`) and `core.fsmonitor`
# turned on in the submodule and the daemon is not yet started in
# the submodule, and someone does a `git submodule absorbgitdirs`
# in the super, Git will recursively invoke `git submodule--helper`
@@ -895,7 +895,7 @@ test_expect_success "submodule absorbgitdirs implicitly starts daemon" '
cat >expect <<-EOF &&
Migrating git directory of '\''dir_1/dir_2/sub'\'' from
'\''$cwd/dir_1/dir_2/sub/.git'\'' to
- '\''$cwd/.git/modules/dir_1/dir_2/sub'\''
+ '\''$cwd/.git/submodules/dir_1/dir_2/sub'\''
EOF
GIT_TRACE2_EVENT="$PWD/super-sub.trace" \
git -C super submodule absorbgitdirs >out 2>actual &&
--
2.50.1.679.gbf363a8fbb.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 3/9] submodule: add gitdir path config override
2025-08-16 21:36 [PATCH 0/9] Encode submodule gitdir names to avoid conflicts Adrian Ratiu
2025-08-16 21:36 ` [PATCH 1/9] submodule--helper: use submodule_name_to_gitdir in add_submodule Adrian Ratiu
2025-08-16 21:36 ` [PATCH 2/9] submodule: create new gitdirs under submodules path Adrian Ratiu
@ 2025-08-16 21:36 ` Adrian Ratiu
2025-08-20 19:37 ` Josh Steadmon
` (2 more replies)
2025-08-16 21:36 ` [PATCH 4/9] t: submodules: add basic mixed gitdir path tests Adrian Ratiu
` (6 subsequent siblings)
9 siblings, 3 replies; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-16 21:36 UTC (permalink / raw)
To: git
Cc: Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Stefan Beller,
Patrick Steinhardt, Adrian Ratiu, Brandon Williams
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.
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 <bmwill@google.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
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 <name>"));
+
+ 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/<name>. */
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
--
2.50.1.679.gbf363a8fbb.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 4/9] t: submodules: add basic mixed gitdir path tests
2025-08-16 21:36 [PATCH 0/9] Encode submodule gitdir names to avoid conflicts Adrian Ratiu
` (2 preceding siblings ...)
2025-08-16 21:36 ` [PATCH 3/9] submodule: add gitdir path config override Adrian Ratiu
@ 2025-08-16 21:36 ` Adrian Ratiu
2025-08-20 22:07 ` Josh Steadmon
2025-09-02 23:02 ` Junio C Hamano
2025-08-16 21:36 ` [PATCH 5/9] strbuf: bring back is_rfc3986_unreserved Adrian Ratiu
` (5 subsequent siblings)
9 siblings, 2 replies; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-16 21:36 UTC (permalink / raw)
To: git
Cc: Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Stefan Beller,
Patrick Steinhardt, Adrian Ratiu
Add some basic submodule tests for mixed gitdir path handling of
legacy (.git/modules) and new-style (.git/submodule) paths.
For now these just test the coexistence, creation and push/pull of
submodules using mixed paths.
More tests will be added later, especially for new-style encoding.
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
t/meson.build | 1 +
t/t7425-submodule-mixed-gitdir-paths.sh | 101 ++++++++++++++++++++++++
2 files changed, 102 insertions(+)
create mode 100755 t/t7425-submodule-mixed-gitdir-paths.sh
diff --git a/t/meson.build b/t/meson.build
index bbeba1a8d5..ffd74f1d3b 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -874,6 +874,7 @@ integration_tests = [
't7422-submodule-output.sh',
't7423-submodule-symlinks.sh',
't7424-submodule-mixed-ref-formats.sh',
+ 't7425-submodule-mixed-gitdir-paths.sh',
't7450-bad-git-dotfiles.sh',
't7500-commit-template-squash-signoff.sh',
't7501-commit-basic-functionality.sh',
diff --git a/t/t7425-submodule-mixed-gitdir-paths.sh b/t/t7425-submodule-mixed-gitdir-paths.sh
new file mode 100755
index 0000000000..801e90522a
--- /dev/null
+++ b/t/t7425-submodule-mixed-gitdir-paths.sh
@@ -0,0 +1,101 @@
+#!/bin/sh
+
+test_description='submodules handle mixed legacy and new (encoded) style gitdir paths'
+
+. ./test-lib.sh
+
+test_expect_success 'setup: allow file protocol' '
+ git config --global protocol.file.allow always
+'
+
+test_expect_success 'create repo with mixed new and legacy submodules' '
+ git init legacy-sub &&
+ test_commit -C legacy-sub legacy-initial &&
+ git -C legacy-sub config receive.denyCurrentBranch updateInstead &&
+ legacy_rev=$(git -C legacy-sub rev-parse HEAD) &&
+
+ git init new-sub &&
+ test_commit -C new-sub new-initial &&
+ git -C new-sub config receive.denyCurrentBranch updateInstead &&
+ new_rev=$(git -C new-sub rev-parse HEAD) &&
+
+ git init main &&
+ (
+ cd main &&
+
+ git config receive.denyCurrentBranch updateInstead &&
+
+ git submodule add ../new-sub new &&
+ test_commit new-sub &&
+
+ git submodule add ../legacy-sub legacy &&
+ test_commit legacy-sub &&
+
+ # simulate legacy .git/modules path by moving submodule
+ mkdir -p .git/modules &&
+ mv .git/submodules/legacy .git/modules/ &&
+ echo "gitdir: ../.git/modules/legacy" > legacy/.git
+ )
+'
+
+test_expect_success 'clone from repo with both legacy and new-style submodules' '
+ git clone --recurse-submodules main cloned &&
+ (
+ cd cloned &&
+
+ # At this point, .git/modules/<name> should not exist as
+ # submodules are checked out into the new path
+ test_path_is_dir .git/submodules/legacy &&
+ test_path_is_dir .git/submodules/new &&
+
+ git submodule status >list &&
+ grep "$legacy_rev legacy" list &&
+ grep "$new_rev new" list
+ )
+'
+
+test_expect_success 'commit and push changes to submodules' '
+ (
+ cd cloned &&
+
+ git -C legacy switch --track -C master origin/master &&
+ test_commit -C legacy second-commit &&
+ git -C legacy push &&
+
+ git -C new switch --track -C master origin/master &&
+ test_commit -C new second-commit &&
+ git -C new push &&
+
+ # Stage and commit submodule changes in superproject
+ git switch --track -C master origin/master &&
+ git add legacy new &&
+ 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/submodules/new &&
+
+ # Verify both submodules are at the expected commits
+ git submodule status >list &&
+ grep "$legacy_rev legacy" list &&
+ grep "$new_rev new" list
+ )
+'
+
+test_done
--
2.50.1.679.gbf363a8fbb.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 5/9] strbuf: bring back is_rfc3986_unreserved
2025-08-16 21:36 [PATCH 0/9] Encode submodule gitdir names to avoid conflicts Adrian Ratiu
` (3 preceding siblings ...)
2025-08-16 21:36 ` [PATCH 4/9] t: submodules: add basic mixed gitdir path tests Adrian Ratiu
@ 2025-08-16 21:36 ` Adrian Ratiu
2025-08-16 21:56 ` Ben Knoble
2025-08-16 21:36 ` [PATCH 6/9] submodule: encode gitdir paths to avoid conflicts Adrian Ratiu
` (4 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-16 21:36 UTC (permalink / raw)
To: git
Cc: Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Stefan Beller,
Patrick Steinhardt, Adrian Ratiu
Commit f89854362c ("credential-store: move related functions to...")
moved the function inside credential-store.c, making it static under
the correct assumption (at the time) that it's the only place used.
However now we need it to apply url encoding to submodule names when
constructing gitdir paths, to avoid conflicts, so bring it back.
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
builtin/credential-store.c | 6 ------
strbuf.c | 6 ++++++
strbuf.h | 2 ++
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/builtin/credential-store.c b/builtin/credential-store.c
index b74e06cc93..0acaf1cc82 100644
--- a/builtin/credential-store.c
+++ b/builtin/credential-store.c
@@ -76,12 +76,6 @@ static void rewrite_credential_file(const char *fn, struct credential *c,
die_errno("unable to write credential store");
}
-static int is_rfc3986_unreserved(char ch)
-{
- return isalnum(ch) ||
- ch == '-' || ch == '_' || ch == '.' || ch == '~';
-}
-
static int is_rfc3986_reserved_or_unreserved(char ch)
{
if (is_rfc3986_unreserved(ch))
diff --git a/strbuf.c b/strbuf.c
index 6c3851a7f8..e8d84cbb6d 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -817,6 +817,12 @@ void strbuf_addstr_xml_quoted(struct strbuf *buf, const char *s)
}
}
+int is_rfc3986_unreserved(char ch)
+{
+ return isalnum(ch) ||
+ ch == '-' || ch == '_' || ch == '.' || ch == '~';
+}
+
static void strbuf_add_urlencode(struct strbuf *sb, const char *s, size_t len,
char_predicate allow_unencoded_fn)
{
diff --git a/strbuf.h b/strbuf.h
index a580ac6084..5139269039 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -640,6 +640,8 @@ static inline void strbuf_complete_line(struct strbuf *sb)
typedef int (*char_predicate)(char ch);
+int is_rfc3986_unreserved(char ch);
+
void strbuf_addstr_urlencode(struct strbuf *sb, const char *name,
char_predicate allow_unencoded_fn);
--
2.50.1.679.gbf363a8fbb.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 6/9] submodule: encode gitdir paths to avoid conflicts
2025-08-16 21:36 [PATCH 0/9] Encode submodule gitdir names to avoid conflicts Adrian Ratiu
` (4 preceding siblings ...)
2025-08-16 21:36 ` [PATCH 5/9] strbuf: bring back is_rfc3986_unreserved Adrian Ratiu
@ 2025-08-16 21:36 ` Adrian Ratiu
2025-08-20 19:29 ` Jeff King
2025-08-16 21:36 ` [PATCH 7/9] submodule: remove validate_submodule_git_dir() Adrian Ratiu
` (3 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-16 21:36 UTC (permalink / raw)
To: git
Cc: Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Stefan Beller,
Patrick Steinhardt, Adrian Ratiu, Brandon Williams
This follows up commit ce125d431a ("submodule: extract path to submodule
gitdir func") to resolve gitdir path conflicts, eg. names "a" and "a/b",
by encoding the submodule names before the paths are built.
Based on previous work by Brandon & all [1].
A custom encoding can become unnecesarily complex, while url-encoding is
relatively well-known, however it needs some extending to support case
insensitive filesystems and quirks like Windows reserving "COM1" names.
Hence why I opted to encode A as _a, B as _b and so on, which also fixes
the COM1 case, as suggested in [1].
The current implementation errors out if the encoded name is too long.
This can be improved, for e.g. the encoded name could be trimmed as the
conflict probability is low at the NAME_MAX length mark, or long names
could be sharded into subdirectories in the future.
This also fixes the existing affected tests and adds a TODO to cleanup
the short-circuit in validate_submodule_git_dir().
A further commit will some more tests to exercise these codepaths.
Link: https://lore.kernel.org/git/20180807230637.247200-1-bmwill@google.com/ [1]
Based-on-patch-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
submodule.c | 65 ++++++++++++++++++++++++-------------
t/t7400-submodule-basic.sh | 2 +-
t/t7406-submodule-update.sh | 10 +++---
t/t7450-bad-git-dotfiles.sh | 39 ++++++++++++----------
4 files changed, 69 insertions(+), 47 deletions(-)
diff --git a/submodule.c b/submodule.c
index bf78636195..722a8e4f2a 100644
--- a/submodule.c
+++ b/submodule.c
@@ -2271,8 +2271,13 @@ int validate_submodule_git_dir(char *git_dir, const char *submodule_name)
if (len <= suffix_len || (p = git_dir + len - suffix_len)[-1] != '/' ||
strcmp(p, submodule_name))
- BUG("submodule name '%s' not a suffix of git dir '%s'",
- submodule_name, git_dir);
+ /*
+ * TODO: revisit and cleanup this test short-circuit, because
+ * submodules with encoded names are expected to take this path.
+ * Likely just move the invariants to submodule_name_to_gitdir()
+ * and delete this entire function in a future commit.
+ */
+ return 0;
/*
* We prevent the contents of sibling submodules' git directories to
@@ -2588,30 +2593,26 @@ int submodule_to_gitdir(struct repository *repo,
return ret;
}
+static void strbuf_addstr_case_encode(struct strbuf *dst, const char *src)
+{
+ for (; *src; src++) {
+ unsigned char c = *src;
+ if (c >= 'A' && c <= 'Z') {
+ strbuf_addch(dst, '_');
+ strbuf_addch(dst, c - 'A' + 'a');
+ } else {
+ strbuf_addch(dst, c);
+ }
+ }
+}
+
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.
- */
+ struct strbuf encoded_sub_name = STRBUF_INIT, tmp = STRBUF_INIT;
+ size_t base_len, encoded_len;
char *gitdir_path, *key;
+ long name_max;
/* Allow config override. */
key = xstrfmt("submodule.%s.gitdirpath", submodule_name);
@@ -2632,5 +2633,23 @@ void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r,
/* New style (encoded) paths go under submodules/<encoded>. */
strbuf_reset(buf);
repo_git_path_append(r, buf, "submodules/");
- strbuf_addstr(buf, submodule_name);
+ base_len = buf->len;
+
+ /* URL-encode then case case-encode A to _a, B to _b and so on */
+ strbuf_addstr_urlencode(&tmp, submodule_name, is_rfc3986_unreserved);
+ strbuf_addstr_case_encode(&encoded_sub_name, tmp.buf);
+ strbuf_release(&tmp);
+ strbuf_addbuf(buf, &encoded_sub_name);
+
+ /* Ensure final path length is below NAME_MAX after encoding */
+ name_max = pathconf(buf->buf, _PC_NAME_MAX);
+ if (name_max == -1)
+ name_max = NAME_MAX;
+
+ encoded_len = buf->len - base_len;
+ if (encoded_len >= name_max)
+ die(_("encoded submodule name '%s' is too long (%zu bytes, limit is %ld)"),
+ encoded_sub_name.buf, encoded_len, name_max);
+
+ strbuf_release(&encoded_sub_name);
}
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index f4d4fb8397..607fdd26bd 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -1047,7 +1047,7 @@ test_expect_success 'recursive relative submodules stay relative' '
cd clone2 &&
git submodule update --init --recursive &&
echo "gitdir: ../.git/submodules/sub3" >./sub3/.git_expect &&
- echo "gitdir: ../../../.git/submodules/sub3/submodules/dirdir/subsub" >./sub3/dirdir/subsub/.git_expect
+ echo "gitdir: ../../../.git/submodules/sub3/submodules/dirdir%2fsubsub" >./sub3/dirdir/subsub/.git_expect
) &&
test_cmp clone2/sub3/.git_expect clone2/sub3/.git &&
test_cmp clone2/sub3/dirdir/subsub/.git_expect clone2/sub3/dirdir/subsub/.git
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index f0c4da1ffa..c44a7e9513 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -864,8 +864,8 @@ test_expect_success 'submodule add places git-dir in superprojects git-dir' '
(cd deeper/submodule &&
git log > ../../expected
) &&
- (cd .git/submodules/deeper/submodule &&
- git log > ../../../../actual
+ (cd .git/submodules/deeper%2fsubmodule &&
+ git log > ../../../actual
) &&
test_cmp expected actual
)
@@ -882,8 +882,8 @@ test_expect_success 'submodule update places git-dir in superprojects git-dir' '
(cd deeper/submodule &&
git log > ../../expected
) &&
- (cd .git/submodules/deeper/submodule &&
- git log > ../../../../actual
+ (cd .git/submodules/deeper%2fsubmodule &&
+ git log > ../../../actual
) &&
test_cmp expected actual
)
@@ -899,7 +899,7 @@ test_expect_success 'submodule add places git-dir in superprojects git-dir recur
git commit -m "added subsubmodule" &&
git push origin :
) &&
- (cd .git/submodules/deeper/submodule/submodules/subsubmodule &&
+ (cd .git/submodules/deeper%2fsubmodule/submodules/subsubmodule &&
git log > ../../../../../actual
) &&
git add deeper/submodule &&
diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh
index 4e2ced3636..27254300f8 100755
--- a/t/t7450-bad-git-dotfiles.sh
+++ b/t/t7450-bad-git-dotfiles.sh
@@ -15,6 +15,7 @@ Such as:
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-pack.sh
+. "$TEST_DIRECTORY"/lib-verify-submodule-gitdir-path.sh
test_expect_success 'setup' '
git config --global protocol.file.allow always
@@ -319,6 +320,8 @@ test_expect_success WINDOWS 'prevent git~1 squatting on Windows' '
fi
'
+# TODO: move these nested gitdir tests to another location in a later commit because
+# they are not pathological cases anymore: by encoding the gitdir paths do not conflict.
test_expect_success 'setup submodules with nested git dirs' '
git init nested &&
test_commit -C nested nested &&
@@ -341,35 +344,35 @@ test_expect_success 'setup submodules with nested git dirs' '
'
test_expect_success 'git dirs of sibling submodules must not be nested' '
- test_must_fail git clone --recurse-submodules nested clone 2>err &&
- test_grep "is inside git dir" err
+ git clone --recurse-submodules nested clone_nested &&
+ verify_submodule_gitdir_path clone_nested hippo submodules/hippo &&
+ verify_submodule_gitdir_path clone_nested hippo/hooks submodules/hippo%2fhooks
'
test_expect_success 'submodule git dir nesting detection must work with parallel cloning' '
- test_must_fail git clone --recurse-submodules --jobs=2 nested clone_parallel 2>err &&
- cat err &&
- grep -E "(already exists|is inside git dir|not a git repository)" err &&
- {
- test_path_is_missing .git/submodules/hippo/HEAD ||
- test_path_is_missing .git/submodules/hippo/hooks/HEAD
- }
+ git clone --recurse-submodules --jobs=2 nested clone_parallel &&
+ verify_submodule_gitdir_path clone_nested hippo submodules/hippo &&
+ verify_submodule_gitdir_path clone_nested hippo/hooks submodules/hippo%2fhooks
'
-test_expect_success 'checkout -f --recurse-submodules must not use a nested gitdir' '
- git clone nested nested_checkout &&
+test_expect_success 'checkout -f --recurse-submodules must corectly handle nested gitdirs' '
+ git clone nested clone_recursive_checkout &&
(
- cd nested_checkout &&
+ cd clone_recursive_checkout &&
+
git submodule init &&
- git submodule update thing1 &&
+ git submodule update thing1 thing2 &&
+
+ # simulate a malicious nested alternate which git should not follow
mkdir -p .git/submodules/hippo/hooks/refs &&
mkdir -p .git/submodules/hippo/hooks/objects/info &&
echo "../../../../objects" >.git/submodules/hippo/hooks/objects/info/alternates &&
- echo "ref: refs/heads/master" >.git/submodules/hippo/hooks/HEAD
+ echo "ref: refs/heads/master" >.git/submodules/hippo/hooks/HEAD &&
+
+ git checkout -f --recurse-submodules HEAD
) &&
- test_must_fail git -C nested_checkout checkout -f --recurse-submodules HEAD 2>err &&
- cat err &&
- grep "is inside git dir" err &&
- test_path_is_missing nested_checkout/thing2/.git
+ verify_submodule_gitdir_path clone_nested hippo submodules/hippo &&
+ verify_submodule_gitdir_path clone_nested hippo/hooks submodules/hippo%2fhooks
'
test_expect_success SYMLINKS,!WINDOWS,!MINGW 'submodule must not checkout into different directory' '
--
2.50.1.679.gbf363a8fbb.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 7/9] submodule: remove validate_submodule_git_dir()
2025-08-16 21:36 [PATCH 0/9] Encode submodule gitdir names to avoid conflicts Adrian Ratiu
` (5 preceding siblings ...)
2025-08-16 21:36 ` [PATCH 6/9] submodule: encode gitdir paths to avoid conflicts Adrian Ratiu
@ 2025-08-16 21:36 ` Adrian Ratiu
2025-08-16 21:36 ` [PATCH 8/9] t: move nested gitdir tests to proper location Adrian Ratiu
` (2 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-16 21:36 UTC (permalink / raw)
To: git
Cc: Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Stefan Beller,
Patrick Steinhardt, Adrian Ratiu
The validate_submodule_git_dir test is not very useful anymore, after
submodule names are encoded to resolve gitdir path conflicts.
In other words, the purpouse of gitdir path encoding is precisely to
avoid such conflicts as this function tries to also prevent.
The first test from the function can be kept though, because it just
verifies invariants which should always be true and raise a BUG if:
- no "/" separator is between dirs/names.
- len(full_gitdir) < len(name).
- name does not match the gitdir path suffix.
Thus we move the invariant checks to submodule_name_to_gitdir() and
clean up the rest of validate_submodule_git_dir() and its uses.
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
builtin/submodule--helper.c | 21 -----------
submodule.c | 74 ++++---------------------------------
submodule.h | 5 ---
3 files changed, 7 insertions(+), 93 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 30e40d6c79..d1ae864e8f 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1725,10 +1725,6 @@ static int clone_submodule(const struct module_clone_data *clone_data,
clone_data_path = to_free = xstrfmt("%s/%s", repo_get_work_tree(the_repository),
clone_data->path);
- if (validate_submodule_git_dir(sm_gitdir, clone_data->name) < 0)
- die(_("refusing to create/use '%s' in another submodule's "
- "git dir"), sm_gitdir);
-
if (!file_exists(sm_gitdir)) {
if (clone_data->require_init && !stat(clone_data_path, &st) &&
!is_empty_dir(clone_data_path))
@@ -1802,23 +1798,6 @@ static int clone_submodule(const struct module_clone_data *clone_data,
free(path);
}
- /*
- * We already performed this check at the beginning of this function,
- * before cloning the objects. This tries to detect racy behavior e.g.
- * in parallel clones, where another process could easily have made the
- * gitdir nested _after_ it was created.
- *
- * To prevent further harm coming from this unintentionally-nested
- * gitdir, let's disable it by deleting the `HEAD` file.
- */
- if (validate_submodule_git_dir(sm_gitdir, clone_data->name) < 0) {
- 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);
- }
-
connect_work_tree_and_git_dir(clone_data_path, sm_gitdir, 0);
p = repo_submodule_path(the_repository, clone_data_path, "config");
diff --git a/submodule.c b/submodule.c
index 722a8e4f2a..cfb45a8f9f 100644
--- a/submodule.c
+++ b/submodule.c
@@ -2163,27 +2163,10 @@ int submodule_move_head(const char *path, const char *super_prefix,
if (!submodule_uses_gitfile(path))
absorb_git_dir_into_superproject(path,
super_prefix);
- else {
- char *dotgit = xstrfmt("%s/.git", path);
- char *git_dir = xstrdup(read_gitfile(dotgit));
-
- free(dotgit);
- if (validate_submodule_git_dir(git_dir,
- sub->name) < 0)
- die(_("refusing to create/use '%s' in "
- "another submodule's git dir"),
- git_dir);
- free(git_dir);
- }
} else {
struct strbuf gitdir = STRBUF_INIT;
submodule_name_to_gitdir(&gitdir, the_repository,
sub->name);
- if (validate_submodule_git_dir(gitdir.buf,
- sub->name) < 0)
- die(_("refusing to create/use '%s' in another "
- "submodule's git dir"),
- gitdir.buf);
connect_work_tree_and_git_dir(path, gitdir.buf, 0);
strbuf_release(&gitdir);
@@ -2263,52 +2246,6 @@ int submodule_move_head(const char *path, const char *super_prefix,
return ret;
}
-int validate_submodule_git_dir(char *git_dir, const char *submodule_name)
-{
- size_t len = strlen(git_dir), suffix_len = strlen(submodule_name);
- char *p;
- int ret = 0;
-
- if (len <= suffix_len || (p = git_dir + len - suffix_len)[-1] != '/' ||
- strcmp(p, submodule_name))
- /*
- * TODO: revisit and cleanup this test short-circuit, because
- * submodules with encoded names are expected to take this path.
- * Likely just move the invariants to submodule_name_to_gitdir()
- * and delete this entire function in a future commit.
- */
- return 0;
-
- /*
- * We prevent the contents of sibling submodules' git directories to
- * clash.
- *
- * Example: having a submodule named `hippo` and another one named
- * `hippo/hooks` would result in the git directories
- * `.git/submodules/hippo/` and `.git/submodules/hippo/hooks/`, respectively,
- * but the latter directory is already designated to contain the hooks
- * of the former.
- */
- for (; *p; p++) {
- if (is_dir_sep(*p)) {
- char c = *p;
-
- *p = '\0';
- if (is_git_directory(git_dir))
- ret = -1;
- *p = c;
-
- if (ret < 0)
- return error(_("submodule git dir '%s' is "
- "inside git dir '%.*s'"),
- git_dir,
- (int)(p - git_dir), git_dir);
- }
- }
-
- return 0;
-}
-
int validate_submodule_path(const char *path)
{
char *p = xstrdup(path);
@@ -2367,9 +2304,6 @@ static void relocate_single_git_dir_into_superproject(const char *path,
die(_("could not lookup name for submodule '%s'"), path);
submodule_name_to_gitdir(&new_gitdir, the_repository, sub->name);
- if (validate_submodule_git_dir(new_gitdir.buf, sub->name) < 0)
- die(_("refusing to move '%s' into an existing git dir"),
- real_old_git_dir);
if (safe_create_leading_directories_const(the_repository, new_gitdir.buf) < 0)
die(_("could not create directory '%s'"), new_gitdir.buf);
real_new_git_dir = real_pathdup(new_gitdir.buf, 1);
@@ -2611,7 +2545,7 @@ void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r,
{
struct strbuf encoded_sub_name = STRBUF_INIT, tmp = STRBUF_INIT;
size_t base_len, encoded_len;
- char *gitdir_path, *key;
+ char *gitdir_path, *key, *p;
long name_max;
/* Allow config override. */
@@ -2651,5 +2585,11 @@ void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r,
die(_("encoded submodule name '%s' is too long (%zu bytes, limit is %ld)"),
encoded_sub_name.buf, encoded_len, name_max);
+ /* Trigger a BUG if these invariants do not hold */
+ p = buf->buf + buf->len - encoded_len;
+ if (buf->len <= encoded_len || p[-1] != '/' || strcmp(p, encoded_sub_name.buf))
+ BUG("encoded submodule name '%s' is not a suffix of git dir '%s'",
+ encoded_sub_name.buf, buf->buf);
+
strbuf_release(&encoded_sub_name);
}
diff --git a/submodule.h b/submodule.h
index b10e16e6c0..0b7692bc20 100644
--- a/submodule.h
+++ b/submodule.h
@@ -137,11 +137,6 @@ int submodule_to_gitdir(struct repository *repo,
void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r,
const char *submodule_name);
-/*
- * Make sure that no submodule's git dir is nested in a sibling submodule's.
- */
-int validate_submodule_git_dir(char *git_dir, const char *submodule_name);
-
/*
* Make sure that the given submodule path does not follow symlinks.
*/
--
2.50.1.679.gbf363a8fbb.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 8/9] t: move nested gitdir tests to proper location
2025-08-16 21:36 [PATCH 0/9] Encode submodule gitdir names to avoid conflicts Adrian Ratiu
` (6 preceding siblings ...)
2025-08-16 21:36 ` [PATCH 7/9] submodule: remove validate_submodule_git_dir() Adrian Ratiu
@ 2025-08-16 21:36 ` Adrian Ratiu
2025-08-16 21:36 ` [PATCH 9/9] t: add gitdir encoding tests Adrian Ratiu
2025-08-17 13:01 ` [PATCH 0/9] Encode submodule gitdir names to avoid conflicts Adrian Ratiu
9 siblings, 0 replies; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-16 21:36 UTC (permalink / raw)
To: git
Cc: Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Stefan Beller,
Patrick Steinhardt, Adrian Ratiu
Now that we are encoding gitdir paths, these tests are not handling
pathological cases anymore, because nested git dirs shouldn't cause
conflicts, so move them from t7450-bad-git-dotfiles.sh to a more
appropriate location where we test mixed gitdir path & encoding use.
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
t/t7425-submodule-mixed-gitdir-paths.sh | 54 ++++++++++++++++++++++++
t/t7450-bad-git-dotfiles.sh | 56 -------------------------
2 files changed, 54 insertions(+), 56 deletions(-)
diff --git a/t/t7425-submodule-mixed-gitdir-paths.sh b/t/t7425-submodule-mixed-gitdir-paths.sh
index 801e90522a..902b2560ca 100755
--- a/t/t7425-submodule-mixed-gitdir-paths.sh
+++ b/t/t7425-submodule-mixed-gitdir-paths.sh
@@ -3,6 +3,7 @@
test_description='submodules handle mixed legacy and new (encoded) style gitdir paths'
. ./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
@@ -98,4 +99,57 @@ test_expect_success 'fetch mixed submodule changes and verify updates' '
)
'
+test_expect_success 'setup submodules with nested git dirs' '
+ git init nested &&
+ test_commit -C nested nested &&
+ (
+ cd nested &&
+ cat >.gitmodules <<-EOF &&
+ [submodule "hippo"]
+ url = .
+ path = thing1
+ [submodule "hippo/hooks"]
+ url = .
+ path = thing2
+ EOF
+ git clone . thing1 &&
+ git clone . thing2 &&
+ git add .gitmodules thing1 thing2 &&
+ test_tick &&
+ git commit -m nested
+ )
+'
+
+test_expect_success 'git dirs of sibling submodules must not be nested' '
+ git clone --recurse-submodules nested clone_nested &&
+ verify_submodule_gitdir_path clone_nested hippo submodules/hippo &&
+ verify_submodule_gitdir_path clone_nested hippo/hooks submodules/hippo%2fhooks
+'
+
+test_expect_success 'submodule git dir nesting detection must work with parallel cloning' '
+ git clone --recurse-submodules --jobs=2 nested clone_parallel &&
+ verify_submodule_gitdir_path clone_nested hippo submodules/hippo &&
+ verify_submodule_gitdir_path clone_nested hippo/hooks submodules/hippo%2fhooks
+'
+
+test_expect_success 'checkout -f --recurse-submodules must corectly handle nested gitdirs' '
+ git clone nested clone_recursive_checkout &&
+ (
+ cd clone_recursive_checkout &&
+
+ git submodule init &&
+ git submodule update thing1 thing2 &&
+
+ # simulate a malicious nested alternate which git should not follow
+ mkdir -p .git/submodules/hippo/hooks/refs &&
+ mkdir -p .git/submodules/hippo/hooks/objects/info &&
+ echo "../../../../objects" >.git/submodules/hippo/hooks/objects/info/alternates &&
+ echo "ref: refs/heads/master" >.git/submodules/hippo/hooks/HEAD &&
+
+ git checkout -f --recurse-submodules HEAD
+ ) &&
+ verify_submodule_gitdir_path clone_nested hippo submodules/hippo &&
+ verify_submodule_gitdir_path clone_nested hippo/hooks submodules/hippo%2fhooks
+'
+
test_done
diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh
index 27254300f8..18624fabc4 100755
--- a/t/t7450-bad-git-dotfiles.sh
+++ b/t/t7450-bad-git-dotfiles.sh
@@ -15,7 +15,6 @@ Such as:
. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-pack.sh
-. "$TEST_DIRECTORY"/lib-verify-submodule-gitdir-path.sh
test_expect_success 'setup' '
git config --global protocol.file.allow always
@@ -320,61 +319,6 @@ test_expect_success WINDOWS 'prevent git~1 squatting on Windows' '
fi
'
-# TODO: move these nested gitdir tests to another location in a later commit because
-# they are not pathological cases anymore: by encoding the gitdir paths do not conflict.
-test_expect_success 'setup submodules with nested git dirs' '
- git init nested &&
- test_commit -C nested nested &&
- (
- cd nested &&
- cat >.gitmodules <<-EOF &&
- [submodule "hippo"]
- url = .
- path = thing1
- [submodule "hippo/hooks"]
- url = .
- path = thing2
- EOF
- git clone . thing1 &&
- git clone . thing2 &&
- git add .gitmodules thing1 thing2 &&
- test_tick &&
- git commit -m nested
- )
-'
-
-test_expect_success 'git dirs of sibling submodules must not be nested' '
- git clone --recurse-submodules nested clone_nested &&
- verify_submodule_gitdir_path clone_nested hippo submodules/hippo &&
- verify_submodule_gitdir_path clone_nested hippo/hooks submodules/hippo%2fhooks
-'
-
-test_expect_success 'submodule git dir nesting detection must work with parallel cloning' '
- git clone --recurse-submodules --jobs=2 nested clone_parallel &&
- verify_submodule_gitdir_path clone_nested hippo submodules/hippo &&
- verify_submodule_gitdir_path clone_nested hippo/hooks submodules/hippo%2fhooks
-'
-
-test_expect_success 'checkout -f --recurse-submodules must corectly handle nested gitdirs' '
- git clone nested clone_recursive_checkout &&
- (
- cd clone_recursive_checkout &&
-
- git submodule init &&
- git submodule update thing1 thing2 &&
-
- # simulate a malicious nested alternate which git should not follow
- mkdir -p .git/submodules/hippo/hooks/refs &&
- mkdir -p .git/submodules/hippo/hooks/objects/info &&
- echo "../../../../objects" >.git/submodules/hippo/hooks/objects/info/alternates &&
- echo "ref: refs/heads/master" >.git/submodules/hippo/hooks/HEAD &&
-
- git checkout -f --recurse-submodules HEAD
- ) &&
- verify_submodule_gitdir_path clone_nested hippo submodules/hippo &&
- verify_submodule_gitdir_path clone_nested hippo/hooks submodules/hippo%2fhooks
-'
-
test_expect_success SYMLINKS,!WINDOWS,!MINGW 'submodule must not checkout into different directory' '
test_when_finished "rm -rf sub repo bad-clone" &&
--
2.50.1.679.gbf363a8fbb.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 9/9] t: add gitdir encoding tests
2025-08-16 21:36 [PATCH 0/9] Encode submodule gitdir names to avoid conflicts Adrian Ratiu
` (7 preceding siblings ...)
2025-08-16 21:36 ` [PATCH 8/9] t: move nested gitdir tests to proper location Adrian Ratiu
@ 2025-08-16 21:36 ` Adrian Ratiu
2025-08-18 22:06 ` Junio C Hamano
2025-08-17 13:01 ` [PATCH 0/9] Encode submodule gitdir names to avoid conflicts Adrian Ratiu
9 siblings, 1 reply; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-16 21:36 UTC (permalink / raw)
To: git
Cc: Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Stefan Beller,
Patrick Steinhardt, Adrian Ratiu
Add some tests to further exercise the gitdir encoding functionality
alongside the existing mixed directory and nested gitdir tests.
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
t/t7425-submodule-mixed-gitdir-paths.sh | 52 +++++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/t/t7425-submodule-mixed-gitdir-paths.sh b/t/t7425-submodule-mixed-gitdir-paths.sh
index 902b2560ca..cfdf487a56 100755
--- a/t/t7425-submodule-mixed-gitdir-paths.sh
+++ b/t/t7425-submodule-mixed-gitdir-paths.sh
@@ -152,4 +152,56 @@ test_expect_success 'checkout -f --recurse-submodules must corectly handle neste
verify_submodule_gitdir_path clone_nested hippo/hooks submodules/hippo%2fhooks
'
+test_expect_success 'new style submodule gitdir paths are properly encoded' '
+ (
+ cd main &&
+
+ # add new-style submodule name containing /
+ git submodule add ../new-sub foo/bar &&
+ git commit -m "add foo/bar" &&
+
+ # simulate existing legacy submodule name containing escaping char %
+ git clone --separate-git-dir .git/modules/foo%bar ../legacy-sub foo%bar &&
+ cat >>.gitmodules <<-EOF &&
+ [submodule "foo%bar"]
+ path = foo%bar
+ url = ../legacy-sub
+ EOF
+ git add .gitmodules &&
+ git commit -m "add foo%bar" &&
+
+ # add new style submodule name containing escaping char %
+ git submodule add ../new-sub fooish%bar &&
+ git commit -m "add fooish%bar" &&
+
+ # add a mixed case submdule name
+ git submodule add ../new-sub FooBar &&
+ git commit -m "add FooBar" &&
+
+ # add a reserved name on Windows
+ git submodule add ../new-sub COM1 &&
+ git commit -m "add COM1"
+ ) &&
+ verify_submodule_gitdir_path main foo/bar submodules/foo%2fbar &&
+ verify_submodule_gitdir_path main foo%bar modules/foo%bar &&
+ verify_submodule_gitdir_path main fooish%bar submodules/fooish%25bar &&
+ verify_submodule_gitdir_path main FooBar submodules/_foo_bar &&
+ verify_submodule_gitdir_path main COM1 submodules/_c_o_m1
+'
+
+test_expect_success 'submodule encoded name exceeds max name limit' '
+ (
+ cd main &&
+
+ # find the system NAME_MAX (fall back to 255 if unknown)
+ name_max=$(getconf NAME_MAX . 2>/dev/null || echo 255) &&
+
+ # each "%" char encodes to "%25" (3 chars), ensure we exceed NAME_MAX
+ count=$((name_max + 10)) &&
+ longname=$(printf "%%%0.s" $(seq 1 $count)) &&
+
+ test_must_fail git submodule add ../new-sub "$longname"
+ )
+'
+
test_done
--
2.50.1.679.gbf363a8fbb.dirty
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH 5/9] strbuf: bring back is_rfc3986_unreserved
2025-08-16 21:36 ` [PATCH 5/9] strbuf: bring back is_rfc3986_unreserved Adrian Ratiu
@ 2025-08-16 21:56 ` Ben Knoble
2025-08-21 13:08 ` Adrian Ratiu
0 siblings, 1 reply; 27+ messages in thread
From: Ben Knoble @ 2025-08-16 21:56 UTC (permalink / raw)
To: Adrian Ratiu
Cc: git, Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Stefan Beller,
Patrick Steinhardt
> Le 16 août 2025 à 17:39, Adrian Ratiu <adrian.ratiu@collabora.com> a écrit :
>
> Commit f89854362c ("credential-store: move related functions to...")
Here and elsewhere, we refer to commits by the output of “git show -s --format=reference <object>”
Best,
Ben
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/9] Encode submodule gitdir names to avoid conflicts
2025-08-16 21:36 [PATCH 0/9] Encode submodule gitdir names to avoid conflicts Adrian Ratiu
` (8 preceding siblings ...)
2025-08-16 21:36 ` [PATCH 9/9] t: add gitdir encoding tests Adrian Ratiu
@ 2025-08-17 13:01 ` Adrian Ratiu
9 siblings, 0 replies; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-17 13:01 UTC (permalink / raw)
To: git
Cc: Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Stefan Beller,
Patrick Steinhardt
---- On Sun, 17 Aug 2025 00:36:33 +0300 Adrian Ratiu <adrian.ratiu@collabora.com> wrote ---
> Hello,
>
> This is a continuation of work done back in 2018 [1], so a big thank you to
> everyone who participated in the initial thread, especially Brandon on whose
> code this is partially based upon. Hope you are still around and doing well. :)
>
> It's mostly a rewrite from scratch addressig open feedback. I decided to
> iterate upen Brandon's url-encoding design instead of pursuing alternatives
> like a custom encoding, name hashing or round-trip encoding/decoding using
> an in-memory git mapping (we'd still have to encode/hash the paths to avoid
> colflicts so IIUC this last one is more complicated for little gain).
>
> I tried to organize and explain the commits in a logical way which is also
> easy to review, keeping the encoding parts, new tests, code moving around
> and path update churn as clearly separated as possible.
>
> This is based on master and I've merged and succesfully run all tests in
> both the next and seen branches.
I also ran the GitHub CI pipeline and noticed there are failures on Win + Mac.
I will address those in v2.
In the meantime I'll leave v1 for a while on the ML to gather more feedback.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 9/9] t: add gitdir encoding tests
2025-08-16 21:36 ` [PATCH 9/9] t: add gitdir encoding tests Adrian Ratiu
@ 2025-08-18 22:06 ` Junio C Hamano
2025-08-21 13:17 ` Adrian Ratiu
0 siblings, 1 reply; 27+ messages in thread
From: Junio C Hamano @ 2025-08-18 22:06 UTC (permalink / raw)
To: Adrian Ratiu
Cc: git, Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Aaron Schrab, Jonathan Nieder, Stefan Beller, Patrick Steinhardt
Adrian Ratiu <adrian.ratiu@collabora.com> writes:
> Add some tests to further exercise the gitdir encoding functionality
> alongside the existing mixed directory and nested gitdir tests.
>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
> t/t7425-submodule-mixed-gitdir-paths.sh | 52 +++++++++++++++++++++++++
> 1 file changed, 52 insertions(+)
>
> diff --git a/t/t7425-submodule-mixed-gitdir-paths.sh b/t/t7425-submodule-mixed-gitdir-paths.sh
> index 902b2560ca..cfdf487a56 100755
> --- a/t/t7425-submodule-mixed-gitdir-paths.sh
> +++ b/t/t7425-submodule-mixed-gitdir-paths.sh
> @@ -152,4 +152,56 @@ test_expect_success 'checkout -f --recurse-submodules must corectly handle neste
> ...
> + longname=$(printf "%%%0.s" $(seq 1 $count)) &&
Use of 'seq' gets complaint from
$ make -C t test-lint-shell-syntax
See the commit message of d17cf5f3 (tests: Introduce test_seq,
2012-08-04) and b32c7ec0 (test-lib: teach test_seq the -f option,
2025-06-23). I think you should be able to do something like
longname=$(test_seq -f "%%%0.s" 1 $count) &&
but I haven't even run the test with such a fix, so take it with a
grain of salt, please.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/9] submodule--helper: use submodule_name_to_gitdir in add_submodule
2025-08-16 21:36 ` [PATCH 1/9] submodule--helper: use submodule_name_to_gitdir in add_submodule Adrian Ratiu
@ 2025-08-20 19:04 ` Josh Steadmon
2025-08-21 11:26 ` Adrian Ratiu
0 siblings, 1 reply; 27+ messages in thread
From: Josh Steadmon @ 2025-08-20 19:04 UTC (permalink / raw)
To: Adrian Ratiu
Cc: git, Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Stefan Beller,
Patrick Steinhardt
On 2025.08.17 00:36, Adrian Ratiu wrote:
> While testing submodule gitdir path encoding, I noticed submodule--helper
> is still using a hardcoded name-based path leading to test failures, so
> convert it to the common helper function introduced by commit ce125d431a
> ("submodule: extract path to submodule gitdir func") and used in other
> locations accross the source tree.
>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
> builtin/submodule--helper.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 07a1935cbe..7243429c6f 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -3213,10 +3213,11 @@ static int add_submodule(const struct add_data *add_data)
> free(submod_gitdir_path);
> } else {
> struct child_process cp = CHILD_PROCESS_INIT;
> + struct strbuf submod_gitdir = STRBUF_INIT;
>
> - submod_gitdir_path = xstrfmt(".git/modules/%s", add_data->sm_name);
> + submodule_name_to_gitdir(&submod_gitdir, the_repository, add_data->sm_name);
I believe submod_gitdir_path is now only used in the `if (...) {...}`
side corresponding to this `else` branch, so perhaps we should make it
local to that block?
> - if (is_directory(submod_gitdir_path)) {
> + if (is_directory(submod_gitdir.buf)) {
> if (!add_data->force) {
> struct strbuf msg = STRBUF_INIT;
> char *die_msg;
> @@ -3225,8 +3226,8 @@ static int add_submodule(const struct add_data *add_data)
> "locally with remote(s):\n"),
> add_data->sm_name);
>
> - append_fetch_remotes(&msg, submod_gitdir_path);
> - free(submod_gitdir_path);
> + append_fetch_remotes(&msg, submod_gitdir.buf);
> + strbuf_release(&submod_gitdir);
>
> strbuf_addf(&msg, _("If you want to reuse this local git "
> "directory instead of cloning again from\n"
> @@ -3244,7 +3245,7 @@ static int add_submodule(const struct add_data *add_data)
> "submodule '%s'\n"), add_data->sm_name);
> }
> }
> - free(submod_gitdir_path);
> + strbuf_release(&submod_gitdir);
>
> clone_data.prefix = add_data->prefix;
> clone_data.path = add_data->sm_path;
> --
> 2.50.1.679.gbf363a8fbb.dirty
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 6/9] submodule: encode gitdir paths to avoid conflicts
2025-08-16 21:36 ` [PATCH 6/9] submodule: encode gitdir paths to avoid conflicts Adrian Ratiu
@ 2025-08-20 19:29 ` Jeff King
2025-08-21 13:14 ` Adrian Ratiu
0 siblings, 1 reply; 27+ messages in thread
From: Jeff King @ 2025-08-20 19:29 UTC (permalink / raw)
To: Adrian Ratiu
Cc: git, Emily Shaffer, Rodrigo Damazio Bovendorp, Junio C Hamano,
Aaron Schrab, Jonathan Nieder, Stefan Beller, Patrick Steinhardt,
Brandon Williams
On Sun, Aug 17, 2025 at 12:36:39AM +0300, Adrian Ratiu wrote:
> @@ -2632,5 +2633,23 @@ void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r,
> /* New style (encoded) paths go under submodules/<encoded>. */
> strbuf_reset(buf);
> repo_git_path_append(r, buf, "submodules/");
> - strbuf_addstr(buf, submodule_name);
> + base_len = buf->len;
> +
> + /* URL-encode then case case-encode A to _a, B to _b and so on */
> + strbuf_addstr_urlencode(&tmp, submodule_name, is_rfc3986_unreserved);
> + strbuf_addstr_case_encode(&encoded_sub_name, tmp.buf);
> + strbuf_release(&tmp);
> + strbuf_addbuf(buf, &encoded_sub_name);
> +
> + /* Ensure final path length is below NAME_MAX after encoding */
> + name_max = pathconf(buf->buf, _PC_NAME_MAX);
> + if (name_max == -1)
> + name_max = NAME_MAX;
This patch seems to break the Windows CI builds, as they don't have
pathconf() there. I guess we'd need a compat wrapper that returns -1 in
this case. And likewise protects _PC_NAME_MAX from being seen on systems
that don't have it.
> + encoded_len = buf->len - base_len;
> + if (encoded_len >= name_max)
> + die(_("encoded submodule name '%s' is too long (%zu bytes, limit is %ld)"),
> + encoded_sub_name.buf, encoded_len, name_max);
It also complained about %z here. I think you have to use PRIuMAX
instead. Likewise size_t is a "long long" on Windows (LLP64). So "%ld"
probably also needs to be PRIuMAX.
I also saw failures on the osx jobs for t7527.62 (submodule
absorbgitdirs implicitly starts daemon). I didn't dig in, but I can
guess they may be related to this series.
-Peff
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] submodule: add gitdir path config override
2025-08-16 21:36 ` [PATCH 3/9] submodule: add gitdir path config override Adrian Ratiu
@ 2025-08-20 19:37 ` Josh Steadmon
2025-08-21 12:18 ` Adrian Ratiu
2025-08-20 21:38 ` Josh Steadmon
2025-08-20 21:50 ` Josh Steadmon
2 siblings, 1 reply; 27+ messages in thread
From: Josh Steadmon @ 2025-08-20 19:37 UTC (permalink / raw)
To: Adrian Ratiu
Cc: git, Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Stefan Beller,
Patrick Steinhardt, Brandon Williams
On 2025.08.17 00:36, Adrian Ratiu wrote:
[snip]
> 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
This comment is a bit inaccurate, right? If I'm reading correctly, we
only verify that the submodule's gitdir actually exists in the "legacy"
.git/modules/$path case. If we don't see anything there, we fall through
to .git/submodules/$encoded_path, but we never verify it actually
exists.
> +
> +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
> --
> 2.50.1.679.gbf363a8fbb.dirty
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] submodule: add gitdir path config override
2025-08-16 21:36 ` [PATCH 3/9] submodule: add gitdir path config override Adrian Ratiu
2025-08-20 19:37 ` Josh Steadmon
@ 2025-08-20 21:38 ` Josh Steadmon
2025-08-21 13:04 ` Adrian Ratiu
2025-08-20 21:50 ` Josh Steadmon
2 siblings, 1 reply; 27+ messages in thread
From: Josh Steadmon @ 2025-08-20 21:38 UTC (permalink / raw)
To: Adrian Ratiu
Cc: git, Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Stefan Beller,
Patrick Steinhardt, Brandon Williams
On 2025.08.17 00:36, Adrian Ratiu wrote:
[snip]
> 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
> +'
> +
Rather than `( cd test-submodule && git config ... )` here, you should
use `test_config -C test-submodule ...` and `test_unconfig -C
test-submodule ...`
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] submodule: add gitdir path config override
2025-08-16 21:36 ` [PATCH 3/9] submodule: add gitdir path config override Adrian Ratiu
2025-08-20 19:37 ` Josh Steadmon
2025-08-20 21:38 ` Josh Steadmon
@ 2025-08-20 21:50 ` Josh Steadmon
2025-08-21 13:05 ` Adrian Ratiu
2 siblings, 1 reply; 27+ messages in thread
From: Josh Steadmon @ 2025-08-20 21:50 UTC (permalink / raw)
To: Adrian Ratiu
Cc: git, Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Patrick Steinhardt
On 2025.08.17 00: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.
>
> 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 <bmwill@google.com>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
> 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
Sorry to keep sending piecemeal feedback. You should also document the
new config option in `Documentation/config/submodule.adoc`
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/9] t: submodules: add basic mixed gitdir path tests
2025-08-16 21:36 ` [PATCH 4/9] t: submodules: add basic mixed gitdir path tests Adrian Ratiu
@ 2025-08-20 22:07 ` Josh Steadmon
2025-09-02 23:02 ` Junio C Hamano
1 sibling, 0 replies; 27+ messages in thread
From: Josh Steadmon @ 2025-08-20 22:07 UTC (permalink / raw)
To: Adrian Ratiu
Cc: git, Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Stefan Beller,
Patrick Steinhardt
On 2025.08.17 00:36, Adrian Ratiu wrote:
> Add some basic submodule tests for mixed gitdir path handling of
> legacy (.git/modules) and new-style (.git/submodule) paths.
>
> For now these just test the coexistence, creation and push/pull of
> submodules using mixed paths.
>
> More tests will be added later, especially for new-style encoding.
>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> ---
> t/meson.build | 1 +
> t/t7425-submodule-mixed-gitdir-paths.sh | 101 ++++++++++++++++++++++++
> 2 files changed, 102 insertions(+)
> create mode 100755 t/t7425-submodule-mixed-gitdir-paths.sh
>
> diff --git a/t/meson.build b/t/meson.build
> index bbeba1a8d5..ffd74f1d3b 100644
> --- a/t/meson.build
> +++ b/t/meson.build
> @@ -874,6 +874,7 @@ integration_tests = [
> 't7422-submodule-output.sh',
> 't7423-submodule-symlinks.sh',
> 't7424-submodule-mixed-ref-formats.sh',
> + 't7425-submodule-mixed-gitdir-paths.sh',
> 't7450-bad-git-dotfiles.sh',
> 't7500-commit-template-squash-signoff.sh',
> 't7501-commit-basic-functionality.sh',
> diff --git a/t/t7425-submodule-mixed-gitdir-paths.sh b/t/t7425-submodule-mixed-gitdir-paths.sh
> new file mode 100755
> index 0000000000..801e90522a
> --- /dev/null
> +++ b/t/t7425-submodule-mixed-gitdir-paths.sh
> @@ -0,0 +1,101 @@
> +#!/bin/sh
> +
> +test_description='submodules handle mixed legacy and new (encoded) style gitdir paths'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup: allow file protocol' '
> + git config --global protocol.file.allow always
> +'
> +
> +test_expect_success 'create repo with mixed new and legacy submodules' '
> + git init legacy-sub &&
> + test_commit -C legacy-sub legacy-initial &&
> + git -C legacy-sub config receive.denyCurrentBranch updateInstead &&
> + legacy_rev=$(git -C legacy-sub rev-parse HEAD) &&
> +
> + git init new-sub &&
> + test_commit -C new-sub new-initial &&
> + git -C new-sub config receive.denyCurrentBranch updateInstead &&
> + new_rev=$(git -C new-sub rev-parse HEAD) &&
> +
> + git init main &&
> + (
> + cd main &&
> +
> + git config receive.denyCurrentBranch updateInstead &&
> +
> + git submodule add ../new-sub new &&
> + test_commit new-sub &&
> +
> + git submodule add ../legacy-sub legacy &&
> + test_commit legacy-sub &&
> +
> + # simulate legacy .git/modules path by moving submodule
> + mkdir -p .git/modules &&
> + mv .git/submodules/legacy .git/modules/ &&
> + echo "gitdir: ../.git/modules/legacy" > legacy/.git
> + )
> +'
> +
> +test_expect_success 'clone from repo with both legacy and new-style submodules' '
> + git clone --recurse-submodules main cloned &&
> + (
> + cd cloned &&
> +
> + # At this point, .git/modules/<name> should not exist as
> + # submodules are checked out into the new path
> + test_path_is_dir .git/submodules/legacy &&
> + test_path_is_dir .git/submodules/new &&
> +
> + git submodule status >list &&
> + grep "$legacy_rev legacy" list &&
> + grep "$new_rev new" list
> + )
You probably want to use `test_grep` here and below.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/9] submodule--helper: use submodule_name_to_gitdir in add_submodule
2025-08-20 19:04 ` Josh Steadmon
@ 2025-08-21 11:26 ` Adrian Ratiu
0 siblings, 0 replies; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-21 11:26 UTC (permalink / raw)
To: Josh Steadmon
Cc: git, Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Stefan Beller,
Patrick Steinhardt
On Wed, 20 Aug 2025, Josh Steadmon <steadmon@google.com> wrote:
> On 2025.08.17 00:36, Adrian Ratiu wrote:
>> While testing submodule gitdir path encoding, I noticed submodule--helper
>> is still using a hardcoded name-based path leading to test failures, so
>> convert it to the common helper function introduced by commit ce125d431a
>> ("submodule: extract path to submodule gitdir func") and used in other
>> locations accross the source tree.
>>
>> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
>> ---
>> builtin/submodule--helper.c | 11 ++++++-----
>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
>> index 07a1935cbe..7243429c6f 100644
>> --- a/builtin/submodule--helper.c
>> +++ b/builtin/submodule--helper.c
>> @@ -3213,10 +3213,11 @@ static int add_submodule(const struct add_data *add_data)
>> free(submod_gitdir_path);
>> } else {
>> struct child_process cp = CHILD_PROCESS_INIT;
>> + struct strbuf submod_gitdir = STRBUF_INIT;
>>
>> - submod_gitdir_path = xstrfmt(".git/modules/%s", add_data->sm_name);
>> + submodule_name_to_gitdir(&submod_gitdir, the_repository, add_data->sm_name);
>
> I believe submod_gitdir_path is now only used in the `if (...) {...}`
> side corresponding to this `else` branch, so perhaps we should make it
> local to that block?
Yes, I'll move it in v2. Thanks!
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] submodule: add gitdir path config override
2025-08-20 19:37 ` Josh Steadmon
@ 2025-08-21 12:18 ` Adrian Ratiu
0 siblings, 0 replies; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-21 12:18 UTC (permalink / raw)
To: Josh Steadmon
Cc: git, Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Patrick Steinhardt,
Brandon Williams
On Wed, 20 Aug 2025, Josh Steadmon <steadmon@google.com> wrote:
> On 2025.08.17 00:36, Adrian Ratiu wrote: [snip]
>> 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
>
> This comment is a bit inaccurate, right? If I'm reading
> correctly, we only verify that the submodule's gitdir actually
> exists in the "legacy" .git/modules/$path case. If we don't see
> anything there, we fall through to
> .git/submodules/$encoded_path, but we never verify it actually
> exists.
>
It was not my intention to imply gitdir existence is verified
here. :)
We just verify where the gitdirs are expected vs configured,
regardless of existence (eg when adding a new submodule it won't
exist beforehand).
The behavior you describe is correct: we only verify existence in
the legacy case, yes, in submodule_name_to_gitdir(), because only
those must exist beforehand and that's how we know we're in the
legacy (backwards compatibility) case, otherwise we default to the
newly encoded paths.
Hope this makes sense. I'll expand the description to clarify in v2.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] submodule: add gitdir path config override
2025-08-20 21:38 ` Josh Steadmon
@ 2025-08-21 13:04 ` Adrian Ratiu
0 siblings, 0 replies; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-21 13:04 UTC (permalink / raw)
To: Josh Steadmon
Cc: git, Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Patrick Steinhardt
On Wed, 20 Aug 2025, Josh Steadmon <steadmon@google.com> wrote:
> On 2025.08.17 00:36, Adrian Ratiu wrote: [snip]
>> 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 +' +
>
> Rather than `( cd test-submodule && git config ... )` here, you
> should use `test_config -C test-submodule ...` and
> `test_unconfig -C test-submodule ...`
ack, will do in v2.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/9] submodule: add gitdir path config override
2025-08-20 21:50 ` Josh Steadmon
@ 2025-08-21 13:05 ` Adrian Ratiu
0 siblings, 0 replies; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-21 13:05 UTC (permalink / raw)
To: Josh Steadmon
Cc: git, Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Patrick Steinhardt
On Wed, 20 Aug 2025, Josh Steadmon <steadmon@google.com> wrote:
> On 2025.08.17 00: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.
>> 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 <bmwill@google.com>
>> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com> ---
>> 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
>
> Sorry to keep sending piecemeal feedback. You should also
> document the new config option in
> `Documentation/config/submodule.adoc`
No problem, thank you for taking the time to review & give
feedback.
Will do in v2.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 5/9] strbuf: bring back is_rfc3986_unreserved
2025-08-16 21:56 ` Ben Knoble
@ 2025-08-21 13:08 ` Adrian Ratiu
0 siblings, 0 replies; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-21 13:08 UTC (permalink / raw)
To: Ben Knoble
Cc: git, Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Junio C Hamano, Aaron Schrab, Jonathan Nieder, Patrick Steinhardt
On Sat, 16 Aug 2025, Ben Knoble <ben.knoble@gmail.com> wrote:
>> Le 16 août 2025 à 17:39, Adrian Ratiu
>> <adrian.ratiu@collabora.com> a écrit : Commit f89854362c
>> ("credential-store: move related functions to...")
>
> Here and elsewhere, we refer to commits by the output of “git
> show -s --format=reference <object>”
>
> Best, Ben
Ack, will fix in v2.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 6/9] submodule: encode gitdir paths to avoid conflicts
2025-08-20 19:29 ` Jeff King
@ 2025-08-21 13:14 ` Adrian Ratiu
0 siblings, 0 replies; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-21 13:14 UTC (permalink / raw)
To: Jeff King
Cc: git, Emily Shaffer, Rodrigo Damazio Bovendorp, Junio C Hamano,
Aaron Schrab, Jonathan Nieder, Patrick Steinhardt
On Wed, 20 Aug 2025, Jeff King <peff@peff.net> wrote:
> On Sun, Aug 17, 2025 at 12:36:39AM +0300, Adrian Ratiu wrote:
>
>> @@ -2632,5 +2633,23 @@ void submodule_name_to_gitdir(struct
>> strbuf *buf, struct repository *r,
>> /* New style (encoded) paths go under
>> submodules/<encoded>. */ strbuf_reset(buf);
>> repo_git_path_append(r, buf, "submodules/");
>> - strbuf_addstr(buf, submodule_name); + base_len =
>> buf->len; + + /* URL-encode then case case-encode A to
>> _a, B to _b and so on */ + strbuf_addstr_urlencode(&tmp,
>> submodule_name, is_rfc3986_unreserved); +
>> strbuf_addstr_case_encode(&encoded_sub_name, tmp.buf); +
>> strbuf_release(&tmp); + strbuf_addbuf(buf,
>> &encoded_sub_name); + + /* Ensure final path length is
>> below NAME_MAX after encoding */ + name_max =
>> pathconf(buf->buf, _PC_NAME_MAX); + if (name_max == -1) +
>> name_max = NAME_MAX;
>
> This patch seems to break the Windows CI builds, as they don't
> have pathconf() there. I guess we'd need a compat wrapper that
> returns -1 in this case. And likewise protects _PC_NAME_MAX from
> being seen on systems that don't have it.
>
Ack, will fix in v2.
>> + encoded_len = buf->len - base_len; + if (encoded_len >=
>> name_max) + die(_("encoded submodule name '%s' is too
>> long (%zu bytes, limit is %ld)"), +
>> encoded_sub_name.buf, encoded_len, name_max);
>
> It also complained about %z here. I think you have to use
> PRIuMAX instead. Likewise size_t is a "long long" on Windows
> (LLP64). So "%ld" probably also needs to be PRIuMAX.
>
> I also saw failures on the osx jobs for t7527.62 (submodule
> absorbgitdirs implicitly starts daemon). I didn't dig in, but I
> can guess they may be related to this series.
It's possible. I'm so sorry for these breakages and will address
them in v2. I'll use the GitHub CI since I don't have access to
win+mac systems.
Also thank you for your patience, this is my first git project patch. :)
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 9/9] t: add gitdir encoding tests
2025-08-18 22:06 ` Junio C Hamano
@ 2025-08-21 13:17 ` Adrian Ratiu
0 siblings, 0 replies; 27+ messages in thread
From: Adrian Ratiu @ 2025-08-21 13:17 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Aaron Schrab, Jonathan Nieder, Patrick Steinhardt
On Mon, 18 Aug 2025, Junio C Hamano <gitster@pobox.com> wrote:
> Adrian Ratiu <adrian.ratiu@collabora.com> writes:
>
>> Add some tests to further exercise the gitdir encoding
>> functionality alongside the existing mixed directory and nested
>> gitdir tests.
>>
>> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com> ---
>> t/t7425-submodule-mixed-gitdir-paths.sh | 52
>> +++++++++++++++++++++++++ 1 file changed, 52 insertions(+)
>>
>> diff --git a/t/t7425-submodule-mixed-gitdir-paths.sh
>> b/t/t7425-submodule-mixed-gitdir-paths.sh index
>> 902b2560ca..cfdf487a56 100755 ---
>> a/t/t7425-submodule-mixed-gitdir-paths.sh +++
>> b/t/t7425-submodule-mixed-gitdir-paths.sh @@ -152,4 +152,56 @@
>> test_expect_success 'checkout -f --recurse-submodules must
>> corectly handle neste ... + longname=$(printf "%%%0.s"
>> $(seq 1 $count)) &&
>
> Use of 'seq' gets complaint from
>
> $ make -C t test-lint-shell-syntax
>
> See the commit message of d17cf5f3 (tests: Introduce test_seq,
> 2012-08-04) and b32c7ec0 (test-lib: teach test_seq the -f
> option, 2025-06-23). I think you should be able to do something
> like
>
> longname=$(test_seq -f "%%%0.s" 1 $count) &&
>
> but I haven't even run the test with such a fix, so take it with
> a grain of salt, please.
Ack and thank you for the pointers. Will address in v2.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/9] t: submodules: add basic mixed gitdir path tests
2025-08-16 21:36 ` [PATCH 4/9] t: submodules: add basic mixed gitdir path tests Adrian Ratiu
2025-08-20 22:07 ` Josh Steadmon
@ 2025-09-02 23:02 ` Junio C Hamano
1 sibling, 0 replies; 27+ messages in thread
From: Junio C Hamano @ 2025-09-02 23:02 UTC (permalink / raw)
To: Adrian Ratiu
Cc: git, Emily Shaffer, Rodrigo Damazio Bovendorp, Jeff King,
Aaron Schrab, Jonathan Nieder, Stefan Beller, Patrick Steinhardt
Adrian Ratiu <adrian.ratiu@collabora.com> writes:
> +test_expect_success 'commit and push changes to submodules' '
> + (
> + cd cloned &&
> +
> + git -C legacy switch --track -C master origin/master &&
This test needs to future-proof itself, perhaps with something like
to force the initial branch name to a known value.
t/t7425-submodule-mixed-gitdir-paths.sh | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/t/t7425-submodule-mixed-gitdir-paths.sh b/t/t7425-submodule-mixed-gitdir-paths.sh
index 8a2d2e917f..02bd48aeeb 100755
--- a/t/t7425-submodule-mixed-gitdir-paths.sh
+++ b/t/t7425-submodule-mixed-gitdir-paths.sh
@@ -2,9 +2,13 @@
test_description='submodules handle mixed legacy and new (encoded) style gitdir paths'
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
+
. ./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
'
--
2.51.0-302-ga83f9e55f9
^ permalink raw reply related [flat|nested] 27+ messages in thread
end of thread, other threads:[~2025-09-02 23:02 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-16 21:36 [PATCH 0/9] Encode submodule gitdir names to avoid conflicts Adrian Ratiu
2025-08-16 21:36 ` [PATCH 1/9] submodule--helper: use submodule_name_to_gitdir in add_submodule Adrian Ratiu
2025-08-20 19:04 ` Josh Steadmon
2025-08-21 11:26 ` Adrian Ratiu
2025-08-16 21:36 ` [PATCH 2/9] submodule: create new gitdirs under submodules path Adrian Ratiu
2025-08-16 21:36 ` [PATCH 3/9] submodule: add gitdir path config override Adrian Ratiu
2025-08-20 19:37 ` Josh Steadmon
2025-08-21 12:18 ` Adrian Ratiu
2025-08-20 21:38 ` Josh Steadmon
2025-08-21 13:04 ` Adrian Ratiu
2025-08-20 21:50 ` Josh Steadmon
2025-08-21 13:05 ` Adrian Ratiu
2025-08-16 21:36 ` [PATCH 4/9] t: submodules: add basic mixed gitdir path tests Adrian Ratiu
2025-08-20 22:07 ` Josh Steadmon
2025-09-02 23:02 ` Junio C Hamano
2025-08-16 21:36 ` [PATCH 5/9] strbuf: bring back is_rfc3986_unreserved Adrian Ratiu
2025-08-16 21:56 ` Ben Knoble
2025-08-21 13:08 ` Adrian Ratiu
2025-08-16 21:36 ` [PATCH 6/9] submodule: encode gitdir paths to avoid conflicts Adrian Ratiu
2025-08-20 19:29 ` Jeff King
2025-08-21 13:14 ` Adrian Ratiu
2025-08-16 21:36 ` [PATCH 7/9] submodule: remove validate_submodule_git_dir() Adrian Ratiu
2025-08-16 21:36 ` [PATCH 8/9] t: move nested gitdir tests to proper location Adrian Ratiu
2025-08-16 21:36 ` [PATCH 9/9] t: add gitdir encoding tests Adrian Ratiu
2025-08-18 22:06 ` Junio C Hamano
2025-08-21 13:17 ` Adrian Ratiu
2025-08-17 13:01 ` [PATCH 0/9] Encode submodule gitdir names to avoid conflicts Adrian Ratiu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).