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

> Stefan Beller <sbeller@google.com> writes:
>
>> Armin Kunaschik <megabreit@googlemail.com> wrote:
>>> I'm trying to compile/test/use git 2.8.2 on AIX 6.1 with
>>> no bash available.
>> ...
>>> make test does not make it through t3513-revert-submodule.sh anymore.
>>> The test is not portable since it uses the z-flags of GNU-tar. When -z
>>> is removed, (and extension is changed back to tar) everything runs and
>>> tests smoothly.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>
> Thanks for a quick fix.  Even though "no bash" and "AIX 6.1" are
> interesting details that are part of a good bug report, these are
> irrelevant noise for a commit that fixes a bug that is unrelated to
> them, so let's rephrase the message and queue it, like this:
>
>     t3513: do not compress backup tar file
>
>     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>
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>

>
>>  t/t3513-revert-submodule.sh | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/t/t3513-revert-submodule.sh b/t/t3513-revert-submodule.sh
>> index a1c4e02..db93781 100755
>> --- a/t/t3513-revert-submodule.sh
>> +++ b/t/t3513-revert-submodule.sh
>> @@ -14,11 +14,11 @@ test_description='revert can handle submodules'
>>  git_revert () {
>>       git status -su >expect &&
>>       ls -1pR * >>expect &&
>> -     tar czf "$TRASH_DIRECTORY/tmp.tgz" * &&
>> +     tar cf "$TRASH_DIRECTORY/tmp.tar" * &&
>>       git checkout "$1" &&
>>       git revert HEAD &&
>>       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 &&
>
> This is not a new problem, but these "ls -1pR" and "rm -rf *" makes
> me wonder if it is the best way to test what is being tested.
>
> The title says "revert can handle submodules", but when it sees that
> revert finishes successfully, it discards the resulting working tree
> state with "rm -rf *" (Yuck) and repopulates with the state before
> the 'checkout && revert' sequence, so the 'status' and 'ls' are not
> testing what 'revert' did at all.
>
> Shouldn't it be testing HEAD^{tree} before "checkout && revert" and
> after and make sure they match, and checking the working state left
> by 'revert' without clobbering it with tarball extract?
>