git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH v3 1/4] submodule: verify support for superproject origin URLs of the form ./foo/bar or ../foo/bar
@ 2012-05-21 13:31 Jon Seymour
  2012-05-21 13:31 ` [PATCH v3 2/4] submodule: support " Jon Seymour
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jon Seymour @ 2012-05-21 13:31 UTC (permalink / raw)
  To: git; +Cc: gitster, Jens.Lehmann, Jon Seymour

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(+)

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/4] submodule: support superproject origin URLs of the form ./foo/bar or ../foo/bar
  2012-05-21 13:31 [PATCH v3 1/4] submodule: verify support for superproject origin URLs of the form ./foo/bar or ../foo/bar Jon Seymour
@ 2012-05-21 13:31 ` Jon Seymour
  2012-05-21 16:35   ` Jon Seymour
  2012-05-21 13:31 ` [PATCH v3 3/4] submodule: verify support for superproject URLs of the form foo/bar Jon Seymour
  2012-05-21 13:31 ` [PATCH v3 4/4] submodule: support superproject origin " Jon Seymour
  2 siblings, 1 reply; 6+ messages in thread
From: Jon Seymour @ 2012-05-21 13:31 UTC (permalink / raw)
  To: git; +Cc: gitster, Jens.Lehmann, Jon Seymour

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 3/4] submodule: verify support for superproject URLs of the form foo/bar.
  2012-05-21 13:31 [PATCH v3 1/4] submodule: verify support for superproject origin URLs of the form ./foo/bar or ../foo/bar Jon Seymour
  2012-05-21 13:31 ` [PATCH v3 2/4] submodule: support " Jon Seymour
@ 2012-05-21 13:31 ` Jon Seymour
  2012-05-21 13:31 ` [PATCH v3 4/4] submodule: support superproject origin " Jon Seymour
  2 siblings, 0 replies; 6+ messages in thread
From: Jon Seymour @ 2012-05-21 13:31 UTC (permalink / raw)
  To: git; +Cc: gitster, Jens.Lehmann, Jon Seymour

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(+)

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 4/4] submodule: support superproject origin URLs of the form foo/bar
  2012-05-21 13:31 [PATCH v3 1/4] submodule: verify support for superproject origin URLs of the form ./foo/bar or ../foo/bar Jon Seymour
  2012-05-21 13:31 ` [PATCH v3 2/4] submodule: support " Jon Seymour
  2012-05-21 13:31 ` [PATCH v3 3/4] submodule: verify support for superproject URLs of the form foo/bar Jon Seymour
@ 2012-05-21 13:31 ` Jon Seymour
  2 siblings, 0 replies; 6+ messages in thread
From: Jon Seymour @ 2012-05-21 13:31 UTC (permalink / raw)
  To: git; +Cc: gitster, Jens.Lehmann, Jon Seymour

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(-)

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/4] submodule: support superproject origin URLs of the form ./foo/bar or ../foo/bar
  2012-05-21 13:31 ` [PATCH v3 2/4] submodule: support " Jon Seymour
@ 2012-05-21 16:35   ` Jon Seymour
  2012-05-21 22:20     ` Jon Seymour
  0 siblings, 1 reply; 6+ messages in thread
From: Jon Seymour @ 2012-05-21 16:35 UTC (permalink / raw)
  To: git; +Cc: Jens Lehmann, Junio C Hamano

On Mon, May 21, 2012 at 11:31 PM, Jon Seymour <jon.seymour@gmail.com> wrote:
> 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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/4] submodule: support superproject origin URLs of the form ./foo/bar or ../foo/bar
  2012-05-21 16:35   ` Jon Seymour
@ 2012-05-21 22:20     ` Jon Seymour
  0 siblings, 0 replies; 6+ messages in thread
From: Jon Seymour @ 2012-05-21 22:20 UTC (permalink / raw)
  To: git; +Cc: Jens Lehmann, Junio C Hamano

On Tue, May 22, 2012 at 2:35 AM, Jon Seymour <jon.seymour@gmail.com> wrote:
> 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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-05-21 22:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-21 13:31 [PATCH v3 1/4] submodule: verify support for superproject origin URLs of the form ./foo/bar or ../foo/bar Jon Seymour
2012-05-21 13:31 ` [PATCH v3 2/4] submodule: support " Jon Seymour
2012-05-21 16:35   ` Jon Seymour
2012-05-21 22:20     ` Jon Seymour
2012-05-21 13:31 ` [PATCH v3 3/4] submodule: verify support for superproject URLs of the form foo/bar Jon Seymour
2012-05-21 13:31 ` [PATCH v3 4/4] submodule: support superproject origin " Jon Seymour

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).