On Sat, Sep 13, 2008 at 4:24 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Petr Baudis <pasky@xxxxxxx> 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). > >> 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. Yeah, redirecting to /dev/null seems like a good idea. > 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. I'd just exit with an informative error if submodule.$name already exists in .git/config. > 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? Not really, since `git submodule` must be executed from the toplevel ($SUBDIRECTORY_OK is undefined, so the cd_to_toplevel in cmd_sync and cmd_summary seems to be noops). -- larsh -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
