git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
To: git@vger.kernel.org
Cc: jayatheerthkulkarni2005@gmail.com
Subject: [PATCH] submodule: prevent overwriting .gitmodules entry on path reuse
Date: Sat, 10 May 2025 11:15:42 +0530	[thread overview]
Message-ID: <20250510054542.556401-1-jayatheerthkulkarni2005@gmail.com> (raw)

When a submodule is added at a path that previously hosted another submodule
(e.g., 'child'), Git reuses the submodule name derived from the path and
updates the corresponding entry in .gitmodules. This can silently overwrite
existing configuration if the old submodule was only moved (e.g., to
'child_old') without renaming the submodule.

This patch improves the `module_add()` logic by checking whether the
submodule name already exists in the config but maps to a different path.
In such a case, Git now errors out unless `--force` is specified, thus
preventing accidental overwrites. To proceed safely, the user can provide
a new name via `--name` or use `--force`.

Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
 builtin/submodule--helper.c | 45 ++++++++++++++++++++++++++++---------
 t/t7400-submodule-basic.sh  | 23 +++++++++++++++++++
 2 files changed, 57 insertions(+), 11 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 53da2116dd..0f98ef122b 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 "wildmatch.h"
+
 
 #define OPT_QUIET (1 << 0)
 #define OPT_CACHED (1 << 1)
@@ -3323,6 +3325,23 @@ static int config_submodule_in_gitmodules(const char *name, const char *var, con
 	return ret;
 }
 
+static int submodule_active_matches_path(const char *path)
+{
+	const struct string_list *values;
+	size_t i;
+
+	if (git_config_get_string_multi("submodule.active", &values))
+		return 0;
+
+	for (i = 0; i < values->nr; i++) {
+		const char *pat = values->items[i].string;
+		if (!wildmatch(pat, path, 0))
+			return 1;
+	}
+
+	return 0;
+}
+
 static void configure_added_submodule(struct add_data *add_data)
 {
 	char *key;
@@ -3370,17 +3389,7 @@ 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)) {
-			key = xstrfmt("submodule.%s.active", add_data->sm_name);
-			git_config_set_gently(key, "true");
-			free(key);
-		}
-	} else {
+	if (!submodule_active_matches_path(add_data->sm_path)) {
 		key = xstrfmt("submodule.%s.active", add_data->sm_name);
 		git_config_set_gently(key, "true");
 		free(key);
@@ -3443,6 +3452,7 @@ 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 option options[] = {
 		OPT_STRING('b', "branch", &add_data.branch, N_("branch"),
@@ -3546,6 +3556,19 @@ 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' "
+			"(use --name to choose another or --force to overwrite)"),
+			add_data.sm_name, existing->path);
+		}
+		add_data.sm_name = xstrfmt("%s.%s", add_data.sm_name, basename(add_data.sm_path));
+	}
+
 	if (check_submodule_name(add_data.sm_name))
 		die(_("'%s' is not a valid submodule name"), add_data.sm_name);
 
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.0.533.g80f4e02b4b


             reply	other threads:[~2025-05-10  5:45 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-10  5:45 K Jayatheerth [this message]
2025-05-10  5:57 ` [PATCH] submodule: prevent overwriting .gitmodules entry on path reuse JAYATHEERTH K
2025-05-12 12:32 ` Junio C Hamano
2025-05-12 13:26   ` JAYATHEERTH K
2025-05-13  3:34     ` [PATCH v2] " K Jayatheerth
2025-05-13 21:44       ` Junio C Hamano
2025-05-14  2:01         ` [PATCH v3] " K Jayatheerth
2025-05-14 22:47           ` Junio C Hamano
2025-05-16  8:51             ` JAYATHEERTH K
2025-05-16 17:49               ` [PATCH v4] " K Jayatheerth
2025-05-16 17:53                 ` JAYATHEERTH K
2025-05-18  7:54                   ` [PATCH v5] " K Jayatheerth
2025-05-18  7:58                     ` JAYATHEERTH K
2025-05-19 15:41                     ` Junio C Hamano
2025-05-20  1:31                       ` JAYATHEERTH K
2025-05-20 15:28                         ` Junio C Hamano
2025-05-24  6:48                           ` [PATCH v6 0/2] Avoid submodule overwritten and skip redundant active entries K Jayatheerth
2025-05-24  6:48                             ` [PATCH v6 1/2] submodule: prevent overwriting .gitmodules entry on path reuse K Jayatheerth
2025-05-24  6:48                             ` [PATCH v6 2/2] submodule: skip redundant active entries when pattern covers path K Jayatheerth
2025-05-24  6:54                               ` JAYATHEERTH K
2025-05-24  7:30                                 ` [PATCH v7 0/2] Avoid submodule overwritten and skip redundant active entries K Jayatheerth
2025-05-24  7:30                                   ` [PATCH v7 1/2] The seventeenth batch K Jayatheerth
2025-05-24  7:33                                     ` JAYATHEERTH K
2025-05-24  7:30                                   ` [PATCH v7 2/2] submodule: prevent overwriting .gitmodules entry on path reuse K Jayatheerth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250510054542.556401-1-jayatheerthkulkarni2005@gmail.com \
    --to=jayatheerthkulkarni2005@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).