* [PATCH 1/6] submodule foreach: correct path display in recursive submodules
2016-03-29 22:23 [PATCHv2 0/6] Fix path bugs in submodule commands executed from sub dir Stefan Beller
@ 2016-03-29 22:23 ` Stefan Beller
2016-03-29 22:23 ` [PATCH 2/6] submodule update --init: correct path handling " Stefan Beller
` (5 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Stefan Beller @ 2016-03-29 22:23 UTC (permalink / raw)
To: gitster, jacob.keller; +Cc: git, Stefan Beller
The new test replicates the previous test with the difference of executing
from a sub directory. By executing from a sub directory all we would
expect all displayed paths to be prefixed by '../'.
Prior to this patch the test 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 <sbeller@google.com>
---
git-submodule.sh | 3 ++-
t/t7407-submodule-foreach.sh | 20 ++++++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 43c68de..2838069 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -417,10 +417,11 @@ cmd_foreach()
say "$(eval_gettext "Entering '\$prefix\$displaypath'")"
name=$(git submodule--helper name "$sm_path")
(
- prefix="$prefix$sm_path/"
+ prefix="$(relative_path $prefix$sm_path)/"
clear_local_git_env
cd "$sm_path" &&
sm_path=$(relative_path "$sm_path") &&
+ wt_prefix=
# we make $path available to scripts ...
path=$sm_path &&
if test $# -eq 1
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 7ca10b8..776b349 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -178,6 +178,26 @@ test_expect_success 'test messages from "foreach --recursive"' '
'
cat > expect <<EOF
+Entering '../nested1'
+Entering '../nested1/nested2'
+Entering '../nested1/nested2/nested3'
+Entering '../nested1/nested2/nested3/submodule'
+Entering '../sub1'
+Entering '../sub2'
+Entering '../sub3'
+EOF
+
+test_expect_success 'test messages from "foreach --recursive" from subdirectory' '
+ (
+ cd clone2 &&
+ mkdir untracked &&
+ cd untracked &&
+ git submodule foreach --recursive >../../actual
+ ) &&
+ test_i18ncmp expect actual
+'
+
+cat > expect <<EOF
nested1-nested1
nested2-nested2
nested3-nested3
--
2.8.0.4.g5639dee.dirty
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/6] submodule update --init: correct path handling in recursive submodules
2016-03-29 22:23 [PATCHv2 0/6] Fix path bugs in submodule commands executed from sub dir Stefan Beller
2016-03-29 22:23 ` [PATCH 1/6] submodule foreach: correct path display in recursive submodules Stefan Beller
@ 2016-03-29 22:23 ` Stefan Beller
2016-03-29 22:23 ` [PATCH 3/6] submodule status: " Stefan Beller
` (4 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Stefan Beller @ 2016-03-29 22:23 UTC (permalink / raw)
To: gitster, jacob.keller; +Cc: git, Stefan Beller
The new test demonstrates a failure in the code prior to this patch.
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.
That happens because the prefix is ignored in `git submodule add`, probably
because that function itself cannot recurse; it may however called by
recursive instances of `git submodule update`, so we need to respect the
`prefix`.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
git-submodule.sh | 2 +-
t/t7406-submodule-update.sh | 33 +++++++++++++++++++++++++++++++++
2 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 2838069..fdb5fbd 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -474,7 +474,7 @@ cmd_init()
die_if_unmatched "$mode"
name=$(git submodule--helper name "$sm_path") || exit
- displaypath=$(relative_path "$sm_path")
+ displaypath=$(relative_path "$prefix$sm_path")
# Copy url setting when it is not set yet
if test -z "$(git config "submodule.$name.url")"
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 68ea31d..9a4ba41 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -63,6 +63,10 @@ test_expect_success 'setup a submodule tree' '
git submodule add ../none none &&
test_tick &&
git commit -m "none"
+ ) &&
+ git clone . recursivesuper &&
+ ( cd recursivesuper
+ git submodule add ../super super
)
'
@@ -95,6 +99,35 @@ test_expect_success 'submodule update from subdirectory' '
)
'
+supersha1=$(cd super && git rev-parse HEAD)
+mergingsha1=$(cd super/merging && git rev-parse HEAD)
+nonesha1=$(cd super/none && git rev-parse HEAD)
+rebasingsha1=$(cd super/rebasing && git rev-parse HEAD)
+submodulesha1=$(cd super/submodule && git rev-parse HEAD)
+pwd=$(pwd)
+
+cat <<EOF >expect
+Submodule path '../super': checked out '$supersha1'
+Submodule 'merging' ($pwd/merging) registered for path '../super/merging'
+Submodule 'none' ($pwd/none) registered for path '../super/none'
+Submodule 'rebasing' ($pwd/rebasing) registered for path '../super/rebasing'
+Submodule 'submodule' ($pwd/submodule) registered for path '../super/submodule'
+Submodule path '../super/merging': checked out '$mergingsha1'
+Submodule path '../super/none': checked out '$nonesha1'
+Submodule path '../super/rebasing': checked out '$rebasingsha1'
+Submodule path '../super/submodule': checked out '$submodulesha1'
+EOF
+
+test_expect_success 'submodule update --init --recursive from subdirectory' '
+ git -C recursivesuper/super reset --hard HEAD^ &&
+ (cd recursivesuper &&
+ mkdir tmp &&
+ cd tmp &&
+ git submodule update --init --recursive ../super >../../actual
+ ) &&
+ test_cmp expect actual
+'
+
apos="'";
test_expect_success 'submodule update does not fetch already present commits' '
(cd submodule &&
--
2.8.0.4.g5639dee.dirty
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 3/6] submodule status: correct path handling in recursive submodules
2016-03-29 22:23 [PATCHv2 0/6] Fix path bugs in submodule commands executed from sub dir Stefan Beller
2016-03-29 22:23 ` [PATCH 1/6] submodule foreach: correct path display in recursive submodules Stefan Beller
2016-03-29 22:23 ` [PATCH 2/6] submodule update --init: correct path handling " Stefan Beller
@ 2016-03-29 22:23 ` Stefan Beller
2016-03-29 22:23 ` [PATCH 4/6] t7407: make expectation as clear as possible Stefan Beller
` (3 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Stefan Beller @ 2016-03-29 22:23 UTC (permalink / raw)
To: gitster, jacob.keller; +Cc: git, Stefan Beller
The new test which is a replica of the previous test except
that it executes from a sub directory. Prior to this patch
the test failed by having too many '../' prefixed:
--- expect 2016-03-29 19:02:33.087336115 +0000
+++ actual 2016-03-29 19:02:33.359343311 +0000
@@ -1,7 +1,7 @@
b23f134787d96fae589a6b76da41f4db112fc8db ../nested1 (heads/master)
-+25d56d1ddfb35c3e91ff7d8f12331c2e53147dcc ../nested1/nested2 (file2)
- 5ec83512b76a0b8170b899f8e643913c3e9b72d9 ../nested1/nested2/nested3 (heads/master)
- 509f622a4f36a3e472affcf28fa959174f3dd5b5 ../nested1/nested2/nested3/submodule (heads/master)
++25d56d1ddfb35c3e91ff7d8f12331c2e53147dcc ../../nested1/nested2 (file2)
+ 5ec83512b76a0b8170b899f8e643913c3e9b72d9 ../../../nested1/nested2/nested3 (heads/master)
+ 509f622a4f36a3e472affcf28fa959174f3dd5b5 ../../../../nested1/nested2/nested3/submodule (heads/master)
0c90624ab7f1aaa301d3bb79f60dcfed1ec4897f ../sub1 (0c90624)
0c90624ab7f1aaa301d3bb79f60dcfed1ec4897f ../sub2 (0c90624)
509f622a4f36a3e472affcf28fa959174f3dd5b5 ../sub3 (heads/master)
The path code in question:
displaypath=$(relative_path "$prefix$sm_path")
prefix=$displaypath
if recursive:
eval cmd_status
That way we change `prefix` each iteration to contain another
'../', because of the the relative_path computation is done
on an already computed relative path.
We must call relative_path exactly once with `wt_prefix` non empty.
Further calls in recursive instances to to calculate the displaypath
already incorporate the correct prefix from before. Fix the issue by
clearing `wt_prefix` in recursive calls.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
git-submodule.sh | 1 +
t/t7407-submodule-foreach.sh | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/git-submodule.sh b/git-submodule.sh
index fdb5fbd..11ed32a 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1160,6 +1160,7 @@ cmd_status()
(
prefix="$displaypath/"
clear_local_git_env
+ wt_prefix=
cd "$sm_path" &&
eval cmd_status
) ||
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 776b349..4b35e12 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -277,6 +277,27 @@ test_expect_success 'ensure "status --cached --recursive" preserves the --cached
test_cmp expect actual
'
+nested2sha1=$(git -C clone3/nested1/nested2 rev-parse HEAD)
+
+cat > expect <<EOF
+ $nested1sha1 ../nested1 (heads/master)
++$nested2sha1 ../nested1/nested2 (file2)
+ $nested3sha1 ../nested1/nested2/nested3 (heads/master)
+ $submodulesha1 ../nested1/nested2/nested3/submodule (heads/master)
+ $sub1sha1 ../sub1 ($sub1sha1_short)
+ $sub2sha1 ../sub2 ($sub2sha1_short)
+ $sub3sha1 ../sub3 (heads/master)
+EOF
+
+test_expect_success 'test "status --recursive" from sub directory' '
+ (
+ cd clone3 &&
+ mkdir tmp && cd tmp &&
+ git submodule status --recursive > ../../actual
+ ) &&
+ test_cmp expect actual
+'
+
test_expect_success 'use "git clone --recursive" to checkout all submodules' '
git clone --recursive super clone4 &&
(
--
2.8.0.4.g5639dee.dirty
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 4/6] t7407: make expectation as clear as possible
2016-03-29 22:23 [PATCHv2 0/6] Fix path bugs in submodule commands executed from sub dir Stefan Beller
` (2 preceding siblings ...)
2016-03-29 22:23 ` [PATCH 3/6] submodule status: " Stefan Beller
@ 2016-03-29 22:23 ` Stefan Beller
2016-03-29 22:23 ` [PATCH 5/6] submodule update: align reporting path for custom command execution Stefan Beller
` (2 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Stefan Beller @ 2016-03-29 22:23 UTC (permalink / raw)
To: gitster, jacob.keller; +Cc: git, Stefan Beller
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 <sbeller@google.com>
---
t/t7407-submodule-foreach.sh | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 4b35e12..6ba5daf 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -262,8 +262,12 @@ test_expect_success 'test "status --recursive"' '
test_cmp expect actual
'
-sed -e "/nested2 /s/.*/+$nested2sha1 nested1\/nested2 (file2~1)/;/sub[1-3]/d" < expect > expect2
-mv -f expect2 expect
+cat > expect <<EOF
+ $nested1sha1 nested1 (heads/master)
++$nested2sha1 nested1/nested2 (file2~1)
+ $nested3sha1 nested1/nested2/nested3 (heads/master)
+ $submodulesha1 nested1/nested2/nested3/submodule (heads/master)
+EOF
test_expect_success 'ensure "status --cached --recursive" preserves the --cached flag' '
(
--
2.8.0.4.g5639dee.dirty
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 5/6] submodule update: align reporting path for custom command execution
2016-03-29 22:23 [PATCHv2 0/6] Fix path bugs in submodule commands executed from sub dir Stefan Beller
` (3 preceding siblings ...)
2016-03-29 22:23 ` [PATCH 4/6] t7407: make expectation as clear as possible Stefan Beller
@ 2016-03-29 22:23 ` Stefan Beller
2016-03-29 22:23 ` [PATCH 6/6] submodule update: test recursive path reporting from subdirectory Stefan Beller
2016-03-29 22:29 ` [PATCHv2 0/6] Fix path bugs in submodule commands executed from sub dir Stefan Beller
6 siblings, 0 replies; 11+ messages in thread
From: Stefan Beller @ 2016-03-29 22:23 UTC (permalink / raw)
To: gitster, jacob.keller; +Cc: git, Stefan Beller
In the predefined actions (merge, rebase, none, checkout), we use
the display path, which is relative to the current working directory.
Also use the display path when running a custom command.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
git-submodule.sh | 4 ++--
t/t7406-submodule-update.sh | 29 ++++++++++++++++++++++++++---
2 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 11ed32a..be2a2b5 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -803,8 +803,8 @@ Maybe you want to use 'update --init'?")"
;;
!*)
command="${update_module#!}"
- die_msg="$(eval_gettext "Execution of '\$command \$sha1' failed in submodule path '\$prefix\$sm_path'")"
- say_msg="$(eval_gettext "Submodule path '\$prefix\$sm_path': '\$command \$sha1'")"
+ die_msg="$(eval_gettext "Execution of '\$command \$sha1' failed in submodule path '\$displaypath'")"
+ say_msg="$(eval_gettext "Submodule path '\$displaypath': '\$command \$sha1'")"
must_die_on_failure=yes
;;
*)
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 9a4ba41..f062065 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -344,16 +344,39 @@ test_expect_success 'submodule update - command in .git/config' '
)
'
+cat << EOF >expect
+Execution of 'false $submodulesha1' failed in submodule path 'submodule'
+EOF
+
test_expect_success 'submodule update - command in .git/config catches failure' '
(cd super &&
git config submodule.submodule.update "!false"
) &&
(cd super/submodule &&
- git reset --hard HEAD^
+ git reset --hard $submodulesha1^
) &&
(cd super &&
- test_must_fail git submodule update submodule
- )
+ test_must_fail git submodule update submodule 2>../actual
+ ) &&
+ test_cmp actual expect
+'
+
+cat << EOF >expect
+Execution of 'false $submodulesha1' failed in submodule path '../submodule'
+EOF
+
+test_expect_success 'submodule update - command in .git/config catches failure -- subdirectory' '
+ (cd super &&
+ git config submodule.submodule.update "!false"
+ ) &&
+ (cd super/submodule &&
+ git reset --hard $submodulesha1^
+ ) &&
+ (cd super &&
+ mkdir tmp && cd tmp &&
+ test_must_fail git submodule update ../submodule 2>../../actual
+ ) &&
+ test_cmp actual expect
'
test_expect_success 'submodule init does not copy command into .git/config' '
--
2.8.0.4.g5639dee.dirty
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 6/6] submodule update: test recursive path reporting from subdirectory
2016-03-29 22:23 [PATCHv2 0/6] Fix path bugs in submodule commands executed from sub dir Stefan Beller
` (4 preceding siblings ...)
2016-03-29 22:23 ` [PATCH 5/6] submodule update: align reporting path for custom command execution Stefan Beller
@ 2016-03-29 22:23 ` Stefan Beller
2016-03-29 22:29 ` [PATCHv2 0/6] Fix path bugs in submodule commands executed from sub dir Stefan Beller
6 siblings, 0 replies; 11+ messages in thread
From: Stefan Beller @ 2016-03-29 22:23 UTC (permalink / raw)
To: gitster, jacob.keller; +Cc: git, Stefan Beller
This patch is just a test and fixes no bug as there is currently no bug
in the path handling of `submodule update`.
In `submodule update` we make a call to `submodule--helper list --prefix
"$wt_prefix"` which looks a bit brittle and likely to introduce a bug
for the path handling. It is not a bug as the prefix is ignored inside
the submodule helper for now. If this test breaks eventually, we want
to make sure the `wt_prefix` is passed correctly into recursive submodules.
Hint: In recursive submodules we expect `wt_prefix` to be empty.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
t/t7406-submodule-update.sh | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index f062065..239d31f 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -379,6 +379,27 @@ test_expect_success 'submodule update - command in .git/config catches failure -
test_cmp actual expect
'
+cat << EOF >expect
+Execution of 'false $submodulesha1' failed in submodule path '../super/submodule'
+Failed to recurse into submodule path '../super'
+EOF
+
+test_expect_success 'recursive submodule update - command in .git/config catches failure -- subdirectory' '
+ (cd recursivesuper &&
+ git submodule update --remote super &&
+ git add super &&
+ git commit -m "update to latest to have more than one commit in submodules"
+ ) &&
+ git -C recursivesuper/super config submodule.submodule.update "!false" &&
+ git -C recursivesuper/super/submodule reset --hard $submodulesha1^ &&
+ (cd recursivesuper &&
+ mkdir -p tmp && cd tmp &&
+ test_pause &&
+ test_must_fail git submodule update --recursive ../super 2>../../actual
+ ) &&
+ test_cmp actual expect
+'
+
test_expect_success 'submodule init does not copy command into .git/config' '
(cd super &&
H=$(git ls-files -s submodule | cut -d" " -f2) &&
--
2.8.0.4.g5639dee.dirty
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCHv2 0/6] Fix path bugs in submodule commands executed from sub dir
2016-03-29 22:23 [PATCHv2 0/6] Fix path bugs in submodule commands executed from sub dir Stefan Beller
` (5 preceding siblings ...)
2016-03-29 22:23 ` [PATCH 6/6] submodule update: test recursive path reporting from subdirectory Stefan Beller
@ 2016-03-29 22:29 ` Stefan Beller
6 siblings, 0 replies; 11+ messages in thread
From: Stefan Beller @ 2016-03-29 22:29 UTC (permalink / raw)
To: Junio C Hamano, Jacob Keller; +Cc: git, Stefan Beller
Please ignore this series; it was sent out prematurely. :(
^ permalink raw reply [flat|nested] 11+ messages in thread