{"thread":{"id":"30600","subject":"[PATCH v4 0/2] submodule: fix handling of relative superproject origin URLs","startedAt":"2012-05-23T16:45:52Z","lastAt":"2012-05-24T02:32:31Z","messageCount":10,"participants":["Jon Seymour","Jens Lehmann","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":2},"messages":[{"id":"192002","messageId":"1337791554-31294-1-git-send-email-jon.seymour@gmail.com","threadId":"30600","inReplyTo":null,"subject":"[PATCH v4 0/2] submodule: fix handling of relative superproject origin URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-23T16:45:52Z","receivedAt":"2012-05-23T16:45:52Z","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 handling of relative superproject origin URLs\n  submodule: fix handling of relative superproject origin URLs\n\n git-submodule.sh           | 17 ++++++++++--\n t/t7400-submodule-basic.sh | 60 +++++++++++++++++++++++++++++++++++++++++++\n t/t7403-submodule-sync.sh  | 64 +++++++++++++++++++++++++++++++++++++++++++++-\n 3 files changed, 138 insertions(+), 3 deletions(-)\n\n-- \n1.7.10.2.594.g24e850d\n"},{"id":"192003","messageId":"1337791554-31294-2-git-send-email-jon.seymour@gmail.com","threadId":"30600","inReplyTo":"1337791554-31294-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH v4 1/2] submodule: document handling of relative superproject origin URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-23T16:45:53Z","receivedAt":"2012-05-23T16:45:53Z","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  | 63 +++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 124 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 81827e6..97e7a73 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_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\techo \\\"cannot strip one component off url 'foo'\\\" >expect &&\n+\t\ttest_must_fail git submodule init 2>actual &&\n+\t\tcat actual &&\n+\t\ttest_cmp expect actual\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_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_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..3784c9b 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,63 @@ test_expect_success '\"git submodule sync\" should not vivify uninteresting submod\n \t)\n '\n \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 echo \\\"cannot strip one component off url 'foo'\\\" > expect &&\n+\t test_must_fail git submodule sync 2> actual &&\n+\t test_cmp expect actual\n+\t)\n+\"\n+\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 test \"$(git config remote.origin.url)\" == \"foo/submodule\"\n+\t)\n+\t)\n+'\n+\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 test \"$(git config remote.origin.url)\" == \"./submodule\"\n+\t)\n+\t)\n+'\n+\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 test \"$(git config remote.origin.url)\" == \"./foo/submodule\"\n+\t)\n+\t)\n+'\n+\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 test \"$(git config remote.origin.url)\" == \"../submodule\"\n+\t)\n+\t)\n+'\n+\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 test \"$(git config remote.origin.url)\" == \"../foo/submodule\"\n+\t)\n+\t)\n+'\n+\n test_done\n-- \n1.7.10.2.594.g24e850d\n"},{"id":"192004","messageId":"1337791554-31294-3-git-send-email-jon.seymour@gmail.com","threadId":"30600","inReplyTo":"1337791554-31294-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH v4 2/2] submodule: fix handling of relative superproject origin URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-23T16:45:54Z","receivedAt":"2012-05-23T16:45:54Z","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           | 17 +++++++++++++++--\n t/t7400-submodule-basic.sh | 12 +++++-------\n t/t7403-submodule-sync.sh  | 21 +++++++++++----------\n 3 files changed, 31 insertions(+), 19 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 64a70d6..3e943de 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -37,6 +37,8 @@ resolve_relative_url ()\n \tremoteurl=$(git config \"remote.$remote.url\") ||\n \t\tremoteurl=$(pwd) # the repository is its own authoritative upstream\n \turl=\"$1\"\n+\tprefix=\"$2\"\n+\tremoteurl=$(echo \"$remoteurl\" | sed \"s|^[^/:\\\\.][^:]*\\$|./&|\")\n \tremoteurl=${remoteurl%/}\n \tsep=/\n \twhile test -n \"$url\"\n@@ -45,6 +47,11 @@ 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\tremoteurl=\"${remoteurl%/*}\"\n+\t\t\t\tremoteurl=\"${remoteurl#./}\"\n+\t\t\t\tremoteurl=\"${prefix}${remoteurl}\"\n+\t\t\t\t;;\n \t\t\t*/*)\n \t\t\t\tremoteurl=\"${remoteurl%/*}\"\n \t\t\t\t;;\n@@ -64,7 +71,7 @@ resolve_relative_url ()\n \t\t\tbreak;;\n \t\tesac\n \tdone\n-\techo \"$remoteurl$sep${url%/}\"\n+\techo \"${remoteurl%/.}$sep${url%/}\"\n }\n \n #\n@@ -964,8 +971,14 @@ cmd_sync()\n \t\t# Possibly a url relative to parent\n \t\tcase \"$url\" in\n \t\t./*|../*)\n+\t\t\tup_path=\"$(echo \"$sm_path\" | sed \"s/[^/]*/../g\")\" &&\n+\t\t\tup_path=${up_path%/}/ &&\n+\t\t\tremoteurl=$(resolve_relative_url \"$url\" \"$up_path\") &&\n \t\t\turl=$(resolve_relative_url \"$url\") || exit\n \t\t\t;;\n+\t\t*)\n+\t\t\tremoteurl=\"$url\"\n+\t\t\t;;\n \t\tesac\n \n \t\tif git config \"submodule.$name.url\" >/dev/null 2>/dev/null\n@@ -979,7 +992,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 \"$remoteurl\"\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 97e7a73..f2f907f 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -507,17 +507,15 @@ test_expect_success 'relative path works with user@host:path' '\n \t)\n '\n \n-test_expect_success '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\techo \\\"cannot strip one component off url 'foo'\\\" >expect &&\n-\t\ttest_must_fail git submodule init 2>actual &&\n-\t\tcat actual &&\n-\t\ttest_cmp expect actual\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = ./subrepo\n \t)\n-\"\n+'\n \n test_expect_success 'relative path works with foo/bar' '\n \t(\n@@ -545,7 +543,7 @@ test_expect_success 'relative path works with ./foo/bar' '\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\ttest \"$(git config submodule.sub.url)\" = foo/subrepo\n \t)\n '\n \ndiff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh\nindex 3784c9b..583fe21 100755\n--- a/t/t7403-submodule-sync.sh\n+++ b/t/t7403-submodule-sync.sh\n@@ -88,21 +88,22 @@ test_expect_success '\"git submodule sync\" should not vivify uninteresting submod\n \t)\n '\n \n-test_expect_success '\"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 echo \\\"cannot strip one component off url 'foo'\\\" > expect &&\n-\t test_must_fail git submodule sync 2> actual &&\n-\t test_cmp expect actual\n+\t git submodule sync &&\n+\t(cd submodule &&\n+\t test \"$(git config remote.origin.url)\" == \"../submodule\"\n \t)\n-\"\n+\t)\n+'\n \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 test \"$(git config remote.origin.url)\" == \"foo/submodule\"\n+\t test \"$(git config remote.origin.url)\" == \"../foo/submodule\"\n \t)\n \t)\n '\n@@ -112,7 +113,7 @@ test_expect_success '\"git submodule sync\" handles origin URL of the form ./foo'\n \t git remote set-url origin ./foo\n \t git submodule sync &&\n \t(cd submodule &&\n-\t test \"$(git config remote.origin.url)\" == \"./submodule\"\n+\t test \"$(git config remote.origin.url)\" == \"../submodule\"\n \t)\n \t)\n '\n@@ -122,7 +123,7 @@ test_expect_success '\"git submodule sync\" handles origin URL of the form ./foo/b\n \t git remote set-url origin ./foo/bar\n \t git submodule sync &&\n \t(cd submodule &&\n-\t test \"$(git config remote.origin.url)\" == \"./foo/submodule\"\n+\t test \"$(git config remote.origin.url)\" == \"../foo/submodule\"\n \t)\n \t)\n '\n@@ -132,7 +133,7 @@ test_expect_success '\"git submodule sync\" handles origin URL of the form ../foo'\n \t git remote set-url origin ../foo\n \t git submodule sync &&\n \t(cd submodule &&\n-\t test \"$(git config remote.origin.url)\" == \"../submodule\"\n+\t test \"$(git config remote.origin.url)\" == \"../../submodule\"\n \t)\n \t)\n '\n@@ -142,7 +143,7 @@ test_expect_success '\"git submodule sync\" handles origin URL of the form ../foo/\n \t git remote set-url origin ../foo/bar\n \t git submodule sync &&\n \t(cd submodule &&\n-\t test \"$(git config remote.origin.url)\" == \"../foo/submodule\"\n+\t test \"$(git config remote.origin.url)\" == \"../../foo/submodule\"\n \t)\n \t)\n '\n-- \n1.7.10.2.594.g24e850d\n"},{"id":"192018","messageId":"4FBD4F68.90305@web.de","threadId":"30600","inReplyTo":"1337791554-31294-2-git-send-email-jon.seymour@gmail.com","subject":"Re: [PATCH v4 1/2] submodule: document handling of relative superproject origin URLs","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-05-23T20:58:16Z","receivedAt":"2012-05-23T20:58:16Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Maybe the subject should rather be:\n\"submodule: document failures handling relative superproject origin URLs\"\n\nAm 23.05.2012 18:45, schrieb Jon Seymour:\n> These tests document how submodule sync and init handle various\n> kinds of relative super project origin URLs.  The submodule URL\n> path is ../subrepo.\n> \n> 6 cases are documented:\n>   foo\n>   foo/bar\n>   ./foo\n>   ./foo/bar\n>   ../foo\n>   ../foo/bar\n\nNice test coverage!\n\n> Signed-off-by: Jon Seymour <jon.seymour@gmail.com>\n> ---\n>  t/t7400-submodule-basic.sh | 62 +++++++++++++++++++++++++++++++++++++++++++++\n>  t/t7403-submodule-sync.sh  | 63 +++++++++++++++++++++++++++++++++++++++++++++-\n>  2 files changed, 124 insertions(+), 1 deletion(-)\n> \n> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\n> index 81827e6..97e7a73 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\nBut please use test_expect_failure for all tests that fail due to\ncurrent bugs and document what should succeed in the commands used\ninside the test case (just like you did in v3). The only change\nneeded when the bug is fixed should be changing each\ntest_expect_failure to test_expect_success.\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\techo \\\"cannot strip one component off url 'foo'\\\" >expect &&\n> +\t\ttest_must_fail git submodule init 2>actual &&\n> +\t\tcat actual &&\n> +\t\ttest_cmp expect actual\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_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_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 &&\n> diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh\n> index 3620215..3784c9b 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,63 @@ test_expect_success '\"git submodule sync\" should not vivify uninteresting submod\n>  \t)\n>  '\n>  \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 echo \\\"cannot strip one component off url 'foo'\\\" > expect &&\n> +\t test_must_fail git submodule sync 2> actual &&\n> +\t test_cmp expect actual\n> +\t)\n> +\"\n> +\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 test \"$(git config remote.origin.url)\" == \"foo/submodule\"\n> +\t)\n> +\t)\n> +'\n> +\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 test \"$(git config remote.origin.url)\" == \"./submodule\"\n> +\t)\n> +\t)\n> +'\n> +\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 test \"$(git config remote.origin.url)\" == \"./foo/submodule\"\n> +\t)\n> +\t)\n> +'\n> +\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 test \"$(git config remote.origin.url)\" == \"../submodule\"\n> +\t)\n> +\t)\n> +'\n> +\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 test \"$(git config remote.origin.url)\" == \"../foo/submodule\"\n> +\t)\n> +\t)\n> +'\n> +\n>  test_done\n"},{"id":"192019","messageId":"7vvcjmzjkv.fsf@alter.siamese.dyndns.org","threadId":"30600","inReplyTo":"4FBD4F68.90305@web.de","subject":"Re: [PATCH v4 1/2] submodule: document handling of relative superproject origin URLs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-23T21:14:08Z","receivedAt":"2012-05-23T21:14:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann <Jens.Lehmann@web.de> writes:\n\n> Maybe the subject should rather be:\n> \"submodule: document failures handling relative superproject origin URLs\"\n>\n> Am 23.05.2012 18:45, schrieb Jon Seymour:\n>> These tests document how submodule sync and init handle various\n>> kinds of relative super project origin URLs.  The submodule URL\n>> path is ../subrepo.\n>> \n>> 6 cases are documented:\n>>   foo\n>>   foo/bar\n>>   ./foo\n>>   ./foo/bar\n>>   ../foo\n>>   ../foo/bar\n>\n> Nice test coverage!\n\nI recall correctly, the original use case for relative URL entries in the\n.gitmodules file (to be copied to .git/config as submodule.$name.path) was\nso that by looking at the top-level, the locations of the origins for\nsubmodule repositories can be known from where the top-level was cloned.\nThe above cases do not seem to be relevant, so in the sense, they are of\nsecondary importance (and I do not find the \"sneakernet tool\" example\nconvincing---the sneakernet tool that is distributed in the scenario can\nbe written differently so that it does not require the other repositories\nto be named relative to it).\n\nAs long as you and submodule stakeholders believe this is a reasonable\naddition and does not break the existing use cases, I am perfectly fine\nwith it, though.\n\n>> Signed-off-by: Jon Seymour <jon.seymour@gmail.com>\n>> ---\n>>  t/t7400-submodule-basic.sh | 62 +++++++++++++++++++++++++++++++++++++++++++++\n>>  t/t7403-submodule-sync.sh  | 63 +++++++++++++++++++++++++++++++++++++++++++++-\n>>  2 files changed, 124 insertions(+), 1 deletion(-)\n>> \n>> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\n>> index 81827e6..97e7a73 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> But please use test_expect_failure for all tests that fail due to\n> current bugs and document what should succeed in the commands used\n> inside the test case (just like you did in v3). The only change\n> needed when the bug is fixed should be changing each\n> test_expect_failure to test_expect_success.\n\nVery true.\n\nThanks for a review.\n"},{"id":"192021","messageId":"4FBD5A61.1030101@web.de","threadId":"30600","inReplyTo":"7vvcjmzjkv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v4 1/2] submodule: document handling of relative superproject origin URLs","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-05-23T21:45:05Z","receivedAt":"2012-05-23T21:45:05Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 23.05.2012 23:14, schrieb Junio C Hamano:\n> Jens Lehmann <Jens.Lehmann@web.de> writes:\n>> Am 23.05.2012 18:45, schrieb Jon Seymour:\n>>> These tests document how submodule sync and init handle various\n>>> kinds of relative super project origin URLs.  The submodule URL\n>>> path is ../subrepo.\n>>>\n>>> 6 cases are documented:\n>>>   foo\n>>>   foo/bar\n>>>   ./foo\n>>>   ./foo/bar\n>>>   ../foo\n>>>   ../foo/bar\n>>\n>> Nice test coverage!\n> \n> I recall correctly, the original use case for relative URL entries in the\n> .gitmodules file (to be copied to .git/config as submodule.$name.path) was\n> so that by looking at the top-level, the locations of the origins for\n> submodule repositories can be known from where the top-level was cloned.\n> The above cases do not seem to be relevant, so in the sense, they are of\n> secondary importance (and I do not find the \"sneakernet tool\" example\n> convincing---the sneakernet tool that is distributed in the scenario can\n> be written differently so that it does not require the other repositories\n> to be named relative to it).\n\nRemember this patch series is not about relative /submodule/ urls but\nrelative /superproject/ urls, so the .gitmodules file is not involved\nhere. But while reviewing 2/2 I started to suspect I was a bit too hasty\njudging the coverage, as I realized only \"git submodule init\" and \"git\nsubmodule sync\" are tested. \"git submodule add\" should also be tested\nhere to make sure it behaves well too (and only when the tests cases are\nnot split across two commits they can be properly reviewed). My remark\naimed at the attempt to test all possible relative paths in at least two\ndepths, which I deem very helpful to find problems early.\n\n> As long as you and submodule stakeholders believe this is a reasonable\n> addition and does not break the existing use cases, I am perfectly fine\n> with it, though.\n\nMe thinks relative /superproject/ urls starting with ./ or ../ should\nwork just fine (which they don't right now). And if relative superproject\nurls inside the superproject are handled too without too much hassle I\nwon't argue against it, even though I'm not convinced of the use case\neither.\n"},{"id":"192022","messageId":"4FBD5B8C.60605@web.de","threadId":"30600","inReplyTo":"1337791554-31294-3-git-send-email-jon.seymour@gmail.com","subject":"Re: [PATCH v4 2/2] submodule: fix handling of relative superproject origin URLs","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-05-23T21:50:04Z","receivedAt":"2012-05-23T21:50:04Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 23.05.2012 18:45, schrieb Jon Seymour:\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           | 17 +++++++++++++++--\n>  t/t7400-submodule-basic.sh | 12 +++++-------\n>  t/t7403-submodule-sync.sh  | 21 +++++++++++----------\n>  3 files changed, 31 insertions(+), 19 deletions(-)\n> \n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 64a70d6..3e943de 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -37,6 +37,8 @@ resolve_relative_url ()\n>  \tremoteurl=$(git config \"remote.$remote.url\") ||\n>  \t\tremoteurl=$(pwd) # the repository is its own authoritative upstream\n>  \turl=\"$1\"\n> +\tprefix=\"$2\"\n\nAs mentioned last time I'd rather use $2 directly at the only site\nwhere $prefix is used. (On the other hand it might also make sense\nto give the parameters a descriptive name at the beginning of the\nfunction, but then I'd vote for a descriptive name like \"urlprefix\"\nor similar to make its meaning clearer)\n\n> +\tremoteurl=$(echo \"$remoteurl\" | sed \"s|^[^/:\\\\.][^:]*\\$|./&|\")\n\nA comment describing what this line actually does would be nice.\n\n>  \tremoteurl=${remoteurl%/}\n>  \tsep=/\n>  \twhile test -n \"$url\"\n> @@ -45,6 +47,11 @@ 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\tremoteurl=\"${remoteurl%/*}\"\n> +\t\t\t\tremoteurl=\"${remoteurl#./}\"\n> +\t\t\t\tremoteurl=\"${prefix}${remoteurl}\"\n> +\t\t\t\t;;\n>  \t\t\t*/*)\n>  \t\t\t\tremoteurl=\"${remoteurl%/*}\"\n>  \t\t\t\t;;\n> @@ -64,7 +71,7 @@ resolve_relative_url ()\n>  \t\t\tbreak;;\n>  \t\tesac\n>  \tdone\n> -\techo \"$remoteurl$sep${url%/}\"\n> +\techo \"${remoteurl%/.}$sep${url%/}\"\n\nWouldn't that better be handled in the \".*/*)\" case above to avoid\naccidentally affecting the other cases?\n\n>  }\n>  \n>  #\n> @@ -964,8 +971,14 @@ cmd_sync()\n>  \t\t# Possibly a url relative to parent\n>  \t\tcase \"$url\" in\n>  \t\t./*|../*)\n> +\t\t\tup_path=\"$(echo \"$sm_path\" | sed \"s/[^/]*/../g\")\" &&\n> +\t\t\tup_path=${up_path%/}/ &&\n> +\t\t\tremoteurl=$(resolve_relative_url \"$url\" \"$up_path\") &&\n>  \t\t\turl=$(resolve_relative_url \"$url\") || exit\n>  \t\t\t;;\n> +\t\t*)\n> +\t\t\tremoteurl=\"$url\"\n> +\t\t\t;;\n>  \t\tesac\n>  \n>  \t\tif git config \"submodule.$name.url\" >/dev/null 2>/dev/null\n> @@ -979,7 +992,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 \"$remoteurl\"\n>  \t\t\t)\n>  \t\t\tfi\n>  \t\tfi\n\nI still have to wrap my head around these two hunks (I suspect it's\ntoo late for that in my timezone ;-), but I really wonder how you get\naway without changing cmd_add and cmd_init like you did last time.\nThis looks much different than #2 and #4 of your v3 combined, which\nmakes me suspicious in what direction this is evolving. Maybe you could\ntell us what you found out addressing the last problem you mentioned\nand how you handled it?\n\n\nThe only changes following here should be from test_expect_failure\nto test_expect_success as mentioned in my response to your first patch.\n\n> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\n> index 97e7a73..f2f907f 100755\n> --- a/t/t7400-submodule-basic.sh\n> +++ b/t/t7400-submodule-basic.sh\n> @@ -507,17 +507,15 @@ test_expect_success 'relative path works with user@host:path' '\n>  \t)\n>  '\n>  \n> -test_expect_success '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\techo \\\"cannot strip one component off url 'foo'\\\" >expect &&\n> -\t\ttest_must_fail git submodule init 2>actual &&\n> -\t\tcat actual &&\n> -\t\ttest_cmp expect actual\n> +\t\tgit submodule init &&\n> +\t\ttest \"$(git config submodule.sub.url)\" = ./subrepo\n>  \t)\n> -\"\n> +'\n>  \n>  test_expect_success 'relative path works with foo/bar' '\n>  \t(\n> @@ -545,7 +543,7 @@ test_expect_success 'relative path works with ./foo/bar' '\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\ttest \"$(git config submodule.sub.url)\" = foo/subrepo\n>  \t)\n>  '\n>  \n> diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh\n> index 3784c9b..583fe21 100755\n> --- a/t/t7403-submodule-sync.sh\n> +++ b/t/t7403-submodule-sync.sh\n> @@ -88,21 +88,22 @@ test_expect_success '\"git submodule sync\" should not vivify uninteresting submod\n>  \t)\n>  '\n>  \n> -test_expect_success '\"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 echo \\\"cannot strip one component off url 'foo'\\\" > expect &&\n> -\t test_must_fail git submodule sync 2> actual &&\n> -\t test_cmp expect actual\n> +\t git submodule sync &&\n> +\t(cd submodule &&\n> +\t test \"$(git config remote.origin.url)\" == \"../submodule\"\n>  \t)\n> -\"\n> +\t)\n> +'\n>  \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 test \"$(git config remote.origin.url)\" == \"foo/submodule\"\n> +\t test \"$(git config remote.origin.url)\" == \"../foo/submodule\"\n>  \t)\n>  \t)\n>  '\n> @@ -112,7 +113,7 @@ test_expect_success '\"git submodule sync\" handles origin URL of the form ./foo'\n>  \t git remote set-url origin ./foo\n>  \t git submodule sync &&\n>  \t(cd submodule &&\n> -\t test \"$(git config remote.origin.url)\" == \"./submodule\"\n> +\t test \"$(git config remote.origin.url)\" == \"../submodule\"\n>  \t)\n>  \t)\n>  '\n> @@ -122,7 +123,7 @@ test_expect_success '\"git submodule sync\" handles origin URL of the form ./foo/b\n>  \t git remote set-url origin ./foo/bar\n>  \t git submodule sync &&\n>  \t(cd submodule &&\n> -\t test \"$(git config remote.origin.url)\" == \"./foo/submodule\"\n> +\t test \"$(git config remote.origin.url)\" == \"../foo/submodule\"\n>  \t)\n>  \t)\n>  '\n> @@ -132,7 +133,7 @@ test_expect_success '\"git submodule sync\" handles origin URL of the form ../foo'\n>  \t git remote set-url origin ../foo\n>  \t git submodule sync &&\n>  \t(cd submodule &&\n> -\t test \"$(git config remote.origin.url)\" == \"../submodule\"\n> +\t test \"$(git config remote.origin.url)\" == \"../../submodule\"\n>  \t)\n>  \t)\n>  '\n> @@ -142,7 +143,7 @@ test_expect_success '\"git submodule sync\" handles origin URL of the form ../foo/\n>  \t git remote set-url origin ../foo/bar\n>  \t git submodule sync &&\n>  \t(cd submodule &&\n> -\t test \"$(git config remote.origin.url)\" == \"../foo/submodule\"\n> +\t test \"$(git config remote.origin.url)\" == \"../../foo/submodule\"\n>  \t)\n>  \t)\n>  '\n"},{"id":"192024","messageId":"CAH3AnrpBEnF8BGcfGccmbyh_X-7c4CBN2UOq_v9uDMeWbsnNEQ@mail.gmail.com","threadId":"30600","inReplyTo":"7vvcjmzjkv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v4 1/2] submodule: document handling of relative superproject origin URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-23T21:55:35Z","receivedAt":"2012-05-23T21:55:35Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"On Thu, May 24, 2012 at 7:14 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Jens Lehmann <Jens.Lehmann@web.de> writes:\n>\n>> Maybe the subject should rather be:\n>> \"submodule: document failures handling relative superproject origin URLs\"\n>\n\nJens: Sure.\n\n>\n> I recall correctly, the original use case for relative URL entries in the\n> .gitmodules file (to be copied to .git/config as submodule.$name.path) was\n> so that by looking at the top-level, the locations of the origins for\n> submodule repositories can be known from where the top-level was cloned.\n> The above cases do not seem to be relevant, so in the sense, they are of\n> secondary importance (and I do not find the \"sneakernet tool\" example\n> convincing---the sneakernet tool that is distributed in the scenario can\n> be written differently so that it does not require the other repositories\n> to be named relative to it).\n\nAgreed, that this is not required for the original use case.\n\nIn the 'sneakernet tool' case, it makes sense for me to have ./mnt/usb\naround in any case (for the purposes of repos that are being managed\nby the sneakernet tool). Since that link exists, I can use it for the\npurposes of managing the tool's own origin repo and doing so does, I\nthink, reduce the number of configuration items I have to tweak to the\nabsolute minimum, but you are right that I don't absolutely have to do\nit that way.\n\n>\n> As long as you and submodule stakeholders believe this is a reasonable\n> addition and does not break the existing use cases, I am perfectly fine\n> with it, though.\n\nOk, thanks for that. Whatever the use case, I think the change is a\npositive if simply because, by respecting an implicit invariant, it\nmakes the behaviour consistent and predictable.\n\nI'll fix the tests, per Jens and your comments and resubmit.\n\nJens: thanks for the review.\n\nRegards,\n\njon.\n"},{"id":"192025","messageId":"CAH3AnrrYm_KeP64yDc+gujkYqj-kiuLQ4URHX5Z57vj5XXLfKw@mail.gmail.com","threadId":"30600","inReplyTo":"4FBD5B8C.60605@web.de","subject":"Re: [PATCH v4 2/2] submodule: fix handling of relative superproject origin URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-23T22:17:19Z","receivedAt":"2012-05-23T22:17:19Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"On Thu, May 24, 2012 at 7:50 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:\n> Am 23.05.2012 18:45, schrieb Jon Seymour:\n>\n> As mentioned last time I'd rather use $2 directly at the only site\n> where $prefix is used. (On the other hand it might also make sense\n> to give the parameters a descriptive name at the beginning of the\n> function, but then I'd vote for a descriptive name like \"urlprefix\"\n> or similar to make its meaning clearer)\n\nOk, I was favouring your latter heuristic. Are you ok if I use up_path here?\n\n>\n>> +     remoteurl=$(echo \"$remoteurl\" | sed \"s|^[^/:\\\\.][^:]*\\$|./&|\")\n>\n> A comment describing what this line actually does would be nice.\n>\n\nSure, will do. What it does is to allow URLs of the form foo/ or\nfoo/bar to be handled by the .*/* case in the switch below by\nrewriting the URL to prefix ./ in that case.\n\nIt doesn't do it for URLs that already begin with . (don't need it), /\n(mustn't have it) or : (assuming that any path beginning with : should\nprobably be handled by the *:* case).\n\n>>       remoteurl=${remoteurl%/}\n>>       sep=/\n>>       while test -n \"$url\"\n>> @@ -45,6 +47,11 @@ resolve_relative_url ()\n>>               ../*)\n>>                       url=\"${url#../}\"\n>>                       case \"$remoteurl\" in\n>> +                     .*/*)\n>> +                             remoteurl=\"${remoteurl%/*}\"\n>> +                             remoteurl=\"${remoteurl#./}\"\n>> +                             remoteurl=\"${prefix}${remoteurl}\"\n>> +                             ;;\n>>                       */*)\n>>                               remoteurl=\"${remoteurl%/*}\"\n>>                               ;;\n>> @@ -64,7 +71,7 @@ resolve_relative_url ()\n>>                       break;;\n>>               esac\n>>       done\n>> -     echo \"$remoteurl$sep${url%/}\"\n>> +     echo \"${remoteurl%/.}$sep${url%/}\"\n>\n> Wouldn't that better be handled in the \".*/*)\" case above to avoid\n> accidentally affecting the other cases?\n\nYes, I think you are right. Thanks.\n\n>\n>>  }\n>>\n>>  #\n>> @@ -964,8 +971,14 @@ cmd_sync()\n>>               # Possibly a url relative to parent\n>>               case \"$url\" in\n>>               ./*|../*)\n>> +                     up_path=\"$(echo \"$sm_path\" | sed \"s/[^/]*/../g\")\" &&\n>> +                     up_path=${up_path%/}/ &&\n>> +                     remoteurl=$(resolve_relative_url \"$url\" \"$up_path\") &&\n>>                       url=$(resolve_relative_url \"$url\") || exit\n>>                       ;;\n>> +             *)\n>> +                     remoteurl=\"$url\"\n>> +                     ;;\n>>               esac\n>>\n>>               if git config \"submodule.$name.url\" >/dev/null 2>/dev/null\n>> @@ -979,7 +992,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 \"$remoteurl\"\n>>                       )\n>>                       fi\n>>               fi\n>\n> I still have to wrap my head around these two hunks (I suspect it's\n> too late for that in my timezone ;-), but I really wonder how you get\n> away without changing cmd_add and cmd_init like you did last time.\n> This looks much different than #2 and #4 of your v3 combined, which\n> makes me suspicious in what direction this is evolving. Maybe you could\n> tell us what you found out addressing the last problem you mentioned\n> and how you handled it?\n\nSo, I subsequently noticed that v3 broke the clone done by a\nsubsequent update in the case of paths of the form ../foo/bar That was\nbecause I had violated an implicit invariant - namely that the\nsubmodule.{name}.url configuration variable in the superproject is\nalways a path relative to the working tree of the superproject. In v3,\nI had effectively made this value relative to the working tree of the\nsubmodule. I mostly only need to modify sync behaviour, because that\nis the only path that modifies the remote.origin.url of the submodule\n- the other two paths modify the submodule.{name}.url variable of the\nsuperproject and this behaviour is mostly correct (sans normalisation\nissues).\n\nThis series respects the implicit invariant that relative paths in\nconfiguration properties (submodule.{name}.url and\nremote.{remote}.url) are always relative to the working tree of the\nrepository in which the configuration variable is defined.\n\nI'll try to find a way to inject aspects of this commentary into the comments.\n\n>\n>\n> The only changes following here should be from test_expect_failure\n> to test_expect_success as mentioned in my response to your first patch.\n>\n\nSure.\n\nThanks for your review.\n\njon.\n"},{"id":"192037","messageId":"CAH3AnrqekvFL3qvRh7HFCENG8-kO5xxnG113_xPqnhjHgxYtzA@mail.gmail.com","threadId":"30600","inReplyTo":"CAH3AnrrYm_KeP64yDc+gujkYqj-kiuLQ4URHX5Z57vj5XXLfKw@mail.gmail.com","subject":"Re: [PATCH v4 2/2] submodule: fix handling of relative superproject origin URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-24T02:32:31Z","receivedAt":"2012-05-24T02:32:31Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"On Thu, May 24, 2012 at 8:17 AM, Jon Seymour <jon.seymour@gmail.com> wrote:\n> On Thu, May 24, 2012 at 7:50 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:\n>> Am 23.05.2012 18:45, schrieb Jon Seymour:\n>> Wouldn't that better be handled in the \".*/*)\" case above to avoid\n>> accidentally affecting the other cases?\n>\n> Yes, I think you are right. Thanks.\n>\n\nTurns out this case was necessary, but I don't fully understand why as\nyet. I'll work that out and see if I can avoid doing it, otherwise I\nwill document why it is reasonable.\n\njon.\n"}]}