[PATCH wayland 2/2] wayland-egl: Make symbol test fail on failure

Daniel Stone daniel at fooishbar.org
Mon Mar 19 13:20:59 UTC 2018


Hi Quentin,
Thanks for the quick review!

On 19 March 2018 at 13:06, Quentin Glidic
<sardemff7+wayland at sardemff7.net> wrote:
> On 3/19/18 1:31 PM, Daniel Stone wrote:
>> diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
>> index c47026b2..d1a4a6be 100755
>> --- a/egl/wayland-egl-symbols-check
>> +++ b/egl/wayland-egl-symbols-check
>> @@ -1,11 +1,17 @@
>>   #!/bin/sh
>>   set -eu
>>   +RET=0
>>   LIB=${WAYLAND_EGL_LIB}
>>   -if [ ! -f "$LIB" ]; then
>> -       echo "The test binary \"$LIB\" does no exist"
>> -       exit 1
>> +if ! test -f "$LIB"; then
>> +       echo "Test binary \"$LIB\" does not exist"
>> +       exit 99
>> +fi
>
> Not a big fan of replacing [!-f] with !test-f but it’s a cosmetic issue so
> not a big deal.

Me neither really, but it seemed best for consistency with the rest of
the file which used test rather than [.

>> +if ! test -x "$NM"; then
>> +       echo "nm binary \"$NM\" does not exist"
>> +       exit 99
>>   fi
>
> Here, however, you are introducing an -x test, which is not good for all the
> people (including packagers) that set NM to e.g. <prefix>-nm (so not the
> full path).

I'd not realised AC_PROG_NM didn't set a full path. Brilliant. Maybe
replace test -x "$NM" with $NM -V >/dev/null 2>&1? Or just trusting it
works if it's non-empty is fine too.

Cheers,
Daniel


More information about the wayland-devel mailing list