threads / patch / 21024

patchgit submodule add: make the <path> parameter optional

Subject: [PATCH RESEND] git submodule add: make the <path> parameter optional

## tl;dr

7 messages between Sep 22, 2009 and Oct 5, 2009. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Jens Lehmann· Sep 22, 2009, 15:10 UTC · lore

When <path> is not given, use the "humanish" part of the source repository instead.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
With this patch, git submodule add behaves like git clone in this respect.
Didn't get a response the last weeks, so here is a resend.
 Documentation/git-submodule.txt |    8 ++++++--
 git-submodule.sh                |    7 ++++++-
 t/t7400-submodule-basic.sh      |   16 ++++++++++++++++
 3 files changed, 28 insertions(+), 3 deletions(-)
Show changes to 3 files +28 −3

Documentation/git-submodule.txt, git-submodule.sh, t/t7400-submodule-basic.sh

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 5ccdd18..4ef70c4 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git submodule' [--quiet] add [-b branch]
-	      [--reference <repository>] [--] <repository> <path>
+	      [--reference <repository>] [--] <repository> [<path>]
 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
 'git submodule' [--quiet] init [--] [<path>...]
 'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
@@ -69,7 +69,11 @@ add::
 	to the changeset to be committed next to the current
 	project: the current project is termed the "superproject".
 +
