[PATCH v2 1/2] submodule: add tests for add,sync,init in presence of relative super origin URL

[PATCH v2 1/2] submodule: add tests for add,sync,init in presence of relative super origin URL

[Jon Seymour]

These tests are expected to fail, pending subsequent patch.

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 t/t7400-submodule-basic.sh | 27 +++++++++++++++++++++++++++
 t/t7403-submodule-sync.sh  | 10 ++++++++++
 2 files changed, 37 insertions(+)

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 81827e6..1c40951 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -507,6 +507,33 @@ test_expect_success 'relative path works with user@host:path' '
 	)
 '
 
+test_expect_failure 'relative path works with ../relative/repo' '
+	(
+		cd reltest &&
+		cp pristine-.git-config .git/config &&
+		git config remote.origin.url ../relative/repo &&
+		git submodule init &&
+		test "$(git config submodule.sub.url)" = ../../relative/subrepo
+	)
+'
+
+test_expect_failure 'test that submodule add creates the correct url when super origin url is relative' '
+	mkdir reladd &&
+	(
+		cd reladd &&
+		git init &&
+		git remote add origin ../relative/repo
+		mkdir sub &&
+		(
+			cd sub &&
+			git init &&
+			test_commit foo
+		) &&
+		git submodule add ../subrepo ./sub &&
+		test "$(git config submodule.sub.url)" = ../../relative/subrepo
+	)
+'
+
 test_expect_success 'moving the superproject does not break submodules' '
 	(
 		cd addtest &&
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index 3620215..788bc24 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh
@@ -86,4 +86,14 @@ test_expect_success '"git submodule sync" should not vivify uninteresting submod
 	)
 '
 
+test_expect_failure '"git submodule sync" should handle a super with a relative origin URL' '
+	git clone super relative-clone &&
+	(cd relative-clone &&
+	 git submodule update --init &&
+	 git remote set-url origin ../relative/super.git &&
+	 git submodule sync &&
+	 test "$(git config submodule.submodule.url)" == ../../relative/moved-submodule
+	)
+'
+
 test_done
-- 
1.7.10.2.650.g22d2504

[PATCH v2 2/2] submodule: fix handling of superproject with relative origin URLs

[Jon Seymour]

Prior to this change, an operation such as git submodule add, init or
sync produced the wrong result when the origin URL of the superproject
was itself a relative URL.

The issue arises in these cases because the origin URL of
the superproject needs to be prepended with a prefix that navigates
from the submodule to the superproject so that when the submodule
URL is concatenated the resulting URL is relative to the working tree
of the submodule.

This change ensures that this is done for add, sync and init.

