On 05/21, Prathamesh Chavan wrote: > Additional test cases added to the submodule-foreach test suite > to check the submodule foreach --recursive behavior from a > subdirectory as this was missing from the test suite. > > Mentored-by: Christian Couder <christian.couder@xxxxxxxxx> > Mentored-by: Stefan Beller <sbeller@xxxxxxxxxx> > Signed-off-by: Prathamesh Chavan <pc44800@xxxxxxxxx> > --- > It was observed that after porting the submodule subcommand to > C, it passed all the test from the existing test-suite. > But since there was some observation made, where the output of > the orignal submodule foreach subcommand wasn't matching to that > of the newly ported function, this test has been added. > > After which, it can been seen that the patch fails in test #9 > of t7407-submodule-foreach, which is the newly added > test to that suite. The main reason of adding this test > was to bring the behavior of $path for the submodule > foreach --recursive case. > > The observation made was as follows: > > For a project - super containing dir (not a submodule) > and a submodule sub which contains another submodule > subsub. When we run a command from super/dir: > > git submodule foreach "echo \$path-\$sm_path" > > actual results: > Entering '../sub' > ../sub-../sub > Entering '../sub/subsub' > ../subsub-../subsub > > ported function's result: > Entering '../sub' > sub-../sub > Entering '../sub/subsub' > subsub-../sub/subsub > > This is occurring since in cmd_foreach of git-submodule.sh > when we use to recurse, we call cmd_foreach > and hence the process ran in the same shell. > Because of this, the variable $wt_prefix is set only once > which is at the beginning of the submodule foreach execution. > wt_prefix=$(git rev-parse --show-prefix) > > And since sm_path and path are set using $wt_prefix as : > sm_path=$(git submodule--helper relative-path "$sm_path" "$wt_prefix") && > path=$sm_path > It differs with the value of displaypath as well. > > This make the value of $path confusing and I also feel it > deviates from its documentation: > $path is the name of the submodule directory relative > to the superproject. > > But since in refactoring the code, we wish to maintain the > code in same way, we need to pass wt_prefix on every > recursive call, which may result in complex C code. > Another option could be to first correct the $path value > in git-submodule.sh and then port the updated cmd_foreach. > > t/t7407-submodule-foreach.sh | 35 ++++++++++++++++++++++++++++++++++- > 1 file changed, 34 insertions(+), 1 deletion(-) > > diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh > index 6ba5daf42..58a890e31 100755 > --- a/t/t7407-submodule-foreach.sh > +++ b/t/t7407-submodule-foreach.sh > @@ -79,7 +79,6 @@ test_expect_success 'test basic "submodule foreach" usage' ' > ) && > test_i18ncmp expect actual > ' > - The removal of this line seems unrelated to the rest of this patch. Was this intended? > cat >expect <<EOF > Entering '../sub1' > $pwd/clone-foo1-../sub1-$sub1sha1 > @@ -197,6 +196,40 @@ test_expect_success 'test messages from "foreach --recursive" from subdirectory' > test_i18ncmp expect actual > ' > > +sub1sha1=$(cd clone2/sub1 && git rev-parse HEAD) > +sub2sha1=$(cd clone2/sub2 && git rev-parse HEAD) > +sub3sha1=$(cd clone2/sub3 && git rev-parse HEAD) > +nested1sha1=$(cd clone2/nested1 && git rev-parse HEAD) > +nested2sha1=$(cd clone2/nested1/nested2 && git rev-parse HEAD) > +nested3sha1=$(cd clone2/nested1/nested2/nested3 && git rev-parse HEAD) > +submodulesha1=$(cd clone2/nested1/nested2/nested3/submodule && git rev-parse HEAD) > + > +cat >expect <<EOF > +Entering '../nested1' > +$pwd/clone2-nested1-../nested1-$nested1sha1 > +Entering '../nested1/nested2' > +$pwd/clone2/nested1-nested2-../nested2-$nested2sha1 > +Entering '../nested1/nested2/nested3' > +$pwd/clone2/nested1/nested2-nested3-../nested3-$nested3sha1 > +Entering '../nested1/nested2/nested3/submodule' > +$pwd/clone2/nested1/nested2/nested3-submodule-../submodule-$submodulesha1 > +Entering '../sub1' > +$pwd/clone2-foo1-../sub1-$sub1sha1 > +Entering '../sub2' > +$pwd/clone2-foo2-../sub2-$sub2sha1 > +Entering '../sub3' > +$pwd/clone2-foo3-../sub3-$sub3sha1 > +EOF > + > +test_expect_success 'test "submodule foreach --recursive" from subdirectory' ' > + ( > + cd clone2 && > + cd untracked && > + git submodule foreach --recursive "echo \$toplevel-\$name-\$sm_path-\$sha1" >../../actual > + ) && > + test_i18ncmp expect actual > +' > + > cat > expect <<EOF > nested1-nested1 > nested2-nested2 > -- > 2.11.0 > -- Brandon Williams