-This requires two arguments: <repository> and <path>.
+This requires at least one argument: <repository>. The optional
+argument <path> is the relative location for the cloned submodule
+to exist in the superproject. If <path> is not given, the
+"humanish" part of the source repository is used ("repo" for
+"/path/to/repo.git" and "foo" for "host.xz:foo/.git").
 +
 <repository> is the URL of the new submodule's origin repository.
 This may be either an absolute URL, or (if it begins with ./
diff --git a/git-submodule.sh b/git-submodule.sh
index bfbd36b..0c617eb 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -5,7 +5,7 @@
 # Copyright (c) 2007 Lars Hjemli

 dashless=$(basename "$0" | sed -e 's/-/ /')
-USAGE="[--quiet] add [-b branch] [--reference <repository>] [--] <repository> <path>
+USAGE="[--quiet] add [-b branch] [--reference <repository>] [--] <repository> [<path>]
    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
    or: $dashless [--quiet] init [--] [<path>...]
    or: $dashless [--quiet] update [--init] [-N|--no-fetch] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
@@ -160,6 +160,11 @@ cmd_add()
 	repo=$1
 	path=$2

+	if test -z "$path"; then
+		path=$(echo "$repo" |
+			sed -e 's|/$||' -e 's|:*/*\.git$||' -e 's|.*[/:]||g')
+	fi
+
 	if test -z "$repo" -o -z "$path"; then
 		usage
 	fi
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 0f2ccc6..a0cc99a 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -306,4 +306,20 @@ test_expect_success 'submodule <invalid-path> warns' '

 '

+test_expect_success 'add submodules without specifying an explicit path' '
+	mkdir repo &&
+	cd repo &&
+	git init &&
+	echo r >r &&
+	git add r &&
+	git commit -m "repo commit 1" &&
+	cd .. &&
+	git clone --bare repo/ bare.git &&
+	cd addtest &&
+	git submodule add "$submodurl/repo" &&
+	git config -f .gitmodules submodule.repo.path repo &&
+	git submodule add "$submodurl/bare.git" &&
+	git config -f .gitmodules submodule.bare.path bare
+'
+
 test_done

-- 
1.6.4.1.246.g7bae7.dirty
Junio C Hamano· Sep 22, 2009, 19:25 UTC · re: Jens Lehmann · lore

Re: [PATCH RESEND] git submodule add: make the <path> parameter optional

Jens Lehmann <Jens.Lehmann@web.de> writes:
Show 39 quoted lines
> When <path> is not given, use the "humanish" part of the source repository
> instead.
>
> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
>
> With this patch, git submodule add behaves like git clone in this respect.
>
> Didn't get a response the last weeks, so here is a resend.
>
>
>  Documentation/git-submodule.txt |    8 ++++++--
>  git-submodule.sh                |    7 ++++++-
>  t/t7400-submodule-basic.sh      |   16 ++++++++++++++++
>  3 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index 5ccdd18..4ef70c4 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
>  --------
>  [verse]
>  'git submodule' [--quiet] add [-b branch]
> -	      [--reference <repository>] [--] <repository> <path>
> +	      [--reference <repository>] [--] <repository> [<path>]
>  'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
>  'git submodule' [--quiet] init [--] [<path>...]
>  'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
> @@ -69,7 +69,11 @@ add::
>  	to the changeset to be committed next to the current
>  	project: the current project is termed the "superproject".
>  +
> -This requires two arguments: <repository> and <path>.
> +This requires at least one argument: <repository>. The optional
> +argument <path> is the relative location for the cloned submodule
> +to exist in the superproject. If <path> is not given, the
> +"humanish" part of the source repository is used ("repo" for
> +"/path/to/repo.git" and "foo" for "host.xz:foo/.git").
I do not know if this is useful in practice nor even desired.  Comments?
Jens Lehmann· Oct 4, 2009, 17:51 UTC · re: Junio C Hamano · lore

Re: [PATCH RESEND] git submodule add: make the <path> parameter optional

Junio C Hamano schrieb:
Show 43 quoted lines
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> When <path> is not given, use the "humanish" part of the source repository
>> instead.
>>
>> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
>> ---
>>
>> With this patch, git submodule add behaves like git clone in this respect.
>>
>> Didn't get a response the last weeks, so here is a resend.
>>
>>
>>  Documentation/git-submodule.txt |    8 ++++++--
>>  git-submodule.sh                |    7 ++++++-
>>  t/t7400-submodule-basic.sh      |   16 ++++++++++++++++
>>  3 files changed, 28 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
>> index 5ccdd18..4ef70c4 100644
>> --- a/Documentation/git-submodule.txt
>> +++ b/Documentation/git-submodule.txt
>> @@ -10,7 +10,7 @@ SYNOPSIS
>>  --------
>>  [verse]
>>  'git submodule' [--quiet] add [-b branch]
>> -	      [--reference <repository>] [--] <repository> <path>
>> +	      [--reference <repository>] [--] <repository> [<path>]
>>  'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
>>  'git submodule' [--quiet] init [--] [<path>...]
>>  'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
>> @@ -69,7 +69,11 @@ add::
>>  	to the changeset to be committed next to the current
>>  	project: the current project is termed the "superproject".
>>  +
>> -This requires two arguments: <repository> and <path>.
>> +This requires at least one argument: <repository>. The optional
>> +argument <path> is the relative location for the cloned submodule
>> +to exist in the superproject. If <path> is not given, the
>> +"humanish" part of the source repository is used ("repo" for
>> +"/path/to/repo.git" and "foo" for "host.xz:foo/.git").
> 
> I do not know if this is useful in practice nor even desired.  Comments?
As nobody commented until now, i'll explain my motivation for this patch.

When adding submodules i was surprised to find that i had to explicitly provide the pathname even though it could be easily generated from the reponame as git clone does it. And i see git clone and git submodule add as related commands from a users perspective, they both connect a remote repo to a working directory.

IMHO this patch makes the ui more consistent and doesn't break existing setups or scripts. And it is really useful because i don't do typos in the pathname anymore ;-)

Jens
Johannes Schindelin· Oct 4, 2009, 21:05 UTC · re: Jens Lehmann · lore

Re: [PATCH RESEND] git submodule add: make the <path> parameter optional

