Re: [PATCH v4 1/3] fetch: add test to make sure we stay backwards compatible

2017-10-17 Thread Stefan Beller
On Mon, Oct 16, 2017 at 6:57 AM, Heiko Voigt  wrote:
> The current implementation of submodules supports on-demand fetch if
> there is no .gitmodules entry for a submodule. Let's add a test to
> document this behavior.
>
> Signed-off-by: Heiko Voigt 
> ---
>  t/t5526-fetch-submodules.sh | 42 +-
>  1 file changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
> index 42251f7f3a..43a22f680f 100755
> --- a/t/t5526-fetch-submodules.sh
> +++ b/t/t5526-fetch-submodules.sh
> @@ -478,7 +478,47 @@ test_expect_success "don't fetch submodule when newly 
> recorded commits are alrea
> git fetch >../actual.out 2>../actual.err
> ) &&
> ! test -s actual.out &&
> -   test_i18ncmp expect.err actual.err
> +   test_i18ncmp expect.err actual.err &&
> +   (
> +   cd submodule &&
> +   git checkout -q master
> +   )

For few instructions inside another repo, I tend to use the
-C option:

  git -C submodule checkout -q master

That saves a shell, which is noticeable cost on Windows I was told.
(also fewer lines to type).

Oh, I see, that is consistent with the rest of the file. Oh well.
(Otherwise I would have lobbied to even move it further up and
put it inside a test_when_finished ""


> +'
> +
> +test_expect_success "'fetch.recurseSubmodules=on-demand' works also without 
> .gitmodule entry" '
> +   (
> +   cd downstream &&
> +   git fetch --recurse-submodules
> +   ) &&

This is consistent with the rest of the file as well, so I shall
refrain from complaining. ;)

> +   add_upstream_commit &&
> +   head1=$(git rev-parse --short HEAD) &&
> +   git add submodule &&
> +   git rm .gitmodules &&
> +   git commit -m "new submodule without .gitmodules" &&
> +   printf "" >expect.out &&

This could be just

: >expect.out

no need to invoke a function to print nothing.

> +   head2=$(git rev-parse --short HEAD) &&
> +   echo "From $pwd/." >expect.err.2 &&
> +   echo "   $head1..$head2  master -> origin/master" >>expect.err.2 
> &&
> +   head -3 expect.err >>expect.err.2 &&
> +   (
> +   cd downstream &&
> +   rm .gitmodules &&
> +   git config fetch.recurseSubmodules on-demand &&
> +   # fake submodule configuration to avoid skipping submodule 
> handling
> +   git config -f .gitmodules submodule.fake.path fake &&
> +   git config -f .gitmodules submodule.fake.url fakeurl &&
> +   git add .gitmodules &&
> +   git config --unset submodule.submodule.url &&
> +   git fetch >../actual.out 2>../actual.err &&
> +   # cleanup
> +   git config --unset fetch.recurseSubmodules &&
> +   git reset --hard
> +   ) &&
> +   test_i18ncmp expect.out actual.out &&
> +   test_i18ncmp expect.err.2 actual.err &&
> +   git checkout HEAD^ -- .gitmodules &&
> +   git add .gitmodules &&
> +   git commit -m "new submodule restored .gitmodules"
>  '

Thanks for writing this test.
With or without the nits addressed, this is

Reviewed-by: Stefan Beller 


[PATCH v4 1/3] fetch: add test to make sure we stay backwards compatible

2017-10-16 Thread Heiko Voigt
The current implementation of submodules supports on-demand fetch if
there is no .gitmodules entry for a submodule. Let's add a test to
document this behavior.

Signed-off-by: Heiko Voigt 
---
 t/t5526-fetch-submodules.sh | 42 +-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh
index 42251f7f3a..43a22f680f 100755
--- a/t/t5526-fetch-submodules.sh
+++ b/t/t5526-fetch-submodules.sh
@@ -478,7 +478,47 @@ test_expect_success "don't fetch submodule when newly 
recorded commits are alrea
git fetch >../actual.out 2>../actual.err
) &&
! test -s actual.out &&
-   test_i18ncmp expect.err actual.err
+   test_i18ncmp expect.err actual.err &&
+   (
+   cd submodule &&
+   git checkout -q master
+   )
+'
+
+test_expect_success "'fetch.recurseSubmodules=on-demand' works also without 
.gitmodule entry" '
+   (
+   cd downstream &&
+   git fetch --recurse-submodules
+   ) &&
+   add_upstream_commit &&
+   head1=$(git rev-parse --short HEAD) &&
+   git add submodule &&
+   git rm .gitmodules &&
+   git commit -m "new submodule without .gitmodules" &&
+   printf "" >expect.out &&
+   head2=$(git rev-parse --short HEAD) &&
+   echo "From $pwd/." >expect.err.2 &&
+   echo "   $head1..$head2  master -> origin/master" >>expect.err.2 &&
+   head -3 expect.err >>expect.err.2 &&
+   (
+   cd downstream &&
+   rm .gitmodules &&
+   git config fetch.recurseSubmodules on-demand &&
+   # fake submodule configuration to avoid skipping submodule 
handling
+   git config -f .gitmodules submodule.fake.path fake &&
+   git config -f .gitmodules submodule.fake.url fakeurl &&
+   git add .gitmodules &&
+   git config --unset submodule.submodule.url &&
+   git fetch >../actual.out 2>../actual.err &&
+   # cleanup
+   git config --unset fetch.recurseSubmodules &&
+   git reset --hard
+   ) &&
+   test_i18ncmp expect.out actual.out &&
+   test_i18ncmp expect.err.2 actual.err &&
+   git checkout HEAD^ -- .gitmodules &&
+   git add .gitmodules &&
+   git commit -m "new submodule restored .gitmodules"
 '
 
 test_expect_success 'fetching submodules respects parallel settings' '
-- 
2.14.1.145.gb3622a4