{"thread":{"id":"42252","subject":"[PATCH] t3513: do not compress backup tar file","startedAt":"2016-05-06T18:37:05Z","lastAt":"2016-05-06T23:00:35Z","messageCount":3,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"285768","messageId":"20160506183705.8214-1-sbeller@google.com","threadId":"42252","inReplyTo":null,"subject":"[PATCH] t3513: do not compress backup tar file","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-06T18:37:05Z","receivedAt":"2016-05-06T18:37:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Armin Kunaschik wrote:\n> I'm trying to compile/test/use git 2.8.2 on AIX 6.1 with\n> no bash available.\n...\n> make test does not make it through t3513-revert-submodule.sh anymore.\n> The test is not portable since it uses the z-flags of GNU-tar. When -z\n> is removed, (and extension is changed back to tar) everything runs and\n> tests smoothly.\n\nSigned-off-by: Stefan Beller \n---\n t/t3513-revert-submodule.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3513-revert-submodule.sh b/t/t3513-revert-submodule.sh\nindex a1c4e02..db93781 100755\n--- a/t/t3513-revert-submodule.sh\n+++ b/t/t3513-revert-submodule.sh\n@@ -14,11 +14,11 @@ test_description='revert can handle submodules'\n git_revert () {\n \tgit status -su >expect &&\n \tls -1pR * >>expect &&\n-\ttar czf \"$TRASH_DIRECTORY/tmp.tgz\" * &&\n+\ttar cf \"$TRASH_DIRECTORY/tmp.tar\" * &&\n \tgit checkout \"$1\" &&\n \tgit revert HEAD &&\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.2.335.g4bb51ae.dirty\n"},{"id":"285788","messageId":"xmqqinyq24cj.fsf@gitster.mtv.corp.google.com","threadId":"42252","inReplyTo":"20160506183705.8214-1-sbeller@google.com","subject":"Re: [PATCH] t3513: do not compress backup tar file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-06T22:45:16Z","receivedAt":"2016-05-06T22:45:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> Armin Kunaschik wrote:\n>> I'm trying to compile/test/use git 2.8.2 on AIX 6.1 with\n>> no bash available.\n> ...\n>> make test does not make it through t3513-revert-submodule.sh anymore.\n>> The test is not portable since it uses the z-flags of GNU-tar. When -z\n>> is removed, (and extension is changed back to tar) everything runs and\n>> tests smoothly.\n>\n> Signed-off-by: Stefan Beller \n> ---\n\nThanks for a quick fix. Even though \"no bash\" and \"AIX 6.1\" are\ninteresting details that are part of a good bug report, these are\nirrelevant noise for a commit that fixes a bug that is unrelated to\nthem, so let's rephrase the message and queue it, like this:\n\n t3513: do not compress backup tar file\n\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 \n Signed-off-by: Stefan Beller \n Signed-off-by: Junio C Hamano \n\n> t/t3513-revert-submodule.sh | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t3513-revert-submodule.sh b/t/t3513-revert-submodule.sh\n> index a1c4e02..db93781 100755\n> --- a/t/t3513-revert-submodule.sh\n> +++ b/t/t3513-revert-submodule.sh\n> @@ -14,11 +14,11 @@ test_description='revert can handle submodules'\n> git_revert () {\n> \tgit status -su >expect &&\n> \tls -1pR * >>expect &&\n> -\ttar czf \"$TRASH_DIRECTORY/tmp.tgz\" * &&\n> +\ttar cf \"$TRASH_DIRECTORY/tmp.tar\" * &&\n> \tgit checkout \"$1\" &&\n> \tgit revert HEAD &&\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\nThis is not a new problem, but these \"ls -1pR\" and \"rm -rf *\" makes\nme wonder if it is the best way to test what is being tested.\n\nThe title says \"revert can handle submodules\", but when it sees that\nrevert finishes successfully, it discards the resulting working tree\nstate with \"rm -rf *\" (Yuck) and repopulates with the state before\nthe 'checkout && revert' sequence, so the 'status' and 'ls' are not\ntesting what 'revert' did at all.\n\nShouldn't it be testing HEAD^{tree} before \"checkout && revert\" and\nafter and make sure they match, and checking the working state left\nby 'revert' without clobbering it with tarball extract?\n"},{"id":"285790","messageId":"CAGZ79kbTWsvJy8MA9igpNDsAZhwOzpQBF-rp=vhGYm5hpNz-iQ@mail.gmail.com","threadId":"42252","inReplyTo":"xmqqinyq24cj.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] t3513: do not compress backup tar file","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-06T23:00:35Z","receivedAt":"2016-05-06T23:00:35Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, May 6, 2016 at 3:45 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> Armin Kunaschik wrote:\n>>> I'm trying to compile/test/use git 2.8.2 on AIX 6.1 with\n>>> no bash available.\n>> ...\n>>> make test does not make it through t3513-revert-submodule.sh anymore.\n>>> The test is not portable since it uses the z-flags of GNU-tar. When -z\n>>> is removed, (and extension is changed back to tar) everything runs and\n>>> tests smoothly.\n>>\n>> Signed-off-by: Stefan Beller \n>> ---\n>\n> Thanks for a quick fix. Even though \"no bash\" and \"AIX 6.1\" are\n> interesting details that are part of a good bug report, these are\n> irrelevant noise for a commit that fixes a bug that is unrelated to\n> them, so let's rephrase the message and queue it, like this:\n>\n> t3513: do not compress backup tar file\n>\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 \n> Signed-off-by: Stefan Beller \n> Signed-off-by: Junio C Hamano \n\nThanks for rewriting the message. :)\n\n>\n>> t/t3513-revert-submodule.sh | 4 ++--\n>> 1 file changed, 2 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/t/t3513-revert-submodule.sh b/t/t3513-revert-submodule.sh\n>> index a1c4e02..db93781 100755\n>> --- a/t/t3513-revert-submodule.sh\n>> +++ b/t/t3513-revert-submodule.sh\n>> @@ -14,11 +14,11 @@ test_description='revert can handle submodules'\n>> git_revert () {\n>> git status -su >expect &&\n>> ls -1pR * >>expect &&\n>> - tar czf \"$TRASH_DIRECTORY/tmp.tgz\" * &&\n>> + tar cf \"$TRASH_DIRECTORY/tmp.tar\" * &&\n>> git checkout \"$1\" &&\n>> git revert HEAD &&\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> This is not a new problem, but these \"ls -1pR\" and \"rm -rf *\" makes\n> me wonder if it is the best way to test what is being tested.\n>\n> The title says \"revert can handle submodules\", but when it sees that\n> revert finishes successfully, it discards the resulting working tree\n> state with \"rm -rf *\" (Yuck) and repopulates with the state before\n> the 'checkout && revert' sequence, so the 'status' and 'ls' are not\n> testing what 'revert' did at all.\n>\n> Shouldn't it be testing HEAD^{tree} before \"checkout && revert\" and\n> after and make sure they match, and checking the working state left\n> by 'revert' without clobbering it with tarball extract?\n>\n\nReading the comment above that function indicates that the middle revert\nis not the interesting revert. The later revert is the actually tested revert.\n"}]}