From: Stefan Beller <hidden> Date: 2016-06-15 23:09:05
Junio wrote:
I suspect that the fix in your 1&2 may be demonstratable without
forcing an early failure by switching to "git -C".
So for now I present test coverage and their minimal fixes.
This series follows a "tick-tock" pattern except for patch5,
which I wrote quickly as I was annoying by the bells and whistles.
I expect test code to be dumb, not tricking ourselves by "smart" code there.
The "tick" patches introduce failing tests. They need to fail to demonstrate
the bugs exist, which are fixed in the "tock" patches, which are doing
nothing fancy but just a one or two line correction of the path handling
code.
This applies to 2.8.
As this is taking a completely different turn than I expected in
"[PATCHv3 0/5] submodule helper: cleanup prefix passing", I made this
a new series. (It also doesn't do cleanup any more, but just fixes bugs.)
Thanks,
Stefan
Stefan Beller (7):
submodule foreach: test path handling in recursive submodules
submodule foreach: correct path computation in recursive submodules
submodule update --init: test path handling in recursive submodules
submodule update --init: correct path handling in recursive submodules
t7407: make expectation as clear as possible
submodule status: test path handling in recursive submodules
submodule status: fix path handling in recursive submodules
git-submodule.sh | 9 ++++++---
t/t7406-submodule-update.sh | 33 +++++++++++++++++++++++++++++++
t/t7407-submodule-foreach.sh | 47 ++++++++++++++++++++++++++++++++++++++++++--
3 files changed, 84 insertions(+), 5 deletions(-)
--
2.8.0.rc4.23.gd22361a.dirty
From: Stefan Beller <hidden> Date: 2016-06-15 23:09:05
This replicates the previous test with the difference of executing
from a sub directory. By executing from a sub directory all we would
expect all paths to be prefixed by '../'.
Document this as failure in this patch, to be fixed in a later patch.
Signed-off-by: Stefan Beller <redacted>
---
t/t7407-submodule-foreach.sh | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
From: Stefan Beller <hidden> Date: 2016-06-15 23:09:05
This demonstrates a failure to handle paths correctly.
Instead of getting the expected
Submodule 'submodule' (${pwd}/submodule) registered for path '../super/submodule'
the `super` directory is omitted and you get
Submodule 'submodule' (${pwd}/submodule) registered for path '../submodule'
instead.
Signed-off-by: Stefan Beller <redacted>
---
t/t7406-submodule-update.sh | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
From: Stefan Beller <hidden> Date: 2016-06-15 23:09:05
The test which is fixed by this patch would report
Entering 'nested1/nested2/../nested3'
instead of the expected
Entering '../nested1/nested2/nested3'
because the prefix is put unconditionally in front and after that a
computed display path with is affected by `wt_prefix`. This is wrong as
any relative path computation would need to be at the front. By emptying
the `wt_prefix` in recursive submodules and adding the information of any
relative path into the `prefix` this is fixed.
Signed-off-by: Stefan Beller <redacted>
---
git-submodule.sh | 3 ++-
t/t7407-submodule-foreach.sh | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
@@ -417,10 +417,11 @@ cmd_foreach()say"$(eval_gettext"Entering '\$prefix\$displaypath'")"name=$(gitsubmodule--helpername"$sm_path")(-prefix="$prefix$sm_path/"+prefix="$(relative_path$prefix$sm_path)/"clear_local_git_envcd"$sm_path"&&sm_path=$(relative_path"$sm_path")&&+wt_prefix=# we make $path available to scripts ...path=$sm_path&&iftest$#-eq1
@@ -187,7 +187,7 @@ Entering '../sub2' Entering'../sub3' EOF-test_expect_failure'test messages from "foreach --recursive" from subdirectory''+test_expect_success'test messages from "foreach --recursive" from subdirectory''(cdclone2&&mkdiruntracked&&
From: Stefan Beller <hidden> Date: 2016-06-15 23:09:05
This fixes the test which was introduced in the parent commit.
Signed-off-by: Stefan Beller <redacted>
---
git-submodule.sh | 1 +
t/t7407-submodule-foreach.sh | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
@@ -272,7 +272,7 @@ cat > expect <<EOF$sub3sha1../sub3(heads/master) EOF-test_expect_failure'test "status --recursive" from sub directory''+test_expect_success'test "status --recursive" from sub directory''(cdclone3&&mkdirtmp&&cdtmp&&
From: Stefan Beller <hidden> Date: 2016-06-15 23:09:05
This replicates the previous test except that it executes from a sub
directory. Document the expected outcome, which currently fails
by having too many '../' prefixed.
Signed-off-by: Stefan Beller <redacted>
---
t/t7407-submodule-foreach.sh | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
From: Stefan Beller <hidden> Date: 2016-06-15 23:09:05
Not everyone (including me) grasps the sed expression in a split second as
they would grasp the 4 lines printed as is.
Signed-off-by: Stefan Beller <redacted>
---
t/t7407-submodule-foreach.sh | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
From: Stefan Beller <hidden> Date: 2016-06-15 23:09:05
This fixes the test introduced by the parent commit.
Signed-off-by: Stefan Beller <redacted>
---
git-submodule.sh | 5 +++--
t/t7406-submodule-update.sh | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
@@ -474,7 +474,7 @@ cmd_init()die_if_unmatched"$mode"name=$(gitsubmodule--helpername"$sm_path")||exit-displaypath=$(relative_path"$sm_path")+displaypath=$(relative_path"$prefix$sm_path")# Copy url setting when it is not set yetiftest-z"$(gitconfig"submodule.$name.url")"
@@ -826,8 +826,9 @@ Maybe you want to use 'update --init'?")"iftest-n"$recursive"then(-prefix="$prefix$sm_path/"+prefix="$(relative_path$prefix$sm_path)/"clear_local_git_env+wt_prefix=cd"$sm_path"&&evalcmd_update)