K Jayatheerth <jayatheerthkulkarni2005@xxxxxxxxx> 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 <jayatheerthkulkarni2005@xxxxxxxxx>
> ---
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
