[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