Hi,
On Sun, 4 Oct 2009, Jens Lehmann wrote:
Show 56 quoted lines
> Junio C Hamano schrieb:
> > Jens Lehmann <Jens.Lehmann@web.de> writes:
> > 
> >> When <path> is not given, use the "humanish" part of the source repository
> >> instead.
> >>
> >> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> >> ---
> >>
> >> With this patch, git submodule add behaves like git clone in this respect.
> >>
> >> Didn't get a response the last weeks, so here is a resend.
> >>
> >>
> >>  Documentation/git-submodule.txt |    8 ++++++--
> >>  git-submodule.sh                |    7 ++++++-
> >>  t/t7400-submodule-basic.sh      |   16 ++++++++++++++++
> >>  3 files changed, 28 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> >> index 5ccdd18..4ef70c4 100644
> >> --- a/Documentation/git-submodule.txt
> >> +++ b/Documentation/git-submodule.txt
> >> @@ -10,7 +10,7 @@ SYNOPSIS
> >>  --------
> >>  [verse]
> >>  'git submodule' [--quiet] add [-b branch]
> >> -	      [--reference <repository>] [--] <repository> <path>
> >> +	      [--reference <repository>] [--] <repository> [<path>]
> >>  'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]
> >>  'git submodule' [--quiet] init [--] [<path>...]
> >>  'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
> >> @@ -69,7 +69,11 @@ add::
> >>  	to the changeset to be committed next to the current
> >>  	project: the current project is termed the "superproject".
> >>  +
> >> -This requires two arguments: <repository> and <path>.
> >> +This requires at least one argument: <repository>. The optional
> >> +argument <path> is the relative location for the cloned submodule
> >> +to exist in the superproject. If <path> is not given, the
> >> +"humanish" part of the source repository is used ("repo" for
> >> +"/path/to/repo.git" and "foo" for "host.xz:foo/.git").
> > 
> > I do not know if this is useful in practice nor even desired.  Comments?
> 
> As nobody commented until now, i'll explain my motivation for this patch.
> 
> When adding submodules i was surprised to find that i had to explicitly
> provide the pathname even though it could be easily generated from the
> reponame as git clone does it. And i see git clone and git submodule add
> as related commands from a users perspective, they both connect a remote
> repo to a working directory.
> 
> IMHO this patch makes the ui more consistent and doesn't break existing
> setups or scripts. And it is really useful because i don't do typos in
> the pathname anymore ;-)

So far, I started submodules by cloning them, doing everything in the other files needed to integrate, and then actually wondered why "git submodule add" could not simply take the (relative) path to the checked-out submodule and deduce the URL from the corresponding config?

So I would actually vote for making the <repository> parameter optional...

Ciao, Dscho

Junio C Hamano· Oct 5, 2009, 01:31 UTC · re: Johannes Schindelin · lore

Re: [PATCH RESEND] git submodule add: make the <path> parameter optional

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> So far, I started submodules by cloning them, doing everything in the 
> other files needed to integrate, and then actually wondered why "git 
> submodule add" could not simply take the (relative) path to the 
> checked-out submodule and deduce the URL from the corresponding config?

Let me try to rephrase the above to see if I understand what you are doing. When building a top-level superproject that uses two submodules from other places, you would do:

	$ git init toplevel
        $ cd toplevel
        $ git clone $overthere submodule1
        $ git clone $elsewhere submodule2
        $ edit Makefile common.h
        $ git add Makefile common.h submodule1 submodule2

and by "the corresponding config", you mean "submodule1/.git/config already records that it came from $overthere, and there is no reason to say it again in 'git submodule add $overthere submodule1'".

If that is the case, I think it is a very sane argument. It also makes me wonder why we need "git submodule add" as a separate step to begin with (i.e. "git add" Porcelain could do that behind the scene), but that is probably a separate topic.

> So I would actually vote for making the <repository> parameter optional...

In your "git submodule add submodule1", it would be quite clear that it is a local path and <repository> is being omitted. On the other hand, if you said "git submodule add $overthere" without submodule1, because $overthere is not likely to be an in-tree path, it also would be clear that it is omitting the path. IOW, these two typesavers are not mutually exclusive.

In real life, it is very likely $overthere does _not_ end with submodule1, and your suggestion would probably be more useful than the patch under discussion, I think.

Johannes Schindelin· Oct 5, 2009, 09:16 UTC · re: Junio C Hamano · lore

Re: [PATCH RESEND] git submodule add: make the <path> parameter optional

Hi,
On Sun, 4 Oct 2009, Junio C Hamano wrote:
Show 21 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > So far, I started submodules by cloning them, doing everything in the 
> > other files needed to integrate, and then actually wondered why "git 
> > submodule add" could not simply take the (relative) path to the 
> > checked-out submodule and deduce the URL from the corresponding config?
> 
> Let me try to rephrase the above to see if I understand what you are
> doing.  When building a top-level superproject that uses two submodules
> from other places, you would do:
> 
> 	$ git init toplevel
>         $ cd toplevel
>         $ git clone $overthere submodule1
>         $ git clone $elsewhere submodule2
>         $ edit Makefile common.h
>         $ git add Makefile common.h submodule1 submodule2
> 
> and by "the corresponding config", you mean "submodule1/.git/config
> already records that it came from $overthere, and there is no reason to
> say it again in 'git submodule add $overthere submodule1'".
Yes, that's what I meant.

