@matheusbsbs / matheus-bagliano / commits / e897c9b7f3

submodule: detect conflicts with existing gitdir configs

Credit goes to Emily and Josh for testing and noticing a corner-case which caused conflicts with existing gitdir configs to silently pass validation, then fail later in add_submodule() with a cryptic error: fatal: A git directory for 'nested%2fsub' is found locally with remote(s): origin /.../trash directory.t7425-submodule-gitdir-path-extension/sub This change ensures the validation step checks existing gitdirs for conflicts. We only have to do this for submodules having gitdirs, because those without submodule.%s.gitdir need to be migrated and will throw an error earlier in the submodule codepath. Quoting Josh: My testing setup has been as follows: * Using our locally-built Git with our downstream patch of [1] included: * create a repo "sub" * create a repo "super" * In "super": * mkdir nested * git submodule add ../sub nested/sub * Verify that the submodule's gitdir is .git/modules/nested%2fsub * Using a build of git from upstream `next` plus this series: * git config set --global extensions.submodulepathconfig true * git clone --recurse-submodules super super2 * create a repo "nested%2fsub" * In "super2": * git submodule add ../nested%2fsub At this point I'd expect the collision detection / encoding to take effect, but instead I get the error listed above. End quote Suggested-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Adrian Ratiu committed Jan 12, 2026 at 20:46 UTC e897c9b7f31cf83e93cfefe1f82eb4a18337c9b1
2 files changed +89
submodule.c
+61
@@ -2296,6 +2296,62 @@ cleanup:
2296 return ret;
2297 }
2298
2299 +struct submodule_from_gitdir_cb {
2300 + const char *gitdir;
2301 + const char *submodule_name;
2302 + bool conflict_found;
2303 +};
2304 +
2305 +static int find_conflict_by_gitdir_cb(const char *var, const char *value,
2306 + const struct config_context *ctx UNUSED, void *data)
2307 +{
2308 + struct submodule_from_gitdir_cb *cb = data;
2309 + const char *submodule_name_start;
2310 + size_t submodule_name_len;
2311 + const char *suffix = ".gitdir";
2312 + size_t suffix_len = strlen(suffix);
2313 +
2314 + if (!skip_prefix(var, "submodule.", &submodule_name_start))
2315 + return 0;
2316 +
2317 + /* Check if submodule_name_start ends with ".gitdir" */
2318 + submodule_name_len = strlen(submodule_name_start);
2319 + if (submodule_name_len < suffix_len ||
2320 + strcmp(submodule_name_start + submodule_name_len - suffix_len, suffix) != 0)
2321 + return 0; /* Does not end with ".gitdir" */
2322 +
2323 + submodule_name_len -= suffix_len;
2324 +
2325 + /*
2326 + * A conflict happens if:
2327 + * 1. The submodule names are different and
2328 + * 2. The gitdir paths resolve to the same absolute path
2329 + */
2330 + if (value && strncmp(cb->submodule_name, submodule_name_start, submodule_name_len)) {
2331 + char *abs_path_cb = absolute_pathdup(cb->gitdir);
2332 + char *abs_path_value = absolute_pathdup(value);
2333 +
2334 + cb->conflict_found = !strcmp(abs_path_cb, abs_path_value);
2335 +
2336 + free(abs_path_cb);
2337 + free(abs_path_value);
2338 + }
2339 +
2340 + return cb->conflict_found;
2341 +}
2342 +
2343 +static bool submodule_conflicts_with_existing(const char *gitdir, const char *submodule_name)
2344 +{
2345 + struct submodule_from_gitdir_cb cb = { 0 };
2346 + cb.submodule_name = submodule_name;
2347 + cb.gitdir = gitdir;
2348 +
2349 + /* Find conflicts with existing repo gitdir configs */
2350 + repo_config(the_repository, find_conflict_by_gitdir_cb, &cb);
2351 +
2352 + return cb.conflict_found;
2353 +}
2354 +
2355 /*
2356 * Encoded gitdir validation, only used when extensions.submodulePathConfig is enabled.
2357 * This does not print errors like the non-encoded version, because encoding is supposed
@@ -2321,6 +2377,11 @@ static int validate_submodule_encoded_git_dir(char *git_dir, const char *submodu
2377 if (!last_submodule_name || strchr(last_submodule_name, '/'))
2378 return -1;
2379
2380 + /* Prevent conflicts with existing submodule gitdirs */
2381 + if (is_git_directory(git_dir) &&
2382 + submodule_conflicts_with_existing(git_dir, submodule_name))
2383 + return -1;
2384 +
2385 /* Prevent conflicts on case-folding filesystems */
2386 repo_config_get_bool(the_repository, "core.ignorecase", &config_ignorecase);
2387 if (ignore_case || config_ignorecase) {
t/t7425-submodule-gitdir-path-extension.sh
+28
@@ -497,4 +497,32 @@ test_expect_success CASE_INSENSITIVE_FS 'verify hashing conflict resolution as a
497 verify_submodule_gitdir_path cloned-hash "Foo" "modules/${hash}"
498 '
499
500 +test_expect_success 'submodule gitdir conflicts with previously encoded name (local config)' '
501 + git init -b main super_with_encoded &&
502 + (
503 + cd super_with_encoded &&
504 +
505 + git config core.repositoryformatversion 1 &&
506 + git config extensions.submodulePathConfig true &&
507 +
508 + # Add a submodule with a nested path
509 + git submodule add --name "nested/sub" ../sub nested/sub &&
510 + test_commit add-encoded-gitdir &&
511 +
512 + verify_submodule_gitdir_path . "nested/sub" "modules/nested%2fsub" &&
513 + test_path_is_dir ".git/modules/nested%2fsub"
514 + ) &&
515 +
516 + # create a submodule that will conflict with the encoded gitdir name:
517 + # the existing gitdir is ".git/modules/nested%2fsub", which is used
518 + # by "nested/sub", so the new submod will get another (non-conflicting)
519 + # name: "nested%252fsub".
520 + (
521 + cd super_with_encoded &&
522 + git submodule add ../sub "nested%2fsub" &&
523 + verify_submodule_gitdir_path . "nested%2fsub" "modules/nested%252fsub" &&
524 + test_path_is_dir ".git/modules/nested%252fsub"
525 + )
526 +'
527 +
528 test_done