Hi Quentin, Thanks for the quick review! On 19 March 2018 at 13:06, Quentin Glidic <[email protected]> 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 _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
