{"thread":{"id":"63839","subject":"[PATCH 0/2] Avoid submodule overwritten and skip redundant active entries","startedAt":"2025-07-24T15:24:33Z","lastAt":"2025-07-25T16:24:20Z","messageCount":8,"participants":["K Jayatheerth","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"522681","messageId":"20250724152418.45226-1-jayatheerthkulkarni2005@gmail.com","threadId":"63839","inReplyTo":null,"subject":"[PATCH 0/2] Avoid submodule overwritten and skip redundant active entries","fromName":"K Jayatheerth","fromEmail":"jayatheerthkulkarni2005@gmail.com","sentAt":"2025-07-24T15:24:16Z","receivedAt":"2025-07-24T15:24:33Z","isPatch":true,"sender":{"key":"jayatheerthkulkarni2005@gmail.com","avatar":"https://avatars.githubusercontent.com/u/148841023?v=4"},"body":"Changes:\n\n1. Readded the comment so the patch now shows no added lines at the comments it's \njust the previous one (As clean as I could do it).            \n/*\n * If the submodule being added isn't already covered by the\n * current configured pathspec, set the submodule's active flag\n*/\n\n2. Instead of ! git config I now use test_must_fail\n\n3. For --force used the same logic the incremental logic.\n\n4. I also updated the docs for --force.\n\n5. Removed the white spaces from the lines.\n\nK Jayatheerth (2):\n  submodule: prevent overwriting .gitmodules on path reuse\n  submodule: skip redundant active entries when pattern covers path\n\n Documentation/git-submodule.adoc |  7 +++++\n builtin/submodule--helper.c      | 52 ++++++++++++++++++++++++++++----\n t/t7400-submodule-basic.sh       | 22 ++++++++++++++\n t/t7413-submodule-is-active.sh   | 15 +++++++++\n 4 files changed, 90 insertions(+), 6 deletions(-)\n\n-- \n2.50.GIT\n\n"},{"id":"522682","messageId":"20250724152418.45226-2-jayatheerthkulkarni2005@gmail.com","threadId":"63839","inReplyTo":"20250724152418.45226-1-jayatheerthkulkarni2005@gmail.com","subject":"[PATCH 1/2] submodule: prevent overwriting .gitmodules on path reuse","fromName":"K Jayatheerth","fromEmail":"jayatheerthkulkarni2005@gmail.com","sentAt":"2025-07-24T15:24:17Z","receivedAt":"2025-07-24T15:24:35Z","isPatch":true,"sender":{"key":"jayatheerthkulkarni2005@gmail.com","avatar":"https://avatars.githubusercontent.com/u/148841023?v=4"},"body":"Adding a submodule at a path that previously hosted\nanother submodule (e.g., 'child') reuses the submodule\nname derived from the path. If the original submodule\nwas only moved (e.g., to 'child_old') and not renamed,\nthis silently overwrites its configuration in .gitmodules.\n\nThis behavior loses user configuration and causes\nconfusion when the original submodule is expected\nto remain intact. It assumes that the path-derived\nname is always safe to reuse, even though the name\nmight still be in use elsewhere in the repository.\n\nTeach module_add() to check if the computed submodule\nname already exists in the repository's submodule config,\nand if so, refuse the operation unless the user explicitly\nrenames the submodule or uses the --force option,\nwhich will automatically generate a unique name by\nappending a number (e.g., child1).\n\nSigned-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>\n---\n Documentation/git-submodule.adoc |  7 +++++++\n builtin/submodule--helper.c      | 27 +++++++++++++++++++++++++++\n t/t7400-submodule-basic.sh       | 22 ++++++++++++++++++++++\n 3 files changed, 56 insertions(+)\n\ndiff --git a/Documentation/git-submodule.adoc b/Documentation/git-submodule.adoc\nindex 87d8e0f0c5..503c84a200 100644\n--- a/Documentation/git-submodule.adoc\n+++ b/Documentation/git-submodule.adoc\n@@ -307,6 +307,13 @@ OPTIONS\n --force::\n \tThis option is only valid for add, deinit and update commands.\n \tWhen running add, allow adding an otherwise ignored submodule path.\n+\tThis option is also used to bypass a check that the submodule's name\n+\tis not already in use. By default, 'git submodule add' will fail if\n+\tthe proposed name (which is derived from the path) is already registered\n+\tfor another submodule in the repository. Using '--force' allows the command\n+\tto proceed by automatically generating a unique name by appending a number\n+\tto the conflicting name (e.g., if a submodule named 'child' exists, it will\n+\ttry 'child1', and so on).\n \tWhen running deinit the submodule working trees will be removed even\n \tif they contain local changes.\n \tWhen running update (only effective with the checkout procedure),\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex d8a6fa47e5..b4f5d6e26a 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -3423,6 +3423,10 @@ static int module_add(int argc, const char **argv, const char *prefix,\n \tstruct add_data add_data = ADD_DATA_INIT;\n \tconst char *ref_storage_format = NULL;\n \tchar *to_free = NULL;\n+\tconst struct submodule *existing;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tint i;\n+\tchar *sm_name_to_free = NULL;\n \tstruct option options[] = {\n \t\tOPT_STRING('b', \"branch\", &add_data.branch, N_(\"branch\"),\n \t\t\t   N_(\"branch of repository to add as submodule\")),\n@@ -3525,6 +3529,28 @@ static int module_add(int argc, const char **argv, const char *prefix,\n \tif(!add_data.sm_name)\n \t\tadd_data.sm_name = add_data.sm_path;\n \n+\texisting = submodule_from_name(the_repository,\n+\t\t\t\t\tnull_oid(the_hash_algo),\n+\t\t\t\t\tadd_data.sm_name);\n+\t\n+\tif (existing && strcmp(existing->path, add_data.sm_path)) {\n+\t\tif (!force) {\n+\t\t\tdie(_(\"submodule name '%s' already used for path '%s'\"),\n+\t\t\tadd_data.sm_name, existing->path);\n+\t\t}\n+\t\t/* --force: build <name><n> until unique */\n+\t\tfor (i = 1; ; i++) {\n+\t\t\tstrbuf_reset(&buf);\n+\t\t\tstrbuf_addf(&buf, \"%s%d\", add_data.sm_name, i);\n+\t\t\tif (!submodule_from_name(the_repository,\n+\t\t\t\t\t\tnull_oid(the_hash_algo),\n+\t\t\t\t\t\tbuf.buf)) {\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t\tadd_data.sm_name = sm_name_to_free = strbuf_detach(&buf, NULL);\n+\t}\n+\n \tif (check_submodule_name(add_data.sm_name))\n \t\tdie(_(\"'%s' is not a valid submodule name\"), add_data.sm_name);\n \n@@ -3540,6 +3566,7 @@ static int module_add(int argc, const char **argv, const char *prefix,\n \n \tret = 0;\n cleanup:\n+\tfree(sm_name_to_free);\n \tfree(add_data.sm_path);\n \tfree(to_free);\n \tstrbuf_release(&sb);\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex d6a501d453..6812df3081 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -1482,4 +1482,26 @@ test_expect_success '`submodule init` and `init.templateDir`' '\n \t)\n '\n \n+test_expect_success 'submodule add fails when name is reused' '\n+\tgit init test-submodule &&\n+\t(\n+\t\tcd test-submodule &&\n+\t\tgit commit --allow-empty -m init &&\n+\n+\t\tgit init ../child-origin &&\n+\t\tgit -C ../child-origin commit --allow-empty -m init &&\n+\n+\t\tgit submodule add ../child-origin child &&\n+\t\tgit commit -m \"Add submodule child\" &&\n+\n+\t\tgit mv child child_old &&\n+\t\tgit commit -m \"Move child to child_old\" &&\n+\n+\t\t# Now adding a *new* repo at the old name must fail\n+\t\tgit init ../child2-origin &&\n+\t\tgit -C ../child2-origin commit --allow-empty -m init &&\n+\t\ttest_must_fail git submodule add ../child2-origin child\n+\t)\n+'\n+\n test_done\n-- \n2.50.GIT\n\n"},{"id":"522683","messageId":"20250724152418.45226-3-jayatheerthkulkarni2005@gmail.com","threadId":"63839","inReplyTo":"20250724152418.45226-1-jayatheerthkulkarni2005@gmail.com","subject":"[PATCH 2/2] submodule: skip redundant active entries when pattern covers path","fromName":"K Jayatheerth","fromEmail":"jayatheerthkulkarni2005@gmail.com","sentAt":"2025-07-24T15:24:18Z","receivedAt":"2025-07-24T15:24:37Z","isPatch":true,"sender":{"key":"jayatheerthkulkarni2005@gmail.com","avatar":"https://avatars.githubusercontent.com/u/148841023?v=4"},"body":"configure_added_submodule always writes an explicit\nsubmodule.<name>.active entry, even when the new\npath is already matched by submodule.active\npatterns. This leads to unnecessary and cluttered configuration.\n\nchange the logic to centralize wildmatch-based pattern lookup,\nin configure_added_submodule. Wrap the active-entry write in a conditional\nthat only fires when that helper reports no existing pattern covers the\nsubmodule’s path.\n\nSigned-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>\n---\n builtin/submodule--helper.c    | 25 +++++++++++++++++++------\n t/t7413-submodule-is-active.sh | 15 +++++++++++++++\n 2 files changed, 34 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex b4f5d6e26a..1fb49a2c4c 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -32,6 +32,8 @@\n #include \"advice.h\"\n #include \"branch.h\"\n #include \"list-objects-filter-options.h\"\n+#include \"wildmatch.h\"\n+#include \"strbuf.h\"\n \n #define OPT_QUIET (1 << 0)\n #define OPT_CACHED (1 << 1)\n@@ -3308,6 +3310,9 @@ static void configure_added_submodule(struct add_data *add_data)\n \tstruct child_process add_submod = CHILD_PROCESS_INIT;\n \tstruct child_process add_gitmodules = CHILD_PROCESS_INIT;\n \n+\tconst struct string_list *values;\n+\tsize_t i;\n+\tint matched = 0;\n \tkey = xstrfmt(\"submodule.%s.url\", add_data->sm_name);\n \tgit_config_set_gently(key, add_data->realrepo);\n \tfree(key);\n@@ -3349,20 +3354,28 @@ static void configure_added_submodule(struct add_data *add_data)\n \t * is_submodule_active(), since that function needs to find\n \t * out the value of \"submodule.active\" again anyway.\n \t */\n-\tif (!git_config_get(\"submodule.active\")) {\n+\tif (git_config_get(\"submodule.active\") || /* key absent */\n+\t    git_config_get_string_multi(\"submodule.active\", &values)) {\n \t\t/*\n \t\t * If the submodule being added isn't already covered by the\n \t\t * current configured pathspec, set the submodule's active flag\n \t\t */\n-\t\tif (!is_submodule_active(the_repository, add_data->sm_path)) {\n+\t\tkey = xstrfmt(\"submodule.%s.active\", add_data->sm_name);\n+\t\tgit_config_set_gently(key, \"true\");\n+\t\tfree(key);\n+\t} else {\n+\t\tfor (i = 0; i < values->nr; i++) {\n+\t\t\tconst char *pat = values->items[i].string;\n+\t\t\tif (!wildmatch(pat, add_data->sm_path, 0)) { /* match found */\n+\t\t\t\tmatched = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t\tif (!matched) { /* no pattern matched -> force-enable */\n \t\t\tkey = xstrfmt(\"submodule.%s.active\", add_data->sm_name);\n \t\t\tgit_config_set_gently(key, \"true\");\n \t\t\tfree(key);\n \t\t}\n-\t} else {\n-\t\tkey = xstrfmt(\"submodule.%s.active\", add_data->sm_name);\n-\t\tgit_config_set_gently(key, \"true\");\n-\t\tfree(key);\n \t}\n }\n \ndiff --git a/t/t7413-submodule-is-active.sh b/t/t7413-submodule-is-active.sh\nindex 9509dc18fd..6fd3b870de 100755\n--- a/t/t7413-submodule-is-active.sh\n+++ b/t/t7413-submodule-is-active.sh\n@@ -124,4 +124,19 @@ test_expect_success 'is-active, submodule.active and submodule add' '\n \tgit -C super2 config --get submodule.mod.active\n '\n \n+test_expect_success 'submodule add skips redundant active entry' '\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\t\tgit config submodule.active \"lib/*\" &&\n+\t\tgit commit --allow-empty -m init &&\n+\n+\t\tgit init ../lib-origin &&\n+\t\tgit -C ../lib-origin commit --allow-empty -m init &&\n+\n+\t\tgit submodule add ../lib-origin lib/foo &&\n+\t\ttest_must_fail git config --get submodule.lib/foo.active\n+\t)\n+'\n+\n test_done\n-- \n2.50.GIT\n\n"},{"id":"522701","messageId":"xmqqjz3xz4ww.fsf@gitster.g","threadId":"63839","inReplyTo":"20250724152418.45226-2-jayatheerthkulkarni2005@gmail.com","subject":"Re: [PATCH 1/2] submodule: prevent overwriting .gitmodules on path reuse","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-24T20:53:51Z","receivedAt":"2025-07-24T20:53:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:\n\n> Adding a submodule at a path that previously hosted\n> another submodule (e.g., 'child') reuses the submodule\n> name derived from the path. If the original submodule\n> was only moved (e.g., to 'child_old') and not renamed,\n> this silently overwrites its configuration in .gitmodules.\n>\n> This behavior loses user configuration and causes\n> confusion when the original submodule is expected\n> to remain intact. It assumes that the path-derived\n> name is always safe to reuse, even though the name\n> might still be in use elsewhere in the repository.\n>\n> Teach module_add() to check if the computed submodule\n> name already exists in the repository's submodule config,\n> and if so, refuse the operation unless the user explicitly\n> renames the submodule or uses the --force option,\n> which will automatically generate a unique name by\n> appending a number (e.g., child1).\n>\n> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>\n> ---\n\nVery well described.\n\n> +\texisting = submodule_from_name(the_repository,\n> +\t\t\t\t\tnull_oid(the_hash_algo),\n> +\t\t\t\t\tadd_data.sm_name);\n> +\t\n> +\tif (existing && strcmp(existing->path, add_data.sm_path)) {\n> +\t\tif (!force) {\n> +\t\t\tdie(_(\"submodule name '%s' already used for path '%s'\"),\n> +\t\t\tadd_data.sm_name, existing->path);\n\nI'll locally fix this funny indentation; not a reason to require an\nupdate.\n\n> +\t\t}\n> +\t\t/* --force: build <name><n> until unique */\n> +\t\tfor (i = 1; ; i++) {\n\nI think you can narrow the scope of \"i\" to this loop alone.  I'll\nlocally do so (and if anything breaks, which I doubt); not a reason\nto require an update.\n\n> + ...\n> +\t\t# Now adding a *new* repo at the old name must fail\n> +\t\tgit init ../child2-origin &&\n> +\t\tgit -C ../child2-origin commit --allow-empty -m init &&\n> +\t\ttest_must_fail git submodule add ../child2-origin child\n\nShouldn't we also check what this failed command tell the end-user?  E.g.\n\n\ttest_must_fail git submodule add ../child2-origin child\t2>err &&\n\ttest_grep \"alreayd used for\" err\n\n"},{"id":"522703","messageId":"xmqq1pq5z3n3.fsf@gitster.g","threadId":"63839","inReplyTo":"20250724152418.45226-3-jayatheerthkulkarni2005@gmail.com","subject":"Re: [PATCH 2/2] submodule: skip redundant active entries when pattern covers path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-07-24T21:21:20Z","receivedAt":"2025-07-24T21:21:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:\n\n> @@ -3308,6 +3310,9 @@ static void configure_added_submodule(struct add_data *add_data)\n>  \tstruct child_process add_submod = CHILD_PROCESS_INIT;\n>  \tstruct child_process add_gitmodules = CHILD_PROCESS_INIT;\n>  \n> +\tconst struct string_list *values;\n> +\tsize_t i;\n> +\tint matched = 0;\n>  \tkey = xstrfmt(\"submodule.%s.url\", add_data->sm_name);\n>  \tgit_config_set_gently(key, add_data->realrepo);\n>  \tfree(key);\n\nThe blank line should be between the end of block of decls\n(i.e. \"int matched = 0\") and the first statement (i.e. \"key =\nxstrfmt(...)\"), not there.  You probably do not need \"i\" in such a\nwide scope; just use\n\n\tfor (size_t i = 0; i < values->nr; i++)\n\nin the only loop that uses it.\n\n> @@ -3349,20 +3354,28 @@ static void configure_added_submodule(struct add_data *add_data)\n>  \t * is_submodule_active(), since that function needs to find\n>  \t * out the value of \"submodule.active\" again anyway.\n>  \t */\n> -\tif (!git_config_get(\"submodule.active\")) {\n> +\tif (git_config_get(\"submodule.active\") || /* key absent */\n> +\t    git_config_get_string_multi(\"submodule.active\", &values)) {\n\nHmph, do we need two calls here, or would a single call to\nget_string_multi() sufficient to learn what we want here?  When\nthere is no such key, the function may fail (or succeed and leave\nvalues->nr == 0), and either way, we can tell that there is no such\nkey, right?\n"},{"id":"522756","messageId":"20250725162402.92098-1-jayatheerthkulkarni2005@gmail.com","threadId":"63839","inReplyTo":"20250724152418.45226-1-jayatheerthkulkarni2005@gmail.com","subject":"[PATCH 0/2] Avoid submodule overwritten and skip redundant active entries","fromName":"K Jayatheerth","fromEmail":"jayatheerthkulkarni2005@gmail.com","sentAt":"2025-07-25T16:24:00Z","receivedAt":"2025-07-25T16:24:17Z","isPatch":true,"sender":{"key":"jayatheerthkulkarni2005@gmail.com","avatar":"https://avatars.githubusercontent.com/u/148841023?v=4"},"body":"Changes:\n1. Test now has an explicit error message in patch 1 \n2. Nice catch on the double function calls those were not needed \n   a single function call to git_config_get_string_multi did get job done.\n3. While I am at it also fixed some indentation issues (Only hoping i didn't create any unknown ones)\nand localized the loop variables.\n\nK Jayatheerth (2):\n  submodule: prevent overwriting .gitmodules on path reuse\n  submodule: skip redundant active entries when pattern covers path\n\n Documentation/git-submodule.adoc |  7 +++++\n builtin/submodule--helper.c      | 54 ++++++++++++++++++++++++++------\n t/t7400-submodule-basic.sh       | 22 +++++++++++++\n t/t7413-submodule-is-active.sh   | 15 +++++++++\n 4 files changed, 88 insertions(+), 10 deletions(-)\n\n-- \n2.50.GIT\n\n"},{"id":"522757","messageId":"20250725162402.92098-2-jayatheerthkulkarni2005@gmail.com","threadId":"63839","inReplyTo":"20250725162402.92098-1-jayatheerthkulkarni2005@gmail.com","subject":"[PATCH 1/2] submodule: prevent overwriting .gitmodules on path reuse","fromName":"K Jayatheerth","fromEmail":"jayatheerthkulkarni2005@gmail.com","sentAt":"2025-07-25T16:24:01Z","receivedAt":"2025-07-25T16:24:19Z","isPatch":true,"sender":{"key":"jayatheerthkulkarni2005@gmail.com","avatar":"https://avatars.githubusercontent.com/u/148841023?v=4"},"body":"Adding a submodule at a path that previously hosted\nanother submodule (e.g., 'child') reuses the submodule\nname derived from the path. If the original submodule\nwas only moved (e.g., to 'child_old') and not renamed,\nthis silently overwrites its configuration in .gitmodules.\n\nThis behavior loses user configuration and causes\nconfusion when the original submodule is expected\nto remain intact. It assumes that the path-derived\nname is always safe to reuse, even though the name\nmight still be in use elsewhere in the repository.\n\nTeach module_add() to check if the computed submodule\nname already exists in the repository's submodule config,\nand if so, refuse the operation unless the user explicitly\nrenames the submodule or uses the --force option,\nwhich will automatically generate a unique name by\nappending a number (e.g., child1).\n\nSigned-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>\n---\n Documentation/git-submodule.adoc |  7 +++++++\n builtin/submodule--helper.c      | 26 ++++++++++++++++++++++++++\n t/t7400-submodule-basic.sh       | 22 ++++++++++++++++++++++\n 3 files changed, 55 insertions(+)\n\ndiff --git a/Documentation/git-submodule.adoc b/Documentation/git-submodule.adoc\nindex 87d8e0f0c5..503c84a200 100644\n--- a/Documentation/git-submodule.adoc\n+++ b/Documentation/git-submodule.adoc\n@@ -307,6 +307,13 @@ OPTIONS\n --force::\n \tThis option is only valid for add, deinit and update commands.\n \tWhen running add, allow adding an otherwise ignored submodule path.\n+\tThis option is also used to bypass a check that the submodule's name\n+\tis not already in use. By default, 'git submodule add' will fail if\n+\tthe proposed name (which is derived from the path) is already registered\n+\tfor another submodule in the repository. Using '--force' allows the command\n+\tto proceed by automatically generating a unique name by appending a number\n+\tto the conflicting name (e.g., if a submodule named 'child' exists, it will\n+\ttry 'child1', and so on).\n \tWhen running deinit the submodule working trees will be removed even\n \tif they contain local changes.\n \tWhen running update (only effective with the checkout procedure),\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex d8a6fa47e5..9406e732c4 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -3423,6 +3423,9 @@ static int module_add(int argc, const char **argv, const char *prefix,\n \tstruct add_data add_data = ADD_DATA_INIT;\n \tconst char *ref_storage_format = NULL;\n \tchar *to_free = NULL;\n+\tconst struct submodule *existing;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tchar *sm_name_to_free = NULL;\n \tstruct option options[] = {\n \t\tOPT_STRING('b', \"branch\", &add_data.branch, N_(\"branch\"),\n \t\t\t   N_(\"branch of repository to add as submodule\")),\n@@ -3525,6 +3528,28 @@ static int module_add(int argc, const char **argv, const char *prefix,\n \tif(!add_data.sm_name)\n \t\tadd_data.sm_name = add_data.sm_path;\n \n+\texisting = submodule_from_name(the_repository,\n+\t\t\t\t\tnull_oid(the_hash_algo),\n+\t\t\t\t\tadd_data.sm_name);\n+\t\n+\tif (existing && strcmp(existing->path, add_data.sm_path)) {\n+\t\tif (!force) {\n+\t\t\tdie(_(\"submodule name '%s' already used for path '%s'\"),\n+\t\t\t    add_data.sm_name, existing->path);\n+\t\t}\n+\t\t/* --force: build <name><n> until unique */\n+\t\tfor (int i = 1; ; i++) {\n+\t\t\tstrbuf_reset(&buf);\n+\t\t\tstrbuf_addf(&buf, \"%s%d\", add_data.sm_name, i);\n+\t\t\tif (!submodule_from_name(the_repository,\n+\t\t\t\t\t\tnull_oid(the_hash_algo),\n+\t\t\t\t\t\tbuf.buf)) {\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t\tadd_data.sm_name = sm_name_to_free = strbuf_detach(&buf, NULL);\n+\t}\n+\n \tif (check_submodule_name(add_data.sm_name))\n \t\tdie(_(\"'%s' is not a valid submodule name\"), add_data.sm_name);\n \n@@ -3540,6 +3565,7 @@ static int module_add(int argc, const char **argv, const char *prefix,\n \n \tret = 0;\n cleanup:\n+\tfree(sm_name_to_free);\n \tfree(add_data.sm_path);\n \tfree(to_free);\n \tstrbuf_release(&sb);\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex d6a501d453..0743ccdfe2 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -1482,4 +1482,26 @@ test_expect_success '`submodule init` and `init.templateDir`' '\n \t)\n '\n \n+test_expect_success 'submodule add fails when name is reused' '\n+\tgit init test-submodule &&\n+\t(\n+\t\tcd test-submodule &&\n+\t\tgit commit --allow-empty -m init &&\n+\n+\t\tgit init ../child-origin &&\n+\t\tgit -C ../child-origin commit --allow-empty -m init &&\n+\n+\t\tgit submodule add ../child-origin child &&\n+\t\tgit commit -m \"Add submodule child\" &&\n+\n+\t\tgit mv child child_old &&\n+\t\tgit commit -m \"Move child to child_old\" &&\n+\n+\t\tgit init ../child2-origin &&\n+\t\tgit -C ../child2-origin commit --allow-empty -m init &&\n+\t\ttest_must_fail git submodule add ../child2-origin child 2>err &&\n+\t\ttest_grep \"submodule name '\\''child'\\'' already used for path '\\''child_old'\\''\" err\n+\t)\n+'\n+\n test_done\n-- \n2.50.GIT\n\n"},{"id":"522758","messageId":"20250725162402.92098-3-jayatheerthkulkarni2005@gmail.com","threadId":"63839","inReplyTo":"20250725162402.92098-1-jayatheerthkulkarni2005@gmail.com","subject":"[PATCH 2/2] submodule: skip redundant active entries when pattern covers path","fromName":"K Jayatheerth","fromEmail":"jayatheerthkulkarni2005@gmail.com","sentAt":"2025-07-25T16:24:02Z","receivedAt":"2025-07-25T16:24:20Z","isPatch":true,"sender":{"key":"jayatheerthkulkarni2005@gmail.com","avatar":"https://avatars.githubusercontent.com/u/148841023?v=4"},"body":"configure_added_submodule always writes an explicit\nsubmodule.<name>.active entry, even when the new\npath is already matched by submodule.active\npatterns. This leads to unnecessary and cluttered configuration.\n\nchange the logic to centralize wildmatch-based pattern lookup,\nin configure_added_submodule. Wrap the active-entry write in a conditional\nthat only fires when that helper reports no existing pattern covers the\nsubmodule’s path.\n\nSigned-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>\n---\n builtin/submodule--helper.c    | 28 ++++++++++++++++++----------\n t/t7413-submodule-is-active.sh | 15 +++++++++++++++\n 2 files changed, 33 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 9406e732c4..d4c4d9b0e1 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -32,6 +32,8 @@\n #include \"advice.h\"\n #include \"branch.h\"\n #include \"list-objects-filter-options.h\"\n+#include \"wildmatch.h\"\n+#include \"strbuf.h\"\n \n #define OPT_QUIET (1 << 0)\n #define OPT_CACHED (1 << 1)\n@@ -3307,7 +3309,8 @@ static void configure_added_submodule(struct add_data *add_data)\n \tchar *key;\n \tstruct child_process add_submod = CHILD_PROCESS_INIT;\n \tstruct child_process add_gitmodules = CHILD_PROCESS_INIT;\n-\n+\tconst struct string_list *values;\n+\tint matched = 0;\n \tkey = xstrfmt(\"submodule.%s.url\", add_data->sm_name);\n \tgit_config_set_gently(key, add_data->realrepo);\n \tfree(key);\n@@ -3349,17 +3352,22 @@ static void configure_added_submodule(struct add_data *add_data)\n \t * is_submodule_active(), since that function needs to find\n \t * out the value of \"submodule.active\" again anyway.\n \t */\n-\tif (!git_config_get(\"submodule.active\")) {\n+\tif (!git_config_get_string_multi(\"submodule.active\", &values)) {\n+\t\t/* The key exists and we have its values. Check for a match. */\n+\t\tfor (size_t i = 0; i < values->nr; i++) {\n+\t\t\tconst char *pat = values->items[i].string;\n+\t\t\tif (!wildmatch(pat, add_data->sm_path, 0)) {\n+\t\t\t\tmatched = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tif (!matched) {\n \t\t/*\n-\t\t * If the submodule being added isn't already covered by the\n-\t\t * current configured pathspec, set the submodule's active flag\n+\t\t * No pattern matched (or no 'submodule.active' patterns\n+\t\t * were configured at all), so explicitly activate.\n \t\t */\n-\t\tif (!is_submodule_active(the_repository, add_data->sm_path)) {\n-\t\t\tkey = xstrfmt(\"submodule.%s.active\", add_data->sm_name);\n-\t\t\tgit_config_set_gently(key, \"true\");\n-\t\t\tfree(key);\n-\t\t}\n-\t} else {\n \t\tkey = xstrfmt(\"submodule.%s.active\", add_data->sm_name);\n \t\tgit_config_set_gently(key, \"true\");\n \t\tfree(key);\ndiff --git a/t/t7413-submodule-is-active.sh b/t/t7413-submodule-is-active.sh\nindex 9509dc18fd..6fd3b870de 100755\n--- a/t/t7413-submodule-is-active.sh\n+++ b/t/t7413-submodule-is-active.sh\n@@ -124,4 +124,19 @@ test_expect_success 'is-active, submodule.active and submodule add' '\n \tgit -C super2 config --get submodule.mod.active\n '\n \n+test_expect_success 'submodule add skips redundant active entry' '\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\t\tgit config submodule.active \"lib/*\" &&\n+\t\tgit commit --allow-empty -m init &&\n+\n+\t\tgit init ../lib-origin &&\n+\t\tgit -C ../lib-origin commit --allow-empty -m init &&\n+\n+\t\tgit submodule add ../lib-origin lib/foo &&\n+\t\ttest_must_fail git config --get submodule.lib/foo.active\n+\t)\n+'\n+\n test_done\n-- \n2.50.GIT\n\n"}]}