{"thread":{"id":"30637","subject":"[PATCH v7 0/9] submodule: improve robustness of path handling","startedAt":"2012-05-27T15:34:02Z","lastAt":"2012-06-03T09:51:00Z","messageCount":17,"participants":["Jon Seymour","Johannes Sixt","Jens Lehmann"],"isPatch":true,"patchVersion":7,"patchTotal":9},"messages":[{"id":"192284","messageId":"1338132851-23497-1-git-send-email-jon.seymour@gmail.com","threadId":"30637","inReplyTo":null,"subject":"[PATCH v7 0/9] submodule: improve robustness of path handling","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-27T15:34:02Z","receivedAt":"2012-05-27T15:34:02Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"This series improves the robustness of path handling by 'git submodule' by:\n\n* detecting submodule URLs that will result in non-sensical submodule origin URLs\n\n* improving handling of various kinds of relative superproject origin URLs\n\n* improving handling of various kinds of denormalized superproject origin URLs\n\nThis series differs from v5 in the following ways, by:\n\n* Adding a more extensive set of failure tests to illustrate the conditions \nbeing addressed.\n\n* Modifying the ../ processing loop in resolve_relative_url to exclude the \n'invariant' parts of absolute URLs from relative URL processing and thereby \nenable earlier and more accurate detection of edits that are going to \nproduce a non-sensical output.\n\n* Simplifying relative superproject origin URL support, by taking advantage of\nthe modifications above.\n\n* Adding support for normalizing denormalized superproject origin URLs.\n\n* Adding some additional regression tests to help guard against \nunintended regressions by this series.\n\n* Improving the source code comments to better explain the purpose\nof various code functions and code blocks\n\nThis series differs from v6 by applying the fix in 8/9 to a wider set of tests.\n\nEach patch in the series has been regression tested against the following tests:\n\n   t2013-checkout-submodule.sh\n   t2103-update-index-ignore-missing.sh\n   t2105-update-index-gitfile.sh\n   t2201-add-update-typechange.sh\n   t3000-ls-files-others.sh\n   t3030-merge-recursive.sh\n   t3404-rebase-interactive.sh\n   t4027-diff-submodule.sh\n   t4041-diff-submodule-option.sh\n   t4134-apply-submodule.sh\n   t5526-fetch-submodules.sh\n   t5531-deep-submodule-push.sh\n   t6008-rev-list-submodule.sh\n   t7003-filter-branch.sh\n   t7400-submodule-basic.sh\n   t7401-submodule-summary.sh\n   t7402-submodule-rebase.sh\n   t7403-submodule-sync.sh\n   t7405-submodule-merge.sh\n   t7406-submodule-update.sh\n   t7407-submodule-foreach.sh\n   t7408-submodule-reference.sh\n   t7506-status-submodule.sh\n   t7508-status.sh\n   t7610-mergetool.sh\n   t9300-fast-import.sh\n   t9350-fast-export.sh\n\nwhich are the tests that match a grep search for submodule.\n\nJon Seymour (9):\n  submodule: additional regression tests for relative URLs\n  submodule: document failure to detect invalid submodule URLs\n  submodule: document failure to handle relative superproject origin\n    URLs\n  submodule: document failure to handle improperly normalized remote\n    origin URLs\n  submodule: extract normalize_path into standalone function\n  submodule: fix detection of invalid submodule URL\n  submodule: fix sync handling of relative superproject origin URLs\n  submodule: fix handling of denormalized superproject origin URLs\n  submodule: fix normalization to handle repeated ./\n\n git-submodule.sh             | 118 +++++++++++++----\n t/t7400-submodule-basic.sh   | 297 ++++++++++++++++++++++++++++++++++++++++++-\n t/t7403-submodule-sync.sh    |  97 +++++++++++++-\n t/t7406-submodule-update.sh  |  16 ++-\n t/t7407-submodule-foreach.sh |  14 +-\n t/t7506-status-submodule.sh  |  10 +-\n 6 files changed, 504 insertions(+), 48 deletions(-)\n\n-- \n1.7.10.2.656.g24a6219\n"},{"id":"192286","messageId":"1338132851-23497-2-git-send-email-jon.seymour@gmail.com","threadId":"30637","inReplyTo":"1338132851-23497-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH v7 1/9] submodule: additional regression tests for relative URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-27T15:34:03Z","receivedAt":"2012-05-27T15:34:03Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"Some additional tests are added to support regression testing of the changes in the\nremainder of the series.\n\nWe also add a pristine copy of .gitmodules in anticipation of this being\nrequired by later tests.\n\nSigned-off-by: Jon Seymour <jon.seymour@gmail.com>\n---\n t/t7400-submodule-basic.sh | 110 +++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 107 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 81827e6..9428c7a 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -483,21 +483,67 @@ test_expect_success 'set up for relative path tests' '\n \t\tgit add sub &&\n \t\tgit config -f .gitmodules submodule.sub.path sub &&\n \t\tgit config -f .gitmodules submodule.sub.url ../subrepo &&\n-\t\tcp .git/config pristine-.git-config\n+\t\tcp .git/config pristine-.git-config &&\n+\t\tcp .gitmodules pristine-.gitmodules\n \t)\n '\n \n-test_expect_success 'relative path works with URL' '\n+test_expect_success '../subrepo works with URL - ssh://hostname/repo' '\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n \t\tgit config remote.origin.url ssh://hostname/repo &&\n \t\tgit submodule init &&\n \t\ttest \"$(git config submodule.sub.url)\" = ssh://hostname/subrepo\n \t)\n '\n \n-test_expect_success 'relative path works with user@host:path' '\n+test_expect_success '../subrepo works with port-qualified URL - ssh://hostname:22/repo' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url ssh://hostname:22/repo &&\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = ssh://hostname:22/subrepo\n+\t)\n+'\n+\n+test_expect_success '../subrepo path works with local path - /foo/repo' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url /foo/repo &&\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = /foo/subrepo\n+\t)\n+'\n+\n+test_expect_success '../subrepo works with file URL - file:///tmp/repo' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url file:///tmp/repo &&\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = file:///tmp/subrepo\n+\t)\n+'\n+\n+test_expect_success '../subrepo works with helper URL- helper:://hostname/repo' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url helper:://hostname/repo &&\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = helper:://hostname/subrepo\n+\t)\n+'\n+\n+test_expect_success '../subrepo works with scp-style URL - user@host:repo' '\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n@@ -507,6 +553,64 @@ test_expect_success 'relative path works with user@host:path' '\n \t)\n '\n \n+test_expect_success '../subrepo works with scp-style URL - user@host:path/to/repo' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url user@host:path/to/repo &&\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = user@host:path/to/subrepo\n+\t)\n+'\n+\n+test_expect_success '../subrepo works with relative local path - foo/bar' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\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 '../subrepo works with relative local path - ../foo' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\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 '../subrepo works with relative local path - ../foo/bar' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\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 '../bar/a/b/c works with relative local path - ../foo/bar.git' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tmkdir -p a/b/c &&\n+\t\t(cd a/b/c; git init) &&\n+\t\tgit config remote.origin.url ../foo/bar.git &&\n+\t\tgit submodule add ../bar/a/b/c ./a/b/c &&\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.a/b/c.url)\" = ../foo/bar/a/b/c\n+\t)\n+'\n+\n test_expect_success 'moving the superproject does not break submodules' '\n \t(\n \t\tcd addtest &&\n-- \n1.7.10.2.656.g24a6219\n"},{"id":"192289","messageId":"1338132851-23497-3-git-send-email-jon.seymour@gmail.com","threadId":"30637","inReplyTo":"1338132851-23497-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH v7 2/9] submodule: document failure to detect invalid submodule URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-27T15:34:04Z","receivedAt":"2012-05-27T15:34:04Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"These tests document failures to detect submodule URLs that backtrack\npast the 'invariant' part of a superproject's origin URL.\n\nFor example: if the origin URL is ssh://hostname/repo, then currently\nif a submodule URL is specified as ../../subrepo, then git will\nconstruct a submodule url of the form ssh://subrepo/ without complaint.\n\nSigned-off-by: Jon Seymour <jon.seymour@gmail.com>\n---\n t/t7400-submodule-basic.sh | 70 ++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 70 insertions(+)\n\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 9428c7a..a758c63 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -543,10 +543,80 @@ test_expect_success '../subrepo works with helper URL- helper:://hostname/repo'\n \t)\n '\n \n+test_expect_failure '../../subrepo fails with URL - ssh://hostname/repo' \"\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url ssh://hostname/repo &&\n+\t\tgit config -f .gitmodules submodule.sub.url ../../subrepo &&\n+\t\techo cannot strip one component off url \\'ssh://hostname/\\' > expected &&\n+\t\ttest_must_fail git submodule init 2>actual &&\n+\t\t#actual no failure, url configured as ssh://subrepo\n+\t\ttest_cmp expected actual\n+\t)\n+\"\n+\n+test_expect_failure '../../subrepo fails with absolute local path - /repo' \"\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url /repo &&\n+\t\tgit config -f .gitmodules submodule.sub.url ../../subrepo &&\n+\t\techo cannot strip one component off url \\'/\\' > expected &&\n+\t\ttest_must_fail git submodule init 2>actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+\"\n+\n+test_expect_failure '../../../subrepo fails with URL - ssh://hostname/repo' \"\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url ssh://hostname/repo &&\n+\t\tgit config -f .gitmodules submodule.sub.url ../../../subrepo &&\n+\t\techo cannot strip one component off url \\'ssh://hostname/\\' > expected &&\n+\t\ttest_must_fail git submodule init 2>actual &&\n+\t\t#actual no failure, url configured as ssh:/subrepo\n+\t\ttest_cmp expected actual\n+\t)\n+\"\n+\n+test_expect_failure '../../../../subrepo fails with with URL - ssh://hostname/repo' \"\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url ssh://hostname/repo &&\n+\t\tgit config -f .gitmodules submodule.sub.url ../../../../subrepo &&\n+\t\techo cannot strip one component off url \\'ssh://hostname/\\' > expected &&\n+\t\ttest_must_fail git submodule init 2>actual &&\n+\t\t#actual no failure, url configured as ssh:/subrepo\n+\t\ttest_cmp expected actual\n+\t)\n+\"\n+\n+test_expect_failure '../../../../../subrepo fails with URL - ssh://hostname/repo' \"\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url ssh://hostname/repo &&\n+\t\tgit config -f .gitmodules submodule.sub.url ../../../../../subrepo &&\n+\t\techo cannot strip one component off url \\'ssh://hostname/\\' > expected &&\n+\t\ttest_must_fail git submodule init 2>actual &&\n+\t\t#actual cannot strip one component off url 'ssh'\n+\t\ttest_cmp expected actual\n+\t)\n+\"\n+\n test_expect_success '../subrepo works with scp-style URL - user@host:repo' '\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n \t\tgit config remote.origin.url user@host:repo &&\n \t\tgit submodule init &&\n \t\ttest \"$(git config submodule.sub.url)\" = user@host:subrepo\n-- \n1.7.10.2.656.g24a6219\n"},{"id":"192285","messageId":"1338132851-23497-4-git-send-email-jon.seymour@gmail.com","threadId":"30637","inReplyTo":"1338132851-23497-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH v7 3/9] submodule: document failure to handle relative superproject origin URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-27T15:34:05Z","receivedAt":"2012-05-27T15:34:05Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"This test case documents several cases where handling of relative\nsuperproject origin URLs doesn't produce an expected result.\n\nsubmodule.{sub}.url in the superproject is incorrect in these cases:\n  foo\n  ./foo\n  ./foo/bar\n\nThe remote.origin.url of the submodule is incorrect in the above cases\nand also when the superproject origin URL is like:\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 | 36 +++++++++++++++++++\n t/t7403-submodule-sync.sh  | 90 +++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 125 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex a758c63..80ec0f7 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -634,6 +634,18 @@ test_expect_success '../subrepo works with scp-style URL - user@host:path/to/rep\n \t)\n '\n \n+test_expect_failure '../subrepo works with relative local path - foo' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\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 '../subrepo works with relative local path - foo/bar' '\n \t(\n \t\tcd reltest &&\n@@ -645,6 +657,30 @@ test_expect_success '../subrepo works with relative local path - foo/bar' '\n \t)\n '\n \n+test_expect_failure '../subrepo works with relative local path - ./foo' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url ./foo &&\n+\t\tgit submodule init &&\n+\t\t#actual ./subrepo\n+\t\ttest \"$(git config submodule.sub.url)\" = subrepo\n+\t)\n+'\n+\n+test_expect_failure '../subrepo works with relative local path - ./foo/bar' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\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 '../subrepo works with relative local path - ../foo' '\n \t(\n \t\tcd reltest &&\ndiff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh\nindex 3620215..56b933d 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,90 @@ 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 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 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 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 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 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 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/bar with deeply nested submodule' '\n+\t(cd relative-clone &&\n+\t git remote set-url origin ../foo/bar &&\n+\t mkdir -p a/b/c &&\n+\t ( cd a/b/c &&\n+\t   git init &&\n+\t   :> .gitignore &&\n+\t   git add .gitignore &&\n+\t   test_tick &&\n+\t   git commit -m \"initial commit\" ) &&\n+\t git submodule add ../bar/a/b/c ./a/b/c &&\n+\t git submodule sync &&\n+\t(cd a/b/c &&\n+\t #actual ../foo/bar/a/b/c\n+\t test \"$(git config remote.origin.url)\" = \"../../../../foo/bar/a/b/c\"\n+\t)\n+\t)\n+'\n+\n+\n test_done\n-- \n1.7.10.2.656.g24a6219\n"},{"id":"192290","messageId":"1338132851-23497-5-git-send-email-jon.seymour@gmail.com","threadId":"30637","inReplyTo":"1338132851-23497-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH v7 4/9] submodule: document failure to handle improperly normalized remote origin URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-27T15:34:06Z","receivedAt":"2012-05-27T15:34:06Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"These tests document failures to properly handle improperly normalized\nremote origin URLs.\n\nSigned-off-by: Jon Seymour <jon.seymour@gmail.com>\n---\n t/t7400-submodule-basic.sh | 88 ++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 88 insertions(+)\n\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 80ec0f7..2674088 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -499,6 +499,39 @@ test_expect_success '../subrepo works with URL - ssh://hostname/repo' '\n \t)\n '\n \n+test_expect_failure 'relative path works with URL - ssh://hostname/path/././repo' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url ssh://hostname/path/././repo &&\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = ssh://hostname/path/subrepo\n+\t)\n+'\n+\n+test_expect_failure 'relative path works with URL - ssh://hostname/path/detour/././../repo' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url ssh://hostname/path/detour/././../repo &&\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = ssh://hostname/path/subrepo\n+\t)\n+'\n+\n+test_expect_failure 'relative path works with URL - ssh://hostname/path/repo/.' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url ssh://hostname/path/repo/. &&\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = ssh://hostname/path/subrepo\n+\t)\n+'\n+\n test_expect_success '../subrepo works with port-qualified URL - ssh://hostname:22/repo' '\n \t(\n \t\tcd reltest &&\n@@ -634,6 +667,50 @@ test_expect_success '../subrepo works with scp-style URL - user@host:path/to/rep\n \t)\n '\n \n+test_expect_failure 'relative path works with user@host:path/to/repo/.' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url user@host:path/to/repo/. &&\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = user@host:path/to/subrepo\n+\t)\n+'\n+\n+test_expect_failure 'relative path works with user@host:path/to/./repo' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url user@host:path/to/./repo &&\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = user@host:path/to/subrepo\n+\t)\n+'\n+\n+test_expect_failure 'relative path works with user@host:path/to/././repo' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url user@host:path/to/././repo &&\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = user@host:path/to/subrepo\n+\t)\n+'\n+\n+test_expect_failure 'relative path works with user@host:path/to/detour/../repo' '\n+\t(\n+\t\tcd reltest &&\n+\t\tcp pristine-.git-config .git/config &&\n+\t\tcp pristine-.gitmodules .gitmodules &&\n+\t\tgit config remote.origin.url user@host:path/to/detour/../repo &&\n+\t\tgit submodule init &&\n+\t\ttest \"$(git config submodule.sub.url)\" = user@host:path/to/subrepo\n+\t)\n+'\n+\n test_expect_failure '../subrepo works with relative local path - foo' '\n \t(\n \t\tcd reltest &&\n@@ -703,6 +780,17 @@ test_expect_success '../subrepo works with relative local path - ../foo/bar' '\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\tcp pristine-.gitmodules .gitmodules &&\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 '../bar/a/b/c works with relative local path - ../foo/bar.git' '\n \t(\n \t\tcd reltest &&\n-- \n1.7.10.2.656.g24a6219\n"},{"id":"192288","messageId":"1338132851-23497-6-git-send-email-jon.seymour@gmail.com","threadId":"30637","inReplyTo":"1338132851-23497-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH v7 5/9] submodule: extract normalize_path into standalone function","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-27T15:34:07Z","receivedAt":"2012-05-27T15:34:07Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"Extract the normalize_path function so that it can be re-used elsewhere.\n\nSigned-off-by: Jon Seymour <jon.seymour@gmail.com>\n---\n git-submodule.sh | 28 ++++++++++++++++------------\n 1 file changed, 16 insertions(+), 12 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 64a70d6..dbbc905 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -176,6 +176,21 @@ module_clone()\n \t(clear_local_git_env; cd \"$sm_path\" && GIT_WORK_TREE=. git config core.worktree \"$rel/$b\")\n }\n \n+normalize_path()\n+{\n+\t# normalize path:\n+\t# multiple //; leading ./; /./; /../; trailing /\n+\tprintf '%s/\\n' \"$1\" |\n+\t\tsed -e '\n+\t\t\ts|//*|/|g\n+\t\t\ts|^\\(\\./\\)*||\n+\t\t\ts|/\\./|/|g\n+\t\t\t:start\n+\t\t\ts|\\([^/]*\\)/\\.\\./||\n+\t\t\ttstart\n+\t\t\ts|/*$||\n+\t\t'\n+}\n #\n # Add a new submodule to the working tree, .gitmodules and the index\n #\n@@ -250,18 +265,7 @@ cmd_add()\n \t;;\n \tesac\n \n-\t# normalize path:\n-\t# multiple //; leading ./; /./; /../; trailing /\n-\tsm_path=$(printf '%s/\\n' \"$sm_path\" |\n-\t\tsed -e '\n-\t\t\ts|//*|/|g\n-\t\t\ts|^\\(\\./\\)*||\n-\t\t\ts|/\\./|/|g\n-\t\t\t:start\n-\t\t\ts|\\([^/]*\\)/\\.\\./||\n-\t\t\ttstart\n-\t\t\ts|/*$||\n-\t\t')\n+\tsm_path=\"$(normalize_path \"$sm_path\")\"\n \tgit ls-files --error-unmatch \"$sm_path\" > /dev/null 2>&1 &&\n \tdie \"$(eval_gettext \"'\\$sm_path' already exists in the index\")\"\n \n-- \n1.7.10.2.656.g24a6219\n"},{"id":"192287","messageId":"1338132851-23497-7-git-send-email-jon.seymour@gmail.com","threadId":"30637","inReplyTo":"1338132851-23497-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH v7 6/9] submodule: fix detection of invalid submodule URL","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-27T15:34:08Z","receivedAt":"2012-05-27T15:34:08Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"Currently the superproject origin URL is progressively transformed\nby stepping through parts of the submodule URL and removing parts\nfrom the superproject URL for each leading ../ found in the\nsubmodule URL. No attempt is made to check that the edited URL still\nhas a path part left to remove. This can result in the construction\nof an absolute submodule URL where the hostname part of the URL\nhas been replaced by path components of the submodule URL.\n\nFor example: if the origin URL is ssh://hostname/repo and the\nsubmodule URL is ../../subrepo, then the origin URL of the subrepo\nwill be calculated as ssh://subrepo.\n\nWith this change, editing is only performed on the path part\nof the superproject origin URL. Any attempt by to consume the\nnon-path parts of the origin URL results in a failure.\n\nAs a side effect of preserving correct handling of support for\nURLs of the form user@host:repo, this change also fixes handling,\nby submodule init, of origin super project URLs of the form:\nfoo, foo/bar, ./foo and ./foo/bar.\n\nSigned-off-by: Jon Seymour <jon.seymour@gmail.com>\n---\n git-submodule.sh           | 41 ++++++++++++++++++++++++++++++++---------\n t/t7400-submodule-basic.sh | 23 ++++++++---------------\n 2 files changed, 40 insertions(+), 24 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex dbbc905..2550681 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -37,23 +37,42 @@ 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-\tremoteurl=${remoteurl%/}\n-\tsep=/\n+\tremoteurl=\"${remoteurl%/}\"\n+\n+\tcase \"$remoteurl\" in\n+\t\t*//*/*)\n+\t\t\tvariant=\"${remoteurl#*//*/}\"\n+\t\t;;\n+\t\t*::*)\n+\t\t\tvariant=\"${remoteurl#*::}\"\n+\t\t;;\n+\t\t*:*)\n+\t\t\tvariant=\"${remoteurl#*:}\"\n+\t\t;;\n+\t\t/*)\n+\t\t\tvariant=\"${remoteurl#/}\"\n+\t\t;;\n+\t\t*)\n+\t\t\tvariant=\"${remoteurl}\"\n+\t\t;;\n+\tesac\n+\tinvariant=\"${remoteurl%$variant}\"\n+\n \twhile test -n \"$url\"\n \tdo\n \t\tcase \"$url\" in\n \t\t../*)\n \t\t\turl=\"${url#../}\"\n-\t\t\tcase \"$remoteurl\" in\n+\t\t\tcase \"$variant\" in\n \t\t\t*/*)\n-\t\t\t\tremoteurl=\"${remoteurl%/*}\"\n+\t\t\t\tvariant=\"${variant%/*}\"\n \t\t\t\t;;\n-\t\t\t*:*)\n-\t\t\t\tremoteurl=\"${remoteurl%:*}\"\n-\t\t\t\tsep=:\n+\t\t\t.)\n+\t\t\t\tdie \"$(eval_gettext \"cannot strip one component off url '\\${invariant}'\")\"\n \t\t\t\t;;\n \t\t\t*)\n-\t\t\t\tdie \"$(eval_gettext \"cannot strip one component off url '\\$remoteurl'\")\"\n+\t\t\t\t# add a sentinel when .. matchs foo\n+\t\t\t\tvariant=.\n \t\t\t\t;;\n \t\t\tesac\n \t\t\t;;\n@@ -64,7 +83,11 @@ resolve_relative_url ()\n \t\t\tbreak;;\n \t\tesac\n \tdone\n-\techo \"$remoteurl$sep${url%/}\"\n+\t# ensure a trailing path separator\n+\tvariant=\"${variant}/\"\n+\t# strip the sentinel, if present\n+\tvariant=\"${variant#./}\"\n+\techo \"$invariant$variant${url%/}\"\n }\n \n #\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex 2674088..a94c5e9 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -576,7 +576,7 @@ test_expect_success '../subrepo works with helper URL- helper:://hostname/repo'\n \t)\n '\n \n-test_expect_failure '../../subrepo fails with URL - ssh://hostname/repo' \"\n+test_expect_success '../../subrepo fails with URL - ssh://hostname/repo' \"\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n@@ -585,12 +585,11 @@ test_expect_failure '../../subrepo fails with URL - ssh://hostname/repo' \"\n \t\tgit config -f .gitmodules submodule.sub.url ../../subrepo &&\n \t\techo cannot strip one component off url \\'ssh://hostname/\\' > expected &&\n \t\ttest_must_fail git submodule init 2>actual &&\n-\t\t#actual no failure, url configured as ssh://subrepo\n \t\ttest_cmp expected actual\n \t)\n \"\n \n-test_expect_failure '../../subrepo fails with absolute local path - /repo' \"\n+test_expect_success '../../subrepo fails with absolute local path - /repo' \"\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n@@ -603,7 +602,7 @@ test_expect_failure '../../subrepo fails with absolute local path - /repo' \"\n \t)\n \"\n \n-test_expect_failure '../../../subrepo fails with URL - ssh://hostname/repo' \"\n+test_expect_success '../../../subrepo fails with URL - ssh://hostname/repo' \"\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n@@ -612,12 +611,11 @@ test_expect_failure '../../../subrepo fails with URL - ssh://hostname/repo' \"\n \t\tgit config -f .gitmodules submodule.sub.url ../../../subrepo &&\n \t\techo cannot strip one component off url \\'ssh://hostname/\\' > expected &&\n \t\ttest_must_fail git submodule init 2>actual &&\n-\t\t#actual no failure, url configured as ssh:/subrepo\n \t\ttest_cmp expected actual\n \t)\n \"\n \n-test_expect_failure '../../../../subrepo fails with with URL - ssh://hostname/repo' \"\n+test_expect_success '../../../../subrepo fails with with URL - ssh://hostname/repo' \"\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n@@ -626,12 +624,11 @@ test_expect_failure '../../../../subrepo fails with with URL - ssh://hostname/re\n \t\tgit config -f .gitmodules submodule.sub.url ../../../../subrepo &&\n \t\techo cannot strip one component off url \\'ssh://hostname/\\' > expected &&\n \t\ttest_must_fail git submodule init 2>actual &&\n-\t\t#actual no failure, url configured as ssh:/subrepo\n \t\ttest_cmp expected actual\n \t)\n \"\n \n-test_expect_failure '../../../../../subrepo fails with URL - ssh://hostname/repo' \"\n+test_expect_success '../../../../../subrepo fails with URL - ssh://hostname/repo' \"\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n@@ -640,7 +637,6 @@ test_expect_failure '../../../../../subrepo fails with URL - ssh://hostname/repo\n \t\tgit config -f .gitmodules submodule.sub.url ../../../../../subrepo &&\n \t\techo cannot strip one component off url \\'ssh://hostname/\\' > expected &&\n \t\ttest_must_fail git submodule init 2>actual &&\n-\t\t#actual cannot strip one component off url 'ssh'\n \t\ttest_cmp expected actual\n \t)\n \"\n@@ -711,13 +707,12 @@ test_expect_failure 'relative path works with user@host:path/to/detour/../repo'\n \t)\n '\n \n-test_expect_failure '../subrepo works with relative local path - foo' '\n+test_expect_success '../subrepo works with relative local path - foo' '\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n \t\tcp pristine-.gitmodules .gitmodules &&\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@@ -734,26 +729,24 @@ test_expect_success '../subrepo works with relative local path - foo/bar' '\n \t)\n '\n \n-test_expect_failure '../subrepo works with relative local path - ./foo' '\n+test_expect_success '../subrepo works with relative local path - ./foo' '\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n \t\tcp pristine-.gitmodules .gitmodules &&\n \t\tgit config remote.origin.url ./foo &&\n \t\tgit submodule init &&\n-\t\t#actual ./subrepo\n \t\ttest \"$(git config submodule.sub.url)\" = subrepo\n \t)\n '\n \n-test_expect_failure '../subrepo works with relative local path - ./foo/bar' '\n+test_expect_success '../subrepo works with relative local path - ./foo/bar' '\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n \t\tcp pristine-.gitmodules .gitmodules &&\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-- \n1.7.10.2.656.g24a6219\n"},{"id":"192291","messageId":"1338132851-23497-8-git-send-email-jon.seymour@gmail.com","threadId":"30637","inReplyTo":"1338132851-23497-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH v7 7/9] submodule: fix sync handling of relative superproject origin URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-27T15:34:09Z","receivedAt":"2012-05-27T15:34:09Z","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, git submodule sync\nconfigures the remote.origin.url configuration property of the submodule\nwith a path that is relative to the work tree of the superproject\nrather than the work tree of the submodule.\n\nTo fix this an 'up_path' that navigates from the work tree of the submodule\nto the work tree of the superproject needs to be prepended to the URL\notherwise calculated.\n\nThis change fixes handling for relative superproject origin URLs like\nthe following:\n      foo\n      foo/bar\n      ./foo\n      ./foo/bar\n      ../foo\n      ../foo/bar\n\nThis change also renames the url variable used by git sync to module_url\nand introduces to new variables super_config_url and sub_origin_url to refer\nto the different URLs derived from the module_url.\n\nThe function blurb is expanded to give a more thorough description of what\nresolve_relative_url()'s function is intended to be.\n\nSigned-off-by: Jon Seymour <jon.seymour@gmail.com>\n---\n git-submodule.sh          | 48 ++++++++++++++++++++++++++++++++++++++++-------\n t/t7403-submodule-sync.sh | 23 ++++++++---------------\n 2 files changed, 49 insertions(+), 22 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 2550681..9ca2ffe 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -30,14 +30,32 @@ nofetch=\n update=\n prefix=\n \n-# Resolve relative url by appending to parent's url\n+# The function takes at most 2 arguments. The first argument is the\n+# relative URL that navigates from the superproject origin repo to the\n+# submodule origin repo. The second up_path argument, if specified, is\n+# the relative path that navigates from the submodule working tree to\n+# the superproject working tree.\n+#\n+# The output of the function is the origin URL of the submodule.\n+#\n+# The output will either be an absolute URL or filesystem path (if the\n+# superproject origin URL is an absolute URL or filesystem path,\n+# respectively) or a relative file system path (if the superproject\n+# origin URL is a relative file system path).\n+#\n+# When the output is a relative file system path, the path is either\n+# relative to the submodule working tree, if up_path is specified, or to\n+# the superproject working tree otherwise.\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 \tremoteurl=\"${remoteurl%/}\"\n+\tis_relative=\n \n \tcase \"$remoteurl\" in\n \t\t*//*/*)\n@@ -54,6 +72,7 @@ resolve_relative_url ()\n \t\t;;\n \t\t*)\n \t\t\tvariant=\"${remoteurl}\"\n+\t\t\tis_relative=t\n \t\t;;\n \tesac\n \tinvariant=\"${remoteurl%$variant}\"\n@@ -83,11 +102,13 @@ resolve_relative_url ()\n \t\t\tbreak;;\n \t\tesac\n \tdone\n+\n \t# ensure a trailing path separator\n \tvariant=\"${variant}/\"\n \t# strip the sentinel, if present\n \tvariant=\"${variant#./}\"\n-\techo \"$invariant$variant${url%/}\"\n+\n+\techo \"$invariant${is_relative:+$up_path}$variant${url%/}\"\n }\n \n #\n@@ -986,19 +1007,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@@ -1006,7 +1040,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/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh\nindex 56b933d..b7466ba 100755\n--- a/t/t7403-submodule-sync.sh\n+++ b/t/t7403-submodule-sync.sh\n@@ -88,73 +88,67 @@ 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 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 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 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 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 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 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/bar with deeply nested submodule' '\n+test_expect_success '\"git submodule sync\" handles origin URL of the form ../foo/bar with deeply nested submodule' '\n \t(cd relative-clone &&\n \t git remote set-url origin ../foo/bar &&\n \t mkdir -p a/b/c &&\n@@ -167,11 +161,10 @@ test_expect_failure '\"git submodule sync\" handles origin URL of the form ../foo/\n \t git submodule add ../bar/a/b/c ./a/b/c &&\n \t git submodule sync &&\n \t(cd a/b/c &&\n-\t #actual ../foo/bar/a/b/c\n \t test \"$(git config remote.origin.url)\" = \"../../../../foo/bar/a/b/c\"\n \t)\n \t)\n '\n \n-\n test_done\n+<\n-- \n1.7.10.2.656.g24a6219\n"},{"id":"192292","messageId":"1338132851-23497-9-git-send-email-jon.seymour@gmail.com","threadId":"30637","inReplyTo":"1338132851-23497-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH v7 8/9] submodule: fix handling of denormalized superproject origin URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-27T15:34:10Z","receivedAt":"2012-05-27T15:34:10Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"Currently git calculates the submodule origin URL incorrectly in\nthe case that the superproject origin URL is denormalized.\n\nSo, we normalize the path part of the superproject URL before iterating\nover the leading ../ parts of the submodule URL.\n\nA remaining problem related to the handling of consecutive repeated ./'s\nin the superproject origin URL is deferred to a subsequent commit.\n\nThis change also fixes a subtle error in the setup of some tests which was\nmasked by the denormalization issue that is now fixed.\n\nPrevious behaviour was relying on submodule add to clone trash/submodule\ninto super/submodule, however from the perspective of super's origin (i.e. trash),\nthe origin submodule is actually located at ./submodule not ../submodule.\n\nHowever, because the origin URL of super was denormalized (it had a trailing /.)\nthe incorrect handling of denormalized super URLs actually produced\nthe correct result - a case of two errors cancelling out each other's\neffects.\n\nNow that normalization is fixed, the erroneous use of git submodule add\nby the test setups needs to be fixed. The cleanest way to do this is to\nclone super not from ., but from ./omega. The subsequent invocation of\n\n   git submodule add ../submodule submodule\n\nnow does the expected thing because ../submodule is the correct\npath from omega to the submodule origin repo.\n\nSigned-off-by: Jon Seymour <jon.seymour@gmail.com>\n---\n git-submodule.sh             |  1 +\n t/t7400-submodule-basic.sh   | 10 +++++-----\n t/t7403-submodule-sync.sh    | 14 +++++++++-----\n t/t7406-submodule-update.sh  | 16 ++++++++++------\n t/t7407-submodule-foreach.sh | 14 +++++++++-----\n t/t7506-status-submodule.sh  | 10 +++++++++-\n 6 files changed, 43 insertions(+), 22 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 9ca2ffe..1f0983c 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -76,6 +76,7 @@ resolve_relative_url ()\n \t\t;;\n \tesac\n \tinvariant=\"${remoteurl%$variant}\"\n+\tvariant=\"$(normalize_path \"$variant\")\"\n \n \twhile test -n \"$url\"\n \tdo\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex a94c5e9..b01f479 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -521,7 +521,7 @@ test_expect_failure 'relative path works with URL - ssh://hostname/path/detour/.\n \t)\n '\n \n-test_expect_failure 'relative path works with URL - ssh://hostname/path/repo/.' '\n+test_expect_success 'relative path works with URL - ssh://hostname/path/repo/.' '\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n@@ -663,7 +663,7 @@ test_expect_success '../subrepo works with scp-style URL - user@host:path/to/rep\n \t)\n '\n \n-test_expect_failure 'relative path works with user@host:path/to/repo/.' '\n+test_expect_success 'relative path works with user@host:path/to/repo/.' '\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n@@ -674,7 +674,7 @@ test_expect_failure 'relative path works with user@host:path/to/repo/.' '\n \t)\n '\n \n-test_expect_failure 'relative path works with user@host:path/to/./repo' '\n+test_expect_success 'relative path works with user@host:path/to/./repo' '\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n@@ -696,7 +696,7 @@ test_expect_failure 'relative path works with user@host:path/to/././repo' '\n \t)\n '\n \n-test_expect_failure 'relative path works with user@host:path/to/detour/../repo' '\n+test_expect_success 'relative path works with user@host:path/to/detour/../repo' '\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n@@ -773,7 +773,7 @@ test_expect_success '../subrepo works with relative local path - ../foo/bar' '\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 &&\ndiff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh\nindex b7466ba..d76e49f 100755\n--- a/t/t7403-submodule-sync.sh\n+++ b/t/t7403-submodule-sync.sh\n@@ -11,11 +11,15 @@ These tests exercise the \"git submodule sync\" subcommand.\n . ./test-lib.sh\n \n test_expect_success setup '\n-\techo file > file &&\n-\tgit add file &&\n-\ttest_tick &&\n-\tgit commit -m upstream &&\n-\tgit clone . super &&\n+\tmkdir omega &&\n+\t(cd omega &&\n+\t git init &&\n+\t echo file > file &&\n+\t git add file &&\n+\t test_tick &&\n+\t git commit -m upstream\n+\t) &&\n+\tgit clone omega super &&\n \tgit clone super submodule &&\n \t(cd super &&\n \t git submodule add ../submodule submodule &&\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex dcb195b..8b6c330 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -22,11 +22,15 @@ compare_head()\n \n \n test_expect_success 'setup a submodule tree' '\n-\techo file > file &&\n-\tgit add file &&\n-\ttest_tick &&\n-\tgit commit -m upstream &&\n-\tgit clone . super &&\n+\tmkdir omega &&\n+\t(cd omega &&\n+\t git init &&\n+\t echo file > file &&\n+\t git add file &&\n+\t test_tick &&\n+\t git commit -m upstream\n+\t) &&\n+\tgit clone omega super &&\n \tgit clone super submodule &&\n \tgit clone super rebasing &&\n \tgit clone super merging &&\n@@ -58,7 +62,7 @@ test_expect_success 'setup a submodule tree' '\n \t git submodule add ../merging merging &&\n \t test_tick &&\n \t git commit -m \"rebasing\"\n-\t)\n+\t) &&\n \t(cd super &&\n \t git submodule add ../none none &&\n \t test_tick &&\ndiff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\nindex 9b69fe2..40f957c 100755\n--- a/t/t7407-submodule-foreach.sh\n+++ b/t/t7407-submodule-foreach.sh\n@@ -13,11 +13,15 @@ that are currently checked out.\n \n \n test_expect_success 'setup a submodule tree' '\n-\techo file > file &&\n-\tgit add file &&\n-\ttest_tick &&\n-\tgit commit -m upstream &&\n-\tgit clone . super &&\n+\tmkdir omega &&\n+\t(cd omega &&\n+\t git init &&\n+\t echo file > file &&\n+\t git add file &&\n+\t test_tick &&\n+\t git commit -m upstream\n+\t) &&\n+\tgit clone omega super &&\n \tgit clone super submodule &&\n \t(\n \t\tcd super &&\ndiff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh\nindex d31b34d..764c1d0 100755\n--- a/t/t7506-status-submodule.sh\n+++ b/t/t7506-status-submodule.sh\n@@ -197,7 +197,15 @@ A  sub1\n EOF\n \n test_expect_success 'status with merge conflict in .gitmodules' '\n-\tgit clone . super &&\n+\tmkdir omega &&\n+\t(cd omega &&\n+\t git init &&\n+\t echo file > file &&\n+\t git add file &&\n+\t test_tick &&\n+\t git commit -m upstream\n+\t) &&\n+\tgit clone omega super &&\n \ttest_create_repo_with_commit sub1 &&\n \ttest_tick &&\n \ttest_create_repo_with_commit sub2 &&\n-- \n1.7.10.2.656.g24a6219\n"},{"id":"192293","messageId":"1338132851-23497-10-git-send-email-jon.seymour@gmail.com","threadId":"30637","inReplyTo":"1338132851-23497-1-git-send-email-jon.seymour@gmail.com","subject":"[PATCH v7 9/9] submodule: fix normalization to handle repeated ./","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-27T15:34:11Z","receivedAt":"2012-05-27T15:34:11Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"Currently path/./foo/./bar is denormalized correctly, but path/foo/././bar\nis not.\n\nWe fix the normalization script to allow repeated application of the\n./ -> / normalization in the same way that foo/.. is handled - by\nmoving it inside a sed loop.\n\nThe existing sed label, start, is renamed to fooslashdotdotslash to\nindicate which of the two loops is being refered to by the second\nbranch directive.\n\nSigned-off-by: Jon Seymour <jon.seymour@gmail.com>\n---\n git-submodule.sh           | 8 +++++---\n t/t7400-submodule-basic.sh | 6 +++---\n 2 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 1f0983c..8f3bc71 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -229,10 +229,12 @@ normalize_path()\n \t\tsed -e '\n \t\t\ts|//*|/|g\n \t\t\ts|^\\(\\./\\)*||\n-\t\t\ts|/\\./|/|g\n-\t\t\t:start\n+\t\t\t:slashdotslash\n+\t\t\ts|/\\./|/|\n+\t\t\ttslashdotslash\n+\t\t\t:fooslashdotdotslash\n \t\t\ts|\\([^/]*\\)/\\.\\./||\n-\t\t\ttstart\n+\t\t\ttfooslashdotdotslash\n \t\t\ts|/*$||\n \t\t'\n }\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex b01f479..61887b2 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -499,7 +499,7 @@ test_expect_success '../subrepo works with URL - ssh://hostname/repo' '\n \t)\n '\n \n-test_expect_failure 'relative path works with URL - ssh://hostname/path/././repo' '\n+test_expect_success 'relative path works with URL - ssh://hostname/path/././repo' '\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n@@ -510,7 +510,7 @@ test_expect_failure 'relative path works with URL - ssh://hostname/path/././repo\n \t)\n '\n \n-test_expect_failure 'relative path works with URL - ssh://hostname/path/detour/././../repo' '\n+test_expect_success 'relative path works with URL - ssh://hostname/path/detour/././../repo' '\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n@@ -685,7 +685,7 @@ test_expect_success 'relative path works with user@host:path/to/./repo' '\n \t)\n '\n \n-test_expect_failure 'relative path works with user@host:path/to/././repo' '\n+test_expect_success 'relative path works with user@host:path/to/././repo' '\n \t(\n \t\tcd reltest &&\n \t\tcp pristine-.git-config .git/config &&\n-- \n1.7.10.2.656.g24a6219\n"},{"id":"192299","messageId":"CAH3AnrpTgwHDDLKM=OraEZfBRDyKzf1jjgqaBCJwkudDvsWkEQ@mail.gmail.com","threadId":"30637","inReplyTo":"1338132851-23497-9-git-send-email-jon.seymour@gmail.com","subject":"Re: [PATCH v7 8/9] submodule: fix handling of denormalized superproject origin URLs","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-27T22:57:53Z","receivedAt":"2012-05-27T22:57:53Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"On Mon, May 28, 2012 at 1:34 AM, Jon Seymour <jon.seymour@gmail.com> wrote:\n> This change also fixes a subtle error in the setup of some tests which was\n> masked by the denormalization issue that is now fixed.\n>\n\nI guess this patch could be improved by moving the change to the test\nsetup to a separate, earlier patch and also including a separate test\nwhich demonstrates the\nflawed behaviour described by the commit message. Anyone disagree?\n\njon.\n"},{"id":"192390","messageId":"4FC3CB7E.6000501@kdbg.org","threadId":"30637","inReplyTo":"1338132851-23497-7-git-send-email-jon.seymour@gmail.com","subject":"Re: [PATCH v7 6/9] submodule: fix detection of invalid submodule URL","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-05-28T19:01:18Z","receivedAt":"2012-05-28T19:01:18Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 27.05.2012 17:34, schrieb Jon Seymour:\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index dbbc905..2550681 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -37,23 +37,42 @@ 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> -\tremoteurl=${remoteurl%/}\n> -\tsep=/\n> +\tremoteurl=\"${remoteurl%/}\"\n> +\n> +\tcase \"$remoteurl\" in\n> +\t\t*//*/*)\n> +\t\t\tvariant=\"${remoteurl#*//*/}\"\n> +\t\t;;\n> +\t\t*::*)\n> +\t\t\tvariant=\"${remoteurl#*::}\"\n> +\t\t;;\n> +\t\t*:*)\n> +\t\t\tvariant=\"${remoteurl#*:}\"\n> +\t\t;;\n> +\t\t/*)\n> +\t\t\tvariant=\"${remoteurl#/}\"\n\nWithout understanding in detail what this series is about, I would guess\nthat the previous two case arms are not very Windows friendly. Does the\nright thing happen when $remoteurl is \"c:/path/to/remote\"? Would it help\nto use is_absolute_path?\n\n\tif is_absolute_path \"$remoteurl\"\n\tthen\n\t\tvariant=\"${remoteurl#*/}\"\n\telse\n\t\tcase \"$remoteurl\" in\n\t\t...other cases go here...\n\t\tesac\n\tfi\n\n-- Hannes\n"},{"id":"192396","messageId":"4FC3DAEF.1070508@web.de","threadId":"30637","inReplyTo":"1338132851-23497-1-git-send-email-jon.seymour@gmail.com","subject":"Re: [PATCH v7 0/9] submodule: improve robustness of path handling","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-05-28T20:07:11Z","receivedAt":"2012-05-28T20:07:11Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 27.05.2012 17:34, schrieb Jon Seymour:\n> This series improves the robustness of path handling by 'git submodule' by:\n> \n> * detecting submodule URLs that will result in non-sensical submodule origin URLs\n> \n> * improving handling of various kinds of relative superproject origin URLs\n> \n> * improving handling of various kinds of denormalized superproject origin URLs\n\nHmm, this has become a quite invasive patch series. While I bought the\nuse case of having a superproject with a relative url and was inclined\nto accept that it might even not start \"./\" or \"../\" (even though that\nis a pretty unusual use and can be easily fixed by prepending a \"./\"),\nI'm not sure the in depth check of URLs is worth the code churn. And\nespecially the high probability of breaking other peoples use cases in\nrather subtle ways worry me (this did happen quite often when the\nsubmodule script was changed in the past; as an example take the\nwindows path issues Johannes already pointed out in his email). And I\ncan't remember bug reports that people complained about URL problems\ndue to the issues you intend to fix here, which makes me think they\nmight be well intended but possibly unnecessary (but my memory might\nserver me wrong here).\n\nSo I'd vote for just fixing the relative submodule path issues and to\nnot care about the possible issues with URLs. Opinions?\n\n(And patches 6-8 contain changes to test cases other than just changing\ntest_expect_failure to test_expect_success which makes reviewing this\nseries unnecessarily hard)\n"},{"id":"192402","messageId":"CAH3Anrrg4Fc5GXB_VwOXRfwP=hx5Xn5bqimP56oDB0USn7c4Cg@mail.gmail.com","threadId":"30637","inReplyTo":"4FC3CB7E.6000501@kdbg.org","subject":"Re: [PATCH v7 6/9] submodule: fix detection of invalid submodule URL","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-28T21:39:29Z","receivedAt":"2012-05-28T21:39:29Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"On Tue, May 29, 2012 at 5:01 AM, Johannes Sixt <j6t@kdbg.org> wrote:\n> Am 27.05.2012 17:34, schrieb Jon Seymour:\n>\n> Without understanding in detail what this series is about, I would guess\n> that the previous two case arms are not very Windows friendly. Does the\n> right thing happen when $remoteurl is \"c:/path/to/remote\"? Would it help\n> to use is_absolute_path?\n>\n>        if is_absolute_path \"$remoteurl\"\n>        then\n>                variant=\"${remoteurl#*/}\"\n>        else\n>                case \"$remoteurl\" in\n>                ...other cases go here...\n>                esac\n>        fi\n>\n> -- Hannes\n\nThanks, I will investigate this as an alternative.\n\njon.\n"},{"id":"192404","messageId":"CAH3AnroT1vs-s==ykNyogq6gbVncY0pt5U1=fMp+b6B0jwG19Q@mail.gmail.com","threadId":"30637","inReplyTo":"4FC3DAEF.1070508@web.de","subject":"Re: [PATCH v7 0/9] submodule: improve robustness of path handling","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-05-28T22:01:19Z","receivedAt":"2012-05-28T22:01:19Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"On Tue, May 29, 2012 at 6:07 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:\n> Am 27.05.2012 17:34, schrieb Jon Seymour:\n>> This series improves the robustness of path handling by 'git submodule' by:\n>>\n>> * detecting submodule URLs that will result in non-sensical submodule origin URLs\n>>\n>> * improving handling of various kinds of relative superproject origin URLs\n>>\n>> * improving handling of various kinds of denormalized superproject origin URLs\n>\n> Hmm, this has become a quite invasive patch series. While I bought the\n> use case of having a superproject with a relative url and was inclined\n> to accept that it might even not start \"./\" or \"../\" (even though that\n> is a pretty unusual use and can be easily fixed by prepending a \"./\"),\n> I'm not sure the in depth check of URLs is worth the code churn. And\n> especially the high probability of breaking other peoples use cases in\n> rather subtle ways worry me (this did happen quite often when the\n> submodule script was changed in the past; as an example take the\n> windows path issues Johannes already pointed out in his email). And I\n> can't remember bug reports that people complained about URL problems\n> due to the issues you intend to fix here, which makes me think they\n> might be well intended but possibly unnecessary (but my memory might\n> server me wrong here).\n>\n> So I'd vote for just fixing the relative submodule path issues and to\n> not care about the possible issues with URLs. Opinions?\n\nI'll write a minimal patch to solve my relative path problem without\nfixing the invalid/\"greedy\" submodule url or url normalization issues.\n\nThe reason I went with a more extensive series is that the change in\n6/9 considerably simplified the change I wanted to make in 7/9 while\nat the same time making the path handling of resolve_relative_url more\nprecise, in the sense documented by the tests in 2/9.\n\nThe refactoring in 5/9 and the changes in 8/9 and 9/9 are related to\nrenormalization which I realised was a weakness (if not a problem\npeople were complaining about) in the original code as documented by\nthe tests in 4/9.\n\nDo you have any comments about whether the failures documented in 2/9\nand 4/9 are worth noting, at least, as weaknesses?\n\n>\n> (And patches 6-8 contain changes to test cases other than just changing\n> test_expect_failure to test_expect_success which makes reviewing this\n> series unnecessarily hard)\n\nAgree absolutely about patch 8 - I will re-roll with separate tests to\ndocument the test setup issue I fixed in 8.\n\nThe only other changes to tests in 6 and 7 were the removal of\ncomments about the actual bad behaviour. Would your preference be that\nI removed these #actual comments completely or that I moved\ndocumentation of the actual behaviour to the header of the test?\n\njon.\n"},{"id":"192429","messageId":"4FC521AE.1010707@web.de","threadId":"30637","inReplyTo":"CAH3AnroT1vs-s==ykNyogq6gbVncY0pt5U1=fMp+b6B0jwG19Q@mail.gmail.com","subject":"Re: [PATCH v7 0/9] submodule: improve robustness of path handling","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2012-05-29T19:21:18Z","receivedAt":"2012-05-29T19:21:18Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 29.05.2012 00:01, schrieb Jon Seymour:\n> On Tue, May 29, 2012 at 6:07 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:\n>> So I'd vote for just fixing the relative submodule path issues and to\n>> not care about the possible issues with URLs. Opinions?\n> \n> I'll write a minimal patch to solve my relative path problem without\n> fixing the invalid/\"greedy\" submodule url or url normalization issues.\n\nI'd really appreciate that.\n\n> Do you have any comments about whether the failures documented in 2/9\n> and 4/9 are worth noting, at least, as weaknesses?\n\nSure, they document known problems. Me thinks they all should be\nsquashed into a single patch and submitted separately. The following\nthree tests from 2/9 are redundant and can be dropped (they are\nalready handled by the '../../subrepo' case):\n\n    '../../../subrepo fails with URL - ssh://hostname/repo' \"\n    '../../../../subrepo fails with with URL - ssh://hostname/repo' \"\n    '../../../../../subrepo fails with URL - ssh://hostname/repo' \"\n\n>> (And patches 6-8 contain changes to test cases other than just changing\n>> test_expect_failure to test_expect_success which makes reviewing this\n>> series unnecessarily hard)\n> \n> Agree absolutely about patch 8 - I will re-roll with separate tests to\n> document the test setup issue I fixed in 8.\n> \n> The only other changes to tests in 6 and 7 were the removal of\n> comments about the actual bad behaviour. Would your preference be that\n> I removed these #actual comments completely or that I moved\n> documentation of the actual behaviour to the header of the test?\n\nI'd prefer just to see the failure => success changes, so the comments\nlook superfluous to me and should be dropped from the failure case.\n"},{"id":"192748","messageId":"CAH3Anrp_aUR2O_iEwxHu4bRs83U58X6QsY6+SJ56NXKEC7LA5Q@mail.gmail.com","threadId":"30637","inReplyTo":"CAH3Anrrg4Fc5GXB_VwOXRfwP=hx5Xn5bqimP56oDB0USn7c4Cg@mail.gmail.com","subject":"Re: [PATCH v7 6/9] submodule: fix detection of invalid submodule URL","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2012-06-03T09:51:00Z","receivedAt":"2012-06-03T09:51:00Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"On Tue, May 29, 2012 at 7:39 AM, Jon Seymour <jon.seymour@gmail.com> wrote:\n> On Tue, May 29, 2012 at 5:01 AM, Johannes Sixt <j6t@kdbg.org> wrote:\n>> Am 27.05.2012 17:34, schrieb Jon Seymour:\n>>\n>> Without understanding in detail what this series is about, I would guess\n>> that the previous two case arms are not very Windows friendly. Does the\n>> right thing happen when $remoteurl is \"c:/path/to/remote\"? Would it help\n>> to use is_absolute_path?\n>>\n>>        if is_absolute_path \"$remoteurl\"\n>>        then\n>>                variant=\"${remoteurl#*/}\"\n>>        else\n>>                case \"$remoteurl\" in\n>>                ...other cases go here...\n>>                esac\n>>        fi\n>>\n>> -- Hannes\n>\n> Thanks, I will investigate this as an alternative.\n>\n\nI did investigate is_absolute_path for the v8 roll of this series, but\nI found it wasn't suitable because it doesn't classify URLs of the\nform user@host:repo as absolute. You can find the alternative I did\nuse in v8 3/4.\n\nRegards,\n\njon.\n"}]}