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

Jens Lehmann [off-list ref] writes:

Am 02.06.2011 19:14, schrieb Junio C Hamano:

quoted

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.

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

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 <redacted>
---
 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

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 <redacted>
---
 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

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 <redacted>
---
 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

Signed-off-by: Marc Branchaud <redacted>
---
 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

Signed-off-by: Marc Branchaud <redacted>
---
 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

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.