{"thread":{"id":"42265","subject":"[PATCH] t6041: do not compress backup tar file","startedAt":"2016-05-09T17:09:08Z","lastAt":"2016-05-09T19:04:38Z","messageCount":4,"participants":["Stefan Beller","Armin Kunaschik","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"285941","messageId":"1462813748-27538-1-git-send-email-sbeller@google.com","threadId":"42265","inReplyTo":null,"subject":"[PATCH] t6041: do not compress backup tar file","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-09T17:09:08Z","receivedAt":"2016-05-09T17:09:08Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The test uses the 'z' option, i.e. \"compress the output while at\nit\", which is GNUism and not portable.\n\nReported-by: Armin Kunaschik <megabreit@googlemail.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\n Thanks Armin for reporting these GNUism!\n Are there any more? (So we can do these patches as a\n series instead of one by one:)\n \n developed on top of origin/sb/z-is-gnutar-ism\n \n Thanks,\n Stefan\n\n t/t6041-bisect-submodule.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t6041-bisect-submodule.sh b/t/t6041-bisect-submodule.sh\nindex c6b7aa6..62b8a2e 100755\n--- a/t/t6041-bisect-submodule.sh\n+++ b/t/t6041-bisect-submodule.sh\n@@ -8,7 +8,7 @@ test_description='bisect can handle submodules'\n git_bisect () {\n \tgit status -su >expect &&\n \tls -1pR * >>expect &&\n-\ttar czf \"$TRASH_DIRECTORY/tmp.tgz\" * &&\n+\ttar cf \"$TRASH_DIRECTORY/tmp.tar\" * &&\n \tGOOD=$(git rev-parse --verify HEAD) &&\n \tgit checkout \"$1\" &&\n \techo \"foo\" >bar &&\n@@ -20,7 +20,7 @@ git_bisect () {\n \tgit bisect start &&\n \tgit bisect good $GOOD &&\n \trm -rf * &&\n-\ttar xzf \"$TRASH_DIRECTORY/tmp.tgz\" &&\n+\ttar xf \"$TRASH_DIRECTORY/tmp.tar\" &&\n \tgit status -su >actual &&\n \tls -1pR * >>actual &&\n \ttest_cmp expect actual &&\n-- \n2.8.0.37.g63b3e6f.dirty\n"},{"id":"285943","messageId":"CALR6jEiFqFunqRQxWEU_meptN9XfEKPR64mTyWFtSw7D6DFzAw@mail.gmail.com","threadId":"42265","inReplyTo":"1462813748-27538-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH] t6041: do not compress backup tar file","fromName":"Armin Kunaschik","fromEmail":"megabreit@googlemail.com","sentAt":"2016-05-09T17:20:13Z","receivedAt":"2016-05-09T17:20:13Z","isPatch":true,"sender":{"key":"megabreit@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/10429218?v=4"},"body":"Hi Stefan,\n\nI'm currently in the process of skipping through the failed tests on my AIX box.\nThere are more tests which require GNU tools like mktemp\n(t7610-mergetool.sh) or readlink (t7800-difftool.sh).\nBut I don't have a solution or workaround for these two.\nBut at least there are not more failing compressed tar tests :-)\n\nThanks,\nArmin\n\nOn Mon, May 9, 2016 at 7:09 PM, Stefan Beller <sbeller@google.com> wrote:\n> The test uses the 'z' option, i.e. \"compress the output while at\n> it\", which is GNUism and not portable.\n>\n> Reported-by: Armin Kunaschik <megabreit@googlemail.com>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>\n>  Thanks Armin for reporting these GNUism!\n>  Are there any more? (So we can do these patches as a\n>  series instead of one by one:)\n>\n>  developed on top of origin/sb/z-is-gnutar-ism\n>\n>  Thanks,\n>  Stefan\n"},{"id":"285960","messageId":"xmqq8tzjul0p.fsf@gitster.mtv.corp.google.com","threadId":"42265","inReplyTo":"1462813748-27538-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH] t6041: do not compress backup tar file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-09T18:46:46Z","receivedAt":"2016-05-09T18:46:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> diff --git a/t/t6041-bisect-submodule.sh b/t/t6041-bisect-submodule.sh\n> index c6b7aa6..62b8a2e 100755\n> --- a/t/t6041-bisect-submodule.sh\n> +++ b/t/t6041-bisect-submodule.sh\n> @@ -8,7 +8,7 @@ test_description='bisect can handle submodules'\n>  git_bisect () {\n>  \tgit status -su >expect &&\n>  \tls -1pR * >>expect &&\n> -\ttar czf \"$TRASH_DIRECTORY/tmp.tgz\" * &&\n> +\ttar cf \"$TRASH_DIRECTORY/tmp.tar\" * &&\n>  \tGOOD=$(git rev-parse --verify HEAD) &&\n>  \tgit checkout \"$1\" &&\n>  \techo \"foo\" >bar &&\n> @@ -20,7 +20,7 @@ git_bisect () {\n>  \tgit bisect start &&\n>  \tgit bisect good $GOOD &&\n>  \trm -rf * &&\n> -\ttar xzf \"$TRASH_DIRECTORY/tmp.tgz\" &&\n> +\ttar xf \"$TRASH_DIRECTORY/tmp.tar\" &&\n>  \tgit status -su >actual &&\n>  \tls -1pR * >>actual &&\n>  \ttest_cmp expect actual &&\n\nWhile I am fine with taking this (and the other) change, which are\nthe only sensible response to these bug reports, this makes me\nwonder two things and a half.\n\n * Why do we even run \"tar\", especially without having a\n   test_when_finished clean-up?  Can't there be better ways to test\n   this and the other one without creating a copy of everything in\n   the test directory?\n\n * Are we sure the trash directory contents is kept manageable size?\n   I am primarily worried about this \"we are not sure what we will\n   be clobbering, so let's blindly tar up everything and restore it\n   later\" pattern spreading and people mistakenly use it in tests\n   that simulate our behaviour on a huge blob with a sparse but\n   gigantic file.\n\n * Is an addition of 'test_snapshot $tarball *' and 'test_restore\n   $tarball' pair too much unnecessary refactoring to cater to only\n   two users of this \"let's tar up everything\" pattern, given that\n   we want to _discourage_ use of this pattern in the longer term?\n"},{"id":"285962","messageId":"CAGZ79kZ0gVzb7aqtdFFGt1mNxgNwX+xaWOtHOQt8kB84Yxhwsg@mail.gmail.com","threadId":"42265","inReplyTo":"xmqq8tzjul0p.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t6041: do not compress backup tar file","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-09T19:04:38Z","receivedAt":"2016-05-09T19:04:38Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"+ cc Jens as he authored both t6041 as well as t3513 in\nthe series leading to ad25da009e2a3730 (2014-07-21,\nMerge branch 'jl/submodule-tests')\n\nOn Mon, May 9, 2016 at 11:46 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> diff --git a/t/t6041-bisect-submodule.sh b/t/t6041-bisect-submodule.sh\n>> index c6b7aa6..62b8a2e 100755\n>> --- a/t/t6041-bisect-submodule.sh\n>> +++ b/t/t6041-bisect-submodule.sh\n>> @@ -8,7 +8,7 @@ test_description='bisect can handle submodules'\n>>  git_bisect () {\n>>       git status -su >expect &&\n>>       ls -1pR * >>expect &&\n>> -     tar czf \"$TRASH_DIRECTORY/tmp.tgz\" * &&\n>> +     tar cf \"$TRASH_DIRECTORY/tmp.tar\" * &&\n>>       GOOD=$(git rev-parse --verify HEAD) &&\n>>       git checkout \"$1\" &&\n>>       echo \"foo\" >bar &&\n>> @@ -20,7 +20,7 @@ git_bisect () {\n>>       git bisect start &&\n>>       git bisect good $GOOD &&\n>>       rm -rf * &&\n>> -     tar xzf \"$TRASH_DIRECTORY/tmp.tgz\" &&\n>> +     tar xf \"$TRASH_DIRECTORY/tmp.tar\" &&\n>>       git status -su >actual &&\n>>       ls -1pR * >>actual &&\n>>       test_cmp expect actual &&\n>\n> While I am fine with taking this (and the other) change, which are\n> the only sensible response to these bug reports, this makes me\n> wonder two things and a half.\n>\n>  * Why do we even run \"tar\", especially without having a\n>    test_when_finished clean-up?  Can't there be better ways to test\n>    this and the other one without creating a copy of everything in\n>    the test directory?\n\nI think some of the submodule testing is a test machinery on its own.\nAny submodule test that is not in t74* follows the pattern to\nprovide a short function for its testing and then call  test_submodule_switch\nwith some long option, e.g.\nKNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1\n\nI haven't attempted to look into these tests yet as they seem to test\nfundamentals (\"submodules as files\" in other commands), whereas\nI am more interested in some new features currently.\n\n>\n>  * Are we sure the trash directory contents is kept manageable size?\n\nThe testing in lib-submodule-update.sh do not seem to take care of\nthese tarballs. And the small testing scripts do not either, but there\nwe could inject a\n\n    test_when_finished \"rm ...\"\n\nsnippet IIUC.\n\n>    I am primarily worried about this \"we are not sure what we will\n>    be clobbering, so let's blindly tar up everything and restore it\n>    later\" pattern spreading and people mistakenly use it in tests\n>    that simulate our behaviour on a huge blob with a sparse but\n>    gigantic file.\n>\n>  * Is an addition of 'test_snapshot $tarball *' and 'test_restore\n>    $tarball' pair too much unnecessary refactoring to cater to only\n>    two users of this \"let's tar up everything\" pattern, given that\n>    we want to _discourage_ use of this pattern in the longer term?\n\nAs said before, I did not yet dive into these test areas myself.\n\n>From a birds eye, there was not a lot of discussion around that series\n(as compared to submodule groups for example). Maybe I am missing\nprior or later series though.\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/251682\n\n\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"}]}