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

[Quentin Glidic]

On 3/19/18 1:31 PM, Daniel Stone wrote:
> The previous rewrite of the wayland-egl ABI checker introduced checks
> for removed symbols as well as added symbols, but broke some failure
> conditions. Add an explict return-code variable set in failure paths,
> rather than chaining or conditions.
> 
> If we cannot find the binary or nm, we regard this as an error
> condition, rather than test failure.
> 
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> Reported-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> Fixes: 21b1f22eb056 ("wayland-egl: enhance the symbol test")
> Cc: Emil Velikov <emil.velikov at collabora.com>
> ---
>   egl/wayland-egl-symbols-check | 27 +++++++++++++++++++++------
>   1 file changed, 21 insertions(+), 6 deletions(-)
> 
> 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.


> +
> +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).


>   AVAIL_FUNCS="$($NM -D --format=bsd --defined-only $LIB | awk '{print $3}')"

Maybe just checking for -n "$NM" then -n "$AVAIL_FUNCS" would be enough?


Cheers,


> @@ -32,7 +38,11 @@ NEW_ABI=$(echo "$AVAIL_FUNCS" | while read func; do
>       echo $func
>   done)
>   
> -test ! -n "$NEW_ABI" || echo "New ABI detected - If intentional, update the test."; echo "$NEW_ABI"
> +if test -n "$NEW_ABI"; then
> +	echo "New ABI detected - If intentional, update the test."
> +	echo "$NEW_ABI"
> +	RET=1
> +fi
>   
>   REMOVED_ABI=$(echo "$REQ_FUNCS" | while read func; do
>       echo "$AVAIL_FUNCS" | grep -q "^$func$" && continue
> @@ -40,5 +50,10 @@ REMOVED_ABI=$(echo "$REQ_FUNCS" | while read func; do
>       echo $func
>   done)
>   
> -test ! -n "$REMOVED_ABI" || echo "ABI break detected - Required symbol(s) no longer exported!"; echo "$REMOVED_ABI"
> -test ! -n "$NEW_ABI" || test ! -n "$REMOVED_ABI"
> +if test -n "$REMOVED_ABI"; then
> +	echo "ABI break detected - Required symbol(s) no longer exported!"
> +	echo "$REMOVED_ABI"
> +	RET=1
> +fi
> +
> +exit $RET
> 


-- 

Quentin “Sardem FF7” Glidic