[PATCH 0/2] Tests for some submodule corner cases.

[PATCH 0/2] Tests for some submodule corner cases.

[Marc Branchaud]

Ran across some submodule behavior that seems wrong to me.  I don't have the
chops to fix the issues, so I thought I'd just point them out with some unit
tests.

Patch 1 tests the case where "submodule add" fails if the path to the
submodule repo is relative (i.e. starts with "../").  This currently fails
with "remote (origin) does not have a url defined in .git/config".  Maybe
there's a reason to fail?  If so, a better error message would be appreciated.

Patch 2 exposes an anomaly in "submodule status", which reports that a
submodule is OK even though it has deleted files.  "git status" inside
the submodule (and in the super-repo) both identify any deleted files, but
"submodule status" doesn't prefix the submodule's HEAD SHA-ID with a "+".

(ps. I didn't sign-off these two patches, since they're just failing unit
tests.  Should I sign-off on them?)

		M.

[PATCH 1/2] Added a test for "submodule add" using a ../relative/path/to/the/submodule/repo

[Marc Branchaud]

---
 t/t7400-submodule-basic.sh |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index b2b26b7..bbcfc61 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -86,6 +86,13 @@ test_expect_success 'submodule add' '
 	test_cmp empty untracked
 '
 
+test_expect_failure 'submodule add with relative path to submodule repo' '
+	(
+		cd addtest &&
+		git submodule add ../.subrepo relative_subrepo_path
+	)
+'
+
 test_expect_success 'submodule add to .gitignored path fails' '
 	(
 		cd addtest-ignore &&
-- 
1.7.5.3.686.g35b8

[PATCH 2/2] Added a test for "submodule status" when the submodule's working directory has deleted files.

[Marc Branchaud]

---
 t/t7400-submodule-basic.sh |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index bbcfc61..d321540 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -331,6 +331,12 @@ test_expect_success 'status should be "up-to-date" after update' '
 	grep "^ $rev1" list
 '
 
+test_expect_failure 'status should be "modified" if working dir deleted' '
+	rm -rf init/* &&
+	git submodule status >list &&
+	grep "^+$rev1" list
+'
+
 test_expect_success 'checkout superproject with subproject already present' '
 	git checkout initial &&
 	git checkout master
-- 
1.7.5.3.686.g35b8

Re: [PATCH 0/2] Tests for some submodule corner cases.

[Jens Lehmann]

Am 30.05.2011 23:51, schrieb Marc Branchaud:
> Ran across some submodule behavior that seems wrong to me.  I don't have the
> chops to fix the issues, so I thought I'd just point them out with some unit
> tests.

Thanks for bringing these issues to our attention this way, having a way
to easily reproduce them is very much appreciated.

> Patch 1 tests the case where "submodule add" fails if the path to the
> submodule repo is relative (i.e. starts with "../").  This currently fails
> with "remote (origin) does not have a url defined in .git/config".  Maybe
> there's a reason to fail?  If so, a better error message would be appreciated.

I stumbled across this behavior now and then too, but according to the
commit it added (f31a522a2d) it is intended that adding a relative path
behaves differently than using an absolute path (it resolves relative to
the superproject's origin, not the filesystem, and to be able to do that
the superproject's .git/config has to have an url defined for it). But
you are right about the error message, it really isn't that helpful ...

> Patch 2 exposes an anomaly in "submodule status", which reports that a
> submodule is OK even though it has deleted files.  "git status" inside
> the submodule (and in the super-repo) both identify any deleted files, but
> "submodule status" doesn't prefix the submodule's HEAD SHA-ID with a "+".

That is documented behavior. "git submodule status" only cares about the
commit recorded in the superproject vs the HEAD in the submodule, work
tree modifications are never shown by it.

But try a "git status" in the superproject, that will give you the following
output:
#	modified:   init (modified content)

[PATCH] submodule add: improve message when resolving a relative url fails

[Jens Lehmann]

A "git submodule add ../sub" interprets "../sub" relative to the default
remote of the superproject. To be able to do that, a url for that remote
has to be set in the superprojects .git/config. If that is not the case
the command fails with:
	"remote (origin) does not have a url defined in .git/config"

This neither mentions the relative repository nor that the .git/config of
the superproject is the one with the missing url. And as a novice user
could assume that relative paths would work just like absolute paths do
in the filesystem and run into this by accident, the message is not very
helpful.

So change that to
	"Cannot resolve "../sub" relative to remote (origin), its url
	 is not set in .git/config"
to give the user a clue that "git submodule add" interprets a relative
path as being relative to its default remote, not the work tree.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Am 31.05.2011 21:30, schrieb Jens Lehmann:
> Am 30.05.2011 23:51, schrieb Marc Branchaud:
>> Patch 1 tests the case where "submodule add" fails if the path to the
>> submodule repo is relative (i.e. starts with "../").  This currently fails
>> with "remote (origin) does not have a url defined in .git/config".  Maybe
>> there's a reason to fail?  If so, a better error message would be appreciated.
> 
> I stumbled across this behavior now and then too, but according to the
> commit it added (f31a522a2d) it is intended that adding a relative path
> behaves differently than using an absolute path (it resolves relative to
> the superproject's origin, not the filesystem, and to be able to do that
> the superproject's .git/config has to have an url defined for it). But
> you are right about the error message, it really isn't that helpful ...

What about this patch?


 git-submodule.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index d189a24..14ef1d4 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -34,7 +34,7 @@ resolve_relative_url ()
 {
 	remote=$(get_default_remote)
 	remoteurl=$(git config "remote.$remote.url") ||
-		die "remote ($remote) does not have a url defined in .git/config"
+		die "Cannot resolve \"$1\" relative to remote ($remote), its url is not set in .git/config"
 	url="$1"
 	remoteurl=${remoteurl%/}
 	sep=/
-- 
1.7.5.3.412.gb2afd.dirty

Re: [PATCH] submodule add: improve message when resolving a relative url fails

[Marc Branchaud]

On 11-05-31 04:00 PM, Jens Lehmann wrote:
> A "git submodule add ../sub" interprets "../sub" relative to the default
> remote of the superproject. To be able to do that, a url for that remote
> has to be set in the superprojects .git/config. If that is not the case

Nit: superprojects --> superproject's

> the command fails with:
> 	"remote (origin) does not have a url defined in .git/config"
> 
> This neither mentions the relative repository nor that the .git/config of
> the superproject is the one with the missing url. And as a novice user
> could assume that relative paths would work just like absolute paths do
> in the filesystem and run into this by accident, the message is not very
> helpful.
> 
> So change that to
> 	"Cannot resolve "../sub" relative to remote (origin), its url
> 	 is not set in .git/config"
> to give the user a clue that "git submodule add" interprets a relative
> path as being relative to its default remote, not the work tree.

Thanks for the cogent explanation & patch.  I think the message could be
improved a bit:

	Cannot resolve "../sub" relative to this repository's "origin"
	remote: The remote's URL is not set in .git/config

However, overall I think this is a pretty fragile way to handle relative
paths.  Consider:

 - The super-repo must be a clone in order for this to work at all.

 - The super-repo cannot be checked out on a detached HEAD.

 - The current code rewrites the URL so that any relative path is either
   rejected or munged into an absolute remote URL.

It seems to me that this feature will only work in a fairly narrow set of
circumstances, and even when it does work it's likely to do something
unexpected (think of a super-repo with several remotes).

Back when Junio accepted the original patch, he said "If you maintain and
serve a set related projects you need to give the users a single URL (per
where the user is and how to reach the server)."  I'm not sure I understand
that:  Why would the users be adding their own submodules to the
superproject?  Wouldn't the superproject define the submodules in for them?

I think it would be better to either just reject relative paths entirely, or
accept any relative path as-is and display a warning that the submodule is
only valid on the local machine.  (Perhaps one day receive-pack could even be
taught to reject any pushes with a .gitmodules file containing a relative URL.)

		M.


> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
> ---
> 
> Am 31.05.2011 21:30, schrieb Jens Lehmann:
>> Am 30.05.2011 23:51, schrieb Marc Branchaud:
>>> Patch 1 tests the case where "submodule add" fails if the path to the
>>> submodule repo is relative (i.e. starts with "../").  This currently fails
>>> with "remote (origin) does not have a url defined in .git/config".  Maybe
>>> there's a reason to fail?  If so, a better error message would be appreciated.
>>
>> I stumbled across this behavior now and then too, but according to the
>> commit it added (f31a522a2d) it is intended that adding a relative path
>> behaves differently than using an absolute path (it resolves relative to
>> the superproject's origin, not the filesystem, and to be able to do that
>> the superproject's .git/config has to have an url defined for it). But
>> you are right about the error message, it really isn't that helpful ...
> 
> What about this patch?
> 
> 
>  git-submodule.sh |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/git-submodule.sh b/git-submodule.sh
> index d189a24..14ef1d4 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -34,7 +34,7 @@ resolve_relative_url ()
>  {
>  	remote=$(get_default_remote)
>  	remoteurl=$(git config "remote.$remote.url") ||
> -		die "remote ($remote) does not have a url defined in .git/config"
> +		die "Cannot resolve \"$1\" relative to remote ($remote), its url is not set in .git/config"
>  	url="$1"
>  	remoteurl=${remoteurl%/}
>  	sep=/

[PATCH v2] submodule add: improve message when resolving a relative url fails

[Jens Lehmann]

A "git submodule add ../sub" interprets "../sub" relative to the default
remote of the superproject. To be able to do that, a url for that remote
has to be set in the superproject's .git/config. If that is not the case
the command fails with:
	"remote (origin) does not have a url defined in .git/config"

This neither mentions the relative repository nor that the .git/config of
the superproject is the one with the missing url. And as a novice user
could assume that relative paths would work just like absolute paths do
in the filesystem and run into this by accident, the message is not very
helpful.

So change that to
	Cannot resolve "../sub" relative to this repository's "origin"
	remote: The remote's URL is not set in .git/config
to give the user a clue that "git submodule add" interprets a relative
path as being relative to its default remote, not the work tree.

Thanks-to: Marc Branchaud <marcnarc@xiplink.com>
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---

Thanks for your review, here is the updated patch.

Am 31.05.2011 22:57, schrieb Marc Branchaud:
> However, overall I think this is a pretty fragile way to handle relative
> paths.  Consider:
> 
>  - The super-repo must be a clone in order for this to work at all.
> 
>  - The super-repo cannot be checked out on a detached HEAD.
> 
>  - The current code rewrites the URL so that any relative path is either
>    rejected or munged into an absolute remote URL.
> 
> It seems to me that this feature will only work in a fairly narrow set of
> circumstances, and even when it does work it's likely to do something
> unexpected (think of a super-repo with several remotes).

And even worse: it defies the principle of least surprise when I can't
just replace an absolute filesystem path with a relative one just like
almost everywhere else ... an option enabling this behavior might have
been a better way in hindsight.

> Back when Junio accepted the original patch, he said "If you maintain and
> serve a set related projects you need to give the users a single URL (per
> where the user is and how to reach the server)."  I'm not sure I understand
> that:  Why would the users be adding their own submodules to the
> superproject?  Wouldn't the superproject define the submodules in for them?

I can't tell about that reasoning, but it might make sense in a way I don't
understand yet ...

> I think it would be better to either just reject relative paths entirely, or
> accept any relative path as-is and display a warning that the submodule is
> only valid on the local machine.  (Perhaps one day receive-pack could even be
> taught to reject any pushes with a .gitmodules file containing a relative URL.)

Breaking backwards compatibility should not be considered easily. I tend to
not change such things until I understand the use cases for the solution,
especially as submodule use cases tend to be very different ...


 git-submodule.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index d189a24..587d74e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -34,7 +34,7 @@ resolve_relative_url ()
 {
 	remote=$(get_default_remote)
 	remoteurl=$(git config "remote.$remote.url") ||
-		die "remote ($remote) does not have a url defined in .git/config"
+		die "Cannot resolve \"$1\" relative to this repository's \"$remote\" remote: The remote's URL is not set in .git/config"
 	url="$1"
 	remoteurl=${remoteurl%/}
 	sep=/
-- 
1.7.5.3.445.g9c0c23

Re: [PATCH] submodule add: improve message when resolving a relative url fails

[Phil Hord]

On 05/31/2011 04:57 PM, Marc Branchaud wrote:
> Thanks for the cogent explanation & patch.  I think the message could be
> improved a bit:
>
> 	Cannot resolve "../sub" relative to this repository's "origin"
> 	remote: The remote's URL is not set in .git/config
>
> However, overall I think this is a pretty fragile way to handle relative
> paths.  Consider:
>
>  - The super-repo must be a clone in order for this to work at all.

Yes, but that constraint (mostly) makes sense to me.  But if 'git
submodule add' did not initialize .git/config, this constraint could be
dropped.

>  - The super-repo cannot be checked out on a detached HEAD.

Why do you think that?  I just tried this and it worked fine for me.  I
can't think of a reason for it to fail.

>  - The current code rewrites the URL so that any relative path is either
>    rejected or munged into an absolute remote URL.

I don't see the URL getting munged away from being relative.  Can you
point to an example?

> It seems to me that this feature will only work in a fairly narrow set of
> circumstances, and even when it does work it's likely to do something
> unexpected (think of a super-repo with several remotes).

I use it this way with several remotes. 

> Back when Junio accepted the original patch, he said "If you maintain and
> serve a set related projects you need to give the users a single URL (per
> where the user is and how to reach the server)."  I'm not sure I understand
> that:  Why would the users be adding their own submodules to the
> superproject?  Wouldn't the superproject define the submodules in for them?
I am a user.  I admin a super-project for other users.  This project
lives at three remotes, remotes/public, remotes/shared and remotes/build. 

I add a new submodule to the superproject like this:

   mkdir sub && cd sub && git init
   cd ..
   git submodule add ../sub sub

This results in the new submodule being inserted into my .gitmodules
file and my .git/config:

   tail -3 .gitmodules
   [submodule "sub"]
       path = sub
       url = ../sub

   tail -2 .git/config
   [submodule "sub"]
       url = public:git/sub

I do have to make sure to push my submodule to the correct location on
each remote before pushing my new .gitmodules.

But the exact same commands work for me if I do this first and then do
'git submodule add ../sub' afterwards. 

So, I don't understand your objections.  Do you understand my use case
any better?

Phil

Re: [PATCH] submodule add: improve message when resolving a relative url fails

[Marc Branchaud]

On 11-05-31 06:04 PM, Phil Hord wrote:
> On 05/31/2011 04:57 PM, Marc Branchaud wrote:
>> Thanks for the cogent explanation & patch.  I think the message could be
>> improved a bit:
>>
>> 	Cannot resolve "../sub" relative to this repository's "origin"
>> 	remote: The remote's URL is not set in .git/config
>>
>> However, overall I think this is a pretty fragile way to handle relative
>> paths.  Consider:
>>
>>  - The super-repo must be a clone in order for this to work at all.
> 
> Yes, but that constraint (mostly) makes sense to me.  But if 'git
> submodule add' did not initialize .git/config, this constraint could be
> dropped.
> 
>>  - The super-repo cannot be checked out on a detached HEAD.
> 
> Why do you think that?  I just tried this and it worked fine for me.  I
> can't think of a reason for it to fail.

Whoops, right.  I was confusing a different error with the fact that "git
symbolic-ref HEAD" fails on a detached HEAD.  The code defaults to "origin"
as the remote name in this case (perhaps that's not strictly the right thing
to do, but I'm sure this isn't the only part of git that assumes there's a
remote called "origin").

>>  - The current code rewrites the URL so that any relative path is either
>>    rejected or munged into an absolute remote URL.
> 
> I don't see the URL getting munged away from being relative.  Can you
> point to an example?

I reached this conclusion because if I go into my clone of git.git and do

	git submodule add ../MyThing

where ../MyThing is a regular git repo, I get

Cloning into MyThing...
fatal: The remote end hung up unexpectedly
Clone of 'git://git.kernel.org/pub/scm/git/MyThing' into submodule path
'MyThing' failed

So it seemed the relative URL became an absolute URL.

Looking more closely at a working example, I can see that (as you show below)
the URL in the super-repo's .gitmodules file retains the relative path, but
the submodule's remote.origin.url is an absolute path.

In any case, "submodule add" isn't doing what I expected: make my local
MyThing repo a submodule of my git.git clone.

>> It seems to me that this feature will only work in a fairly narrow set of
>> circumstances, and even when it does work it's likely to do something
>> unexpected (think of a super-repo with several remotes).
> 
> I use it this way with several remotes. 
> 
>> Back when Junio accepted the original patch, he said "If you maintain and
>> serve a set related projects you need to give the users a single URL (per
>> where the user is and how to reach the server)."  I'm not sure I understand
>> that:  Why would the users be adding their own submodules to the
>> superproject?  Wouldn't the superproject define the submodules in for them?
> I am a user.  I admin a super-project for other users.  This project
> lives at three remotes, remotes/public, remotes/shared and remotes/build. 
> 
> I add a new submodule to the superproject like this:
> 
>    mkdir sub && cd sub && git init
>    cd ..
>    git submodule add ../sub sub
> 
> This results in the new submodule being inserted into my .gitmodules
> file and my .git/config:
> 
>    tail -3 .gitmodules
>    [submodule "sub"]
>        path = sub
>        url = ../sub
> 
>    tail -2 .git/config
>    [submodule "sub"]
>        url = public:git/sub
> 
> I do have to make sure to push my submodule to the correct location on
> each remote before pushing my new .gitmodules.
> 
> But the exact same commands work for me if I do this first and then do
> 'git submodule add ../sub' afterwards. 
> 
> So, I don't understand your objections.  Do you understand my use case
> any better?

It's not so much an objection as confusion over how "submodule add" works.

I believe your case works smoothly only because in your super-project you're
careful to make sure you have checked out a branch that remotely tracks a
something in remotes/public.  If you checked out a branch that tracks a
different remote you'd get different results.  This seems fragile to me.

When you tried the detached-HEAD scenario, did you get URLs for
"public:git/sub" or "origin:git/sub"?  Does "origin" just happen to be the
remote you want to use in any case?

My fundamental point is that "git submodule add" seems to do confusing things
with relative paths.  Maybe all that's needed is to clarify the
documentation.  I'll post a patch.

		M.

Re: [PATCH] submodule add: improve message when resolving a relative url fails

[Phil Hord]

On 06/01/2011 11:55 AM, Marc Branchaud wrote:
> On 11-05-31 06:04 PM, Phil Hord wrote:
>> On 05/31/2011 04:57 PM, Marc Branchaud wrote:
>>>  - The current code rewrites the URL so that any relative path is either
>>>    rejected or munged into an absolute remote URL.
>> I don't see the URL getting munged away from being relative.  Can you
>> point to an example?
> I reached this conclusion because if I go into my clone of git.git and do
>
> 	git submodule add ../MyThing
>
> where ../MyThing is a regular git repo, I get
>
> Cloning into MyThing...
> fatal: The remote end hung up unexpectedly
> Clone of 'git://git.kernel.org/pub/scm/git/MyThing' into submodule path
> 'MyThing' failed
>
> So it seemed the relative URL became an absolute URL.
>
> Looking more closely at a working example, I can see that (as you show below)
> the URL in the super-repo's .gitmodules file retains the relative path, but
> the submodule's remote.origin.url is an absolute path.
>
> In any case, "submodule add" isn't doing what I expected: make my local
> MyThing repo a submodule of my git.git clone.

I thought I understood this workflow better than I actually did.  I
think I understand more now, and I'm somewhat disappointed.  But I also
failed to pick up the ball on this old discussion.

If you do this, I think it will work like you were hoping:

    :: ( mkdir MyThing && cd MyThing && git init )
    Initialized empty Git repository in /opc/git/MyThing/.git/
    :: git submodule add ../MyThing
    Adding existing repo at 'MyThing' to the index

I haven't examined the code, but I think this is how it works.  'git
submodule add' takes a URL and a local path.  When you omit the local
path, git infers one from the URL.  So these two commands are equivalent:
    :: git submodule add ../MyThing
    :: git submodule add ../MyThing MyThing


If the path you provide (explicitly or implicitly) already contains a
git repo, it is assumed to be "the" submodule repo and git uses it.  If
it does not contain a git repo, git attempts to clone it from the
(remote) URL.

Furthermore, the relative path only works for URLs.  It does not work
for local filesystems.

Phil

Re: [PATCH] submodule add: improve message when resolving a relative url fails

[Marc Branchaud]

On 11-07-27 03:00 PM, Phil Hord wrote:
> On 06/01/2011 11:55 AM, Marc Branchaud wrote:
>> On 11-05-31 06:04 PM, Phil Hord wrote:
>>> On 05/31/2011 04:57 PM, Marc Branchaud wrote:
>>>>  - The current code rewrites the URL so that any relative path is either
>>>>    rejected or munged into an absolute remote URL.
>>> I don't see the URL getting munged away from being relative.  Can you
>>> point to an example?
>> I reached this conclusion because if I go into my clone of git.git and do
>>
>> 	git submodule add ../MyThing
>>
>> where ../MyThing is a regular git repo, I get
>>
>> Cloning into MyThing...
>> fatal: The remote end hung up unexpectedly
>> Clone of 'git://git.kernel.org/pub/scm/git/MyThing' into submodule path
>> 'MyThing' failed
>>
>> So it seemed the relative URL became an absolute URL.
>>
>> Looking more closely at a working example, I can see that (as you show below)
>> the URL in the super-repo's .gitmodules file retains the relative path, but
>> the submodule's remote.origin.url is an absolute path.
>>
>> In any case, "submodule add" isn't doing what I expected: make my local
>> MyThing repo a submodule of my git.git clone.
> 
> I thought I understood this workflow better than I actually did.  I
> think I understand more now, and I'm somewhat disappointed.  But I also
> failed to pick up the ball on this old discussion.
> 
> If you do this, I think it will work like you were hoping:
> 
>     :: ( mkdir MyThing && cd MyThing && git init )
>     Initialized empty Git repository in /opc/git/MyThing/.git/
>     :: git submodule add ../MyThing
>     Adding existing repo at 'MyThing' to the index

I see how that works, but I don't find that intuitive at all.  For one,
../MyThing doesn't even exist, neither locally or on the origin repo.  It's
really ./MyThing (with one dot).  The last idea that I'd come up with for
adding ./MyThing as a submodule would be to use ../MyThing.

What git does in this case actually looks like a bug to me.  I'd expect "git
submodule add ../MyThing" to fail if there's no local ../MyThing and no
remote ../MyThing.

> Furthermore, the relative path only works for URLs.  It does not work
> for local filesystems.

Well, sort of.  From the current man page: "If the superproject doesn't have
an origin configured the superproject is its own authoritative upstream and
the current working directory is used instead."

I submitted two patches to clarify the documentation on that point:

	http://article.gmane.org/gmane.comp.version-control.git/175163
	http://article.gmane.org/gmane.comp.version-control.git/175162

They seemed to have been lost in the shuffle, though.

		M.

Re: [PATCH] submodule add: improve message when resolving a relative url fails

[Junio C Hamano]

Marc Branchaud <marcnarc@xiplink.com> writes:

> Back when Junio accepted the original patch, he said "If you maintain and
> serve a set related projects you need to give the users a single URL (per
> where the user is and how to reach the server)."

I think it was phrased badly. At least it should have s/need/only need/;

Imagine ta project has many components, all of which are kept as
submodules of a single top-level superproject. You wrote and manage
everything; there is no borrowed code. In that context, imagine that I am
talking to the maintainer of that set of projects and calling the person
"you".

By giving the URL for the top-level superproject, without having to give
any other URL for the subprojects, you can let your users fetch from you,
as everything underneath is relative. Another convenience this may give
you and your users is when the user needs to talk to you over different
transport. You may give "git://your.site/project.git" to the users, but
they may come to "http://your.site/project.git".

By recording submodule.<path>.url as relative to where your users happen
to have fetched your project in the superproject's .gitmodules file, your
users do not have to run around fixing URLs for 47 different component
submodules.

At least I think that is what I meant back then.

[PATCH] Clarified how "git submodule add" handles relative paths.

[Marc Branchaud]

Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>
---
 Documentation/git-submodule.txt |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

On 11-05-31 07:23 PM, Junio C Hamano wrote:
> Marc Branchaud <marcnarc@xiplink.com> writes:
> 
>> Back when Junio accepted the original patch, he said "If you maintain and
>> serve a set related projects you need to give the users a single URL (per
>> where the user is and how to reach the server)."
> 
> I think it was phrased badly. At least it should have s/need/only need/;
> 
> Imagine ta project has many components, all of which are kept as
> submodules of a single top-level superproject. You wrote and manage
> everything; there is no borrowed code. In that context, imagine that I am
> talking to the maintainer of that set of projects and calling the person
> "you".
> 
> By giving the URL for the top-level superproject, without having to give
> any other URL for the subprojects, you can let your users fetch from you,
> as everything underneath is relative. Another convenience this may give
> you and your users is when the user needs to talk to you over different
> transport. You may give "git://your.site/project.git" to the users, but
> they may come to "http://your.site/project.git".
> 
> By recording submodule.<path>.url as relative to where your users happen
> to have fetched your project in the superproject's .gitmodules file, your
> users do not have to run around fixing URLs for 47 different component
> submodules.
> 
> At least I think that is what I meant back then.

I see -- I was confused by the phrase "give ... a single URL" (since the
URLs are already in the .gitmodules file, how is there ever any need to
give more than one URL?).

So this is really about saving the users the hassle of modifying all the
URLs in the .gitmodules file.  Does this patch document what you mean?

		M.


diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 1a16ff6..54dfebb 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -77,8 +77,17 @@ to exist in the superproject. If <path> is not given, the
 +
 <repository> is the URL of the new submodule's origin repository.
 This may be either an absolute URL, or (if it begins with ./
-or ../), the location relative to the superproject's origin
-repository.
+or ../) a URL relative to one of the superproject's remote
+repostories:  If the superprojet's currently checked-out branch tracks
+a remote branch then that remote's URL is used, otherwise the "origin"
+remote's URL is used.  Relative URLs allow users to easily clone the
+superproject and its submodules using a different URL than what the
+superproject's maintainer might use (e.g. the maintainer can use ssh://
+URLs while the users might use git:// URLs).  With relative URLs in the
+.gitmodules file, the users won't have to edit all the submodule URLs.
++
+*NOTE*: This means that you can *not* use a relative path to refer to a
+repository in your local filesystem.
 +
 <path> is the relative location for the cloned submodule to
 exist in the superproject. If <path> does not exist, then the
-- 
1.7.5.3.1.ge85f0

Re: [PATCH] Clarified how "git submodule add" handles relative paths.

[Junio C Hamano]

Marc Branchaud <marcnarc@xiplink.com> writes:

> So this is really about saving the users the hassle of modifying all the
> URLs in the .gitmodules file.  Does this patch document what you mean?

I do not necessarily think it is _solely_ about users. Another obvious
example I left unsaid was what would happen when your project gets popular
and gets mirrored to many places. Surely you need to advise people of
alternate locations of the top-level, but what is recorded in .gitmodules
will be relative to that top-level, so they do not have to be changed.
Even if you have only one public repository, I would imagine that the same
convenience would apply if you ever decide to switch the project hosting
site.

"Users can use a URL different from what was given by the maintainer" is
merely an example; I doubt it deserves stressing that much.

> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> index 1a16ff6..54dfebb 100644
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -77,8 +77,17 @@ to exist in the superproject. If <path> is not given, the
>  +
>  <repository> is the URL of the new submodule's origin repository.
>  This may be either an absolute URL, or (if it begins with ./
> -or ../), the location relative to the superproject's origin
> -repository.
> +or ../) a URL relative to one of the superproject's remote
> +repostories:  If the superprojet's currently checked-out branch tracks
> +a remote branch then that remote's URL is used, otherwise the "origin"
> +remote's URL is used.  Relative URLs allow users to easily clone the
> +superproject and its submodules using a different URL than what the
> +superproject's maintainer might use (e.g. the maintainer can use ssh://
> +URLs while the users might use git:// URLs).  With relative URLs in the
> +.gitmodules file, the users won't have to edit all the submodule URLs.
> ++
> +*NOTE*: This means that you can *not* use a relative path to refer to a
> +repository in your local filesystem.

It is not "can not" but "do not", is it?  More importantly, I do not think
it is limited to the case of relative path at all.

I thought that in general "submodule add" takes a URL and never a _local_
filesystem location, as the whole point [*1*] of running "submodule add"
command is to update the entry in the .gitmodules file for people who may
not have access to _your_ local filesystem in the first place?

I am starting to suspect that the original confusion that started this
thread is coming from misunderstanding in this area.

[Footnote]

*1* The command also adds a corresponding entry to your own .git/config
file so that you can work as if you are just one of your users and as if
you did "submodule init" based on what is in .gitmodules.

Re: [PATCH] Clarified how "git submodule add" handles relative paths.

[Jens Lehmann]

Am 01.06.2011 18:59, schrieb Junio C Hamano:
> I thought that in general "submodule add" takes a URL and never a _local_
> filesystem location, as the whole point [*1*] of running "submodule add"
> command is to update the entry in the .gitmodules file for people who may
> not have access to _your_ local filesystem in the first place?

I often use a local filesystem location for shared code I'm using in some
personal projects, mainly because I want to avoid the hassle of setting
up a server location for it (and the git test suite uses that feature too
for similar reasons). That doesn't make much sense when working together
with others, but that is not an issue in these use cases.

So I see three different location types supported by current submodule
add:
  1) a URL reachable by you and your coworkers
  2) a path relative to the URL of the superproject's default remote
  3) A local filesystem location which can only be shared locally
And each of them has its merits and uses (and using two of them everyday
might make it easy to overlook the third ;-)

Re: [PATCH] Clarified how "git submodule add" handles relative paths.

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> I often use a local filesystem location for shared code I'm using in some
> personal projects, mainly because I want to avoid the hassle of setting
> up a server location for it (and the git test suite uses that feature too
> for similar reasons). That doesn't make much sense when working together
> with others, but that is not an issue in these use cases.
>
> So I see three different location types supported by current submodule
> add:
>   1) a URL reachable by you and your coworkers
>   2) a path relative to the URL of the superproject's default remote
>   3) A local filesystem location which can only be shared locally
> And each of them has its merits and uses (and using two of them everyday
> might make it easy to overlook the third ;-)

I suspect that it would be a relatively easy fix if your toplevel
superproject is its own authoritative upstream.  Something along the line
of this patch, perhaps?  It is obviously untested, and we may want to
issue an "echo >&2 'info:...'" to tell the user what we are assuming in
this codepath.

 git-submodule.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index b010a67..6d27729 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -34,7 +34,7 @@ resolve_relative_url ()
 {
 	remote=$(get_default_remote)
 	remoteurl=$(git config "remote.$remote.url") ||
-		die "remote ($remote) does not have a url defined in .git/config"
+	remoteurl=$(pwd) # the repository is its own authoritative upstream
 	url="$1"
 	remoteurl=${remoteurl%/}
 	sep=/

Re: [PATCH] Clarified how "git submodule add" handles relative paths.

[Jens Lehmann]

Am 02.06.2011 19:14, schrieb Junio C Hamano:
> I suspect that it would be a relatively easy fix if your toplevel
> superproject is its own authoritative upstream.  Something along the line
> of this patch, perhaps?  It is obviously untested, and we may want to
> issue an "echo >&2 'info:...'" to tell the user what we are assuming in
> this codepath.

Maybe it is better to not automagically switch from "path is relative to
url configured in superproject" to "path is relative to $(pwd)" depending
on the presence or absence of a default remote in the superproject. When
a user wants to set up his submodules relative to the superproject and
simply did forget to configure the url of the superproject first he won't
notice that anymore after this patch. But instead he will get a local
submodule url only to find out later that this was not what I wanted (and
an 'info' can easily be missed).

Now I understand this issue better I'd vote for leaving the relative url
like it is, comment it better in the man page and give a better error
message when that happens. After all this issue only surprised a few
people, mostly due to the lack of information in the error message and
man page, so I'd rather prefer to not change the behavior but the wording.

Re: [PATCH] Clarified how "git submodule add" handles relative paths.

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Now I understand this issue better I'd vote for leaving the relative url
> like it is, comment it better in the man page and give a better error
> message when that happens. After all this issue only surprised a few
> people, mostly due to the lack of information in the error message and
> man page, so I'd rather prefer to not change the behavior but the wording.

The "how about this" patch you are voting against (I am neutral by the
way) is a response to your earlier "I have three use cases and the current
implementation is forgetting the third", which in turn was a response to
my "your third use case does not count, so the updated wording of the
documentation is wrong---it should say 'do not', not 'cannot'".

So what should the updated document say?

Re: [PATCH] Clarified how "git submodule add" handles relative paths.

[Mark Levedahl]

On 06/03/2011 07:16 PM, Junio C Hamano wrote:
> Jens Lehmann<Jens.Lehmann@web.de>  writes:
> The "how about this" patch you are voting against (I am neutral by the
> way) is a response to your earlier "I have three use cases and the current
> implementation is forgetting the third", which in turn was a response to
> my "your third use case does not count, so the updated wording of the
> documentation is wrong---it should say 'do not', not 'cannot'".
>
> So what should the updated document say?
>
If I understand this correctly, the third use case isn't actually 
unique: if the upstream repo is on a local file system, why not just use 
a file://... url for the super project's origin?

Mark

Re: [PATCH] Clarified how "git submodule add" handles relative paths.

[Jens Lehmann]

Am 04.06.2011 04:23, schrieb Mark Levedahl:
> On 06/03/2011 07:16 PM, Junio C Hamano wrote:
>> Jens Lehmann<Jens.Lehmann@web.de>  writes:
>> The "how about this" patch you are voting against (I am neutral by the
>> way) is a response to your earlier "I have three use cases and the current
>> implementation is forgetting the third", which in turn was a response to
>> my "your third use case does not count, so the updated wording of the
>> documentation is wrong---it should say 'do not', not 'cannot'".
>>
>> So what should the updated document say?
>>
> If I understand this correctly, the third use case isn't actually unique: if the upstream repo is on a local file system, why not just use a file://... url for the super project's origin?

Right, if you use a "file:/" scheme these two cases become one. But you
can omit the "file:/" scheme part and it will still give the same result.
(So if the file scheme would be regarded as default the third case would
already be covered by the url case)

Re: [PATCH] Clarified how "git submodule add" handles relative paths.

[Jens Lehmann]

Am 04.06.2011 01:16, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> Now I understand this issue better I'd vote for leaving the relative url
>> like it is, comment it better in the man page and give a better error
>> message when that happens. After all this issue only surprised a few
>> people, mostly due to the lack of information in the error message and
>> man page, so I'd rather prefer to not change the behavior but the wording.
> 
> The "how about this" patch you are voting against (I am neutral by the
> way) is a response to your earlier "I have three use cases and the current
> implementation is forgetting the third", which in turn was a response to
> my "your third use case does not count, so the updated wording of the
> documentation is wrong---it should say 'do not', not 'cannot'".
> 
> So what should the updated document say?

I think that for the improved error message the v2 of my 'submodule add:
improve message when resolving a relative url fails' should be sufficient.

For the documentation I'd propose to apply Marc's 'Clarified how "git
submodule add" handles relative paths.' patch with the interdiff below
squashed in. It fixes a typo and explains that absolute paths are allowed
too (and join case three with the absolute url one by using a "file://"
scheme as default, which is what Mark hinted in his email).

If you want me to resend these two as an updated series just let me know.
---------- 8< ----------
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 2294fa6..99d0a83 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -76,9 +76,10 @@ to exist in the superproject. If <path> is not given, the
 "/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 ./
+This may be either an absolute URL (when the scheme part is not
+specified "file://" is assumed) or (if it begins with ./
 or ../) a URL relative to one of the superproject's remote
-repostories:  If the superprojet's currently checked-out branch tracks
+repostories:  If the superproject's currently checked-out branch tracks
 a remote branch then that remote's URL is used, otherwise the "origin"
 remote's URL is used.  Relative URLs allow users to easily clone the
 superproject and its submodules using a different URL than what the

Re: [PATCH] Clarified how "git submodule add" handles relative paths.

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Am 02.06.2011 19:14, schrieb Junio C Hamano:
>> I suspect that it would be a relatively easy fix if your toplevel
>> superproject is its own authoritative upstream.  Something along the line
>> of this patch, perhaps?  It is obviously untested, and we may want to
>> issue an "echo >&2 'info:...'" to tell the user what we are assuming in
>> this codepath.
>
> Maybe it is better to not automagically switch from "path is relative to
> url configured in superproject" to "path is relative to $(pwd)" depending
> on the presence or absence of a default remote in the superproject. When
> a user wants to set up his submodules relative to the superproject and
> simply did forget to configure the url of the superproject first he won't
> notice that anymore after this patch. But instead he will get a local
> submodule url only to find out later that this was not what I wanted (and
> an 'info' can easily be missed).

Sorry, I don't get this. The "how-about-this" patch was not about
"automagically switch depending on ...". Absense of the remote in the
superproject means that the project originates from here, iow, it is its
own "origin" (that is your third use case).

I think I understand the scenario you are worried about; let me illustrate
to make sure I got it right:

 1. You are starting your project that will have subproject locally. You do
    not have "origin" yet.

 2. You create a subproject "xyzzy", still locally, and add it with
    "submodule add ./xyzzy" with a relative URL.

 3. You will deploy your superproject and subproject at "git://host.xz/mine/"
    and "git://host.xz/mine/xyzzy", respectively.

 4. But because in step 2. your .git/config is already set up to point
    your local $(pwd)/xyzzy as the submodule location. This is not what
    you want and you may not notice it.

Is that the problem you are worried about? If so, I think you are solving
it in a wrong way.

By not allowing a relative path, in step 2. you would entice the user to
say "submodule add $(pwd)/xyzzy" (as there is no final upstream location
yet), no? If the project is going to be eventually published at a
different location, not just .git/config but .gitmodules also needs to be
updated as part of step 3. Isn't that going backwards?  If you allow the
user to say "./xyzzy" in step 2., the .gitmodules entry can stay the same
from the get-go.

If you think about "absense of the remote in the superproject means the
project originates from here", what you are doing in step 3. is to
changing the origin of these set of projects. After changing the origin of
these set of projects, isn't "git submodule sync" an established way to
adjust to the change? I was hoping that that would update .git/config in
step 3. so you wouldn't have the problem in step 4. at all.

It is likely I am missing something in the above analysis. Please correct
me.

Thanks.

[PATCH 0/3] submodule add: allow relative repository path even when no url is set

[Jens Lehmann]

Am 05.06.2011 20:27, schrieb Junio C Hamano:
> If you think about "absense of the remote in the superproject means the
> project originates from here", what you are doing in step 3. is to
> changing the origin of these set of projects. After changing the origin of
> these set of projects, isn't "git submodule sync" an established way to
> adjust to the change? I was hoping that that would update .git/config in
> step 3. so you wouldn't have the problem in step 4. at all.

Thanks for explaining that in detail, I think I do get it now.

So what about this series: The first commit adds a test for the error we
are talking about, the second one implements the logic you proposed and
the last one removes some duplicated code I stumbled across while staring
at the code.


Jens Lehmann (3):
  submodule add: test failure when url is not configured in
    superproject
  submodule add: allow relative repository path even when no url is set
  submodule add: clean up duplicated code

 Documentation/git-submodule.txt |    4 +++-
 git-submodule.sh                |   12 ++----------
 t/t7400-submodule-basic.sh      |   10 ++++++++++
 3 files changed, 15 insertions(+), 11 deletions(-)

-- 
1.7.6.rc0.3.g28a66

[PATCH 1/3] submodule add: test failure when url is not configured in superproject

[Jens Lehmann]

This documents the current behavior (submodule add with the url set in the
superproject is already tested in t7403, t7406, t7407 and t7506).

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 t/t7400-submodule-basic.sh |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 874279e..cae5fd0 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -446,6 +446,13 @@ test_expect_success 'add should fail when path is used by an existing directory'
 	)
 '

+test_expect_success 'add should fail when path is relative but no url is set in the superproject' '
+	(
+		cd addtest &&
+		test_must_fail git submodule add ../repo relative
+	)
+'
+
 test_expect_success 'set up for relative path tests' '
 	mkdir reltest &&
 	(
-- 
1.7.6.rc0.3.g28a66

[PATCH 2/3] submodule add: allow relative repository path even when no url is set

[Jens Lehmann]

Adding a submodule with a relative repository path did only succeed when
the superproject's default remote was set. But when that is unset, the
superproject is its own authoritative upstream, so lets use its working
directory as upstream instead.

This allows users to set up a new superpoject where the submodules urls
are configured relative to the superproject's upstream while its default
remote can be configured later.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 Documentation/git-submodule.txt |    4 +++-
 git-submodule.sh                |    2 +-
 t/t7400-submodule-basic.sh      |    7 +++++--
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 5e7a413..408998e 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -78,7 +78,9 @@ to exist in the superproject. If <path> is not given, the
 <repository> is the URL of the new submodule's origin repository.
 This may be either an absolute URL, or (if it begins with ./
 or ../), the location relative to the superproject's origin
-repository.
+repository. If the superproject doesn't have an origin configured
+the superproject is its own authoritative upstream and the current
+working directory is used instead.
 +
 <path> is the relative location for the cloned submodule to
 exist in the superproject. If <path> does not exist, then the
diff --git a/git-submodule.sh b/git-submodule.sh
index d189a24..3fbc21e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -34,7 +34,7 @@ resolve_relative_url ()
 {
 	remote=$(get_default_remote)
 	remoteurl=$(git config "remote.$remote.url") ||
-		die "remote ($remote) does not have a url defined in .git/config"
+		remoteurl=$(pwd) # the repository is its own authoritative upstream
 	url="$1"
 	remoteurl=${remoteurl%/}
 	sep=/
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index cae5fd0..9099e80 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -446,10 +446,13 @@ test_expect_success 'add should fail when path is used by an existing directory'
 	)
 '

-test_expect_success 'add should fail when path is relative but no url is set in the superproject' '
+test_expect_success 'use superproject as upstream when path is relative and no url is set there' '
 	(
 		cd addtest &&
-		test_must_fail git submodule add ../repo relative
+		git submodule add ../repo relative &&
+		test "$(git config -f .gitmodules submodule.relative.url)" = ../repo &&
+		git submodule sync relative &&
+		test "$(git config submodule.relative.url)" = "$submodurl/repo"
 	)
 '

-- 
1.7.6.rc0.3.g28a66

[PATCH 0/2] Improve "git submodule add" documentation.

[Marc Branchaud]

This series applies atop Jens's 2/3 patch.  Jens, please feel free to
squash these into your commit if you like.

The first commit makes the documentation (hopefully) more accurately
describe how git chooses which superproject remote to use.

The second commit moves the paragraph describing the utility of relative
submodule URLs right after their description, making it more likely for
readers to see it (instead of assuming it's part of the <path> parameter's
documentation -- as I did on previous occasions).

		M.

[PATCH 1/2] More precisely described how "git submodule add" handles relative submodule URLs.

[Marc Branchaud]

Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>
---
 Documentation/git-submodule.txt |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 408998e..83c59cc 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -77,8 +77,11 @@ to exist in the superproject. If <path> is not given, the
 +
 <repository> is the URL of the new submodule's origin repository.
 This may be either an absolute URL, or (if it begins with ./
-or ../), the location relative to the superproject's origin
-repository. If the superproject doesn't have an origin configured
+or ../) a location relative to one of the superproject's remote
+repositories:  If the superproject's currently checked-out branch tracks
+a remote branch then that remote's URL is used, otherwise the "origin"
+remote's URL is used.
+If the superproject doesn't have an "origin" remote configured
 the superproject is its own authoritative upstream and the current
 working directory is used instead.
 +
-- 
1.7.6.rc0.17.g3eac3

[PATCH 2/2] Moved paragraph describing the utility of relative submodule URLs.

[Marc Branchaud]

Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>
---
 Documentation/git-submodule.txt |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 83c59cc..ad67541 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -85,6 +85,14 @@ If the superproject doesn't have an "origin" remote configured
 the superproject is its own authoritative upstream and the current
 working directory is used instead.
 +
+In any case, the given URL is recorded into .gitmodules for
+use by subsequent users cloning the superproject. If the URL is
+given relative to the superproject's repository, the presumption
+is the superproject and submodule repositories will be kept
+together in the same relative location, and only the
+superproject's URL needs to be provided: git-submodule will correctly
+locate the submodule using the relative URL in .gitmodules.
++
 <path> is the relative location for the cloned submodule to
 exist in the superproject. If <path> does not exist, then the
 submodule is created by cloning from the named URL. If <path> does
@@ -92,14 +100,6 @@ exist and is already a valid git repository, then this is added
 to the changeset without cloning. This second form is provided
 to ease creating a new submodule from scratch, and presumes
 the user will later push the submodule to the given URL.
-+
-In either case, the given URL is recorded into .gitmodules for
-use by subsequent users cloning the superproject. If the URL is
-given relative to the superproject's repository, the presumption
-is the superproject and submodule repositories will be kept
-together in the same relative location, and only the
-superproject's URL needs to be provided: git-submodule will correctly
-locate the submodule using the relative URL in .gitmodules.
 
 status::
 	Show the status of the submodules. This will print the SHA-1 of the
-- 
1.7.6.rc0.17.g3eac3

[PATCH 3/3] submodule add: clean up duplicated code

[Jens Lehmann]

In cmd_add() the switch statement used to resolve a relative url was
present twice. Remove the second one and use the realrepo variable set
by the first one (lines 194 ff.) instead.

Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
 git-submodule.sh |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 3fbc21e..2a727e1 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -238,15 +238,7 @@ cmd_add()
 			die "'$path' already exists and is not a valid git repo"
 		fi

-		case "$repo" in
-		./*|../*)
-			url=$(resolve_relative_url "$repo") || exit
-		    ;;
-		*)
-			url="$repo"
-			;;
-		esac
-		git config submodule."$path".url "$url"
+		git config submodule."$path".url "$realrepo"
 	else

 		module_clone "$path" "$realrepo" "$reference" || exit
-- 
1.7.6.rc0.3.g28a66

Re: [PATCH 0/3] submodule add: allow relative repository path even when no url is set

[Junio C Hamano]

Jens Lehmann <Jens.Lehmann@web.de> writes:

> Am 05.06.2011 20:27, schrieb Junio C Hamano:
>> If you think about "absense of the remote in the superproject means the
>> project originates from here", what you are doing in step 3. is to
>> changing the origin of these set of projects. After changing the origin of
>> these set of projects, isn't "git submodule sync" an established way to
>> adjust to the change? I was hoping that that would update .git/config in
>> step 3. so you wouldn't have the problem in step 4. at all.
>
> Thanks for explaining that in detail, I think I do get it now.

I actually still have a feeling that I may be missing something from the
discussion.  While I do like a solution that lifts existing limitation to
allow workflows that were hitherto impossible, that only makes sense when
the newly allowed workflow makes sense and useful, and when the lifted
limitation was not protecting some silly mistakes from getting made.

I _think_ our last exchange gave me a fuzzy confirmation that we are not
lifting a useful limitation, but I still do not know if the new workflow
matches the workflow Marc (who kicked off this thread) wanted to use. I
think it does match the set-up Phil Hord mentioned in an earlier message,
though.

Re: [PATCH 0/3] submodule add: allow relative repository path even when no url is set

[Marc Branchaud]

On 11-06-06 05:00 PM, Junio C Hamano wrote:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> Am 05.06.2011 20:27, schrieb Junio C Hamano:
>>> If you think about "absense of the remote in the superproject means the
>>> project originates from here", what you are doing in step 3. is to
>>> changing the origin of these set of projects. After changing the origin of
>>> these set of projects, isn't "git submodule sync" an established way to
>>> adjust to the change? I was hoping that that would update .git/config in
>>> step 3. so you wouldn't have the problem in step 4. at all.
>>
>> Thanks for explaining that in detail, I think I do get it now.
> 
> I actually still have a feeling that I may be missing something from the
> discussion.  While I do like a solution that lifts existing limitation to
> allow workflows that were hitherto impossible, that only makes sense when
> the newly allowed workflow makes sense and useful, and when the lifted
> limitation was not protecting some silly mistakes from getting made.
> 
> I _think_ our last exchange gave me a fuzzy confirmation that we are not
> lifting a useful limitation, but I still do not know if the new workflow
> matches the workflow Marc (who kicked off this thread) wanted to use. I
> think it does match the set-up Phil Hord mentioned in an earlier message,
> though.

Well, Jens's changes do remove the error I encountered, and they also do what
I was expecting in the original context I was in when I started this thread.
 So I think this is a definite improvement.

There may still be a lingering niggle where git might do something the user
doesn't expect.  For example, git might create a submodule out of
git://origin/foo.git instead of the local ../foo.git.  You have to be paying
attention to git's output to notice that difference, and I could see where a
user might get tripped up.  But IMO improving this can be done independently
of Jens's patches.

		M.

Re: [PATCH 0/3] submodule add: allow relative repository path even when no url is set

[Jens Lehmann]

Am 06.06.2011 23:23, schrieb Marc Branchaud:
> On 11-06-06 05:00 PM, Junio C Hamano wrote:
>> Jens Lehmann <Jens.Lehmann@web.de> writes:
>>
>>> Am 05.06.2011 20:27, schrieb Junio C Hamano:
>>>> If you think about "absense of the remote in the superproject means the
>>>> project originates from here", what you are doing in step 3. is to
>>>> changing the origin of these set of projects. After changing the origin of
>>>> these set of projects, isn't "git submodule sync" an established way to
>>>> adjust to the change? I was hoping that that would update .git/config in
>>>> step 3. so you wouldn't have the problem in step 4. at all.
>>>
>>> Thanks for explaining that in detail, I think I do get it now.
>>
>> I actually still have a feeling that I may be missing something from the
>> discussion.  While I do like a solution that lifts existing limitation to
>> allow workflows that were hitherto impossible, that only makes sense when
>> the newly allowed workflow makes sense and useful, and when the lifted
>> limitation was not protecting some silly mistakes from getting made.

That's why I started with an improved error message and documentation ;-)

>> I _think_ our last exchange gave me a fuzzy confirmation that we are not
>> lifting a useful limitation, but I still do not know if the new workflow
>> matches the workflow Marc (who kicked off this thread) wanted to use. I
>> think it does match the set-up Phil Hord mentioned in an earlier message,
>> though.
> 
> Well, Jens's changes do remove the error I encountered, and they also do what
> I was expecting in the original context I was in when I started this thread.
>  So I think this is a definite improvement.

Thanks.

> There may still be a lingering niggle where git might do something the user
> doesn't expect.  For example, git might create a submodule out of
> git://origin/foo.git instead of the local ../foo.git.  You have to be paying
> attention to git's output to notice that difference, and I could see where a
> user might get tripped up.  But IMO improving this can be done independently
> of Jens's patches.

Maybe Phil's opinion could be helpful here as he seems to be a heavy user of
relative submodule urls.

Re: [PATCH 0/3] submodule add: allow relative repository path even when no url is set

[Jens Lehmann]

Am 06.06.2011 23:23, schrieb Marc Branchaud:
> On 11-06-06 05:00 PM, Junio C Hamano wrote:
>> I actually still have a feeling that I may be missing something from the
>> discussion.  While I do like a solution that lifts existing limitation to
>> allow workflows that were hitherto impossible, that only makes sense when
>> the newly allowed workflow makes sense and useful, and when the lifted
>> limitation was not protecting some silly mistakes from getting made.
>>
>> I _think_ our last exchange gave me a fuzzy confirmation that we are not
>> lifting a useful limitation, but I still do not know if the new workflow
>> matches the workflow Marc (who kicked off this thread) wanted to use. I
>> think it does match the set-up Phil Hord mentioned in an earlier message,
>> though.

After thinking about this issue some more I think the change is good. We
only affect relative urls and obviously don't change the case where the
url is already set in the superproject (that case stays like it is and
users like Phil seem to like it that way).

For the case where the url is not set I see two use cases: The first for
people who would like to keep their submodules local (like me and Marc):
the new behavior enables us to use a relative path too, so we are happy.
Then there are those who want to have a submodule relative to the super-
projects url: they won't get an error anymore when using a relative url
without having a default remote in the superproject. But that is easily
fixed later by doing a "submodule sync" when they recognize that fact,
just like Junio explained.

> There may still be a lingering niggle where git might do something the user
> doesn't expect.  For example, git might create a submodule out of
> git://origin/foo.git instead of the local ../foo.git.  You have to be paying
> attention to git's output to notice that difference, and I could see where a
> user might get tripped up.  But IMO improving this can be done independently
> of Jens's patches.

This behavior exists for some time and is not changed by the patches in
question. But maybe when this patch is applied and people get used to
relative paths for local submodules that might become an issue when they
run into it. But I surely won't have an upstream defined for a superproject
I want to add a local submodule too, so I doubt that.

Or am I still missing something?

Re: [PATCH 0/3] submodule add: allow relative repository path even when no url is set

[Phil Hord]

/(Pardon the duplicate; sending this again from my clean MUA.)
/
Sometime last week, Marc Branchaud wrote:
> I believe your case works smoothly only because in your super-project
> you're careful to make sure you have checked out a branch that
> remotely tracks a something in remotes/public.  If you checked out a
> branch that tracks a different remote you'd get different results.
> This seems fragile to me.

Well, I definitely wasn't careful, but in fact I _was_ on a
remote-tracking branch.  The fact that 'git submodule add' is dependent
on the currently-checked-out branch is a surprise to me.

> When you tried the detached-HEAD scenario, did you get URLs for
> "public:git/sub" or "origin:git/sub"?  Does "origin" just happen to be
> the remote you want to use in any case?

Yes, origin happens to be the remote I want to use.  Actually, any of
the remotes would be fine with me because in this case I want to push to
all of them eventually.  The scenario I was working on was where I am
creating a new repository and new submodules to go in it.  All of my
remotes are clones of my repo collection.

git remote -v
git02    git02:foo/tss.git (fetch)
git02    git02:foo/tss.git (push)
origin    public:bar/tss (fetch)
origin    public:bar/tss (push)

> My fundamental point is that "git submodule add" seems to do confusing
> things with relative paths.  Maybe all that's needed is to clarify the
> documentation.  I'll post a patch.

I definitely agree that 'submodule add' does confusing and unexpected
things.   I only got it to work by carefully contriving the exact steps
required to make it work.  Editing .gitmodules directly would have been
much simpler.

Clarifying the documentation is a good first step. But now that you've
made me look at this more closely, I see there is much more to complain
about and, hopefully, get fixed.

I've run out of time today, though.  I'll follow up with another email
on this thread tomorrow.

Thanks for stirring this pot.

Phil

[PATCHv2] Clarified how "git submodule add" handles relative paths.

[Marc Branchaud]

Signed-off-by: Marc Branchaud <marcnarc@xiplink.com>
---
 Documentation/git-submodule.txt |   17 +++++++++++++++--
 1 files changed, 15 insertions(+), 2 deletions(-)

On 11-06-01 12:59 PM, Junio C Hamano wrote:
> Marc Branchaud <marcnarc@xiplink.com> writes:
> 
>> So this is really about saving the users the hassle of modifying all the
>> URLs in the .gitmodules file.  Does this patch document what you mean?
> 
> I do not necessarily think it is _solely_ about users. Another obvious
> example I left unsaid was what would happen when your project gets popular
> and gets mirrored to many places. Surely you need to advise people of
> alternate locations of the top-level, but what is recorded in .gitmodules
> will be relative to that top-level, so they do not have to be changed.
> Even if you have only one public repository, I would imagine that the same
> convenience would apply if you ever decide to switch the project hosting
> site.

I think I understand.  How does this version grab ya?

> "Users can use a URL different from what was given by the maintainer" is
> merely an example; I doubt it deserves stressing that much.
> 
>> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
>> index 1a16ff6..54dfebb 100644
>> --- a/Documentation/git-submodule.txt
>> +++ b/Documentation/git-submodule.txt
>> @@ -77,8 +77,17 @@ to exist in the superproject. If <path> is not given, the
>>  +
>>  <repository> is the URL of the new submodule's origin repository.
>>  This may be either an absolute URL, or (if it begins with ./
>> -or ../), the location relative to the superproject's origin
>> -repository.
>> +or ../) a URL relative to one of the superproject's remote
>> +repostories:  If the superprojet's currently checked-out branch tracks
>> +a remote branch then that remote's URL is used, otherwise the "origin"
>> +remote's URL is used.  Relative URLs allow users to easily clone the
>> +superproject and its submodules using a different URL than what the
>> +superproject's maintainer might use (e.g. the maintainer can use ssh://
>> +URLs while the users might use git:// URLs).  With relative URLs in the
>> +.gitmodules file, the users won't have to edit all the submodule URLs.
>> ++
>> +*NOTE*: This means that you can *not* use a relative path to refer to a
>> +repository in your local filesystem.
> 
> It is not "can not" but "do not", is it?  More importantly, I do not think
> it is limited to the case of relative path at all.

Well, the way the code currently works I think it's "can not".  It'll
happily accept an absolute file path but any relative path is either
successfully "remotified" or the command fails.

(I also agree with Jens that there's utility in allowing local-filesystem
submodules.)

		M.


diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 1a16ff6..8daa33c 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -77,8 +77,21 @@ to exist in the superproject. If <path> is not given, the
 +
 <repository> is the URL of the new submodule's origin repository.
 This may be either an absolute URL, or (if it begins with ./
-or ../), the location relative to the superproject's origin
-repository.
+or ../) a URL relative to one of the superproject's remote
+repostories:  If the superprojet's currently checked-out branch tracks
+a remote branch then that remote's URL is used, otherwise the "origin"
+remote's URL is used.
++
+Relative submodule URLs provide flexibility in cases where the
+super-project and its submodules are hosted together.  For example,
+the relative submodule URLs remain valid if the repository collection
+moves to a different hosting site, or if the collection is mirrored at
+different sites.  Similarly, maintainers can conveniently make clones
+with a different URL (say an ssh:// URL) than users (who might use
+git:// URLs).
++
+*NOTE*: This means that you can *not* use a relative path to refer to a
+repository in your local filesystem.
 +
 <path> is the relative location for the cloned submodule to
 exist in the superproject. If <path> does not exist, then the
-- 
1.7.5.3.1.ge85f0.dirty

Re: [PATCH 0/2] Tests for some submodule corner cases.

[Marc Branchaud]

On 11-05-31 03:30 PM, Jens Lehmann wrote:
> Am 30.05.2011 23:51, schrieb Marc Branchaud:
>> Ran across some submodule behavior that seems wrong to me.  I don't have the
>> chops to fix the issues, so I thought I'd just point them out with some unit
>> tests.
> 
> Thanks for bringing these issues to our attention this way, having a way
> to easily reproduce them is very much appreciated.
> 
>> Patch 1 tests the case where "submodule add" fails if the path to the
>> submodule repo is relative (i.e. starts with "../").  This currently fails
>> with "remote (origin) does not have a url defined in .git/config".  Maybe
>> there's a reason to fail?  If so, a better error message would be appreciated.
> 
> I stumbled across this behavior now and then too, but according to the
> commit it added (f31a522a2d) it is intended that adding a relative path
> behaves differently than using an absolute path (it resolves relative to
> the superproject's origin, not the filesystem, and to be able to do that
> the superproject's .git/config has to have an url defined for it). But
> you are right about the error message, it really isn't that helpful ...
> 
>> Patch 2 exposes an anomaly in "submodule status", which reports that a
>> submodule is OK even though it has deleted files.  "git status" inside
>> the submodule (and in the super-repo) both identify any deleted files, but
>> "submodule status" doesn't prefix the submodule's HEAD SHA-ID with a "+".
> 
> That is documented behavior. "git submodule status" only cares about the
> commit recorded in the superproject vs the HEAD in the submodule, work
> tree modifications are never shown by it.
> 
> But try a "git status" in the superproject, that will give you the following
> output:
> #	modified:   init (modified content)

I understand.  My apologies for not reading the man page closely enough.

I know there's been a lot of recent work on making "git status"
submodule-friendly, but would there be any interest in having another prefix
for submodule status to cover this case?  Maybe ! could indicate that the
submodule's HEAD is correct, but the working directory doesn't match it exactly.

		M.

Re: [PATCH 0/2] Tests for some submodule corner cases.

[Jens Lehmann]

Am 31.05.2011 23:06, schrieb Marc Branchaud:
> On 11-05-31 03:30 PM, Jens Lehmann wrote:
>> Am 30.05.2011 23:51, schrieb Marc Branchaud:
>>> Patch 2 exposes an anomaly in "submodule status", which reports that a
>>> submodule is OK even though it has deleted files.  "git status" inside
>>> the submodule (and in the super-repo) both identify any deleted files, but
>>> "submodule status" doesn't prefix the submodule's HEAD SHA-ID with a "+".
>>
>> That is documented behavior. "git submodule status" only cares about the
>> commit recorded in the superproject vs the HEAD in the submodule, work
>> tree modifications are never shown by it.
>>
>> But try a "git status" in the superproject, that will give you the following
>> output:
>> #	modified:   init (modified content)
> 
> I understand.  My apologies for not reading the man page closely enough.

No problem, maybe that's just an indication that a reference to "git status"
being more capable of telling what is going on inside a submodule is missing
to the man page for "git submodule status".

> I know there's been a lot of recent work on making "git status"
> submodule-friendly, but would there be any interest in having another prefix
> for submodule status to cover this case?  Maybe ! could indicate that the
> submodule's HEAD is correct, but the working directory doesn't match it exactly.

I'd rather leave "git submodule status" as it is and incorporate this kind
of functionality into core git (for "submodule status" it already arrived there
in 1.7.0/1.7.1, so that part is finished ;-). I hope making the "git submodule"
script mostly obsolete in the long run and would want to avoid teaching it new
stuff already covered by core git.

Re: [PATCH 0/2] Tests for some submodule corner cases.

[Marc Branchaud]

On 11-05-31 05:26 PM, Jens Lehmann wrote:
> Am 31.05.2011 23:06, schrieb Marc Branchaud:
>> On 11-05-31 03:30 PM, Jens Lehmann wrote:
>>> Am 30.05.2011 23:51, schrieb Marc Branchaud:
>>>> Patch 2 exposes an anomaly in "submodule status", which reports that a
>>>> submodule is OK even though it has deleted files.  "git status" inside
>>>> the submodule (and in the super-repo) both identify any deleted files, but
>>>> "submodule status" doesn't prefix the submodule's HEAD SHA-ID with a "+".
>>>
>>> That is documented behavior. "git submodule status" only cares about the
>>> commit recorded in the superproject vs the HEAD in the submodule, work
>>> tree modifications are never shown by it.
>>>
>>> But try a "git status" in the superproject, that will give you the following
>>> output:
>>> #	modified:   init (modified content)
>>
>> I understand.  My apologies for not reading the man page closely enough.
> 
> No problem, maybe that's just an indication that a reference to "git status"
> being more capable of telling what is going on inside a submodule is missing
> to the man page for "git submodule status".

Yes, that'd possibly help.

>> I know there's been a lot of recent work on making "git status"
>> submodule-friendly, but would there be any interest in having another prefix
>> for submodule status to cover this case?  Maybe ! could indicate that the
>> submodule's HEAD is correct, but the working directory doesn't match it exactly.
> 
> I'd rather leave "git submodule status" as it is and incorporate this kind
> of functionality into core git (for "submodule status" it already arrived there
> in 1.7.0/1.7.1, so that part is finished ;-). I hope making the "git submodule"
> script mostly obsolete in the long run and would want to avoid teaching it new
> stuff already covered by core git.

A noble goal, certainly.

So here's my basic question: How can my build system be sure that a submodule
contains the correct working directory?  Do I need to do both "git submodule
status" to check the submodule's HEAD, then also use "git status" to see if
that HEAD is correctly checked out?

Note that my automated builds don't really care about possibly-modified files
or anything like that.  They just want the exact tree that corresponds to the
commit ID recorded in the superproject.  (Previous builds might've left some
cruft lying around, and the automated build wants to be sure that's eliminated.)

Maybe I should just forego the status-checking altogether, and do "git
submodule update path/to/sub && (cd path/to/sub; git reset --hard HEAD; git
clean -dx)".

		M.

Re: [PATCH 0/2] Tests for some submodule corner cases.

[Junio C Hamano]

Marc Branchaud <marcnarc@xiplink.com> writes:

> Note that my automated builds don't really care about possibly-modified files
> or anything like that.  They just want the exact tree that corresponds to the
> commit ID recorded in the superproject.  (Previous builds might've left some
> cruft lying around, and the automated build wants to be sure that's eliminated.)
>
> Maybe I should just forego the status-checking altogether, and do "git
> submodule update path/to/sub && (cd path/to/sub; git reset --hard HEAD; git
> clean -dx)".

It really depends on what you really care about. If the working tree is
shared between your build-bot and a human user, the build-bot may want to
make sure it does not overwrite and lose work by a human, but otherwise,
instead of checking if something is stale (and having to design what to do
when it actually finds something is stale), actively causing the state it
wants to see appear in the working tree sounds like a simpler, more robust
and much saner thing to do.

Re: [PATCH 0/2] Tests for some submodule corner cases.

[Jens Lehmann]

Am 01.06.2011 18:11, schrieb Marc Branchaud:
> On 11-05-31 05:26 PM, Jens Lehmann wrote:
>> No problem, maybe that's just an indication that a reference to "git status"
>> being more capable of telling what is going on inside a submodule is missing
>> to the man page for "git submodule status".
> 
> Yes, that'd possibly help.

Ok, I'll see if I can come up with something ...

> So here's my basic question: How can my build system be sure that a submodule
> contains the correct working directory?  Do I need to do both "git submodule
> status" to check the submodule's HEAD, then also use "git status" to see if
> that HEAD is correctly checked out?

No, "git status" will do both. The only thing it will be silent about is when
a submodule isn't initialized at all ("git submodule status" shows this with
a '-').