Re: [PATCH 6/6] git submodule add: Fix naming clash handling
- From
- Lars Hjemli <hjemli@gmail.com>
- Date
- Sep 13, 2008, 11:32 UTC
- Message-ID
- <8c5c35580809130432r11e986e5u4c7300d62ca2896f@mail.gmail.com>
- In-Reply-To
- <7v63p0n5pv.fsf@gitster.siamese.dyndns.org>
On Sat, Sep 13, 2008 at 4:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 49 quoted lines
> 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). > >> 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.
Show 7 quoted lines
> 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