From 712693e8dbf188d4e9337c065617f09f7a0d1a16 Mon Sep 17 00:00:00 2001 From: Jon Seymour Date: Sun, 3 Jun 2012 19:46:47 +1000 Subject: [PATCH 1/5] submodule: additional regression tests for relative URLs Some additional tests are added to support regression testing of the changes in the remainder of the series. We also add a pristine copy of .gitmodules in anticipation of this being required by later tests. Signed-off-by: Jon Seymour Signed-off-by: Junio C Hamano --- t/t7400-submodule-basic.sh | 110 ++++++++++++++++++++++++++++++++++++- 1 file changed, 107 insertions(+), 3 deletions(-) diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 81827e696f2..9428c7a744c 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -483,21 +483,67 @@ test_expect_success 'set up for relative path tests' ' git add sub && git config -f .gitmodules submodule.sub.path sub && git config -f .gitmodules submodule.sub.url ../subrepo && - cp .git/config pristine-.git-config + cp .git/config pristine-.git-config && + cp .gitmodules pristine-.gitmodules ) ' -test_expect_success 'relative path works with URL' ' +test_expect_success '../subrepo works with URL - ssh://hostname/repo' ' ( cd reltest && cp pristine-.git-config .git/config && + cp pristine-.gitmodules .gitmodules && git config remote.origin.url ssh://hostname/repo && git submodule init && test "$(git config submodule.sub.url)" = ssh://hostname/subrepo ) ' -test_expect_success 'relative path works with user@host:path' ' +test_expect_success '../subrepo works with port-qualified URL - ssh://hostname:22/repo' ' + ( + cd reltest && + cp pristine-.git-config .git/config && + cp pristine-.gitmodules .gitmodules && + git config remote.origin.url ssh://hostname:22/repo && + git submodule init && + test "$(git config submodule.sub.url)" = ssh://hostname:22/subrepo + ) +' + +test_expect_success '../subrepo path works with local path - /foo/repo' ' + ( + cd reltest && + cp pristine-.git-config .git/config && + cp pristine-.gitmodules .gitmodules && + git config remote.origin.url /foo/repo && + git submodule init && + test "$(git config submodule.sub.url)" = /foo/subrepo + ) +' + +test_expect_success '../subrepo works with file URL - file:///tmp/repo' ' + ( + cd reltest && + cp pristine-.git-config .git/config && + cp pristine-.gitmodules .gitmodules && + git config remote.origin.url file:///tmp/repo && + git submodule init && + test "$(git config submodule.sub.url)" = file:///tmp/subrepo + ) +' + +test_expect_success '../subrepo works with helper URL- helper:://hostname/repo' ' + ( + cd reltest && + cp pristine-.git-config .git/config && + cp pristine-.gitmodules .gitmodules && + git config remote.origin.url helper:://hostname/repo && + git submodule init && + test "$(git config submodule.sub.url)" = helper:://hostname/subrepo + ) +' + +test_expect_success '../subrepo works with scp-style URL - user@host:repo' ' ( cd reltest && cp pristine-.git-config .git/config && @@ -507,6 +553,64 @@ test_expect_success 'relative path works with user@host:path' ' ) ' +test_expect_success '../subrepo works with scp-style URL - user@host:path/to/repo' ' + ( + cd reltest && + cp pristine-.git-config .git/config && + cp pristine-.gitmodules .gitmodules && + git config remote.origin.url user@host:path/to/repo && + git submodule init && + test "$(git config submodule.sub.url)" = user@host:path/to/subrepo + ) +' + +test_expect_success '../subrepo works with relative local path - foo/bar' ' + ( + cd reltest && + cp pristine-.git-config .git/config && + cp pristine-.gitmodules .gitmodules && + git config remote.origin.url foo/bar && + git submodule init && + test "$(git config submodule.sub.url)" = foo/subrepo + ) +' + +test_expect_success '../subrepo works with relative local path - ../foo' ' + ( + cd reltest && + cp pristine-.git-config .git/config && + cp pristine-.gitmodules .gitmodules && + git config remote.origin.url ../foo && + git submodule init && + test "$(git config submodule.sub.url)" = ../subrepo + ) +' + +test_expect_success '../subrepo works with relative local path - ../foo/bar' ' + ( + cd reltest && + cp pristine-.git-config .git/config && + cp pristine-.gitmodules .gitmodules && + git config remote.origin.url ../foo/bar && + git submodule init && + test "$(git config submodule.sub.url)" = ../foo/subrepo + ) +' + +test_expect_success '../bar/a/b/c works with relative local path - ../foo/bar.git' ' + ( + cd reltest && + cp pristine-.git-config .git/config && + cp pristine-.gitmodules .gitmodules && + mkdir -p a/b/c && + (cd a/b/c; git init) && + git config remote.origin.url ../foo/bar.git && + git submodule add ../bar/a/b/c ./a/b/c && + git submodule init && + test "$(git config submodule.a/b/c.url)" = ../foo/bar/a/b/c + ) +' + test_expect_success 'moving the superproject does not break submodules' ' ( cd addtest && From 49301c64f3612d68058efb428a74133ac6e4a071 Mon Sep 17 00:00:00 2001 From: Jon Seymour Date: Sun, 3 Jun 2012 19:46:48 +1000 Subject: [PATCH 2/5] submodule: document failure to handle relative superproject origin URLs This test case documents several cases where handling of relative superproject origin URLs doesn't produce an expected result. submodule.{sub}.url in the superproject is incorrect in these cases: foo ./foo ./foo/bar The remote.origin.url of the submodule is incorrect in the above cases and also when the superproject origin URL is like: foo/bar ../foo ../foo/bar Signed-off-by: Jon Seymour Signed-off-by: Junio C Hamano --- t/t7400-submodule-basic.sh | 34 ++++++++++++++ t/t7403-submodule-sync.sh | 90 +++++++++++++++++++++++++++++++++++++- 2 files changed, 123 insertions(+), 1 deletion(-) diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 9428c7a744c..09e2b9bf16e 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -564,6 +564,18 @@ test_expect_success '../subrepo works with scp-style URL - user@host:path/to/rep ) ' +test_expect_failure '../subrepo works with relative local path - foo' ' + ( + cd reltest && + cp pristine-.git-config .git/config && + cp pristine-.gitmodules .gitmodules && + git config remote.origin.url foo && + # actual: fails with an error + git submodule init && + test "$(git config submodule.sub.url)" = subrepo + ) +' + test_expect_success '../subrepo works with relative local path - foo/bar' ' ( cd reltest && @@ -575,6 +587,28 @@ test_expect_success '../subrepo works with relative local path - foo/bar' ' ) ' +test_expect_failure '../subrepo works with relative local path - ./foo' ' + ( + cd reltest && + cp pristine-.git-config .git/config && + cp pristine-.gitmodules .gitmodules && + git config remote.origin.url ./foo && + git submodule init && + test "$(git config submodule.sub.url)" = subrepo + ) +' + +test_expect_failure '../subrepo works with relative local path - ./foo/bar' ' + ( + cd reltest && + cp pristine-.git-config .git/config && + cp pristine-.gitmodules .gitmodules && + git config remote.origin.url ./foo/bar && + git submodule init && + test "$(git config submodule.sub.url)" = foo/subrepo + ) +' + test_expect_success '../subrepo works with relative local path - ../foo' ' ( cd reltest && diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh index 3620215c1f8..56b933da45a 100755 --- a/t/t7403-submodule-sync.sh +++ b/t/t7403-submodule-sync.sh @@ -26,7 +26,9 @@ test_expect_success setup ' (cd super-clone && git submodule update --init) && git clone super empty-clone && (cd empty-clone && git submodule init) && - git clone super top-only-clone + git clone super top-only-clone && + git clone super relative-clone && + (cd relative-clone && git submodule update --init) ' test_expect_success 'change submodule' ' @@ -86,4 +88,90 @@ test_expect_success '"git submodule sync" should not vivify uninteresting submod ) ' +test_expect_failure '"git submodule sync" handles origin URL of the form foo' ' + (cd relative-clone && + git remote set-url origin foo && + git submodule sync && + (cd submodule && + #actual fails with: "cannot strip off url foo + test "$(git config remote.origin.url)" = "../submodule" + ) + ) +' + +test_expect_failure '"git submodule sync" handles origin URL of the form foo/bar' ' + (cd relative-clone && + git remote set-url origin foo/bar && + git submodule sync && + (cd submodule && + #actual foo/submodule + test "$(git config remote.origin.url)" = "../foo/submodule" + ) + ) +' + +test_expect_failure '"git submodule sync" handles origin URL of the form ./foo' ' + (cd relative-clone && + git remote set-url origin ./foo && + git submodule sync && + (cd submodule && + #actual ./submodule + test "$(git config remote.origin.url)" = "../submodule" + ) + ) +' + +test_expect_failure '"git submodule sync" handles origin URL of the form ./foo/bar' ' + (cd relative-clone && + git remote set-url origin ./foo/bar && + git submodule sync && + (cd submodule && + #actual ./foo/submodule + test "$(git config remote.origin.url)" = "../foo/submodule" + ) + ) +' + +test_expect_failure '"git submodule sync" handles origin URL of the form ../foo' ' + (cd relative-clone && + git remote set-url origin ../foo && + git submodule sync && + (cd submodule && + #actual ../submodule + test "$(git config remote.origin.url)" = "../../submodule" + ) + ) +' + +test_expect_failure '"git submodule sync" handles origin URL of the form ../foo/bar' ' + (cd relative-clone && + git remote set-url origin ../foo/bar && + git submodule sync && + (cd submodule && + #actual ../foo/submodule + test "$(git config remote.origin.url)" = "../../foo/submodule" + ) + ) +' + +test_expect_failure '"git submodule sync" handles origin URL of the form ../foo/bar with deeply nested submodule' ' + (cd relative-clone && + git remote set-url origin ../foo/bar && + mkdir -p a/b/c && + ( cd a/b/c && + git init && + :> .gitignore && + git add .gitignore && + test_tick && + git commit -m "initial commit" ) && + git submodule add ../bar/a/b/c ./a/b/c && + git submodule sync && + (cd a/b/c && + #actual ../foo/bar/a/b/c + test "$(git config remote.origin.url)" = "../../../../foo/bar/a/b/c" + ) + ) +' + + test_done From 967b2c66738fe6b168ead0c3106ac47fb9ae22c7 Mon Sep 17 00:00:00 2001 From: Jon Seymour Date: Wed, 6 Jun 2012 21:57:29 +1000 Subject: [PATCH 3/5] submodule: fix sync handling of some relative superproject origin URLs When the origin URL of the superproject is itself relative, git submodule sync configures the remote.origin.url configuration property of the submodule with a path that is relative to the work tree of the superproject rather than the work tree of the submodule. To fix this an 'up_path' that navigates from the work tree of the submodule to the work tree of the superproject needs to be prepended to the URL otherwise calculated. Correct handling of superproject origin URLs like foo, ./foo and ./foo/bar is left to a subsequent patch since an additional change is required to handle these cases. The documentation of resolve_relative_url() is expanded to give a more thorough description of the function's objective. Signed-off-by: Jon Seymour Signed-off-by: Junio C Hamano --- git-submodule.sh | 48 +++++++++++++++++++++++++++++++++++---- t/t7403-submodule-sync.sh | 8 +++---- 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 64a70d621aa..0a3e1465e39 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -30,7 +30,22 @@ nofetch= update= prefix= -# Resolve relative url by appending to parent's url +# The function takes at most 2 arguments. The first argument is the +# URL that navigates to the submodule origin repo. When relative, this URL +# is relative to the superproject origin URL repo. The second up_path +# argument, if specified, is the relative path that navigates +# from the submodule working tree to the superproject working tree. +# +# The output of the function is the origin URL of the submodule. +# +# The output will either be an absolute URL or filesystem path (if the +# superproject origin URL is an absolute URL or filesystem path, +# respectively) or a relative file system path (if the superproject +# origin URL is a relative file system path). +# +# When the output is a relative file system path, the path is either +# relative to the submodule working tree, if up_path is specified, or to +# the superproject working tree otherwise. resolve_relative_url () { remote=$(get_default_remote) @@ -39,6 +54,17 @@ resolve_relative_url () url="$1" remoteurl=${remoteurl%/} sep=/ + up_path="$2" + + case "$remoteurl" in + *:*|/*) + is_relative= + ;; + *) + is_relative=t + ;; + esac + while test -n "$url" do case "$url" in @@ -64,7 +90,7 @@ resolve_relative_url () break;; esac done - echo "$remoteurl$sep${url%/}" + echo "${is_relative:+${up_path}}$remoteurl$sep${url%/}" } # @@ -964,14 +990,26 @@ cmd_sync() # Possibly a url relative to parent case "$url" in ./*|../*) - url=$(resolve_relative_url "$url") || exit + # rewrite foo/bar as ../.. to find path from + # submodule work tree to superproject work tree + up_path="$(echo "$sm_path" | sed "s/[^/][^/]*/../g")" && + # guarantee a trailing / + up_path=${up_path%/}/ && + # path from submodule work tree to submodule origin repo + sub_origin_url=$(resolve_relative_url "$url" "$up_path") && + # path from superproject work tree to submodule origin repo + super_config_url=$(resolve_relative_url "$url") || exit + ;; + *) + sub_origin_url="$url" + super_config_url="$url" ;; esac if git config "submodule.$name.url" >/dev/null 2>/dev/null then say "$(eval_gettext "Synchronizing submodule url for '\$name'")" - git config submodule."$name".url "$url" + git config submodule."$name".url "$super_config_url" if test -e "$sm_path"/.git then @@ -979,7 +1017,7 @@ cmd_sync() clear_local_git_env cd "$sm_path" remote=$(get_default_remote) - git config remote."$remote".url "$url" + git config remote."$remote".url "$sub_origin_url" ) fi fi diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh index 56b933da45a..98bc74a4e9c 100755 --- a/t/t7403-submodule-sync.sh +++ b/t/t7403-submodule-sync.sh @@ -99,7 +99,7 @@ test_expect_failure '"git submodule sync" handles origin URL of the form foo' ' ) ' -test_expect_failure '"git submodule sync" handles origin URL of the form foo/bar' ' +test_expect_success '"git submodule sync" handles origin URL of the form foo/bar' ' (cd relative-clone && git remote set-url origin foo/bar && git submodule sync && @@ -132,7 +132,7 @@ test_expect_failure '"git submodule sync" handles origin URL of the form ./foo/b ) ' -test_expect_failure '"git submodule sync" handles origin URL of the form ../foo' ' +test_expect_success '"git submodule sync" handles origin URL of the form ../foo' ' (cd relative-clone && git remote set-url origin ../foo && git submodule sync && @@ -143,7 +143,7 @@ test_expect_failure '"git submodule sync" handles origin URL of the form ../foo' ) ' -test_expect_failure '"git submodule sync" handles origin URL of the form ../foo/bar' ' +test_expect_success '"git submodule sync" handles origin URL of the form ../foo/bar' ' (cd relative-clone && git remote set-url origin ../foo/bar && git submodule sync && @@ -154,7 +154,7 @@ test_expect_failure '"git submodule sync" handles origin URL of the form ../foo/ ) ' -test_expect_failure '"git submodule sync" handles origin URL of the form ../foo/bar with deeply nested submodule' ' +test_expect_success '"git submodule sync" handles origin URL of the form ../foo/bar with deeply nested submodule' ' (cd relative-clone && git remote set-url origin ../foo/bar && mkdir -p a/b/c && From 758615e2517db8b9fda9218afded06a3e1b42e20 Mon Sep 17 00:00:00 2001 From: Jon Seymour Date: Wed, 6 Jun 2012 21:57:30 +1000 Subject: [PATCH 4/5] submodule: fix handling of superproject origin URLs like foo, ./foo and ./foo/bar Currently git submodule init and git submodule sync fail with an error if the superproject origin URL is of the form foo but succeed if the superproject origin URL is of the form ./foo or ./foo/bar or foo/bar. This change makes handling of the foo case behave like the handling of the ./foo case and also ensures that superfluous leading and embedded ./'s are removed from the resulting derived URLs. Signed-off-by: Jon Seymour Signed-off-by: Junio C Hamano --- git-submodule.sh | 14 ++++++++++++-- t/t7400-submodule-basic.sh | 6 +++--- t/t7403-submodule-sync.sh | 6 +++--- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 0a3e1465e39..b0b6ccbe723 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -60,8 +60,12 @@ resolve_relative_url () *:*|/*) is_relative= ;; + ./*|../*) + is_relative=t + ;; *) is_relative=t + remoteurl="./$remoteurl" ;; esac @@ -79,7 +83,12 @@ resolve_relative_url () sep=: ;; *) - die "$(eval_gettext "cannot strip one component off url '\$remoteurl'")" + if test -z "$is_relative" || test "." = "$remoteurl" + then + die "$(eval_gettext "cannot strip one component off url '\$remoteurl'")" + else + remoteurl=. + fi ;; esac ;; @@ -90,7 +99,8 @@ resolve_relative_url () break;; esac done - echo "${is_relative:+${up_path}}$remoteurl$sep${url%/}" + remoteurl="$remoteurl$sep${url%/}" + echo "${is_relative:+${up_path}}${remoteurl#./}" } # diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 09e2b9bf16e..a899e6dbf58 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -564,7 +564,7 @@ test_expect_success '../subrepo works with scp-style URL - user@host:path/to/rep ) ' -test_expect_failure '../subrepo works with relative local path - foo' ' +test_expect_success '../subrepo works with relative local path - foo' ' ( cd reltest && cp pristine-.git-config .git/config && @@ -587,7 +587,7 @@ test_expect_success '../subrepo works with relative local path - foo/bar' ' ) ' -test_expect_failure '../subrepo works with relative local path - ./foo' ' +test_expect_success '../subrepo works with relative local path - ./foo' ' ( cd reltest && cp pristine-.git-config .git/config && @@ -598,7 +598,7 @@ test_expect_failure '../subrepo works with relative local path - ./foo' ' ) ' -test_expect_failure '../subrepo works with relative local path - ./foo/bar' ' +test_expect_success '../subrepo works with relative local path - ./foo/bar' ' ( cd reltest && cp pristine-.git-config .git/config && diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh index 98bc74a4e9c..524d5c1b21a 100755 --- a/t/t7403-submodule-sync.sh +++ b/t/t7403-submodule-sync.sh @@ -88,7 +88,7 @@ test_expect_success '"git submodule sync" should not vivify uninteresting submod ) ' -test_expect_failure '"git submodule sync" handles origin URL of the form foo' ' +test_expect_success '"git submodule sync" handles origin URL of the form foo' ' (cd relative-clone && git remote set-url origin foo && git submodule sync && @@ -110,7 +110,7 @@ test_expect_success '"git submodule sync" handles origin URL of the form foo/bar ) ' -test_expect_failure '"git submodule sync" handles origin URL of the form ./foo' ' +test_expect_success '"git submodule sync" handles origin URL of the form ./foo' ' (cd relative-clone && git remote set-url origin ./foo && git submodule sync && @@ -121,7 +121,7 @@ test_expect_failure '"git submodule sync" handles origin URL of the form ./foo' ) ' -test_expect_failure '"git submodule sync" handles origin URL of the form ./foo/bar' ' +test_expect_success '"git submodule sync" handles origin URL of the form ./foo/bar' ' (cd relative-clone && git remote set-url origin ./foo/bar && git submodule sync && From c517e73d0f116f8293f1fbc7f92bb60ea8047b0f Mon Sep 17 00:00:00 2001 From: Johannes Sixt Date: Thu, 14 Jun 2012 14:10:27 +0200 Subject: [PATCH 5/5] t7400: avoid path mangling issues A recently introduced test uses an absolute path. But when run on Windows using the MSYS bash, such a path is mangled into a Windows style path when it is passed to 'git config'. The subsequent 'test' then compares the mangled path to the unmangled version and reports a failure. A path beginning with two slashes denotes a network directory (//server/share path) and is not mangled. Use that trick to side-step the issue. Just in case that 'git submodule init' regresses in such a way that it accesses the URL, use a path name that is unlikely to exist on POSIX systems, and that cannot be a server name on Windows. Signed-off-by: Johannes Sixt Signed-off-by: Junio C Hamano --- t/t7400-submodule-basic.sh | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index a899e6dbf58..c73bec9551e 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -510,14 +510,19 @@ test_expect_success '../subrepo works with port-qualified URL - ssh://hostname:2 ) ' -test_expect_success '../subrepo path works with local path - /foo/repo' ' +# About the choice of the path in the next test: +# - double-slash side-steps path mangling issues on Windows +# - it is still an absolute local path +# - there cannot be a server with a blank in its name just in case the +# path is used erroneously to access a //server/share style path +test_expect_success '../subrepo path works with local path - //somewhere else/repo' ' ( cd reltest && cp pristine-.git-config .git/config && cp pristine-.gitmodules .gitmodules && - git config remote.origin.url /foo/repo && + git config remote.origin.url "//somewhere else/repo" && git submodule init && - test "$(git config submodule.sub.url)" = /foo/subrepo + test "$(git config submodule.sub.url)" = "//somewhere else/subrepo" ) '