But I do like your idea of enhancing "git add" to do this. Then we can keep "git submodule add" as-is, and then it actually makes sense to make the <path> parameter optional: "git submodule add" is then about _cloning_ and adding the submodule. Which is not my scenario.

Show 8 quoted lines
> > So I would actually vote for making the <repository> parameter 
> > optional...
> 
> In your "git submodule add submodule1", it would be quite clear that it is
> a local path and <repository> is being omitted.  On the other hand, if you
> said "git submodule add $overthere" without submodule1, because $overthere
> is not likely to be an in-tree path, it also would be clear that it is
> omitting the path.  IOW, these two typesavers are not mutually exclusive.

I thought about that, but it seems a bit too magical, and it is obvious that

	git submodule add $(pwd)/bla
would test that magic.

Ciao, Dscho

Jens Lehmann· Oct 5, 2009, 10:28 UTC · re: Junio C Hamano · lore

Re: [PATCH RESEND] git submodule add: make the <path> parameter optional

Junio C Hamano schrieb:
Show 21 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
>> So far, I started submodules by cloning them, doing everything in the 
>> other files needed to integrate, and then actually wondered why "git 
>> submodule add" could not simply take the (relative) path to the 
>> checked-out submodule and deduce the URL from the corresponding config?
> 
> Let me try to rephrase the above to see if I understand what you are
> doing.  When building a top-level superproject that uses two submodules
> from other places, you would do:
> 
> 	$ git init toplevel
>         $ cd toplevel
>         $ git clone $overthere submodule1
>         $ git clone $elsewhere submodule2
>         $ edit Makefile common.h
>         $ git add Makefile common.h submodule1 submodule2
> 
> and by "the corresponding config", you mean "submodule1/.git/config
> already records that it came from $overthere, and there is no reason to
> say it again in 'git submodule add $overthere submodule1'".

Right, no reason to use git submodule add here. But in this example submodule1 & submodule2 aren't real submodules, because the .gitmodules file is not created. Or am i missing something?

> If that is the case, I think it is a very sane argument.  It also makes me
> wonder why we need "git submodule add" as a separate step to begin with
> (i.e. "git add" Porcelain could do that behind the scene), but that is
> probably a separate topic.

I think we need git submodule add because it is doing much more than a plain git add. It also does the clone and creates the .gitmodules file needed later.

My everyday use case looks different. When i start a new project where i want to use two of libraries living in their own git repo, i do:

   $ git init toplevel
   $ cd toplevel
   $ git submodule add git://mygitserver/submodule1.git submodule1
   $ git submodule add git://mygitserver/submodule2.git submodule2
   $ git commit -m 'Initial setup with submodule1 & submodule2'

After the submodule add submodule1, submodule2 and .gitmodules are added to the index and the two repositories are cloned, so i just have to do a commit when ready.

And with my patch the two submodule add lines read:
   $ git submodule add git://mygitserver/submodule1.git
   $ git submodule add git://mygitserver/submodule2.git

Which then resembles the command i would use when i want to clone them on their own:

   $ git clone git://mygitserver/submodule1.git
Show 11 quoted lines
>> So I would actually vote for making the <repository> parameter optional...
> 
> In your "git submodule add submodule1", it would be quite clear that it is
> a local path and <repository> is being omitted.  On the other hand, if you
> said "git submodule add $overthere" without submodule1, because $overthere
> is not likely to be an in-tree path, it also would be clear that it is
> omitting the path.  IOW, these two typesavers are not mutually exclusive.
> 
> In real life, it is very likely $overthere does _not_ end with submodule1,
> and your suggestion would probably be more useful than the patch under
> discussion, I think.

Actually, for me $overthere is _always_ a treeish path (e.g. ends with submodule1 or submodule1.git). And almost always i have no reason to name the local directory different than the repo.

Jens

← back to recent threads