From ea2fa1040d14f1b7aab8fd78cc3ff4d41abc57a1 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 29 Mar 2016 18:27:41 -0700 Subject: [PATCH 1/6] submodule foreach: correct path display in recursive submodules The `prefix` was put in front of the display path unconditionally. This is wrong as any relative path computation would need to be at the front, so include the prefix into the display path. 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' Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- git-submodule.sh | 6 +++--- t/t7407-submodule-foreach.sh | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 43c68deee9..b3f248c3fe 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -413,8 +413,8 @@ cmd_foreach() die_if_unmatched "$mode" if test -e "$sm_path"/.git then - displaypath=$(relative_path "$sm_path") - say "$(eval_gettext "Entering '\$prefix\$displaypath'")" + displaypath=$(relative_path "$prefix$sm_path") + say "$(eval_gettext "Entering '\$displaypath'")" name=$(git submodule--helper name "$sm_path") ( prefix="$prefix$sm_path/" @@ -434,7 +434,7 @@ cmd_foreach() cmd_foreach "--recursive" "$@" fi ) <&3 3<&- || - die "$(eval_gettext "Stopping at '\$prefix\$displaypath'; script returned non-zero status.")" + die "$(eval_gettext "Stopping at '\$displaypath'; script returned non-zero status.")" fi done } diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh index 7ca10b8606..776b349508 100755 --- a/t/t7407-submodule-foreach.sh +++ b/t/t7407-submodule-foreach.sh @@ -177,6 +177,26 @@ test_expect_success 'test messages from "foreach --recursive"' ' test_i18ncmp expect actual ' +cat > expect <../../actual + ) && + test_i18ncmp expect actual +' + cat > expect < Date: Tue, 29 Mar 2016 18:27:42 -0700 Subject: [PATCH 2/6] submodule update --init: correct path handling in recursive submodules When calling `git submodule init` from a recursive instance of `git submodule update --recursive`, the reported path is wrong as it skips the nested submodules. 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. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- 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 b3f248c3fe..9fffa5c54a 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -473,7 +473,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 68ea31d693..d4745f4d93 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=$(git -C super rev-parse HEAD) +mergingsha1=$(git -C super/merging rev-parse HEAD) +nonesha1=$(git -C super/none rev-parse HEAD) +rebasingsha1=$(git -C super/rebasing rev-parse HEAD) +submodulesha1=$(git -C super/submodule rev-parse HEAD) +pwd=$(pwd) + +cat <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 && From 10450cf72b51baf3bac6a779fb4e47241af7ae5e Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 29 Mar 2016 18:27:43 -0700 Subject: [PATCH 3/6] submodule status: correct path handling in recursive submodules 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 Signed-off-by: Junio C Hamano --- 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 9fffa5c54a..1024774f61 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -1159,6 +1159,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 776b349508..4b35e12a73 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 < ../../actual + ) && + test_cmp expect actual +' + test_expect_success 'use "git clone --recursive" to checkout all submodules' ' git clone --recursive super clone4 && ( From b08238ac3f5e4033d26824afc65e7397fbb61847 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 29 Mar 2016 18:27:44 -0700 Subject: [PATCH 4/6] submodule update: align reporting path for custom command execution 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 Signed-off-by: Junio C Hamano --- 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 1024774f61..753a90d307 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -802,8 +802,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 d4745f4d93..01dd3243d6 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' ' From c1e06d11c7ab67080f0a08cd60b84c329442abb4 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 29 Mar 2016 18:27:45 -0700 Subject: [PATCH 5/6] submodule update: test recursive path reporting from subdirectory 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 Signed-off-by: Junio C Hamano --- t/t7406-submodule-update.sh | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 01dd3243d6..e5af4b4976 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -379,6 +379,26 @@ 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_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) && From 2ab56603bffb7aed6ec6d2a36deddfdf49cfd5ad Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 29 Mar 2016 18:27:46 -0700 Subject: [PATCH 6/6] t7407: make expectation as clear as possible 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 Signed-off-by: Junio C Hamano --- 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 4b35e12a73..6ba5daf42e 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 <