[PATCH] t6041: do not compress backup tar file

[PATCH] t6041: do not compress backup tar file

[Stefan Beller]

The test uses the 'z' option, i.e. "compress the output while at
it", which is GNUism and not portable.

Reported-by: Armin Kunaschik <megabreit@googlemail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---

 Thanks Armin for reporting these GNUism!
 Are there any more? (So we can do these patches as a
 series instead of one by one:)
 
 developed on top of origin/sb/z-is-gnutar-ism
 
 Thanks,
 Stefan

 t/t6041-bisect-submodule.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t6041-bisect-submodule.sh b/t/t6041-bisect-submodule.sh
index c6b7aa6..62b8a2e 100755
--- a/t/t6041-bisect-submodule.sh
+++ b/t/t6041-bisect-submodule.sh
@@ -8,7 +8,7 @@ test_description='bisect can handle submodules'
 git_bisect () {
 	git status -su >expect &&
 	ls -1pR * >>expect &&
-	tar czf "$TRASH_DIRECTORY/tmp.tgz" * &&
+	tar cf "$TRASH_DIRECTORY/tmp.tar" * &&
 	GOOD=$(git rev-parse --verify HEAD) &&
 	git checkout "$1" &&
 	echo "foo" >bar &&
@@ -20,7 +20,7 @@ git_bisect () {
 	git bisect start &&
 	git bisect good $GOOD &&
 	rm -rf * &&
-	tar xzf "$TRASH_DIRECTORY/tmp.tgz" &&
+	tar xf "$TRASH_DIRECTORY/tmp.tar" &&
 	git status -su >actual &&
 	ls -1pR * >>actual &&
 	test_cmp expect actual &&
-- 
2.8.0.37.g63b3e6f.dirty

Re: [PATCH] t6041: do not compress backup tar file

[Armin Kunaschik]

Hi Stefan,

I'm currently in the process of skipping through the failed tests on my AIX box.
There are more tests which require GNU tools like mktemp
(t7610-mergetool.sh) or readlink (t7800-difftool.sh).
But I don't have a solution or workaround for these two.
But at least there are not more failing compressed tar tests :-)

Thanks,
Armin

On Mon, May 9, 2016 at 7:09 PM, Stefan Beller <sbeller@google.com> wrote:
> The test uses the 'z' option, i.e. "compress the output while at
> it", which is GNUism and not portable.
>
> Reported-by: Armin Kunaschik <megabreit@googlemail.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
>  Thanks Armin for reporting these GNUism!
>  Are there any more? (So we can do these patches as a
>  series instead of one by one:)
>
>  developed on top of origin/sb/z-is-gnutar-ism
>
>  Thanks,
>  Stefan

Re: [PATCH] t6041: do not compress backup tar file

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> diff --git a/t/t6041-bisect-submodule.sh b/t/t6041-bisect-submodule.sh
> index c6b7aa6..62b8a2e 100755
> --- a/t/t6041-bisect-submodule.sh
> +++ b/t/t6041-bisect-submodule.sh
> @@ -8,7 +8,7 @@ test_description='bisect can handle submodules'
>  git_bisect () {
>  	git status -su >expect &&
>  	ls -1pR * >>expect &&
> -	tar czf "$TRASH_DIRECTORY/tmp.tgz" * &&
> +	tar cf "$TRASH_DIRECTORY/tmp.tar" * &&
>  	GOOD=$(git rev-parse --verify HEAD) &&
>  	git checkout "$1" &&
>  	echo "foo" >bar &&
> @@ -20,7 +20,7 @@ git_bisect () {
>  	git bisect start &&
>  	git bisect good $GOOD &&
>  	rm -rf * &&
> -	tar xzf "$TRASH_DIRECTORY/tmp.tgz" &&
> +	tar xf "$TRASH_DIRECTORY/tmp.tar" &&
>  	git status -su >actual &&
>  	ls -1pR * >>actual &&
>  	test_cmp expect actual &&

While I am fine with taking this (and the other) change, which are
the only sensible response to these bug reports, this makes me
wonder two things and a half.

 * Why do we even run "tar", especially without having a
   test_when_finished clean-up?  Can't there be better ways to test
   this and the other one without creating a copy of everything in
   the test directory?

 * Are we sure the trash directory contents is kept manageable size?
   I am primarily worried about this "we are not sure what we will
   be clobbering, so let's blindly tar up everything and restore it
   later" pattern spreading and people mistakenly use it in tests
   that simulate our behaviour on a huge blob with a sparse but
   gigantic file.

 * Is an addition of 'test_snapshot $tarball *' and 'test_restore
   $tarball' pair too much unnecessary refactoring to cater to only
   two users of this "let's tar up everything" pattern, given that
   we want to _discourage_ use of this pattern in the longer term?

Re: [PATCH] t6041: do not compress backup tar file

[Stefan Beller]

+ cc Jens as he authored both t6041 as well as t3513 in
the series leading to ad25da009e2a3730 (2014-07-21,
Merge branch 'jl/submodule-tests')

On Mon, May 9, 2016 at 11:46 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> diff --git a/t/t6041-bisect-submodule.sh b/t/t6041-bisect-submodule.sh
>> index c6b7aa6..62b8a2e 100755
>> --- a/t/t6041-bisect-submodule.sh
>> +++ b/t/t6041-bisect-submodule.sh
>> @@ -8,7 +8,7 @@ test_description='bisect can handle submodules'
>>  git_bisect () {
>>       git status -su >expect &&
>>       ls -1pR * >>expect &&
>> -     tar czf "$TRASH_DIRECTORY/tmp.tgz" * &&
>> +     tar cf "$TRASH_DIRECTORY/tmp.tar" * &&
>>       GOOD=$(git rev-parse --verify HEAD) &&
>>       git checkout "$1" &&
>>       echo "foo" >bar &&
>> @@ -20,7 +20,7 @@ git_bisect () {
>>       git bisect start &&
>>       git bisect good $GOOD &&
>>       rm -rf * &&
>> -     tar xzf "$TRASH_DIRECTORY/tmp.tgz" &&
>> +     tar xf "$TRASH_DIRECTORY/tmp.tar" &&
>>       git status -su >actual &&
>>       ls -1pR * >>actual &&
>>       test_cmp expect actual &&
>
> While I am fine with taking this (and the other) change, which are
> the only sensible response to these bug reports, this makes me
> wonder two things and a half.
>
>  * Why do we even run "tar", especially without having a
>    test_when_finished clean-up?  Can't there be better ways to test
>    this and the other one without creating a copy of everything in
>    the test directory?

I think some of the submodule testing is a test machinery on its own.
Any submodule test that is not in t74* follows the pattern to
provide a short function for its testing and then call  test_submodule_switch
with some long option, e.g.
KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1

I haven't attempted to look into these tests yet as they seem to test
fundamentals ("submodules as files" in other commands), whereas
I am more interested in some new features currently.

>
>  * Are we sure the trash directory contents is kept manageable size?

The testing in lib-submodule-update.sh do not seem to take care of
these tarballs. And the small testing scripts do not either, but there
we could inject a

    test_when_finished "rm ..."

snippet IIUC.

>    I am primarily worried about this "we are not sure what we will
>    be clobbering, so let's blindly tar up everything and restore it
>    later" pattern spreading and people mistakenly use it in tests
>    that simulate our behaviour on a huge blob with a sparse but
>    gigantic file.
>
>  * Is an addition of 'test_snapshot $tarball *' and 'test_restore
>    $tarball' pair too much unnecessary refactoring to cater to only
>    two users of this "let's tar up everything" pattern, given that
>    we want to _discourage_ use of this pattern in the longer term?

As said before, I did not yet dive into these test areas myself.

>From a birds eye, there was not a lot of discussion around that series
(as compared to submodule groups for example). Maybe I am missing
prior or later series though.

http://thread.gmane.org/gmane.comp.version-control.git/251682


>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html