* [PATCHv3 0/6] Fix path bugs in submodule commands executed from sub dir
@ 2016-03-29 23:02 Stefan Beller
2016-03-29 23:02 ` [PATCH 1/6] submodule foreach: correct path display in recursive submodules Stefan Beller
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Stefan Beller @ 2016-03-29 23:02 UTC (permalink / raw)
To: gitster, jacob.keller; +Cc: git, Stefan Beller
v3:
Resending without `test_pause` in the last test.
v2:
The first 3 commits are
* Test and bugfix in one commit each
* better explained than before
The expansion of an expected test result moved to the back of the series.
There are two new commits
* one being another bugfix of the display path for `submodule update`
* another test for `submodule update` as I suspect it may break further on
refactoring and we currently have no tests for it.
Thanks,
Stefan
Stefan Beller (6):
submodule foreach: correct path display in recursive submodules
submodule update --init: correct path handling in recursive submodules
submodule status: correct path handling in recursive submodules
t7407: make expectation as clear as possible
submodule update: align reporting path for custom command execution
submodule update: test recursive path reporting from subdirectory
git-submodule.sh | 10 +++---
t/t7406-submodule-update.sh | 83 ++++++++++++++++++++++++++++++++++++++++++--
t/t7407-submodule-foreach.sh | 49 ++++++++++++++++++++++++--
3 files changed, 133 insertions(+), 9 deletions(-)
--
2.8.0.4.g5639dee.dirty
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/6] submodule foreach: correct path display in recursive submodules
2016-03-29 23:02 [PATCHv3 0/6] Fix path bugs in submodule commands executed from sub dir Stefan Beller
@ 2016-03-29 23:02 ` Stefan Beller
2016-03-29 23:49 ` Junio C Hamano
2016-03-29 23:02 ` [PATCH 2/6] submodule update --init: correct path handling " Stefan Beller
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Stefan Beller @ 2016-03-29 23:02 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.6.g3d0b0ba
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/6] submodule update --init: correct path handling in recursive submodules
2016-03-29 23:02 [PATCHv3 0/6] Fix path bugs in submodule commands executed from sub dir Stefan Beller
2016-03-29 23:02 ` [PATCH 1/6] submodule foreach: correct path display in recursive submodules Stefan Beller
@ 2016-03-29 23:02 ` Stefan Beller
2016-03-29 23:54 ` Junio C Hamano
2016-03-29 23:02 ` [PATCH 3/6] submodule status: " Stefan Beller
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Stefan Beller @ 2016-03-29 23:02 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.6.g3d0b0ba
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/6] submodule status: correct path handling in recursive submodules
2016-03-29 23:02 [PATCHv3 0/6] Fix path bugs in submodule commands executed from sub dir Stefan Beller
2016-03-29 23:02 ` [PATCH 1/6] submodule foreach: correct path display in recursive submodules Stefan Beller
2016-03-29 23:02 ` [PATCH 2/6] submodule update --init: correct path handling " Stefan Beller
@ 2016-03-29 23:02 ` Stefan Beller
2016-03-29 23:55 ` Junio C Hamano
2016-03-29 23:02 ` [PATCH 4/6] submodule update: align reporting path for custom command execution Stefan Beller
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Stefan Beller @ 2016-03-29 23:02 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.6.g3d0b0ba
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/6] submodule update: align reporting path for custom command execution
2016-03-29 23:02 [PATCHv3 0/6] Fix path bugs in submodule commands executed from sub dir Stefan Beller
` (2 preceding siblings ...)
2016-03-29 23:02 ` [PATCH 3/6] submodule status: " Stefan Beller
@ 2016-03-29 23:02 ` Stefan Beller
2016-03-29 23:57 ` Junio C Hamano
2016-03-29 23:02 ` [PATCH 5/6] submodule update: test recursive path reporting from subdirectory Stefan Beller
2016-03-29 23:02 ` [PATCH 6/6] t7407: make expectation as clear as possible Stefan Beller
5 siblings, 1 reply; 13+ messages in thread
From: Stefan Beller @ 2016-03-29 23:02 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.6.g3d0b0ba
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/6] submodule update: test recursive path reporting from subdirectory
2016-03-29 23:02 [PATCHv3 0/6] Fix path bugs in submodule commands executed from sub dir Stefan Beller
` (3 preceding siblings ...)
2016-03-29 23:02 ` [PATCH 4/6] submodule update: align reporting path for custom command execution Stefan Beller
@ 2016-03-29 23:02 ` Stefan Beller
2016-03-29 23:02 ` [PATCH 6/6] t7407: make expectation as clear as possible Stefan Beller
5 siblings, 0 replies; 13+ messages in thread
From: Stefan Beller @ 2016-03-29 23:02 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 | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index f062065..6dcf2d4 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) &&
--
2.8.0.6.g3d0b0ba
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 6/6] t7407: make expectation as clear as possible
2016-03-29 23:02 [PATCHv3 0/6] Fix path bugs in submodule commands executed from sub dir Stefan Beller
` (4 preceding siblings ...)
2016-03-29 23:02 ` [PATCH 5/6] submodule update: test recursive path reporting from subdirectory Stefan Beller
@ 2016-03-29 23:02 ` Stefan Beller
5 siblings, 0 replies; 13+ messages in thread
From: Stefan Beller @ 2016-03-29 23:02 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.6.g3d0b0ba
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/6] submodule foreach: correct path display in recursive submodules
2016-03-29 23:02 ` [PATCH 1/6] submodule foreach: correct path display in recursive submodules Stefan Beller
@ 2016-03-29 23:49 ` Junio C Hamano
0 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2016-03-29 23:49 UTC (permalink / raw)
To: Stefan Beller; +Cc: jacob.keller, git
Stefan Beller <sbeller@google.com> writes:
> 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 '../'.
-ECANTPARSE on the last sentence. Remove the first "all" perhaps?
As this patch is no longer about a new test but is about a fix of a
problem, for which a new test serves as a typical example, it sounds
somewhat funny to start the log message with description of the test.
> 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`.
-ECANTPARSE even though I can guess what you want to say.
> 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)/"
What happens when prefix or sm_path has $IFS whitespace? Imitate the
correct quoting you do three lines below, i.e.
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/6] submodule update --init: correct path handling in recursive submodules
2016-03-29 23:02 ` [PATCH 2/6] submodule update --init: correct path handling " Stefan Beller
@ 2016-03-29 23:54 ` Junio C Hamano
2016-03-30 1:00 ` Stefan Beller
0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2016-03-29 23:54 UTC (permalink / raw)
To: Stefan Beller; +Cc: jacob.keller, git
Stefan Beller <sbeller@google.com> writes:
> 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.
Same "is this about test?" comment applies here.
> That happens because the prefix is ignored in `git submodule add`, probably
> because that function itself cannot recurse;
"probably"???
> it may however called by
Probably "be" needs to be somewhere on this line.
> 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)
Perhaps "git -C super rev-parse HEAD"?
> +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 &&
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/6] submodule status: correct path handling in recursive submodules
2016-03-29 23:02 ` [PATCH 3/6] submodule status: " Stefan Beller
@ 2016-03-29 23:55 ` Junio C Hamano
0 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2016-03-29 23:55 UTC (permalink / raw)
To: Stefan Beller; +Cc: jacob.keller, git
Stefan Beller <sbeller@google.com> writes:
> 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.
OK, nicely analyzed and explained.
> 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
> ) ||
Makes sense.
Thanks.
> 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 &&
> (
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/6] submodule update: align reporting path for custom command execution
2016-03-29 23:02 ` [PATCH 4/6] submodule update: align reporting path for custom command execution Stefan Beller
@ 2016-03-29 23:57 ` Junio C Hamano
0 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2016-03-29 23:57 UTC (permalink / raw)
To: Stefan Beller; +Cc: jacob.keller, git
Stefan Beller <sbeller@google.com> writes:
> 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.
Very sensible.
> 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' '
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/6] submodule update --init: correct path handling in recursive submodules
2016-03-29 23:54 ` Junio C Hamano
@ 2016-03-30 1:00 ` Stefan Beller
0 siblings, 0 replies; 13+ messages in thread
From: Stefan Beller @ 2016-03-30 1:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Keller, git
On Tue, Mar 29, 2016 at 4:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> 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.
>
> Same "is this about test?" comment applies here.
>
>> That happens because the prefix is ignored in `git submodule add`, probably
>> because that function itself cannot recurse;
>
> "probably"???
I am speculating there. I have no idea why it originally was written that way
but I would assume this is the most likeliest explanation.
I'll reword the whole commit message.
>
>>
>> +supersha1=$(cd super && git rev-parse HEAD)
>
> Perhaps "git -C super rev-parse HEAD"?
done
^ permalink raw reply [flat|nested] 13+ 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 ` Stefan Beller
0 siblings, 0 replies; 13+ 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] 13+ messages in thread
end of thread, other threads:[~2016-03-30 1:00 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-29 23:02 [PATCHv3 0/6] Fix path bugs in submodule commands executed from sub dir Stefan Beller
2016-03-29 23:02 ` [PATCH 1/6] submodule foreach: correct path display in recursive submodules Stefan Beller
2016-03-29 23:49 ` Junio C Hamano
2016-03-29 23:02 ` [PATCH 2/6] submodule update --init: correct path handling " Stefan Beller
2016-03-29 23:54 ` Junio C Hamano
2016-03-30 1:00 ` Stefan Beller
2016-03-29 23:02 ` [PATCH 3/6] submodule status: " Stefan Beller
2016-03-29 23:55 ` Junio C Hamano
2016-03-29 23:02 ` [PATCH 4/6] submodule update: align reporting path for custom command execution Stefan Beller
2016-03-29 23:57 ` Junio C Hamano
2016-03-29 23:02 ` [PATCH 5/6] submodule update: test recursive path reporting from subdirectory Stefan Beller
2016-03-29 23:02 ` [PATCH 6/6] t7407: make expectation as clear as possible Stefan Beller
-- strict thread matches above, loose matches on Subject: below --
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 2/6] submodule update --init: correct path handling in recursive submodules Stefan Beller
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.