* [PATCH v7 0/9] submodule: improve robustness of path handling
@ 2012-05-27 15:34 Jon Seymour
2012-05-27 15:34 ` [PATCH v7 1/9] submodule: additional regression tests for relative URLs Jon Seymour
` (9 more replies)
0 siblings, 10 replies; 17+ messages in thread
From: Jon Seymour @ 2012-05-27 15:34 UTC (permalink / raw)
To: git; +Cc: Jens.Lehmann, gitster, phil.hord, ramsay, Jon Seymour
This series improves the robustness of path handling by 'git submodule' by:
* detecting submodule URLs that will result in non-sensical submodule origin URLs
* improving handling of various kinds of relative superproject origin URLs
* improving handling of various kinds of denormalized superproject origin URLs
This series differs from v5 in the following ways, by:
* Adding a more extensive set of failure tests to illustrate the conditions
being addressed.
* Modifying the ../ processing loop in resolve_relative_url to exclude the
'invariant' parts of absolute URLs from relative URL processing and thereby
enable earlier and more accurate detection of edits that are going to
produce a non-sensical output.
* Simplifying relative superproject origin URL support, by taking advantage of
the modifications above.
* Adding support for normalizing denormalized superproject origin URLs.
* Adding some additional regression tests to help guard against
unintended regressions by this series.
* Improving the source code comments to better explain the purpose
of various code functions and code blocks
This series differs from v6 by applying the fix in 8/9 to a wider set of tests.
Each patch in the series has been regression tested against the following tests:
t2013-checkout-submodule.sh
t2103-update-index-ignore-missing.sh
t2105-update-index-gitfile.sh
t2201-add-update-typechange.sh
t3000-ls-files-others.sh
t3030-merge-recursive.sh
t3404-rebase-interactive.sh
t4027-diff-submodule.sh
t4041-diff-submodule-option.sh
t4134-apply-submodule.sh
t5526-fetch-submodules.sh
t5531-deep-submodule-push.sh
t6008-rev-list-submodule.sh
t7003-filter-branch.sh
t7400-submodule-basic.sh
t7401-submodule-summary.sh
t7402-submodule-rebase.sh
t7403-submodule-sync.sh
t7405-submodule-merge.sh
t7406-submodule-update.sh
t7407-submodule-foreach.sh
t7408-submodule-reference.sh
t7506-status-submodule.sh
t7508-status.sh
t7610-mergetool.sh
t9300-fast-import.sh
t9350-fast-export.sh
which are the tests that match a grep search for submodule.
Jon Seymour (9):
submodule: additional regression tests for relative URLs
submodule: document failure to detect invalid submodule URLs
submodule: document failure to handle relative superproject origin
URLs
submodule: document failure to handle improperly normalized remote
origin URLs
submodule: extract normalize_path into standalone function
submodule: fix detection of invalid submodule URL
submodule: fix sync handling of relative superproject origin URLs
submodule: fix handling of denormalized superproject origin URLs
submodule: fix normalization to handle repeated ./
git-submodule.sh | 118 +++++++++++++----
t/t7400-submodule-basic.sh | 297 ++++++++++++++++++++++++++++++++++++++++++-
t/t7403-submodule-sync.sh | 97 +++++++++++++-
t/t7406-submodule-update.sh | 16 ++-
t/t7407-submodule-foreach.sh | 14 +-
t/t7506-status-submodule.sh | 10 +-
6 files changed, 504 insertions(+), 48 deletions(-)
--
1.7.10.2.656.g24a6219
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v7 1/9] submodule: additional regression tests for relative URLs
2012-05-27 15:34 [PATCH v7 0/9] submodule: improve robustness of path handling Jon Seymour
@ 2012-05-27 15:34 ` Jon Seymour
2012-05-27 15:34 ` [PATCH v7 2/9] submodule: document failure to detect invalid submodule URLs Jon Seymour
` (8 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Jon Seymour @ 2012-05-27 15:34 UTC (permalink / raw)
To: git; +Cc: Jens.Lehmann, gitster, phil.hord, ramsay, Jon Seymour
Some additional tests are added to support regression testing of the changes in the
remainder of the series.
We also add a pristine copy of .gitmodules in anticipation of this being
required by later tests.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
t/t7400-submodule-basic.sh | 110 +++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 107 insertions(+), 3 deletions(-)
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 81827e6..9428c7a 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -483,21 +483,67 @@ test_expect_success 'set up for relative path tests' '
git add sub &&
git config -f .gitmodules submodule.sub.path sub &&
git config -f .gitmodules submodule.sub.url ../subrepo &&
- cp .git/config pristine-.git-config
+ cp .git/config pristine-.git-config &&
+ cp .gitmodules pristine-.gitmodules
)
'
-test_expect_success 'relative path works with URL' '
+test_expect_success '../subrepo works with URL - ssh://hostname/repo' '
(
cd reltest &&
cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
git config remote.origin.url ssh://hostname/repo &&
git submodule init &&
test "$(git config submodule.sub.url)" = ssh://hostname/subrepo
)
'
-test_expect_success 'relative path works with user@host:path' '
+test_expect_success '../subrepo works with port-qualified URL - ssh://hostname:22/repo' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url ssh://hostname:22/repo &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = ssh://hostname:22/subrepo
+ )
+'
+
+test_expect_success '../subrepo path works with local path - /foo/repo' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url /foo/repo &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = /foo/subrepo
+ )
+'
+
+test_expect_success '../subrepo works with file URL - file:///tmp/repo' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url file:///tmp/repo &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = file:///tmp/subrepo
+ )
+'
+
+test_expect_success '../subrepo works with helper URL- helper:://hostname/repo' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url helper:://hostname/repo &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = helper:://hostname/subrepo
+ )
+'
+
+test_expect_success '../subrepo works with scp-style URL - user@host:repo' '
(
cd reltest &&
cp pristine-.git-config .git/config &&
@@ -507,6 +553,64 @@ test_expect_success 'relative path works with user@host:path' '
)
'
+test_expect_success '../subrepo works with scp-style URL - user@host:path/to/repo' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url user@host:path/to/repo &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = user@host:path/to/subrepo
+ )
+'
+
+test_expect_success '../subrepo works with relative local path - foo/bar' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url foo/bar &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = foo/subrepo
+ )
+'
+
+test_expect_success '../subrepo works with relative local path - ../foo' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url ../foo &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = ../subrepo
+ )
+'
+
+test_expect_success '../subrepo works with relative local path - ../foo/bar' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url ../foo/bar &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = ../foo/subrepo
+ )
+'
+
+test_expect_success '../bar/a/b/c works with relative local path - ../foo/bar.git' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ mkdir -p a/b/c &&
+ (cd a/b/c; git init) &&
+ git config remote.origin.url ../foo/bar.git &&
+ git submodule add ../bar/a/b/c ./a/b/c &&
+ git submodule init &&
+ test "$(git config submodule.a/b/c.url)" = ../foo/bar/a/b/c
+ )
+'
+
test_expect_success 'moving the superproject does not break submodules' '
(
cd addtest &&
--
1.7.10.2.656.g24a6219
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 2/9] submodule: document failure to detect invalid submodule URLs
2012-05-27 15:34 [PATCH v7 0/9] submodule: improve robustness of path handling Jon Seymour
2012-05-27 15:34 ` [PATCH v7 1/9] submodule: additional regression tests for relative URLs Jon Seymour
@ 2012-05-27 15:34 ` Jon Seymour
2012-05-27 15:34 ` [PATCH v7 3/9] submodule: document failure to handle relative superproject origin URLs Jon Seymour
` (7 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Jon Seymour @ 2012-05-27 15:34 UTC (permalink / raw)
To: git; +Cc: Jens.Lehmann, gitster, phil.hord, ramsay, Jon Seymour
These tests document failures to detect submodule URLs that backtrack
past the 'invariant' part of a superproject's origin URL.
For example: if the origin URL is ssh://hostname/repo, then currently
if a submodule URL is specified as ../../subrepo, then git will
construct a submodule url of the form ssh://subrepo/ without complaint.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
t/t7400-submodule-basic.sh | 70 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 9428c7a..a758c63 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -543,10 +543,80 @@ test_expect_success '../subrepo works with helper URL- helper:://hostname/repo'
)
'
+test_expect_failure '../../subrepo fails with URL - ssh://hostname/repo' "
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url ssh://hostname/repo &&
+ git config -f .gitmodules submodule.sub.url ../../subrepo &&
+ echo cannot strip one component off url \'ssh://hostname/\' > expected &&
+ test_must_fail git submodule init 2>actual &&
+ #actual no failure, url configured as ssh://subrepo
+ test_cmp expected actual
+ )
+"
+
+test_expect_failure '../../subrepo fails with absolute local path - /repo' "
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url /repo &&
+ git config -f .gitmodules submodule.sub.url ../../subrepo &&
+ echo cannot strip one component off url \'/\' > expected &&
+ test_must_fail git submodule init 2>actual &&
+ test_cmp expected actual
+ )
+"
+
+test_expect_failure '../../../subrepo fails with URL - ssh://hostname/repo' "
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url ssh://hostname/repo &&
+ git config -f .gitmodules submodule.sub.url ../../../subrepo &&
+ echo cannot strip one component off url \'ssh://hostname/\' > expected &&
+ test_must_fail git submodule init 2>actual &&
+ #actual no failure, url configured as ssh:/subrepo
+ test_cmp expected actual
+ )
+"
+
+test_expect_failure '../../../../subrepo fails with with URL - ssh://hostname/repo' "
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url ssh://hostname/repo &&
+ git config -f .gitmodules submodule.sub.url ../../../../subrepo &&
+ echo cannot strip one component off url \'ssh://hostname/\' > expected &&
+ test_must_fail git submodule init 2>actual &&
+ #actual no failure, url configured as ssh:/subrepo
+ test_cmp expected actual
+ )
+"
+
+test_expect_failure '../../../../../subrepo fails with URL - ssh://hostname/repo' "
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url ssh://hostname/repo &&
+ git config -f .gitmodules submodule.sub.url ../../../../../subrepo &&
+ echo cannot strip one component off url \'ssh://hostname/\' > expected &&
+ test_must_fail git submodule init 2>actual &&
+ #actual cannot strip one component off url 'ssh'
+ test_cmp expected actual
+ )
+"
+
test_expect_success '../subrepo works with scp-style URL - user@host:repo' '
(
cd reltest &&
cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
git config remote.origin.url user@host:repo &&
git submodule init &&
test "$(git config submodule.sub.url)" = user@host:subrepo
--
1.7.10.2.656.g24a6219
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 3/9] submodule: document failure to handle relative superproject origin URLs
2012-05-27 15:34 [PATCH v7 0/9] submodule: improve robustness of path handling Jon Seymour
2012-05-27 15:34 ` [PATCH v7 1/9] submodule: additional regression tests for relative URLs Jon Seymour
2012-05-27 15:34 ` [PATCH v7 2/9] submodule: document failure to detect invalid submodule URLs Jon Seymour
@ 2012-05-27 15:34 ` Jon Seymour
2012-05-27 15:34 ` [PATCH v7 4/9] submodule: document failure to handle improperly normalized remote " Jon Seymour
` (6 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Jon Seymour @ 2012-05-27 15:34 UTC (permalink / raw)
To: git; +Cc: Jens.Lehmann, gitster, phil.hord, ramsay, Jon Seymour
This test case documents several cases where handling of relative
superproject origin URLs doesn't produce an expected result.
submodule.{sub}.url in the superproject is incorrect in these cases:
foo
./foo
./foo/bar
The remote.origin.url of the submodule is incorrect in the above cases
and also when the superproject origin URL is like:
foo/bar
../foo
../foo/bar
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
t/t7400-submodule-basic.sh | 36 +++++++++++++++++++
t/t7403-submodule-sync.sh | 90 +++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 125 insertions(+), 1 deletion(-)
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index a758c63..80ec0f7 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -634,6 +634,18 @@ test_expect_success '../subrepo works with scp-style URL - user@host:path/to/rep
)
'
+test_expect_failure '../subrepo works with relative local path - foo' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url foo &&
+ # actual: fails with an error
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = subrepo
+ )
+'
+
test_expect_success '../subrepo works with relative local path - foo/bar' '
(
cd reltest &&
@@ -645,6 +657,30 @@ test_expect_success '../subrepo works with relative local path - foo/bar' '
)
'
+test_expect_failure '../subrepo works with relative local path - ./foo' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url ./foo &&
+ git submodule init &&
+ #actual ./subrepo
+ test "$(git config submodule.sub.url)" = subrepo
+ )
+'
+
+test_expect_failure '../subrepo works with relative local path - ./foo/bar' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url ./foo/bar &&
+ git submodule init &&
+ #actual: ./foo/subrepo
+ test "$(git config submodule.sub.url)" = foo/subrepo
+ )
+'
+
test_expect_success '../subrepo works with relative local path - ../foo' '
(
cd reltest &&
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index 3620215..56b933d 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh
@@ -26,7 +26,9 @@ test_expect_success setup '
(cd super-clone && git submodule update --init) &&
git clone super empty-clone &&
(cd empty-clone && git submodule init) &&
- git clone super top-only-clone
+ git clone super top-only-clone &&
+ git clone super relative-clone &&
+ (cd relative-clone && git submodule update --init)
'
test_expect_success 'change submodule' '
@@ -86,4 +88,90 @@ test_expect_success '"git submodule sync" should not vivify uninteresting submod
)
'
+test_expect_failure '"git submodule sync" handles origin URL of the form foo' '
+ (cd relative-clone &&
+ git remote set-url origin foo &&
+ git submodule sync &&
+ (cd submodule &&
+ #actual fails with: "cannot strip off url foo
+ test "$(git config remote.origin.url)" = "../submodule"
+ )
+ )
+'
+
+test_expect_failure '"git submodule sync" handles origin URL of the form foo/bar' '
+ (cd relative-clone &&
+ git remote set-url origin foo/bar &&
+ git submodule sync &&
+ (cd submodule &&
+ #actual foo/submodule
+ test "$(git config remote.origin.url)" = "../foo/submodule"
+ )
+ )
+'
+
+test_expect_failure '"git submodule sync" handles origin URL of the form ./foo' '
+ (cd relative-clone &&
+ git remote set-url origin ./foo &&
+ git submodule sync &&
+ (cd submodule &&
+ #actual ./submodule
+ test "$(git config remote.origin.url)" = "../submodule"
+ )
+ )
+'
+
+test_expect_failure '"git submodule sync" handles origin URL of the form ./foo/bar' '
+ (cd relative-clone &&
+ git remote set-url origin ./foo/bar &&
+ git submodule sync &&
+ (cd submodule &&
+ #actual ./foo/submodule
+ test "$(git config remote.origin.url)" = "../foo/submodule"
+ )
+ )
+'
+
+test_expect_failure '"git submodule sync" handles origin URL of the form ../foo' '
+ (cd relative-clone &&
+ git remote set-url origin ../foo &&
+ git submodule sync &&
+ (cd submodule &&
+ #actual ../submodule
+ test "$(git config remote.origin.url)" = "../../submodule"
+ )
+ )
+'
+
+test_expect_failure '"git submodule sync" handles origin URL of the form ../foo/bar' '
+ (cd relative-clone &&
+ git remote set-url origin ../foo/bar &&
+ git submodule sync &&
+ (cd submodule &&
+ #actual ../foo/submodule
+ test "$(git config remote.origin.url)" = "../../foo/submodule"
+ )
+ )
+'
+
+test_expect_failure '"git submodule sync" handles origin URL of the form ../foo/bar with deeply nested submodule' '
+ (cd relative-clone &&
+ git remote set-url origin ../foo/bar &&
+ mkdir -p a/b/c &&
+ ( cd a/b/c &&
+ git init &&
+ :> .gitignore &&
+ git add .gitignore &&
+ test_tick &&
+ git commit -m "initial commit" ) &&
+ git submodule add ../bar/a/b/c ./a/b/c &&
+ git submodule sync &&
+ (cd a/b/c &&
+ #actual ../foo/bar/a/b/c
+ test "$(git config remote.origin.url)" = "../../../../foo/bar/a/b/c"
+ )
+ )
+'
+
+
test_done
--
1.7.10.2.656.g24a6219
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 4/9] submodule: document failure to handle improperly normalized remote origin URLs
2012-05-27 15:34 [PATCH v7 0/9] submodule: improve robustness of path handling Jon Seymour
` (2 preceding siblings ...)
2012-05-27 15:34 ` [PATCH v7 3/9] submodule: document failure to handle relative superproject origin URLs Jon Seymour
@ 2012-05-27 15:34 ` Jon Seymour
2012-05-27 15:34 ` [PATCH v7 5/9] submodule: extract normalize_path into standalone function Jon Seymour
` (5 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Jon Seymour @ 2012-05-27 15:34 UTC (permalink / raw)
To: git; +Cc: Jens.Lehmann, gitster, phil.hord, ramsay, Jon Seymour
These tests document failures to properly handle improperly normalized
remote origin URLs.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
t/t7400-submodule-basic.sh | 88 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 88 insertions(+)
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 80ec0f7..2674088 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -499,6 +499,39 @@ test_expect_success '../subrepo works with URL - ssh://hostname/repo' '
)
'
+test_expect_failure 'relative path works with URL - ssh://hostname/path/././repo' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url ssh://hostname/path/././repo &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = ssh://hostname/path/subrepo
+ )
+'
+
+test_expect_failure 'relative path works with URL - ssh://hostname/path/detour/././../repo' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url ssh://hostname/path/detour/././../repo &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = ssh://hostname/path/subrepo
+ )
+'
+
+test_expect_failure 'relative path works with URL - ssh://hostname/path/repo/.' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url ssh://hostname/path/repo/. &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = ssh://hostname/path/subrepo
+ )
+'
+
test_expect_success '../subrepo works with port-qualified URL - ssh://hostname:22/repo' '
(
cd reltest &&
@@ -634,6 +667,50 @@ test_expect_success '../subrepo works with scp-style URL - user@host:path/to/rep
)
'
+test_expect_failure 'relative path works with user@host:path/to/repo/.' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url user@host:path/to/repo/. &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = user@host:path/to/subrepo
+ )
+'
+
+test_expect_failure 'relative path works with user@host:path/to/./repo' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url user@host:path/to/./repo &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = user@host:path/to/subrepo
+ )
+'
+
+test_expect_failure 'relative path works with user@host:path/to/././repo' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url user@host:path/to/././repo &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = user@host:path/to/subrepo
+ )
+'
+
+test_expect_failure 'relative path works with user@host:path/to/detour/../repo' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url user@host:path/to/detour/../repo &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = user@host:path/to/subrepo
+ )
+'
+
test_expect_failure '../subrepo works with relative local path - foo' '
(
cd reltest &&
@@ -703,6 +780,17 @@ test_expect_success '../subrepo works with relative local path - ../foo/bar' '
)
'
+test_expect_failure 'relative path works with ../foo/./bar' '
+ (
+ cd reltest &&
+ cp pristine-.git-config .git/config &&
+ cp pristine-.gitmodules .gitmodules &&
+ git config remote.origin.url ../foo/./bar &&
+ git submodule init &&
+ test "$(git config submodule.sub.url)" = ../foo/subrepo
+ )
+'
+
test_expect_success '../bar/a/b/c works with relative local path - ../foo/bar.git' '
(
cd reltest &&
--
1.7.10.2.656.g24a6219
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 5/9] submodule: extract normalize_path into standalone function
2012-05-27 15:34 [PATCH v7 0/9] submodule: improve robustness of path handling Jon Seymour
` (3 preceding siblings ...)
2012-05-27 15:34 ` [PATCH v7 4/9] submodule: document failure to handle improperly normalized remote " Jon Seymour
@ 2012-05-27 15:34 ` Jon Seymour
2012-05-27 15:34 ` [PATCH v7 6/9] submodule: fix detection of invalid submodule URL Jon Seymour
` (4 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Jon Seymour @ 2012-05-27 15:34 UTC (permalink / raw)
To: git; +Cc: Jens.Lehmann, gitster, phil.hord, ramsay, Jon Seymour
Extract the normalize_path function so that it can be re-used elsewhere.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
git-submodule.sh | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 64a70d6..dbbc905 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -176,6 +176,21 @@ module_clone()
(clear_local_git_env; cd "$sm_path" && GIT_WORK_TREE=. git config core.worktree "$rel/$b")
}
+normalize_path()
+{
+ # normalize path:
+ # multiple //; leading ./; /./; /../; trailing /
+ printf '%s/\n' "$1" |
+ sed -e '
+ s|//*|/|g
+ s|^\(\./\)*||
+ s|/\./|/|g
+ :start
+ s|\([^/]*\)/\.\./||
+ tstart
+ s|/*$||
+ '
+}
#
# Add a new submodule to the working tree, .gitmodules and the index
#
@@ -250,18 +265,7 @@ cmd_add()
;;
esac
- # normalize path:
- # multiple //; leading ./; /./; /../; trailing /
- sm_path=$(printf '%s/\n' "$sm_path" |
- sed -e '
- s|//*|/|g
- s|^\(\./\)*||
- s|/\./|/|g
- :start
- s|\([^/]*\)/\.\./||
- tstart
- s|/*$||
- ')
+ sm_path="$(normalize_path "$sm_path")"
git ls-files --error-unmatch "$sm_path" > /dev/null 2>&1 &&
die "$(eval_gettext "'\$sm_path' already exists in the index")"
--
1.7.10.2.656.g24a6219
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 6/9] submodule: fix detection of invalid submodule URL
2012-05-27 15:34 [PATCH v7 0/9] submodule: improve robustness of path handling Jon Seymour
` (4 preceding siblings ...)
2012-05-27 15:34 ` [PATCH v7 5/9] submodule: extract normalize_path into standalone function Jon Seymour
@ 2012-05-27 15:34 ` Jon Seymour
2012-05-28 19:01 ` Johannes Sixt
2012-05-27 15:34 ` [PATCH v7 7/9] submodule: fix sync handling of relative superproject origin URLs Jon Seymour
` (3 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Jon Seymour @ 2012-05-27 15:34 UTC (permalink / raw)
To: git; +Cc: Jens.Lehmann, gitster, phil.hord, ramsay, Jon Seymour
Currently the superproject origin URL is progressively transformed
by stepping through parts of the submodule URL and removing parts
from the superproject URL for each leading ../ found in the
submodule URL. No attempt is made to check that the edited URL still
has a path part left to remove. This can result in the construction
of an absolute submodule URL where the hostname part of the URL
has been replaced by path components of the submodule URL.
For example: if the origin URL is ssh://hostname/repo and the
submodule URL is ../../subrepo, then the origin URL of the subrepo
will be calculated as ssh://subrepo.
With this change, editing is only performed on the path part
of the superproject origin URL. Any attempt by to consume the
non-path parts of the origin URL results in a failure.
As a side effect of preserving correct handling of support for
URLs of the form user@host:repo, this change also fixes handling,
by submodule init, of origin super project URLs of the form:
foo, foo/bar, ./foo and ./foo/bar.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
git-submodule.sh | 41 ++++++++++++++++++++++++++++++++---------
t/t7400-submodule-basic.sh | 23 ++++++++---------------
2 files changed, 40 insertions(+), 24 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index dbbc905..2550681 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -37,23 +37,42 @@ resolve_relative_url ()
remoteurl=$(git config "remote.$remote.url") ||
remoteurl=$(pwd) # the repository is its own authoritative upstream
url="$1"
- remoteurl=${remoteurl%/}
- sep=/
+ remoteurl="${remoteurl%/}"
+
+ case "$remoteurl" in
+ *//*/*)
+ variant="${remoteurl#*//*/}"
+ ;;
+ *::*)
+ variant="${remoteurl#*::}"
+ ;;
+ *:*)
+ variant="${remoteurl#*:}"
+ ;;
+ /*)
+ variant="${remoteurl#/}"
+ ;;
+ *)
+ variant="${remoteurl}"
+ ;;
+ esac
+ invariant="${remoteurl%$variant}"
+
while test -n "$url"
do
case "$url" in
../*)
url="${url#../}"
- case "$remoteurl" in
+ case "$variant" in
*/*)
- remoteurl="${remoteurl%/*}"
+ variant="${variant%/*}"
;;
- *:*)
- remoteurl="${remoteurl%:*}"
- sep=:
+ .)
+ die "$(eval_gettext "cannot strip one component off url '\${invariant}'")"
;;
*)
- die "$(eval_gettext "cannot strip one component off url '\$remoteurl'")"
+ # add a sentinel when .. matchs foo
+ variant=.
;;
esac
;;
@@ -64,7 +83,11 @@ resolve_relative_url ()
break;;
esac
done
- echo "$remoteurl$sep${url%/}"
+ # ensure a trailing path separator
+ variant="${variant}/"
+ # strip the sentinel, if present
+ variant="${variant#./}"
+ echo "$invariant$variant${url%/}"
}
#
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 2674088..a94c5e9 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -576,7 +576,7 @@ test_expect_success '../subrepo works with helper URL- helper:://hostname/repo'
)
'
-test_expect_failure '../../subrepo fails with URL - ssh://hostname/repo' "
+test_expect_success '../../subrepo fails with URL - ssh://hostname/repo' "
(
cd reltest &&
cp pristine-.git-config .git/config &&
@@ -585,12 +585,11 @@ test_expect_failure '../../subrepo fails with URL - ssh://hostname/repo' "
git config -f .gitmodules submodule.sub.url ../../subrepo &&
echo cannot strip one component off url \'ssh://hostname/\' > expected &&
test_must_fail git submodule init 2>actual &&
- #actual no failure, url configured as ssh://subrepo
test_cmp expected actual
)
"
-test_expect_failure '../../subrepo fails with absolute local path - /repo' "
+test_expect_success '../../subrepo fails with absolute local path - /repo' "
(
cd reltest &&
cp pristine-.git-config .git/config &&
@@ -603,7 +602,7 @@ test_expect_failure '../../subrepo fails with absolute local path - /repo' "
)
"
-test_expect_failure '../../../subrepo fails with URL - ssh://hostname/repo' "
+test_expect_success '../../../subrepo fails with URL - ssh://hostname/repo' "
(
cd reltest &&
cp pristine-.git-config .git/config &&
@@ -612,12 +611,11 @@ test_expect_failure '../../../subrepo fails with URL - ssh://hostname/repo' "
git config -f .gitmodules submodule.sub.url ../../../subrepo &&
echo cannot strip one component off url \'ssh://hostname/\' > expected &&
test_must_fail git submodule init 2>actual &&
- #actual no failure, url configured as ssh:/subrepo
test_cmp expected actual
)
"
-test_expect_failure '../../../../subrepo fails with with URL - ssh://hostname/repo' "
+test_expect_success '../../../../subrepo fails with with URL - ssh://hostname/repo' "
(
cd reltest &&
cp pristine-.git-config .git/config &&
@@ -626,12 +624,11 @@ test_expect_failure '../../../../subrepo fails with with URL - ssh://hostname/re
git config -f .gitmodules submodule.sub.url ../../../../subrepo &&
echo cannot strip one component off url \'ssh://hostname/\' > expected &&
test_must_fail git submodule init 2>actual &&
- #actual no failure, url configured as ssh:/subrepo
test_cmp expected actual
)
"
-test_expect_failure '../../../../../subrepo fails with URL - ssh://hostname/repo' "
+test_expect_success '../../../../../subrepo fails with URL - ssh://hostname/repo' "
(
cd reltest &&
cp pristine-.git-config .git/config &&
@@ -640,7 +637,6 @@ test_expect_failure '../../../../../subrepo fails with URL - ssh://hostname/repo
git config -f .gitmodules submodule.sub.url ../../../../../subrepo &&
echo cannot strip one component off url \'ssh://hostname/\' > expected &&
test_must_fail git submodule init 2>actual &&
- #actual cannot strip one component off url 'ssh'
test_cmp expected actual
)
"
@@ -711,13 +707,12 @@ test_expect_failure 'relative path works with user@host:path/to/detour/../repo'
)
'
-test_expect_failure '../subrepo works with relative local path - foo' '
+test_expect_success '../subrepo works with relative local path - foo' '
(
cd reltest &&
cp pristine-.git-config .git/config &&
cp pristine-.gitmodules .gitmodules &&
git config remote.origin.url foo &&
- # actual: fails with an error
git submodule init &&
test "$(git config submodule.sub.url)" = subrepo
)
@@ -734,26 +729,24 @@ test_expect_success '../subrepo works with relative local path - foo/bar' '
)
'
-test_expect_failure '../subrepo works with relative local path - ./foo' '
+test_expect_success '../subrepo works with relative local path - ./foo' '
(
cd reltest &&
cp pristine-.git-config .git/config &&
cp pristine-.gitmodules .gitmodules &&
git config remote.origin.url ./foo &&
git submodule init &&
- #actual ./subrepo
test "$(git config submodule.sub.url)" = subrepo
)
'
-test_expect_failure '../subrepo works with relative local path - ./foo/bar' '
+test_expect_success '../subrepo works with relative local path - ./foo/bar' '
(
cd reltest &&
cp pristine-.git-config .git/config &&
cp pristine-.gitmodules .gitmodules &&
git config remote.origin.url ./foo/bar &&
git submodule init &&
- #actual: ./foo/subrepo
test "$(git config submodule.sub.url)" = foo/subrepo
)
'
--
1.7.10.2.656.g24a6219
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 7/9] submodule: fix sync handling of relative superproject origin URLs
2012-05-27 15:34 [PATCH v7 0/9] submodule: improve robustness of path handling Jon Seymour
` (5 preceding siblings ...)
2012-05-27 15:34 ` [PATCH v7 6/9] submodule: fix detection of invalid submodule URL Jon Seymour
@ 2012-05-27 15:34 ` Jon Seymour
2012-05-27 15:34 ` [PATCH v7 8/9] submodule: fix handling of denormalized " Jon Seymour
` (2 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Jon Seymour @ 2012-05-27 15:34 UTC (permalink / raw)
To: git; +Cc: Jens.Lehmann, gitster, phil.hord, ramsay, Jon Seymour
When the origin URL of the superproject is itself relative, git submodule sync
configures the remote.origin.url configuration property of the submodule
with a path that is relative to the work tree of the superproject
rather than the work tree of the submodule.
To fix this an 'up_path' that navigates from the work tree of the submodule
to the work tree of the superproject needs to be prepended to the URL
otherwise calculated.
This change fixes handling for relative superproject origin URLs like
the following:
foo
foo/bar
./foo
./foo/bar
../foo
../foo/bar
This change also renames the url variable used by git sync to module_url
and introduces to new variables super_config_url and sub_origin_url to refer
to the different URLs derived from the module_url.
The function blurb is expanded to give a more thorough description of what
resolve_relative_url()'s function is intended to be.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
git-submodule.sh | 48 ++++++++++++++++++++++++++++++++++++++++-------
t/t7403-submodule-sync.sh | 23 ++++++++---------------
2 files changed, 49 insertions(+), 22 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 2550681..9ca2ffe 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -30,14 +30,32 @@ nofetch=
update=
prefix=
-# Resolve relative url by appending to parent's url
+# The function takes at most 2 arguments. The first argument is the
+# relative URL that navigates from the superproject origin repo to the
+# submodule origin repo. The second up_path argument, if specified, is
+# the relative path that navigates from the submodule working tree to
+# the superproject working tree.
+#
+# The output of the function is the origin URL of the submodule.
+#
+# The output will either be an absolute URL or filesystem path (if the
+# superproject origin URL is an absolute URL or filesystem path,
+# respectively) or a relative file system path (if the superproject
+# origin URL is a relative file system path).
+#
+# When the output is a relative file system path, the path is either
+# relative to the submodule working tree, if up_path is specified, or to
+# the superproject working tree otherwise.
resolve_relative_url ()
{
remote=$(get_default_remote)
remoteurl=$(git config "remote.$remote.url") ||
remoteurl=$(pwd) # the repository is its own authoritative upstream
url="$1"
+ up_path="$2"
+
remoteurl="${remoteurl%/}"
+ is_relative=
case "$remoteurl" in
*//*/*)
@@ -54,6 +72,7 @@ resolve_relative_url ()
;;
*)
variant="${remoteurl}"
+ is_relative=t
;;
esac
invariant="${remoteurl%$variant}"
@@ -83,11 +102,13 @@ resolve_relative_url ()
break;;
esac
done
+
# ensure a trailing path separator
variant="${variant}/"
# strip the sentinel, if present
variant="${variant#./}"
- echo "$invariant$variant${url%/}"
+
+ echo "$invariant${is_relative:+$up_path}$variant${url%/}"
}
#
@@ -986,19 +1007,32 @@ cmd_sync()
while read mode sha1 stage sm_path
do
name=$(module_name "$sm_path")
- url=$(git config -f .gitmodules --get submodule."$name".url)
+ # path from superproject origin repo to submodule origin repo
+ module_url=$(git config -f .gitmodules --get submodule."$name".url)
# Possibly a url relative to parent
- case "$url" in
+ case "$module_url" in
./*|../*)
- url=$(resolve_relative_url "$url") || exit
+ # rewrite foo/bar as ../.. to find path from
+ # submodule work tree to superproject work tree
+ up_path="$(echo "$sm_path" | sed "s/[^/]*/../g")" &&
+ # guarantee a trailing /
+ up_path=${up_path%/}/ &&
+ # path from submodule work tree to submodule origin repo
+ sub_origin_url=$(resolve_relative_url "$module_url" "$up_path") &&
+ # path from superproject work tree to submodule origin repo
+ super_config_url=$(resolve_relative_url "$module_url") || exit
+ ;;
+ *)
+ sub_origin_url="$module_url"
+ super_config_url="$module_url"
;;
esac
if git config "submodule.$name.url" >/dev/null 2>/dev/null
then
say "$(eval_gettext "Synchronizing submodule url for '\$name'")"
- git config submodule."$name".url "$url"
+ git config submodule."$name".url "$super_config_url"
if test -e "$sm_path"/.git
then
@@ -1006,7 +1040,7 @@ cmd_sync()
clear_local_git_env
cd "$sm_path"
remote=$(get_default_remote)
- git config remote."$remote".url "$url"
+ git config remote."$remote".url "$sub_origin_url"
)
fi
fi
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index 56b933d..b7466ba 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh
@@ -88,73 +88,67 @@ test_expect_success '"git submodule sync" should not vivify uninteresting submod
)
'
-test_expect_failure '"git submodule sync" handles origin URL of the form foo' '
+test_expect_success '"git submodule sync" handles origin URL of the form foo' '
(cd relative-clone &&
git remote set-url origin foo &&
git submodule sync &&
(cd submodule &&
- #actual fails with: "cannot strip off url foo
test "$(git config remote.origin.url)" = "../submodule"
)
)
'
-test_expect_failure '"git submodule sync" handles origin URL of the form foo/bar' '
+test_expect_success '"git submodule sync" handles origin URL of the form foo/bar' '
(cd relative-clone &&
git remote set-url origin foo/bar &&
git submodule sync &&
(cd submodule &&
- #actual foo/submodule
test "$(git config remote.origin.url)" = "../foo/submodule"
)
)
'
-test_expect_failure '"git submodule sync" handles origin URL of the form ./foo' '
+test_expect_success '"git submodule sync" handles origin URL of the form ./foo' '
(cd relative-clone &&
git remote set-url origin ./foo &&
git submodule sync &&
(cd submodule &&
- #actual ./submodule
test "$(git config remote.origin.url)" = "../submodule"
)
)
'
-test_expect_failure '"git submodule sync" handles origin URL of the form ./foo/bar' '
+test_expect_success '"git submodule sync" handles origin URL of the form ./foo/bar' '
(cd relative-clone &&
git remote set-url origin ./foo/bar &&
git submodule sync &&
(cd submodule &&
- #actual ./foo/submodule
test "$(git config remote.origin.url)" = "../foo/submodule"
)
)
'
-test_expect_failure '"git submodule sync" handles origin URL of the form ../foo' '
+test_expect_success '"git submodule sync" handles origin URL of the form ../foo' '
(cd relative-clone &&
git remote set-url origin ../foo &&
git submodule sync &&
(cd submodule &&
- #actual ../submodule
test "$(git config remote.origin.url)" = "../../submodule"
)
)
'
-test_expect_failure '"git submodule sync" handles origin URL of the form ../foo/bar' '
+test_expect_success '"git submodule sync" handles origin URL of the form ../foo/bar' '
(cd relative-clone &&
git remote set-url origin ../foo/bar &&
git submodule sync &&
(cd submodule &&
- #actual ../foo/submodule
test "$(git config remote.origin.url)" = "../../foo/submodule"
)
)
'
-test_expect_failure '"git submodule sync" handles origin URL of the form ../foo/bar with deeply nested submodule' '
+test_expect_success '"git submodule sync" handles origin URL of the form ../foo/bar with deeply nested submodule' '
(cd relative-clone &&
git remote set-url origin ../foo/bar &&
mkdir -p a/b/c &&
@@ -167,11 +161,10 @@ test_expect_failure '"git submodule sync" handles origin URL of the form ../foo/
git submodule add ../bar/a/b/c ./a/b/c &&
git submodule sync &&
(cd a/b/c &&
- #actual ../foo/bar/a/b/c
test "$(git config remote.origin.url)" = "../../../../foo/bar/a/b/c"
)
)
'
-
test_done
+<
--
1.7.10.2.656.g24a6219
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 8/9] submodule: fix handling of denormalized superproject origin URLs
2012-05-27 15:34 [PATCH v7 0/9] submodule: improve robustness of path handling Jon Seymour
` (6 preceding siblings ...)
2012-05-27 15:34 ` [PATCH v7 7/9] submodule: fix sync handling of relative superproject origin URLs Jon Seymour
@ 2012-05-27 15:34 ` Jon Seymour
2012-05-27 22:57 ` Jon Seymour
2012-05-27 15:34 ` [PATCH v7 9/9] submodule: fix normalization to handle repeated ./ Jon Seymour
2012-05-28 20:07 ` [PATCH v7 0/9] submodule: improve robustness of path handling Jens Lehmann
9 siblings, 1 reply; 17+ messages in thread
From: Jon Seymour @ 2012-05-27 15:34 UTC (permalink / raw)
To: git; +Cc: Jens.Lehmann, gitster, phil.hord, ramsay, Jon Seymour
Currently git calculates the submodule origin URL incorrectly in
the case that the superproject origin URL is denormalized.
So, we normalize the path part of the superproject URL before iterating
over the leading ../ parts of the submodule URL.
A remaining problem related to the handling of consecutive repeated ./'s
in the superproject origin URL is deferred to a subsequent commit.
This change also fixes a subtle error in the setup of some tests which was
masked by the denormalization issue that is now fixed.
Previous behaviour was relying on submodule add to clone trash/submodule
into super/submodule, however from the perspective of super's origin (i.e. trash),
the origin submodule is actually located at ./submodule not ../submodule.
However, because the origin URL of super was denormalized (it had a trailing /.)
the incorrect handling of denormalized super URLs actually produced
the correct result - a case of two errors cancelling out each other's
effects.
Now that normalization is fixed, the erroneous use of git submodule add
by the test setups needs to be fixed. The cleanest way to do this is to
clone super not from ., but from ./omega. The subsequent invocation of
git submodule add ../submodule submodule
now does the expected thing because ../submodule is the correct
path from omega to the submodule origin repo.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
git-submodule.sh | 1 +
t/t7400-submodule-basic.sh | 10 +++++-----
t/t7403-submodule-sync.sh | 14 +++++++++-----
t/t7406-submodule-update.sh | 16 ++++++++++------
t/t7407-submodule-foreach.sh | 14 +++++++++-----
t/t7506-status-submodule.sh | 10 +++++++++-
6 files changed, 43 insertions(+), 22 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 9ca2ffe..1f0983c 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -76,6 +76,7 @@ resolve_relative_url ()
;;
esac
invariant="${remoteurl%$variant}"
+ variant="$(normalize_path "$variant")"
while test -n "$url"
do
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index a94c5e9..b01f479 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -521,7 +521,7 @@ test_expect_failure 'relative path works with URL - ssh://hostname/path/detour/.
)
'
-test_expect_failure 'relative path works with URL - ssh://hostname/path/repo/.' '
+test_expect_success 'relative path works with URL - ssh://hostname/path/repo/.' '
(
cd reltest &&
cp pristine-.git-config .git/config &&
@@ -663,7 +663,7 @@ test_expect_success '../subrepo works with scp-style URL - user@host:path/to/rep
)
'
-test_expect_failure 'relative path works with user@host:path/to/repo/.' '
+test_expect_success 'relative path works with user@host:path/to/repo/.' '
(
cd reltest &&
cp pristine-.git-config .git/config &&
@@ -674,7 +674,7 @@ test_expect_failure 'relative path works with user@host:path/to/repo/.' '
)
'
-test_expect_failure 'relative path works with user@host:path/to/./repo' '
+test_expect_success 'relative path works with user@host:path/to/./repo' '
(
cd reltest &&
cp pristine-.git-config .git/config &&
@@ -696,7 +696,7 @@ test_expect_failure 'relative path works with user@host:path/to/././repo' '
)
'
-test_expect_failure 'relative path works with user@host:path/to/detour/../repo' '
+test_expect_success 'relative path works with user@host:path/to/detour/../repo' '
(
cd reltest &&
cp pristine-.git-config .git/config &&
@@ -773,7 +773,7 @@ test_expect_success '../subrepo works with relative local path - ../foo/bar' '
)
'
-test_expect_failure 'relative path works with ../foo/./bar' '
+test_expect_success 'relative path works with ../foo/./bar' '
(
cd reltest &&
cp pristine-.git-config .git/config &&
diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh
index b7466ba..d76e49f 100755
--- a/t/t7403-submodule-sync.sh
+++ b/t/t7403-submodule-sync.sh
@@ -11,11 +11,15 @@ These tests exercise the "git submodule sync" subcommand.
. ./test-lib.sh
test_expect_success setup '
- echo file > file &&
- git add file &&
- test_tick &&
- git commit -m upstream &&
- git clone . super &&
+ mkdir omega &&
+ (cd omega &&
+ git init &&
+ echo file > file &&
+ git add file &&
+ test_tick &&
+ git commit -m upstream
+ ) &&
+ git clone omega super &&
git clone super submodule &&
(cd super &&
git submodule add ../submodule submodule &&
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index dcb195b..8b6c330 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -22,11 +22,15 @@ compare_head()
test_expect_success 'setup a submodule tree' '
- echo file > file &&
- git add file &&
- test_tick &&
- git commit -m upstream &&
- git clone . super &&
+ mkdir omega &&
+ (cd omega &&
+ git init &&
+ echo file > file &&
+ git add file &&
+ test_tick &&
+ git commit -m upstream
+ ) &&
+ git clone omega super &&
git clone super submodule &&
git clone super rebasing &&
git clone super merging &&
@@ -58,7 +62,7 @@ test_expect_success 'setup a submodule tree' '
git submodule add ../merging merging &&
test_tick &&
git commit -m "rebasing"
- )
+ ) &&
(cd super &&
git submodule add ../none none &&
test_tick &&
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 9b69fe2..40f957c 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -13,11 +13,15 @@ that are currently checked out.
test_expect_success 'setup a submodule tree' '
- echo file > file &&
- git add file &&
- test_tick &&
- git commit -m upstream &&
- git clone . super &&
+ mkdir omega &&
+ (cd omega &&
+ git init &&
+ echo file > file &&
+ git add file &&
+ test_tick &&
+ git commit -m upstream
+ ) &&
+ git clone omega super &&
git clone super submodule &&
(
cd super &&
diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
index d31b34d..764c1d0 100755
--- a/t/t7506-status-submodule.sh
+++ b/t/t7506-status-submodule.sh
@@ -197,7 +197,15 @@ A sub1
EOF
test_expect_success 'status with merge conflict in .gitmodules' '
- git clone . super &&
+ mkdir omega &&
+ (cd omega &&
+ git init &&
+ echo file > file &&
+ git add file &&
+ test_tick &&
+ git commit -m upstream
+ ) &&
+ git clone omega super &&
test_create_repo_with_commit sub1 &&
test_tick &&
test_create_repo_with_commit sub2 &&
--
1.7.10.2.656.g24a6219
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 9/9] submodule: fix normalization to handle repeated ./
2012-05-27 15:34 [PATCH v7 0/9] submodule: improve robustness of path handling Jon Seymour
` (7 preceding siblings ...)
2012-05-27 15:34 ` [PATCH v7 8/9] submodule: fix handling of denormalized " Jon Seymour
@ 2012-05-27 15:34 ` Jon Seymour
2012-05-28 20:07 ` [PATCH v7 0/9] submodule: improve robustness of path handling Jens Lehmann
9 siblings, 0 replies; 17+ messages in thread
From: Jon Seymour @ 2012-05-27 15:34 UTC (permalink / raw)
To: git; +Cc: Jens.Lehmann, gitster, phil.hord, ramsay, Jon Seymour
Currently path/./foo/./bar is denormalized correctly, but path/foo/././bar
is not.
We fix the normalization script to allow repeated application of the
./ -> / normalization in the same way that foo/.. is handled - by
moving it inside a sed loop.
The existing sed label, start, is renamed to fooslashdotdotslash to
indicate which of the two loops is being refered to by the second
branch directive.
Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
git-submodule.sh | 8 +++++---
t/t7400-submodule-basic.sh | 6 +++---
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 1f0983c..8f3bc71 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -229,10 +229,12 @@ normalize_path()
sed -e '
s|//*|/|g
s|^\(\./\)*||
- s|/\./|/|g
- :start
+ :slashdotslash
+ s|/\./|/|
+ tslashdotslash
+ :fooslashdotdotslash
s|\([^/]*\)/\.\./||
- tstart
+ tfooslashdotdotslash
s|/*$||
'
}
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index b01f479..61887b2 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -499,7 +499,7 @@ test_expect_success '../subrepo works with URL - ssh://hostname/repo' '
)
'
-test_expect_failure 'relative path works with URL - ssh://hostname/path/././repo' '
+test_expect_success 'relative path works with URL - ssh://hostname/path/././repo' '
(
cd reltest &&
cp pristine-.git-config .git/config &&
@@ -510,7 +510,7 @@ test_expect_failure 'relative path works with URL - ssh://hostname/path/././repo
)
'
-test_expect_failure 'relative path works with URL - ssh://hostname/path/detour/././../repo' '
+test_expect_success 'relative path works with URL - ssh://hostname/path/detour/././../repo' '
(
cd reltest &&
cp pristine-.git-config .git/config &&
@@ -685,7 +685,7 @@ test_expect_success 'relative path works with user@host:path/to/./repo' '
)
'
-test_expect_failure 'relative path works with user@host:path/to/././repo' '
+test_expect_success 'relative path works with user@host:path/to/././repo' '
(
cd reltest &&
cp pristine-.git-config .git/config &&
--
1.7.10.2.656.g24a6219
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v7 8/9] submodule: fix handling of denormalized superproject origin URLs
2012-05-27 15:34 ` [PATCH v7 8/9] submodule: fix handling of denormalized " Jon Seymour
@ 2012-05-27 22:57 ` Jon Seymour
0 siblings, 0 replies; 17+ messages in thread
From: Jon Seymour @ 2012-05-27 22:57 UTC (permalink / raw)
To: git; +Cc: Jens.Lehmann, gitster, phil.hord, ramsay, Jon Seymour
On Mon, May 28, 2012 at 1:34 AM, Jon Seymour <jon.seymour@gmail.com> wrote:
> This change also fixes a subtle error in the setup of some tests which was
> masked by the denormalization issue that is now fixed.
>
I guess this patch could be improved by moving the change to the test
setup to a separate, earlier patch and also including a separate test
which demonstrates the
flawed behaviour described by the commit message. Anyone disagree?
jon.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 6/9] submodule: fix detection of invalid submodule URL
2012-05-27 15:34 ` [PATCH v7 6/9] submodule: fix detection of invalid submodule URL Jon Seymour
@ 2012-05-28 19:01 ` Johannes Sixt
2012-05-28 21:39 ` Jon Seymour
0 siblings, 1 reply; 17+ messages in thread
From: Johannes Sixt @ 2012-05-28 19:01 UTC (permalink / raw)
To: Jon Seymour; +Cc: git, Jens.Lehmann, gitster, phil.hord, ramsay
Am 27.05.2012 17:34, schrieb Jon Seymour:
> diff --git a/git-submodule.sh b/git-submodule.sh
> index dbbc905..2550681 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -37,23 +37,42 @@ resolve_relative_url ()
> remoteurl=$(git config "remote.$remote.url") ||
> remoteurl=$(pwd) # the repository is its own authoritative upstream
> url="$1"
> - remoteurl=${remoteurl%/}
> - sep=/
> + remoteurl="${remoteurl%/}"
> +
> + case "$remoteurl" in
> + *//*/*)
> + variant="${remoteurl#*//*/}"
> + ;;
> + *::*)
> + variant="${remoteurl#*::}"
> + ;;
> + *:*)
> + variant="${remoteurl#*:}"
> + ;;
> + /*)
> + variant="${remoteurl#/}"
Without understanding in detail what this series is about, I would guess
that the previous two case arms are not very Windows friendly. Does the
right thing happen when $remoteurl is "c:/path/to/remote"? Would it help
to use is_absolute_path?
if is_absolute_path "$remoteurl"
then
variant="${remoteurl#*/}"
else
case "$remoteurl" in
...other cases go here...
esac
fi
-- Hannes
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 0/9] submodule: improve robustness of path handling
2012-05-27 15:34 [PATCH v7 0/9] submodule: improve robustness of path handling Jon Seymour
` (8 preceding siblings ...)
2012-05-27 15:34 ` [PATCH v7 9/9] submodule: fix normalization to handle repeated ./ Jon Seymour
@ 2012-05-28 20:07 ` Jens Lehmann
2012-05-28 22:01 ` Jon Seymour
9 siblings, 1 reply; 17+ messages in thread
From: Jens Lehmann @ 2012-05-28 20:07 UTC (permalink / raw)
To: Jon Seymour; +Cc: git, gitster, phil.hord, Ramsay Jones, Johannes Sixt
Am 27.05.2012 17:34, schrieb Jon Seymour:
> This series improves the robustness of path handling by 'git submodule' by:
>
> * detecting submodule URLs that will result in non-sensical submodule origin URLs
>
> * improving handling of various kinds of relative superproject origin URLs
>
> * improving handling of various kinds of denormalized superproject origin URLs
Hmm, this has become a quite invasive patch series. While I bought the
use case of having a superproject with a relative url and was inclined
to accept that it might even not start "./" or "../" (even though that
is a pretty unusual use and can be easily fixed by prepending a "./"),
I'm not sure the in depth check of URLs is worth the code churn. And
especially the high probability of breaking other peoples use cases in
rather subtle ways worry me (this did happen quite often when the
submodule script was changed in the past; as an example take the
windows path issues Johannes already pointed out in his email). And I
can't remember bug reports that people complained about URL problems
due to the issues you intend to fix here, which makes me think they
might be well intended but possibly unnecessary (but my memory might
server me wrong here).
So I'd vote for just fixing the relative submodule path issues and to
not care about the possible issues with URLs. Opinions?
(And patches 6-8 contain changes to test cases other than just changing
test_expect_failure to test_expect_success which makes reviewing this
series unnecessarily hard)
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 6/9] submodule: fix detection of invalid submodule URL
2012-05-28 19:01 ` Johannes Sixt
@ 2012-05-28 21:39 ` Jon Seymour
2012-06-03 9:51 ` Jon Seymour
0 siblings, 1 reply; 17+ messages in thread
From: Jon Seymour @ 2012-05-28 21:39 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Jens.Lehmann, gitster, phil.hord, ramsay
On Tue, May 29, 2012 at 5:01 AM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 27.05.2012 17:34, schrieb Jon Seymour:
>
> Without understanding in detail what this series is about, I would guess
> that the previous two case arms are not very Windows friendly. Does the
> right thing happen when $remoteurl is "c:/path/to/remote"? Would it help
> to use is_absolute_path?
>
> if is_absolute_path "$remoteurl"
> then
> variant="${remoteurl#*/}"
> else
> case "$remoteurl" in
> ...other cases go here...
> esac
> fi
>
> -- Hannes
Thanks, I will investigate this as an alternative.
jon.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 0/9] submodule: improve robustness of path handling
2012-05-28 20:07 ` [PATCH v7 0/9] submodule: improve robustness of path handling Jens Lehmann
@ 2012-05-28 22:01 ` Jon Seymour
2012-05-29 19:21 ` Jens Lehmann
0 siblings, 1 reply; 17+ messages in thread
From: Jon Seymour @ 2012-05-28 22:01 UTC (permalink / raw)
To: Jens Lehmann; +Cc: git, gitster, phil.hord, Ramsay Jones, Johannes Sixt
On Tue, May 29, 2012 at 6:07 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 27.05.2012 17:34, schrieb Jon Seymour:
>> This series improves the robustness of path handling by 'git submodule' by:
>>
>> * detecting submodule URLs that will result in non-sensical submodule origin URLs
>>
>> * improving handling of various kinds of relative superproject origin URLs
>>
>> * improving handling of various kinds of denormalized superproject origin URLs
>
> Hmm, this has become a quite invasive patch series. While I bought the
> use case of having a superproject with a relative url and was inclined
> to accept that it might even not start "./" or "../" (even though that
> is a pretty unusual use and can be easily fixed by prepending a "./"),
> I'm not sure the in depth check of URLs is worth the code churn. And
> especially the high probability of breaking other peoples use cases in
> rather subtle ways worry me (this did happen quite often when the
> submodule script was changed in the past; as an example take the
> windows path issues Johannes already pointed out in his email). And I
> can't remember bug reports that people complained about URL problems
> due to the issues you intend to fix here, which makes me think they
> might be well intended but possibly unnecessary (but my memory might
> server me wrong here).
>
> So I'd vote for just fixing the relative submodule path issues and to
> not care about the possible issues with URLs. Opinions?
I'll write a minimal patch to solve my relative path problem without
fixing the invalid/"greedy" submodule url or url normalization issues.
The reason I went with a more extensive series is that the change in
6/9 considerably simplified the change I wanted to make in 7/9 while
at the same time making the path handling of resolve_relative_url more
precise, in the sense documented by the tests in 2/9.
The refactoring in 5/9 and the changes in 8/9 and 9/9 are related to
renormalization which I realised was a weakness (if not a problem
people were complaining about) in the original code as documented by
the tests in 4/9.
Do you have any comments about whether the failures documented in 2/9
and 4/9 are worth noting, at least, as weaknesses?
>
> (And patches 6-8 contain changes to test cases other than just changing
> test_expect_failure to test_expect_success which makes reviewing this
> series unnecessarily hard)
Agree absolutely about patch 8 - I will re-roll with separate tests to
document the test setup issue I fixed in 8.
The only other changes to tests in 6 and 7 were the removal of
comments about the actual bad behaviour. Would your preference be that
I removed these #actual comments completely or that I moved
documentation of the actual behaviour to the header of the test?
jon.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 0/9] submodule: improve robustness of path handling
2012-05-28 22:01 ` Jon Seymour
@ 2012-05-29 19:21 ` Jens Lehmann
0 siblings, 0 replies; 17+ messages in thread
From: Jens Lehmann @ 2012-05-29 19:21 UTC (permalink / raw)
To: Jon Seymour; +Cc: git, gitster, phil.hord, Ramsay Jones, Johannes Sixt
Am 29.05.2012 00:01, schrieb Jon Seymour:
> On Tue, May 29, 2012 at 6:07 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>> So I'd vote for just fixing the relative submodule path issues and to
>> not care about the possible issues with URLs. Opinions?
>
> I'll write a minimal patch to solve my relative path problem without
> fixing the invalid/"greedy" submodule url or url normalization issues.
I'd really appreciate that.
> Do you have any comments about whether the failures documented in 2/9
> and 4/9 are worth noting, at least, as weaknesses?
Sure, they document known problems. Me thinks they all should be
squashed into a single patch and submitted separately. The following
three tests from 2/9 are redundant and can be dropped (they are
already handled by the '../../subrepo' case):
'../../../subrepo fails with URL - ssh://hostname/repo' "
'../../../../subrepo fails with with URL - ssh://hostname/repo' "
'../../../../../subrepo fails with URL - ssh://hostname/repo' "
>> (And patches 6-8 contain changes to test cases other than just changing
>> test_expect_failure to test_expect_success which makes reviewing this
>> series unnecessarily hard)
>
> Agree absolutely about patch 8 - I will re-roll with separate tests to
> document the test setup issue I fixed in 8.
>
> The only other changes to tests in 6 and 7 were the removal of
> comments about the actual bad behaviour. Would your preference be that
> I removed these #actual comments completely or that I moved
> documentation of the actual behaviour to the header of the test?
I'd prefer just to see the failure => success changes, so the comments
look superfluous to me and should be dropped from the failure case.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 6/9] submodule: fix detection of invalid submodule URL
2012-05-28 21:39 ` Jon Seymour
@ 2012-06-03 9:51 ` Jon Seymour
0 siblings, 0 replies; 17+ messages in thread
From: Jon Seymour @ 2012-06-03 9:51 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Jens.Lehmann, gitster, phil.hord, ramsay
On Tue, May 29, 2012 at 7:39 AM, Jon Seymour <jon.seymour@gmail.com> wrote:
> On Tue, May 29, 2012 at 5:01 AM, Johannes Sixt <j6t@kdbg.org> wrote:
>> Am 27.05.2012 17:34, schrieb Jon Seymour:
>>
>> Without understanding in detail what this series is about, I would guess
>> that the previous two case arms are not very Windows friendly. Does the
>> right thing happen when $remoteurl is "c:/path/to/remote"? Would it help
>> to use is_absolute_path?
>>
>> if is_absolute_path "$remoteurl"
>> then
>> variant="${remoteurl#*/}"
>> else
>> case "$remoteurl" in
>> ...other cases go here...
>> esac
>> fi
>>
>> -- Hannes
>
> Thanks, I will investigate this as an alternative.
>
I did investigate is_absolute_path for the v8 roll of this series, but
I found it wasn't suitable because it doesn't classify URLs of the
form user@host:repo as absolute. You can find the alternative I did
use in v8 3/4.
Regards,
jon.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2012-06-03 9:51 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-27 15:34 [PATCH v7 0/9] submodule: improve robustness of path handling Jon Seymour
2012-05-27 15:34 ` [PATCH v7 1/9] submodule: additional regression tests for relative URLs Jon Seymour
2012-05-27 15:34 ` [PATCH v7 2/9] submodule: document failure to detect invalid submodule URLs Jon Seymour
2012-05-27 15:34 ` [PATCH v7 3/9] submodule: document failure to handle relative superproject origin URLs Jon Seymour
2012-05-27 15:34 ` [PATCH v7 4/9] submodule: document failure to handle improperly normalized remote " Jon Seymour
2012-05-27 15:34 ` [PATCH v7 5/9] submodule: extract normalize_path into standalone function Jon Seymour
2012-05-27 15:34 ` [PATCH v7 6/9] submodule: fix detection of invalid submodule URL Jon Seymour
2012-05-28 19:01 ` Johannes Sixt
2012-05-28 21:39 ` Jon Seymour
2012-06-03 9:51 ` Jon Seymour
2012-05-27 15:34 ` [PATCH v7 7/9] submodule: fix sync handling of relative superproject origin URLs Jon Seymour
2012-05-27 15:34 ` [PATCH v7 8/9] submodule: fix handling of denormalized " Jon Seymour
2012-05-27 22:57 ` Jon Seymour
2012-05-27 15:34 ` [PATCH v7 9/9] submodule: fix normalization to handle repeated ./ Jon Seymour
2012-05-28 20:07 ` [PATCH v7 0/9] submodule: improve robustness of path handling Jens Lehmann
2012-05-28 22:01 ` Jon Seymour
2012-05-29 19:21 ` Jens Lehmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.