Re: [PATCH] iotests: Work around failing readlink -f

[Max Reitz]

On 14.09.20 14:31, Peter Maydell wrote:
> On Mon, 14 Sep 2020 at 12:39, Max Reitz <mreitz@redhat.com> wrote:
>>
>> On macOS, (out of the box) readlink does not have -f.  If the recent
>> "readlink -f" call introduced by b1cbc33a397 fails, just fall back to
>> the old behavior (which means you can run the iotests only from the
>> build tree, but that worked fine for six years, so it should be fine
>> still).
>>
>> Keep any potential error message on stderr.  If users want to run the
>> iotests from outside the build tree, this may point them to what's wrong
>> (with their system).
>>
>> Fixes: b1cbc33a3971b6bb005d5ac3569feae35a71de0f
>>        ("iotests: Allow running from different directory")
>> Reported-by: Claudio Fontana <cfontana@suse.de>
>> Reported-by: Thomas Huth <thuth@redhat.com>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> Hi Thomas,
>>
>> I thought this would be quicker than writing a witty response on whether
>> you or me should write this patch. O:)
>> ---
>>  tests/qemu-iotests/check | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
>> index e14a1f354d..75675e1a18 100755
>> --- a/tests/qemu-iotests/check
>> +++ b/tests/qemu-iotests/check
>> @@ -45,6 +45,10 @@ then
>>      fi
>>      source_iotests=$(cd "$source_iotests"; pwd) || _init_error "failed to 
>> enter source tree"
>>      build_iotests=$(readlink -f $(dirname "$0"))
>> +    if [ "$?" -ne 0 ]; then
>> +        # Perhaps -f is unsupported, revert to pre-b1cbc33a397 behavior
>> +        build_iotests=$PWD
>> +    fi
>>  else
> 
> This still prints
>   readlink: illegal option -- f
>   usage: readlink [-n] [file ...]
> 
> (you can see it in the build log that Thomas links to).
> 
>    build_iotests=$(readlink -f $(dirname "$0") 2>/dev/null)
> 
> should avoid that, I think.

I mentioned in the commit message that I find this useful and desirable
behavior.  Something isn’t working that perhaps users are expecting to
work (because it will work on other systems), so I don’t think the error
message should be suppressed.

Max

signature.asc
Description: OpenPGP digital signature