threads / patch / 30577

v3, 4 partssubmodule: verify support for superproject origin URLs of the form ./foo/bar or ../foo/bar

Subject: [PATCH v3 1/4] submodule: verify support for superproject origin URLs of the form ./foo/bar or ../foo/bar

## tl;dr

6 messages between May 21, 2012 and May 21, 2012. Diffs are folded; open one to read it.

replies: 5people: 1as markdown or json

Jon Seymour· May 21, 2012, 13:31 UTC · lore
These tests are expected to fail, pending a subsequent corrective patch.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 t/t7400-submodule-basic.sh | 26 ++++++++++++++++++++++++++
 t/t7403-submodule-sync.sh  | 10 ++++++++++
 2 files changed, 36 insertions(+)
Show changes to 2 files +36 −0

t/t7400-submodule-basic.sh, t/t7403-submodule-sync.sh

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 81827e6..02e6428 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -507,6 +507,32 @@ 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/repo' '
+	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.594.g5c52315
Jon Seymour· May 21, 2012, 13:31 UTC · re: Jon Seymour · lore

[PATCH v3 2/4] submodule: support superproject origin URLs of the form ./foo/bar or ../foo/bar

When the origin URL of the superproject is itself relative, an operation such as git submodule add, init or sync produces a relative URL for the submodule that doesn't point at the correct location.

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.
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(-)
Show changes to 3 files +23 −18

git-submodule.sh, t/t7400-submodule-basic.sh, t/t7403-submodule-sync.sh

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 02e6428..b838f43 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/repo' '
+test_expect_sucess 'test that submodule add creates the correct url when super origin url is ../relative/repo' '
 	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.594.g5c52315
Jon Seymour· May 21, 2012, 16:35 UTC · re: Jon Seymour · lore

Re: [PATCH v3 2/4] submodule: support superproject origin URLs of the form ./foo/bar or ../foo/bar

On Mon, May 21, 2012 at 11:31 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
Show 135 quoted lines
> When the origin URL of the superproject is itself relative, an operation
> such as git submodule add, init or sync produces a relative
> URL for the submodule that doesn't point at the correct location.
>
> 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.
>
> 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 02e6428..b838f43 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/repo' '
> +test_expect_sucess 'test that submodule add creates the correct url when super origin url is ../relative/repo' '
>        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.594.g5c52315
>

Mmmm. Better hold off on this one for the moment, I have detected a break during a subsequent git submodule update that my selection of regression tests didn't pick up. The issue is that the URL used for cloning the submodule during the update is now not correct.

jon.
Jon Seymour· May 21, 2012, 22:20 UTC · re: Jon Seymour · lore

Re: [PATCH v3 2/4] submodule: support superproject origin URLs of the form ./foo/bar or ../foo/bar

On Tue, May 22, 2012 at 2:35 AM, Jon Seymour <jon.seymour@gmail.com> wrote:
Show 8 quoted lines
> On Mon, May 21, 2012 at 11:31 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
>
> Mmmm. Better hold off on this one for the moment, I have detected a
> break during a subsequent git submodule update that my selection of
> regression tests didn't pick up. The issue is that the URL used for
> cloning the submodule during the update is now not correct.
>
> jon.

I guess the issue here is that whereas the origin url of the submodule (as in remote.origin.url in the submodule) should be relative to the submodule's working tree, it might actually be better to keep submodule.{modulename}.url relative to the working directory of superproject (which is current behaviour).

Thoughts?
jon.
Jon Seymour· May 21, 2012, 13:31 UTC · re: Jon Seymour · lore

[PATCH v3 3/4] submodule: verify support for superproject URLs of the form foo/bar.

This test shows that git submodule add produces the wrong submodule URL when the origin URL of the superproject is of the form: foo/bar.

The problem is fixed by a subsequent patch.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 t/t7400-submodule-basic.sh | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
Show changes to t/t7400-submodule-basic.sh +17 −0
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index b838f43..71f30d8 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -533,6 +533,23 @@ test_expect_sucess 'test that submodule add creates the correct url when super o
 		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/repo' '
+	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 &&

-- 
1.7.10.2.594.g5c52315
Jon Seymour· May 21, 2012, 13:31 UTC · re: Jon Seymour · lore

[PATCH v3 4/4] submodule: support superproject origin URLs of the form foo/bar

When the origin URL of a superproject is of the form foo/bar, submodule operations such as add, sync or init will configure the URL of any submodule with an origin URL that points at the incorrect location.

Origin URLs of this form are not expected in most cases, since the origin URL of a repo will usually point to a location that is not nested within the current working tree.

However, the situation can arise when the physical location of an upstream repo is managed with a symbolic link nested within the working tree itself.

For example, suppose that a toolset is installed in ~/tools and ~/tools/mnt/usb is a occasionally used as the origin for a refresh of the toolset. Different users with different USB devices may map ~/tools/mnt/usb to a different subdirectory of /media, but the auto-update function of the toolset may simply pull an update from mnt/usb, assuming that the user has done what is required to initialise mnt/usb to refer to the correct subdirectory of /media.

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 git-submodule.sh           | 2 ++
 t/t7400-submodule-basic.sh | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)
Show changes to 2 files +3 −1

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

diff --git a/git-submodule.sh b/git-submodule.sh
index 230c219..b8a7403 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -37,6 +37,7 @@ resolve_relative_url ()
 	remoteurl=$(git config "remote.$remote.url") ||
 		remoteurl=$(pwd) # the repository is its own authoritative upstream
 	url="$1"
+	remoteurl=$(echo "$remoteurl" | sed "s|^[^/][^:]*\$|./&|")
 	remoteurl=${remoteurl%/}
 	sep=/
 	while test -n "$url"
@@ -47,6 +48,7 @@ resolve_relative_url ()
 			case "$remoteurl" in
 			.*/*)
 				up_path="$(echo "$2" | sed "s/[^/]*/../g")"
+				remoteurl=${remoteurl#./}
 				remoteurl="${up_path%/}/${remoteurl%/*}"
 				;;
 			*/*)
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 71f30d8..a4a8363 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -533,7 +533,7 @@ test_expect_sucess 'test that submodule add creates the correct url when super o
 		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/repo' '
+test_expect_success 'test that submodule add creates the correct url when super origin url is relative/repo' '
 	mkdir reladd &&
 	(
 		cd reladd &&

-- 
1.7.10.2.594.g5c52315

← back to recent threads