Re: [PATCH 6/6] git submodule add: Fix naming clash handling

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 13, 2008, 02:24 UTC
Message-ID: <7v63p0n5pv.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080912210924.31628.61593.stgit@localhost>

Petr Baudis <pasky@suse.cz> writes:

> This patch fixes git submodule add behaviour when we add submodule
> living at a same path as logical name of existing submodule. This
> can happen e.g. in case the user git mv's the previous submodule away
> and then git submodule add's another under the same name.

In short, name "example" was used to name the submodule bound at path "init" in earlier tests, and the new one adds another submodule whose name and path are both "example" and makes sure they do not get mixed up (and the original code did mix them up).

Show 29 quoted lines

>  git-submodule.sh           |   15 ++++++++++++---
>  t/t7400-submodule-basic.sh |   11 +++++++++++
>  2 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 1c39b59..3e4d839 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -201,10 +201,19 @@ cmd_add()
>  	git add "$path" ||
>  	die "Failed to add submodule '$path'"
>  
> -	git config -f .gitmodules submodule."$path".path "$path" &&
> -	git config -f .gitmodules submodule."$path".url "$repo" &&
> +	name="$path"
> +	if git config -f .gitmodules submodule."$name".path; then
> +		name="$path~"; i=1;
> +		while git config -f .gitmodules submodule."$name".path; do
> +			name="$path~$i"
> +			i=$((i+1))
> +		done
> +	fi
> +
> +	git config -f .gitmodules submodule."$name".path "$path" &&
> +	git config -f .gitmodules submodule."$name".url "$repo" &&
>  	git add .gitmodules ||
> -	die "Failed to register submodule '$path'"
> +	die "Failed to register submodule '$path' (name '$name')"
>  }

The logic of the fix seems to be correct, but shouldn't the test be like this instead?

	if git config -f .gitmodules "submodule.$name.path" >/dev/null
        then

The same thing for "git config" used in the "while" loop.

Also I am not sure if name="$path~" is a good idea for two reasons:

 - name suffixed with tilde and number looks too much like revision
   expression.

 - A, A~, A~1, A~2... looks ugly; A, A-0, A-1, A-2,... (or start counting
   from 1 or 2) I would understand.

By the way, I noticed that cmd_add does not seem to cd_to_toplevel, and accesses .gitmodules in the current working directory. Isn't this a bug? How should "git submodule add" work from inside a subdirectory?