Re: [PATCH 1/2] submodule: prevent overwriting .gitmodules on path reuse

K Jayatheerth [off-list ref] writes:

Adding a submodule at a path that previously hosted
another submodule (e.g., 'child') reuses the submodule
name derived from the path. If the original submodule
was only moved (e.g., to 'child_old') and not renamed,
this silently overwrites its configuration in .gitmodules.

This behavior loses user configuration and causes
confusion when the original submodule is expected
to remain intact. It assumes that the path-derived
name is always safe to reuse, even though the name
might still be in use elsewhere in the repository.

Teach module_add() to check if the computed submodule
name already exists in the repository's submodule config,
and if so, refuse the operation unless the user explicitly
renames the submodule or uses the --force option,
which will automatically generate a unique name by
appending a number (e.g., child1).

Signed-off-by: K Jayatheerth <redacted>
---

Very well described.

+	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);

I'll locally fix this funny indentation; not a reason to require an
update.

+		}
+		/* --force: build <name><n> until unique */
+		for (i = 1; ; i++) {

I think you can narrow the scope of "i" to this loop alone.  I'll
locally do so (and if anything breaks, which I doubt); not a reason
to require an update.

+ ...
+		# Now adding a *new* repo at the old name must fail
+		git init ../child2-origin &&
+		git -C ../child2-origin commit --allow-empty -m init &&
+		test_must_fail git submodule add ../child2-origin child

Shouldn't we also check what this failed command tell the end-user?  E.g.

	test_must_fail git submodule add ../child2-origin child	2>err &&
	test_grep "alreayd used for" err