git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
LHLars 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

Previous: Junio C Hamano
Message 18 of 18 in “Submodule support for git mv and git rm”
  1. 0/6 Submodule support for git mv and git rmPetr Baudis, Sep 12, 2008
  2. 1/6 submodule.*: Introduce simple C interface for submodule lookup by pathPetr Baudis, Sep 12, 2008
  3. Junio C HamanoSep 12, 2008
  4. Jakub NarebskiSep 12, 2008
  5. Petr BaudisSep 12, 2008
  6. 2/6 git rm: Support for removing submodulesPetr Baudis, Sep 12, 2008
  7. Junio C HamanoSep 12, 2008
  8. Junio C HamanoSep 12, 2008
  9. Petr BaudisSep 12, 2008
  10. Junio C HamanoSep 12, 2008
  11. 3/6 git mv: Support moving submodulesPetr Baudis, Sep 12, 2008
  12. git mv: Support moving submodulesPetr Baudis, Sep 12, 2008
  13. Junio C HamanoSep 12, 2008
  14. 4/6 t7403: Submodule git mv, git rm testsuitePetr Baudis, Sep 12, 2008
  15. 5/6 t7400: Add short "git submodule add" testsuitePetr Baudis, Sep 12, 2008
  16. 6/6 git submodule add: Fix naming clash handlingPetr Baudis, Sep 12, 2008
  17. Junio C HamanoSep 13, 2008
  18. Lars HjemliSep 13, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.