[PATCH v5] submodule: prevent overwriting .gitmodules entry on path reuse
- From
K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
- Date
- May 18, 2025, 07:54 UTC
- Message-ID
- <20250518075436.75139-1-jayatheerthkulkarni2005@gmail.com>
- In-Reply-To
- <CA+rGoLfLA_s_xYN1N41w=vaOg0WPTNo_9TGKvG6ZQDqqWmzFPA@mail.gmail.com>
When a submodule is added, Git writes submodule.<name>.active = true to the repository configuration to mark it as active. This happens even when the submodule path already matches a pattern in submodule.active. This results in redundant configuration entries that are unnecessary and clutter the config, especially when pattern-based activation is used.
Avoid writing the submodule.<name>.active entry if the path is already covered by a pattern in submodule.active.
Reported-by: Moritz <mlell08@gmail.com> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com> --- builtin/submodule--helper.c | 59 ++++++++++++++++++++++++++++++------- t/t7400-submodule-basic.sh | 23 +++++++++++++++ 2 files changed, 72 insertions(+), 10 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 53da2116dd..5a9c8bdc0c 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -32,6 +32,8 @@ #include "advice.h" #include "branch.h" #include "list-objects-filter-options.h" +#include "strbuf.h" +#include "wildmatch.h" #define OPT_QUIET (1 << 0) #define OPT_CACHED (1 << 1) @@ -3328,6 +3330,9 @@ static void configure_added_submodule(struct add_data *add_data) char *key; struct child_process add_submod = CHILD_PROCESS_INIT; struct child_process add_gitmodules = CHILD_PROCESS_INIT; + const struct string_list *values; + size_t i; + int matched = 0; key = xstrfmt("submodule.%s.url", add_data->sm_name); git_config_set_gently(key, add_data->realrepo); @@ -3370,20 +3375,25 @@ static void configure_added_submodule(struct add_data *add_data) * is_submodule_active(), since that function needs to find * out the value of "submodule.active" again anyway. */ - if (!git_config_get("submodule.active")) { - /* - * If the submodule being added isn't already covered by the - * current configured pathspec, set the submodule's active flag - */ - if (!is_submodule_active(the_repository, add_data->sm_path)) { + if (git_config_get("submodule.active") || /* key absent */ + git_config_get_string_multi("submodule.active", &values)) { + /* submodule.active is missing -> force-enable */ + key = xstrfmt("submodule.%s.active", add_data->sm_name); + git_config_set_gently(key, "true"); + free(key); + } else { + for (i = 0; i < values->nr; i++) { + const char *pat = values->items[i].string; + if (!wildmatch(pat, add_data->sm_path, 0)) { /* match found */ + matched = 1; + break; + } + } + if (!matched) { /* no pattern matched -> force-enable */ key = xstrfmt("submodule.%s.active", add_data->sm_name); git_config_set_gently(key, "true"); free(key); } - } else { - key = xstrfmt("submodule.%s.active", add_data->sm_name); - git_config_set_gently(key, "true"); - free(key); } } @@ -3443,7 +3453,11 @@ static int module_add(int argc, const char **argv, const char *prefix, int force = 0, quiet = 0, progress = 0, dissociate = 0; struct add_data add_data = ADD_DATA_INIT; const char *ref_storage_format = NULL; + const struct submodule *existing; char *to_free = NULL; + struct strbuf buf = STRBUF_INIT; + int i; + char *sm_name_to_free = NULL; struct option options[] = { OPT_STRING('b', "branch", &add_data.branch, N_("branch"), N_("branch of repository to add as submodule")), @@ -3546,6 +3560,30 @@ static int module_add(int argc, const char **argv, const char *prefix, if(!add_data.sm_name) add_data.sm_name = add_data.sm_path; + existing = submodule_from_name(the_repository, + null_oid(the_hash_algo), + add_data.sm_name); + + if (existing && strcmp(existing->path, add_data.sm_path)) { + if (!force) { + die(_("submodule name '%s' already used for path '%s'"), + add_data.sm_name, existing->path); + } + + /* --force: build <name><n> until unique */ + for (i = 1; ; i++) { + strbuf_reset(&buf); + strbuf_addf(&buf, "%s%d", add_data.sm_name, i); + if (!submodule_from_name(the_repository, + null_oid(the_hash_algo), + buf.buf)) { + break; + } + } + + add_data.sm_name = sm_name_to_free = strbuf_detach(&buf, NULL); + } + if (check_submodule_name(add_data.sm_name)) die(_("'%s' is not a valid submodule name"), add_data.sm_name); @@ -3561,6 +3599,7 @@ static int module_add(int argc, const char **argv, const char *prefix, ret = 0; cleanup: + free(sm_name_to_free); free(add_data.sm_path); free(to_free); strbuf_release(&sb); diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index d6a501d453..5c3f471338 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -1482,4 +1482,27 @@ test_expect_success '`submodule init` and `init.templateDir`' ' ) ' +test_expect_success 'submodule add fails when name is reused' ' + git init test-submodule && + ( + cd test-submodule && + git commit --allow-empty -m "initial commit" && + + git init ../child-origin && + git -C ../child-origin commit --allow-empty -m "initial commit" && + + git submodule add ../child-origin child && + git commit -m "Add submodule child" && + + git mv child child_old && + git commit -m "Move child to child_old" && + + # Create another submodule repo + git init ../child2-origin && + git -C ../child2-origin commit --allow-empty -m "initial commit" && + + test_must_fail git submodule add ../child2-origin child + ) +' + test_done
-- 2.49.GIT