{"thread":{"id":"30605","subject":"[PATCH v5 0/2] series: submodule: fix handling of relative superproject origin URLs","startedAt":"2012-05-24T03:37:35Z","lastAt":"2012-05-24T23:07:11Z","messageCount":7,"participants":["Jon Seymour","Phil Hord"],"isPatch":true,"patchVersion":5,"patchTotal":2},"messages":[{"id":"192039","messageId":"1337830657-16400-1-git-send-email-jon.seymour@gmail.com","threadId":"30605","inReplyTo":null,"subject":"[PATCH v5 0/2] series: submodule: fix handling of relative superproject origin URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-24T03:37:35Z","receivedAt":"2012-05-24T03:37:35Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"This series ensures that git submodule handles relative superproject origin URLs correctly.\n\nCurrent behaviour:\n\n* does not guarantee that the submodule.{name}.url property of the superproject is \nalways confgured with a valid path to the submodule's origin repo that is \nrelative to the working tree of the superproject.\n* does not guarantee that the remote.origin.url property of the submodule is\nalways configured with a valid path to the submodule's origin repo that is \nrelative to the working tree of the submodule\n* errors out (origin URL = foo) even though this is not strictly necessary.\n\nThese changes:\n\n* guarantee that the configured paths are always valid relative paths\nfrom the working tree of the repo containing to the configuration to the\norigin repo of the described submodule.\n\nJon Seymour (2):\n  submodule: document failures handling relative superproject origin\n    URLs\n  submodule: fix handling of relative superproject origin URLs\n\n git-submodule.sh           | 57 +++++++++++++++++++++++++++++++++----\n t/t7400-submodule-basic.sh | 60 +++++++++++++++++++++++++++++++++++++++\n t/t7403-submodule-sync.sh  | 70 +++++++++++++++++++++++++++++++++++++++++++++-\n 3 files changed, 180 insertions(+), 7 deletions(-)\n\nv4->v5 changes:\n       Used test_expect_failure for failing tests.\n       Added comments to justify statements.\n       Isolated changes to the relative branch.\n       Used role-based variable names to help explain transformations.\t\n\n-- \n1.7.10.2.649.g5ca7d80\n"},{"id":"192041","messageId":"1337830657-16400-2-git-send-email-jon.seymour@gmail.com","threadId":"30605","inReplyTo":"1337830657-16400-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH v5 1/2] submodule: document failures handling relative superproject origin URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-24T03:37:36Z","receivedAt":"2012-05-24T03:37:36Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"These tests document how submodule sync and init handle various\nkinds of relative super project origin URLs.  The submodule URL\npath is ../subrepo.\n\n6 cases are documented:\n  foo\n  foo/bar\n  ./foo\n  ./foo/bar\n  ../foo\n  ../foo/bar\n\nSigned-off-by: Jon Seymour <jon.seymour@gmail.com>\n---\n t/t7400-submodule-basic.sh | 62 +++++++++++++++++++++++++++++++++++++\n t/t7403-submodule-sync.sh  | 76 +++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 137 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 81827e6..08b8d2f 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -507,6 +507,68 @@ test_expect_success 'relative path works with user@host:path' '\n \t)\n '\n \n+test_expect_failure 'relative path works with foo' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tgit config remote.origin.url foo &&\n+\t\t# actual: fails with an error\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = ./subrepo\n+\t)\n+'\n+\n+test_expect_success 'relative path works with foo/bar' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tgit config remote.origin.url foo/bar &&\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = foo/subrepo\n+\t)\n+'\n+\n+test_expect_success 'relative path works with ./foo' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tgit config remote.origin.url ./foo &&\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = ./subrepo\n+\t)\n+'\n+\n+test_expect_failure 'relative path works with ./foo/bar' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tgit config remote.origin.url ./foo/bar &&\n+\t\tgit submodule init &&\n+\t\t#actual: ./foo/subrepo\n+\t\ttest \"$(git config submodule.sub.url)\" = foo/subrepo\n+\t)\n+'\n+\n+test_expect_success 'relative path works with ../foo' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tgit config remote.origin.url ../foo &&\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = ../subrepo\n+\t)\n+'\n+\n+test_expect_success 'relative path works with ../foo/bar' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tgit config remote.origin.url ../foo/bar &&\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = ../foo/subrepo\n+\t)\n+'\n+\n test_expect_success 'moving the superproject does not break submodules' '\n \t(\n \t\tcd addtest &&\ndiff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh\nindex 3620215..9fa4e58 100755\n--- a/t/t7403-submodule-sync.sh\n+++ b/t/t7403-submodule-sync.sh\n@@ -26,7 +26,9 @@ test_expect_success setup '\n \t(cd super-clone && git submodule update --init) &&\n \tgit clone super empty-clone &&\n \t(cd empty-clone && git submodule init) &&\n-\tgit clone super top-only-clone\n+\tgit clone super top-only-clone &&\n+\tgit clone super relative-clone &&\n+\t(cd relative-clone && git submodule update --init)\n '\n \n test_expect_success 'change submodule' '\n@@ -86,4 +88,76 @@ test_expect_success '\"git submodule sync\" should not vivify uninteresting submod\n \t)\n '\n \n+test_expect_failure '\"git submodule sync\" handles origin URL of the form foo' '\n+\t(cd relative-clone &&\n+\t git remote set-url origin foo\n+\t git submodule sync &&\n+\t(cd submodule &&\n+\t #actual fails with: \"cannot strip off url foo\n+\t git config remote.origin.url &&\n+\t test \"$(git config remote.origin.url)\" == \"../submodule\"\n+\t)\n+\t)\n+'\n+\n+test_expect_failure '\"git submodule sync\" handles origin URL of the form foo/bar' '\n+\t(cd relative-clone &&\n+\t git remote set-url origin foo/bar\n+\t git submodule sync &&\n+\t(cd submodule &&\n+\t #actual foo/submodule\n+\t git config remote.origin.url &&\n+\t test \"$(git config remote.origin.url)\" == \"../foo/submodule\"\n+\t)\n+\t)\n+'\n+\n+test_expect_failure '\"git submodule sync\" handles origin URL of the form ./foo' '\n+\t(cd relative-clone &&\n+\t git remote set-url origin ./foo\n+\t git submodule sync &&\n+\t(cd submodule &&\n+\t #actual ./submodule\n+\t git config remote.origin.url &&\n+\t test \"$(git config remote.origin.url)\" == \"../submodule\"\n+\t)\n+\t)\n+'\n+\n+test_expect_failure '\"git submodule sync\" handles origin URL of the form ./foo/bar' '\n+\t(cd relative-clone &&\n+\t git remote set-url origin ./foo/bar\n+\t git submodule sync &&\n+\t(cd submodule &&\n+\t #actual ./foo/submodule\n+\t git config remote.origin.url &&\n+\t test \"$(git config remote.origin.url)\" == \"../foo/submodule\"\n+\t)\n+\t)\n+'\n+\n+test_expect_failure '\"git submodule sync\" handles origin URL of the form ../foo' '\n+\t(cd relative-clone &&\n+\t git remote set-url origin ../foo\n+\t git submodule sync &&\n+\t(cd submodule &&\n+\t #actual ../submodule\n+\t git config remote.origin.url &&\n+\t test \"$(git config remote.origin.url)\" == \"../../submodule\"\n+\t)\n+\t)\n+'\n+\n+test_expect_failure '\"git submodule sync\" handles origin URL of the form ../foo/bar' '\n+\t(cd relative-clone &&\n+\t git remote set-url origin ../foo/bar\n+\t git submodule sync &&\n+\t(cd submodule &&\n+\t #actual ../foo/submodule\n+\t git config remote.origin.url &&\n+\t test \"$(git config remote.origin.url)\" == \"../../foo/submodule\"\n+\t)\n+\t)\n+'\n+\n test_done\n-- \n1.7.10.2.649.g5ca7d80\n"},{"id":"192040","messageId":"1337830657-16400-3-git-send-email-jon.seymour@gmail.com","threadId":"30605","inReplyTo":"1337830657-16400-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH v5 2/2] submodule: fix handling of relative superproject origin URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-24T03:37:37Z","receivedAt":"2012-05-24T03:37:37Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"When the origin URL of the superproject is itself relative, an operation\nsuch as git submodule add, init or sync may result in either the\nsubmodule.{name}.url configuration property of the superproject\nreferring to the incorrect location or remote.origin.url configuration\nproperty of the submodule referring to the incorrect location or both\nthese conditions. In some cases, git submodule fails to update\nthe configuration and fails with an error condition.\n\nThe issue arises in these cases because the origin URL of\nthe superproject needs to be prepended with a prefix that navigates\nfrom the submodule to the superproject so that when the submodule\nURL is concatenated the resulting URL is relative to the working tree\nof the submodule.\n\nThis change fixes handling for relative superproject origin URLs\nfor 6 cases:\n  foo\n  foo/bar\n  ./foo\n  ./foo/bar\n  ../foo\n  ../foo/bar\n\nIn each case, the configuration properties in the superproject's\nconfiguration and the submodule's configuration refer to the\ncorrect, relative, location of the submodule's origin repo. In all cases,\nthe configured paths are relative to the working trees of the\nrepositories containing the configuration.\n\nSigned-off-by: Jon Seymour <jon.seymour@gmail.com>\n---\n git-submodule.sh           | 57 +++++++++++++++++++++++++++++++++++++++++-----\n t/t7400-submodule-basic.sh |  6 ++---\n t/t7403-submodule-sync.sh  | 18 +++++----------\n 3 files changed, 59 insertions(+), 22 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 64a70d6..738eba3 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -30,13 +30,35 @@ nofetch=\n update=\n prefix=\n \n-# Resolve relative url by appending to parent's url\n+# Resolve relative url by appending the submodule url\n+# to the superproject's origin URL\n+#\n+# If the origin URL is itself a relative URL prepend\n+# an additional prefix, if present, that represents\n+# the relative path from the submodule's working tree\n+# to the superprojects' working tree.\n+#\n+# This behaviour is required to ensure that the origin URL\n+# of a submodule, when relative, is relative to the\n+# submodule's work tree and not to the superproject's work tree.\n+#\n resolve_relative_url ()\n {\n \tremote=$(get_default_remote)\n \tremoteurl=$(git config \"remote.$remote.url\") ||\n \t\tremoteurl=$(pwd) # the repository is its own authoritative upstream\n \turl=\"$1\"\n+\tup_path=\"$2\"\n+\n+\t#\n+\t# ensure all relative paths begin with ./ to enable\n+\t# selection relative branch of subsequent case \"$remoteurl\"\n+\t# statement.\n+\t#\n+\t# rewrites foo/bar to ./foo/bar but leaves /foo, :foo ./foo\n+\t# and ../foo untouched.\n+\t#\n+\tremoteurl=$(echo \"$remoteurl\" | sed \"s|^[^/:\\\\.][^:]*\\$|./&|\")\n \tremoteurl=${remoteurl%/}\n \tsep=/\n \twhile test -n \"$url\"\n@@ -45,6 +67,16 @@ resolve_relative_url ()\n \t\t../*)\n \t\t\turl=\"${url#../}\"\n \t\t\tcase \"$remoteurl\" in\n+\t\t\t.*/*)\n+\t\t\t\t# remove last part\n+\t\t\t\tremoteurl=\"${remoteurl%/*}\"\n+\t\t\t\t# remove redundant leading ./\n+\t\t\t\tremoteurl=\"${remoteurl#./}\"\n+\t\t\t\t# prefix path from submodule work tree to superproject work tree\n+\t\t\t\tremoteurl=\"${up_path}${remoteurl}\"\n+\t\t\t\t# remove trailing /.\n+\t\t\t\tremoteurl=\"${remoteurl%/.}\"\n+\t\t\t\t;;\n \t\t\t*/*)\n \t\t\t\tremoteurl=\"${remoteurl%/*}\"\n \t\t\t\t;;\n@@ -959,19 +991,32 @@ cmd_sync()\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tname=$(module_name \"$sm_path\")\n-\t\turl=$(git config -f .gitmodules --get submodule.\"$name\".url)\n+\t\t# path from superproject origin repo to submodule origin repo\n+\t\tmodule_url=$(git config -f .gitmodules --get submodule.\"$name\".url)\n \n \t\t# Possibly a url relative to parent\n-\t\tcase \"$url\" in\n+\t\tcase \"$module_url\" in\n \t\t./*|../*)\n-\t\t\turl=$(resolve_relative_url \"$url\") || exit\n+\t\t\t# rewrite foo/bar as ../.. to find path from\n+\t\t\t# submodule work tree to superproject work tree\n+\t\t\tup_path=\"$(echo \"$sm_path\" | sed \"s/[^/]*/../g\")\" &&\n+\t\t\t# guarantee a trailing /\n+\t\t\tup_path=${up_path%/}/ &&\n+\t\t\t# path from submodule work tree to submodule origin repo\n+\t\t\tsub_origin_url=$(resolve_relative_url \"$module_url\" \"$up_path\") &&\n+\t\t\t# path from superproject work tree to submodule origin repo\n+\t\t\tsuper_config_url=$(resolve_relative_url \"$module_url\") || exit\n+\t\t\t;;\n+\t\t*)\n+\t\t\tsub_origin_url=\"$module_url\"\n+\t\t\tsuper_config_url=\"$module_url\"\n \t\t\t;;\n \t\tesac\n \n \t\tif git config \"submodule.$name.url\" >/dev/null 2>/dev/null\n \t\tthen\n \t\t\tsay \"$(eval_gettext \"Synchronizing submodule url for '\\$name'\")\"\n-\t\t\tgit config submodule.\"$name\".url \"$url\"\n+\t\t\tgit config submodule.\"$name\".url \"$super_config_url\"\n \n \t\t\tif test -e \"$sm_path\"/.git\n \t\t\tthen\n@@ -979,7 +1024,7 @@ cmd_sync()\n \t\t\t\tclear_local_git_env\n \t\t\t\tcd \"$sm_path\"\n \t\t\t\tremote=$(get_default_remote)\n-\t\t\t\tgit config remote.\"$remote\".url \"$url\"\n+\t\t\t\tgit config remote.\"$remote\".url \"$sub_origin_url\"\n \t\t\t)\n \t\t\tfi\n \t\tfi\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 08b8d2f..f2f907f 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -507,12 +507,11 @@ test_expect_success 'relative path works with user@host:path' '\n \t)\n '\n \n-test_expect_failure 'relative path works with foo' '\n+test_expect_success 'relative path works with foo' '\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n \t\tgit config remote.origin.url foo &&\n-\t\t# actual: fails with an error\n \t\tgit submodule init &&\n \t\ttest \"$(git config submodule.sub.url)\" = ./subrepo\n \t)\n@@ -538,13 +537,12 @@ test_expect_success 'relative path works with ./foo' '\n \t)\n '\n \n-test_expect_failure 'relative path works with ./foo/bar' '\n+test_expect_success 'relative path works with ./foo/bar' '\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n \t\tgit config remote.origin.url ./foo/bar &&\n \t\tgit submodule init &&\n-\t\t#actual: ./foo/subrepo\n \t\ttest \"$(git config submodule.sub.url)\" = foo/subrepo\n \t)\n '\ndiff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh\nindex 9fa4e58..9526f8d 100755\n--- a/t/t7403-submodule-sync.sh\n+++ b/t/t7403-submodule-sync.sh\n@@ -88,72 +88,66 @@ test_expect_success '\"git submodule sync\" should not vivify uninteresting submod\n \t)\n '\n \n-test_expect_failure '\"git submodule sync\" handles origin URL of the form foo' '\n+test_expect_success '\"git submodule sync\" handles origin URL of the form foo' '\n \t(cd relative-clone &&\n \t git remote set-url origin foo\n \t git submodule sync &&\n \t(cd submodule &&\n-\t #actual fails with: \"cannot strip off url foo\n \t git config remote.origin.url &&\n \t test \"$(git config remote.origin.url)\" == \"../submodule\"\n \t)\n \t)\n '\n \n-test_expect_failure '\"git submodule sync\" handles origin URL of the form foo/bar' '\n+test_expect_success '\"git submodule sync\" handles origin URL of the form foo/bar' '\n \t(cd relative-clone &&\n \t git remote set-url origin foo/bar\n \t git submodule sync &&\n \t(cd submodule &&\n-\t #actual foo/submodule\n \t git config remote.origin.url &&\n \t test \"$(git config remote.origin.url)\" == \"../foo/submodule\"\n \t)\n \t)\n '\n \n-test_expect_failure '\"git submodule sync\" handles origin URL of the form ./foo' '\n+test_expect_success '\"git submodule sync\" handles origin URL of the form ./foo' '\n \t(cd relative-clone &&\n \t git remote set-url origin ./foo\n \t git submodule sync &&\n \t(cd submodule &&\n-\t #actual ./submodule\n \t git config remote.origin.url &&\n \t test \"$(git config remote.origin.url)\" == \"../submodule\"\n \t)\n \t)\n '\n \n-test_expect_failure '\"git submodule sync\" handles origin URL of the form ./foo/bar' '\n+test_expect_success '\"git submodule sync\" handles origin URL of the form ./foo/bar' '\n \t(cd relative-clone &&\n \t git remote set-url origin ./foo/bar\n \t git submodule sync &&\n \t(cd submodule &&\n-\t #actual ./foo/submodule\n \t git config remote.origin.url &&\n \t test \"$(git config remote.origin.url)\" == \"../foo/submodule\"\n \t)\n \t)\n '\n \n-test_expect_failure '\"git submodule sync\" handles origin URL of the form ../foo' '\n+test_expect_success '\"git submodule sync\" handles origin URL of the form ../foo' '\n \t(cd relative-clone &&\n \t git remote set-url origin ../foo\n \t git submodule sync &&\n \t(cd submodule &&\n-\t #actual ../submodule\n \t git config remote.origin.url &&\n \t test \"$(git config remote.origin.url)\" == \"../../submodule\"\n \t)\n \t)\n '\n \n-test_expect_failure '\"git submodule sync\" handles origin URL of the form ../foo/bar' '\n+test_expect_success '\"git submodule sync\" handles origin URL of the form ../foo/bar' '\n \t(cd relative-clone &&\n \t git remote set-url origin ../foo/bar\n \t git submodule sync &&\n \t(cd submodule &&\n-\t #actual ../foo/submodule\n \t git config remote.origin.url &&\n \t test \"$(git config remote.origin.url)\" == \"../../foo/submodule\"\n \t)\n-- \n1.7.10.2.649.g5ca7d80\n"},{"id":"192042","messageId":"CAH3AnrpweAah9GojN2QodTx6qW2rHE4wadxbc23zrwj=uPCP-A@mail.gmail.com","threadId":"30605","inReplyTo":"1337830657-16400-3-git-send-email-jon.seymour@gmail.com","subject":"Re: [PATCH v5 2/2] submodule: fix handling of relative superproject origin URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-24T03:40:42Z","receivedAt":"2012-05-24T03:40:42Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"On Thu, May 24, 2012 at 1:37 PM, Jon Seymour <jon.seymour@gmail.com> wrote:\n> When the origin URL of the superproject is itself relative, an operation\n> such as git submodule add, init or sync may result in either the\n> submodule.{name}.url configuration property of the superproject\n> referring to the incorrect location or remote.origin.url configuration\n> property of the submodule referring to the incorrect location or both\n> these conditions. In some cases, git submodule fails to update\n> the configuration and fails with an error condition.\n>\n> The issue arises in these cases because the origin URL of\n> the superproject needs to be prepended with a prefix that navigates\n> from the submodule to the superproject so that when the submodule\n> URL is concatenated the resulting URL is relative to the working tree\n> of the submodule.\n>\n> This change fixes handling for relative superproject origin URLs\n> for 6 cases:\n>  foo\n>  foo/bar\n>  ./foo\n>  ./foo/bar\n>  ../foo\n>  ../foo/bar\n>\n> In each case, the configuration properties in the superproject's\n> configuration and the submodule's configuration refer to the\n> correct, relative, location of the submodule's origin repo. In all cases,\n> the configured paths are relative to the working trees of the\n> repositories containing the configuration.\n>\n> Signed-off-by: Jon Seymour <jon.seymour@gmail.com>\n> ---\n>  git-submodule.sh           | 57 +++++++++++++++++++++++++++++++++++++++++-----\n>  t/t7400-submodule-basic.sh |  6 ++---\n>  t/t7403-submodule-sync.sh  | 18 +++++----------\n>  3 files changed, 59 insertions(+), 22 deletions(-)\n>\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 64a70d6..738eba3 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -30,13 +30,35 @@ nofetch=\n>  update=\n>  prefix=\n>\n> -# Resolve relative url by appending to parent's url\n> +# Resolve relative url by appending the submodule url\n> +# to the superproject's origin URL\n> +#\n> +# If the origin URL is itself a relative URL prepend\n> +# an additional prefix, if present, that represents\n> +# the relative path from the submodule's working tree\n> +# to the superprojects' working tree.\n> +#\n> +# This behaviour is required to ensure that the origin URL\n> +# of a submodule, when relative, is relative to the\n> +# submodule's work tree and not to the superproject's work tree.\n> +#\n>  resolve_relative_url ()\n>  {\n>        remote=$(get_default_remote)\n>        remoteurl=$(git config \"remote.$remote.url\") ||\n>                remoteurl=$(pwd) # the repository is its own authoritative upstream\n>        url=\"$1\"\n> +       up_path=\"$2\"\n> +\n> +       #\n> +       # ensure all relative paths begin with ./ to enable\n> +       # selection relative branch of subsequent case \"$remoteurl\"\n> +       # statement.\n> +       #\n> +       # rewrites foo/bar to ./foo/bar but leaves /foo, :foo ./foo\n> +       # and ../foo untouched.\n> +       #\n> +       remoteurl=$(echo \"$remoteurl\" | sed \"s|^[^/:\\\\.][^:]*\\$|./&|\")\n>        remoteurl=${remoteurl%/}\n>        sep=/\n>        while test -n \"$url\"\n> @@ -45,6 +67,16 @@ resolve_relative_url ()\n>                ../*)\n>                        url=\"${url#../}\"\n>                        case \"$remoteurl\" in\n> +                       .*/*)\n> +                               # remove last part\n> +                               remoteurl=\"${remoteurl%/*}\"\n> +                               # remove redundant leading ./\n> +                               remoteurl=\"${remoteurl#./}\"\n> +                               # prefix path from submodule work tree to superproject work tree\n> +                               remoteurl=\"${up_path}${remoteurl}\"\n> +                               # remove trailing /.\n> +                               remoteurl=\"${remoteurl%/.}\"\n> +                               ;;\n>                        */*)\n>                                remoteurl=\"${remoteurl%/*}\"\n>                                ;;\n> @@ -959,19 +991,32 @@ cmd_sync()\n>        while read mode sha1 stage sm_path\n>        do\n>                name=$(module_name \"$sm_path\")\n> -               url=$(git config -f .gitmodules --get submodule.\"$name\".url)\n> +               # path from superproject origin repo to submodule origin repo\n> +               module_url=$(git config -f .gitmodules --get submodule.\"$name\".url)\n>\n>                # Possibly a url relative to parent\n> -               case \"$url\" in\n> +               case \"$module_url\" in\n>                ./*|../*)\n> -                       url=$(resolve_relative_url \"$url\") || exit\n> +                       # rewrite foo/bar as ../.. to find path from\n> +                       # submodule work tree to superproject work tree\n> +                       up_path=\"$(echo \"$sm_path\" | sed \"s/[^/]*/../g\")\" &&\n> +                       # guarantee a trailing /\n> +                       up_path=${up_path%/}/ &&\n> +                       # path from submodule work tree to submodule origin repo\n> +                       sub_origin_url=$(resolve_relative_url \"$module_url\" \"$up_path\") &&\n> +                       # path from superproject work tree to submodule origin repo\n> +                       super_config_url=$(resolve_relative_url \"$module_url\") || exit\n> +                       ;;\n> +               *)\n> +                       sub_origin_url=\"$module_url\"\n> +                       super_config_url=\"$module_url\"\n>                        ;;\n>                esac\n>\n>                if git config \"submodule.$name.url\" >/dev/null 2>/dev/null\n>                then\n>                        say \"$(eval_gettext \"Synchronizing submodule url for '\\$name'\")\"\n> -                       git config submodule.\"$name\".url \"$url\"\n> +                       git config submodule.\"$name\".url \"$super_config_url\"\n>\n>                        if test -e \"$sm_path\"/.git\n>                        then\n> @@ -979,7 +1024,7 @@ cmd_sync()\n>                                clear_local_git_env\n>                                cd \"$sm_path\"\n>                                remote=$(get_default_remote)\n> -                               git config remote.\"$remote\".url \"$url\"\n> +                               git config remote.\"$remote\".url \"$sub_origin_url\"\n>                        )\n>                        fi\n>                fi\n> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\n> index 08b8d2f..f2f907f 100755\n> --- a/t/t7400-submodule-basic.sh\n> +++ b/t/t7400-submodule-basic.sh\n> @@ -507,12 +507,11 @@ test_expect_success 'relative path works with user@host:path' '\n>        )\n>  '\n>\n> -test_expect_failure 'relative path works with foo' '\n> +test_expect_success 'relative path works with foo' '\n>        (\n>                cd reltest &&\n>                cp pristine-.git-config .git/config &&\n>                git config remote.origin.url foo &&\n> -               # actual: fails with an error\n>                git submodule init &&\n>                test \"$(git config submodule.sub.url)\" = ./subrepo\n>        )\n> @@ -538,13 +537,12 @@ test_expect_success 'relative path works with ./foo' '\n>        )\n>  '\n>\n> -test_expect_failure 'relative path works with ./foo/bar' '\n> +test_expect_success 'relative path works with ./foo/bar' '\n>        (\n>                cd reltest &&\n>                cp pristine-.git-config .git/config &&\n>                git config remote.origin.url ./foo/bar &&\n>                git submodule init &&\n> -               #actual: ./foo/subrepo\n>                test \"$(git config submodule.sub.url)\" = foo/subrepo\n>        )\n>  '\n> diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh\n> index 9fa4e58..9526f8d 100755\n> --- a/t/t7403-submodule-sync.sh\n> +++ b/t/t7403-submodule-sync.sh\n> @@ -88,72 +88,66 @@ test_expect_success '\"git submodule sync\" should not vivify uninteresting submod\n>        )\n>  '\n>\n> -test_expect_failure '\"git submodule sync\" handles origin URL of the form foo' '\n> +test_expect_success '\"git submodule sync\" handles origin URL of the form foo' '\n>        (cd relative-clone &&\n>         git remote set-url origin foo\n>         git submodule sync &&\n>        (cd submodule &&\n> -        #actual fails with: \"cannot strip off url foo\n>         git config remote.origin.url &&\n\nSorry, it appears I left some debug output in the tests. Will roll\nthis into a v6 with any updates you suggest.\n\nRegards,\n\njon.\n"},{"id":"192100","messageId":"CABURp0pE7TjSOx7aWNqak4iyK5dBS-70wf0xvBU4KXtsGu5tRw@mail.gmail.com","threadId":"30605","inReplyTo":"1337830657-16400-3-git-send-email-jon.seymour@gmail.com","subject":"Re: [PATCH v5 2/2] submodule: fix handling of relative superproject origin URLs","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2012-05-24T18:58:25Z","receivedAt":"2012-05-24T18:58:25Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Wed, May 23, 2012 at 11:37 PM, Jon Seymour <jon.seymour@gmail.com> wrote:\n> -# Resolve relative url by appending to parent's url\n> +# Resolve relative url by appending the submodule url\n> +# to the superproject's origin URL\n> +#\n> +# If the origin URL is itself a relative URL prepend\n> +# an additional prefix, if present, that represents\n> +# the relative path from the submodule's working tree\n> +# to the superprojects' working tree.\n> +#\n> +# This behaviour is required to ensure that the origin URL\n\n\"Required behavior\" always seems overstated to me when I\nread it in comments and so I tend to distrust it.  I'd prefer to\nsee \"This behaviour is intended to ensure...\"  But this is\nonly my personal preference.\n\n\n> +# of a submodule, when relative, is relative to the\n> +# submodule's work tree and not to the superproject's work tree.\n> +#\n>  resolve_relative_url ()\n>  {\n>        remote=$(get_default_remote)\n>        remoteurl=$(git config \"remote.$remote.url\") ||\n>                remoteurl=$(pwd) # the repository is its own authoritative upstream\n>        url=\"$1\"\n> +       up_path=\"$2\"\n> +\n> +       #\n> +       # ensure all relative paths begin with ./ to enable\n\nAre we talking about remote urls or local filesystem paths?  I think this\nis all confusing enough without the comments also using terminology\ninconsistently.  Would it be more correct to say \"all relatiive urls\" here?\nOr is this function only interested in local filesystem paths?\n\nI realize that urls are paths of a different nature.  I pick this nit\nonly in the cause of clarity.\n\n> +       # selection relative branch of subsequent case \"$remoteurl\"\n> +       # statement.\n> +       #\n> +       # rewrites foo/bar to ./foo/bar but leaves /foo, :foo ./foo\n> +       # and ../foo untouched.\n> +       #\n\nIn many filesystems, \":foo\" and \"\\foo\" are valid filenames.\nI suspect it is unwise to employ them and expect them not\nto cause trouble, so I don't know if we should make\nspecial efforts to accept them here.  But I think it\nis worth noting.\n\nOn the other hand, I do not know how these special\ncharacters are represented in urls.\n\n> +       remoteurl=$(echo \"$remoteurl\" | sed \"s|^[^/:\\\\.][^:]*\\$|./&|\")\n>        remoteurl=${remoteurl%/}\n>        sep=/\n>        while test -n \"$url\"\n> @@ -45,6 +67,16 @@ resolve_relative_url ()\n>                ../*)\n>                        url=\"${url#../}\"\n>                        case \"$remoteurl\" in\n> +                       .*/*)\n> +                               # remove last part\n> +                               remoteurl=\"${remoteurl%/*}\"\n> +                               # remove redundant leading ./\n> +                               remoteurl=\"${remoteurl#./}\"\n> +                               # prefix path from submodule work tree to superproject work tree\n> +                               remoteurl=\"${up_path}${remoteurl}\"\n\nHere we seem to be talking about paths since we are concerned\nwith the worktrees.  So maybe my earlier concern about \"urls\"\nvs. \"paths\" was miguided.  My head swims.\n\n\n> +                               # remove trailing /.\n> +                               remoteurl=\"${remoteurl%/.}\"\n> +                               ;;\n>                        */*)\n>                                remoteurl=\"${remoteurl%/*}\"\n>                                ;;\n\nI haven't tested it, but the rest of this is making more sense to me now.\n\nPhil\n"},{"id":"192128","messageId":"CAH3Anror5Q0HYWXGX+-Es2n3zabST5k35cATKrTZzW30dN85oQ@mail.gmail.com","threadId":"30605","inReplyTo":"CABURp0pE7TjSOx7aWNqak4iyK5dBS-70wf0xvBU4KXtsGu5tRw@mail.gmail.com","subject":"Re: [PATCH v5 2/2] submodule: fix handling of relative superproject origin URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-24T22:09:12Z","receivedAt":"2012-05-24T22:09:12Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"On Fri, May 25, 2012 at 4:58 AM, Phil Hord <phil.hord@gmail.com> wrote:\n> On Wed, May 23, 2012 at 11:37 PM, Jon Seymour <jon.seymour@gmail.com> wrote:\n>> +#\n>> +# This behaviour is required to ensure that the origin URL\n>\n> \"Required behavior\" always seems overstated to me when I\n> read it in comments and so I tend to distrust it.  I'd prefer to\n> see \"This behaviour is intended to ensure...\"  But this is\n> only my personal preference.\n\nSure.\n\n>> +       #\n>> +       # ensure all relative paths begin with ./ to enable\n>\n\nFor clarity, this should be:\n\n    ensure all relative superproject origin URLs begin with ./ to enable ...\n\n> Are we talking about remote urls or local filesystem paths?  I think this\n> is all confusing enough without the comments also using terminology\n> inconsistently.  Would it be more correct to say \"all relatiive urls\" here?\n> Or is this function only interested in local filesystem paths?\n>\n> I realize that urls are paths of a different nature.  I pick this nit\n> only in the cause of clarity.\n\nHow about I rewrite the function blurb as something like:\n\n\"The function takes at most 2 arguments. The first argument is the\nrelative URL that navigates from the superproject origin repo to the\nsubmodule origin repo. The second up_path argument, if specified, is\nthe relative path that navigates from the submodule working tree to\nthe superproject working tree.\n\nThe output of the function is the origin URL of the submodule.\n\nThe output will either be an absolute URL or filesystem path (if the\nsuperproject origin URL is an absolute URL or filesystem path,\nrespectively) or a relative file system path (if the superproject\norigin URL is a relative file system path).\n\nWhen the output is a relative file system path, the path is either\nrelative to the submodule working tree, if up_path is specified, or to\nthe superproject working tree otherwise.\"\n\nPhil/Jens: Perhaps we should rename the function to be:\nsubmodule_origin_url to reflect its intended use?\n\n>\n>> +       # selection relative branch of subsequent case \"$remoteurl\"\n>> +       # statement.\n>> +       #\n>> +       # rewrites foo/bar to ./foo/bar but leaves /foo, :foo ./foo\n>> +       # and ../foo untouched.\n>> +       #\n>\n> In many filesystems, \":foo\" and \"\\foo\" are valid filenames.\n> I suspect it is unwise to employ them and expect them not\n> to cause trouble, so I don't know if we should make\n> special efforts to accept them here.  But I think it\n> is worth noting.\n>\n> On the other hand, I do not know how these special\n> characters are represented in urls.\n\nPerhaps I should indicate 'why' these paths are not touched? Something like:\n\n\"Rewrites foo/bar foo/bar as ./foo/bar but leaves ./foo, ../foo, /foo,\nfoo:bar, :bar untouched because the first 2 forms do not require\nadditional qualification and the last forms\nare assumed to be absolute URLs or paths.\"\n\nPerhaps I shouldn't be treating :foo as a possible absolute file\nsystem path ? My knowledge of filesystems is not good enough to know\nif there are filesystems that git supports where :foo might be\nregarded a valid absolute path?\n\n>\n>> +       remoteurl=$(echo \"$remoteurl\" | sed \"s|^[^/:\\\\.][^:]*\\$|./&|\")\n>>        remoteurl=${remoteurl%/}\n>>        sep=/\n>>        while test -n \"$url\"\n>> @@ -45,6 +67,16 @@ resolve_relative_url ()\n>>                ../*)\n>>                        url=\"${url#../}\"\n>>                        case \"$remoteurl\" in\n>> +                       .*/*)\n>> +                               # remove last part\n>> +                               remoteurl=\"${remoteurl%/*}\"\n>> +                               # remove redundant leading ./\n>> +                               remoteurl=\"${remoteurl#./}\"\n>> +                               # prefix path from submodule work tree to superproject work tree\n>> +                               remoteurl=\"${up_path}${remoteurl}\"\n>\n> Here we seem to be talking about paths since we are concerned\n> with the worktrees.  So maybe my earlier concern about \"urls\"\n> vs. \"paths\" was miguided.  My head swims.\n>\n\nThis branch of the case statement is specifically when the origin\nsuperproject URL is a relative file system path.\n\n>\n>> +                               # remove trailing /.\n>> +                               remoteurl=\"${remoteurl%/.}\"\n>> +                               ;;\n>>                        */*)\n>>                                remoteurl=\"${remoteurl%/*}\"\n>>                                ;;\n>\n> I haven't tested it, but the rest of this is making more sense to me now.\n>\n> Phil\n\nThanks for your review and suggestions.\n\njon.\n"},{"id":"192132","messageId":"CAH3AnrqBtzALZjk_sRDbCfHrPmnGbdstUuKWaHiaJJayfCw9=Q@mail.gmail.com","threadId":"30605","inReplyTo":"1337830657-16400-3-git-send-email-jon.seymour@gmail.com","subject":"Re: [PATCH v5 2/2] submodule: fix handling of relative superproject origin URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-24T23:07:11Z","receivedAt":"2012-05-24T23:07:11Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"On Thu, May 24, 2012 at 1:37 PM, Jon Seymour <jon.seymour@gmail.com> wrote:\n>        sep=/\n>        while test -n \"$url\"\n> @@ -45,6 +67,16 @@ resolve_relative_url ()\n>                ../*)\n>                        url=\"${url#../}\"\n>                        case \"$remoteurl\" in\n> +                       .*/*)\n> +                               # remove last part\n> +                               remoteurl=\"${remoteurl%/*}\"\n> +                               # remove redundant leading ./\n> +                               remoteurl=\"${remoteurl#./}\"\n> +                               # prefix path from submodule work tree to superproject work tree\n> +                               remoteurl=\"${up_path}${remoteurl}\"\n> +                               # remove trailing /.\n> +                               remoteurl=\"${remoteurl%/.}\"\n> +                               ;;\n\nMmmm. Probably shouldn't be doing these _all_ of these edits on each\niteration of the while loop, methinks.\n\nI'll write a test, and fix.\n\njon.\n"}]}