Note that superproject relative origin URLs of the form foo/bar
are still not handled correctly. The user can workaround this case
by adding ./ prefix to the origin URL.

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 git-submodule.sh           | 35 ++++++++++++++++++++---------------
 t/t7400-submodule-basic.sh |  4 ++--
 t/t7403-submodule-sync.sh  |  2 +-
 3 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 64a70d6..230c219 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -45,6 +45,10 @@ resolve_relative_url ()
 		../*)
 			url="${url#../}"
 			case "$remoteurl" in
+			.*/*)
+				up_path="$(echo "$2" | sed "s/[^/]*/../g")"
+				remoteurl="${up_path%/}/${remoteurl%/*}"
+				;;
 			*/*)
 				remoteurl="${remoteurl%/*}"
 				;;
@@ -235,11 +239,24 @@ cmd_add()
 		usage
 	fi
 
+	# normalize path:
+	# multiple //; leading ./; /./; /../; trailing /
+	sm_path=$(printf '%s/\n' "$sm_path" |
+		sed -e '
+			s|//*|/|g
+			s|^\(\./\)*||
+			s|/\./|/|g
+			:start
+			s|\([^/]*\)/\.\./||
+			tstart
+			s|/*$||
+		')
+
 	# assure repo is absolute or relative to parent
 	case "$repo" in
 	./*|../*)
 		# dereference source url relative to parent's url
-		realrepo=$(resolve_relative_url "$repo") || exit
+		realrepo=$(resolve_relative_url "$repo" "$sm_path") || exit
 		;;
 	*:*|/*)
 		# absolute url
@@ -250,18 +267,6 @@ cmd_add()
 	;;
 	esac
 
-	# normalize path:
-	# multiple //; leading ./; /./; /../; trailing /
-	sm_path=$(printf '%s/\n' "$sm_path" |
-		sed -e '
-			s|//*|/|g
-			s|^\(\./\)*||
-			s|/\./|/|g
-			:start
-			s|\([^/]*\)/\.\./||
-			tstart
-			s|/*$||
-		')
 	git ls-files --error-unmatch "$sm_path" > /dev/null 2>&1 &&
 	die "$(eval_gettext "'\$sm_path' already exists in the index")"
 
@@ -407,7 +412,7 @@ cmd_init()
 			# Possibly a url relative to parent
 			case "$url" in
 			./*|../*)
-				url=$(resolve_relative_url "$url") || exit
+				url=$(resolve_relative_url "$url" "$sm_path") || exit
 				;;
 			esac
 			git config submodule."$name".url "$url" ||
@@ -964,7 +969,7 @@ cmd_sync()
 		# Possibly a url relative to parent
 		case "$url" in
 		./*|../*)
-			url=$(resolve_relative_url "$url") || exit
+			url=$(resolve_relative_url "$url" "$sm_path") || exit
 			;;
 		esac
 
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 1c40951..e10abc4 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -507,7 +507,7 @@ test_expect_success 'relative path works with user@host:path' '
 	)
 '
 
-test_expect_failure 'relative path works with ../relative/repo' '
+test_expect_success 'relative path works with ../relative/repo' '
 	(
 		cd reltest &&
 		cp pristine-.git-config .git/config &&
@@ -517,7 +517,7 @@ test_expect_failure 'relative path works with ../relative/repo' '
 	)
 '
 
-test_expect_failure 'test that submodule add creates the correct url when super origin url is relative' '
+test_expect_success 'test that submodule add creates the correct url when super origin url is relative' '
 	mkdir reladd &&
 	(
 		cd reladd &&
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index 788bc24..35700ef 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh
@@ -86,7 +86,7 @@ test_expect_success '"git submodule sync" should not vivify uninteresting submod
 	)
 '
 
-test_expect_failure '"git submodule sync" should handle a super with a relative origin URL' '
+test_expect_success '"git submodule sync" should handle a super with a relative origin URL' '
 	git clone super relative-clone &&
 	(cd relative-clone &&
 	 git submodule update --init &&
-- 
1.7.10.2.650.g22d2504

Re: [PATCH v2 2/2] submodule: fix handling of superproject with relative origin URLs

[Junio C Hamano]

Jon Seymour <jon.seymour@gmail.com> writes:

> Prior to this change, an operation such as git submodule add, init or
> sync produced the wrong result when the origin URL of the superproject
> was itself a relative URL.

If you say you are "fixing" something in the title, it is already known to
the reader that a broken behaviour exists in the code without the patch in
question.  Instead of spending four useless words "Prior to this change",
could "the wrong result" be clarified with either saying "wrong in what
way" and/or "because of this and that reason"?

In the case of this patch, you explain "because..." part in the second
paragraph (which is good), so

	An operation such as A and B does this when it should do that
	instead.

stated in the present tense, as a statement of the fact, is sufficient.
"does this instead of that, which is wrong" is a lot more important.

> Note that superproject relative origin URLs of the form foo/bar
> are still not handled correctly.

I am not sure what the use case of such a layout is.  A project that has a
"bar" repository as its superproject (or its one of submodules for that
matter) may advertise that the other repository lives at ../bar.git, so
that when these two projects are served at a random hosting service, such
a cross-project pointer does not have to be rewritten as long as their
relative location at the hosting service remains the same.  But what does
it mean to say a related "foo" project lives in foo/bar.git directory
relative to one project in the first place?  Does the project's $GIT_DIR/
have a "foo" directory next to its "refs" and "objects"?  Probably I am
missing what you are trying to achieve.  Puzzled.

Re: [PATCH v2 2/2] submodule: fix handling of superproject with relative origin URLs

[Jon Seymour]

On Mon, May 21, 2012 at 12:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jon Seymour <jon.seymour@gmail.com> writes:
>
>> Prior to this change, an operation such as git submodule add, init or
>> sync produced the wrong result when the origin URL of the superproject
>> was itself a relative URL.
>
> If you say you are "fixing" something in the title, it is already known to
> the reader that a broken behaviour exists in the code without the patch in
> question.  Instead of spending four useless words "Prior to this change",
> could "the wrong result" be clarified with either saying "wrong in what
> way" and/or "because of this and that reason"?
>

I hope v3 is closer to what you would like to see.

>> Note that superproject relative origin URLs of the form foo/bar
>> are still not handled correctly.
>
> I am not sure what the use case of such a layout is.  A project that has a
> "bar" repository as its superproject (or its one of submodules for that
> matter) may advertise that the other repository lives at ../bar.git, so
> that when these two projects are served at a random hosting service, such
> a cross-project pointer does not have to be rewritten as long as their
> relative location at the hosting service remains the same.  But what does
> it mean to say a related "foo" project lives in foo/bar.git directory
> relative to one project in the first place?  Does the project's $GIT_DIR/
> have a "foo" directory next to its "refs" and "objects"?  Probably I am
> missing what you are trying to achieve.  Puzzled.

I have tried to explain the use case again in 4/4 of the v3 patch (my
previous attempt
was in a previous reply to one of Jen's posts). I don't claim that
this is a common
use case at all, but it is a use case that I have used several times myself.

It is particularly useful in cases where there is a need to regularly
synchronize
a collection of git repos and other files across an "air gap" (for example, via
an intermediary USB drive) where one of the repos being synchronized is the
synchronization toolset itself.

In this case, it may well be that the origin repo for a particular
toolset instance
itself is actually located on a USB drive whose physical location changes as
the removable device used to  update the tool set changes.

The user may choose to manage the relative location of the USB drive with a
symbolic link (relative to the toolset) rather than a git origin URL.
(The advantage of
using a symbolic link in this case is that you can update the actual physical
location of the USB drive with a single update to a single symbolic
link - this may
be more convenient in cases where are multiple git repos that need to be synced
to the USB or if there are resources other than git repos on the USB).

So, for example, suppose the toolset is managed within ~/tools with directories
such as:

  ~/tools/bin
  ~/tools/lib

then, perhaps the origin copy of the toolset is found by:

  ~/tools/mnt/usb -> /media/MY_USB_DEVICE/tools

remote.origin.url in ~/tools/.git/config might refer to mnt/usb/tools so that
if the device occasionally changes to /media/YOUR_USB_DEVICE/tools
then this can be accomodated by updating a single symbolic link
(e.g. ~/tools/mnt/usb)

Apologies if this explanation is not clear enough.

Again, I don't claim it is a common use case, but then I don't see
any harm with git supporting it either since it doesn't require much
work.

jon