Re: [PATCH wayland v3 1/3] wayland-egl: fail the symbol check if lib is missing

[Daniel Stone]

Hi,

On 19 March 2018 at 11:43, Emil Velikov  wrote:
> On 19 March 2018 at 09:56, Pekka Paalanen  wrote:
>> On Fri, 16 Mar 2018 16:18:57 +
>> Emil Velikov  wrote:
>>> Right - I was aiming to remove the bonkers envvar and forgot about
>>> this preexisting bug.
>>> Patch (to be applied before the series) coming in a second.
>>
>> In egl/wayland-egl-symbols-check.log I have:
>>
>> ./egl/wayland-egl-symbols-check: line 11: NM: unbound variable
>>
> Seems like a 'fun' side-effect of the $NM patch. The series predates
> the patch by a week.
> First suggestion that comes to mind - use $(NM-nm}
>
> Eric any suggestions?

The root cause is that NM is only ever set as a make variable (by
AC_PROG_NM); it's never actually added to the shell-based test
environment.

Thanks for catching this, Pekka - I just noted that the tests
succeeded, and didn't think to check the failure conditions. :\ I've
sent out two patches now, one fixing NM and one fixing the failure to
fail.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland v3 1/3] wayland-egl: fail the symbol check if lib is missing

[Eric Engestrom]

On Monday, 2018-03-19 12:11:50 +, Eric Engestrom wrote:
> On Monday, 2018-03-19 11:43:08 +, Emil Velikov wrote:
> > On 19 March 2018 at 09:56, Pekka Paalanen  wrote:
> > > On Fri, 16 Mar 2018 16:18:57 +
> > > Emil Velikov  wrote:
> > >
> > >> On 16 March 2018 at 13:52, Pekka Paalanen  wrote:
> > >> > On Thu, 15 Mar 2018 14:30:27 +
> > >> > Emil Velikov  wrote:
> > >> >
> > >> >> From: Emil Velikov 
> > >> >>
> > >> >> Based on a similar patch (in Mesa) by Eric Engestrom.
> > >> >>
> > >> >> v2: Rebase on top of $NM patch
> > >> >> v3: Rebase
> > >> >>
> > >> >> Reviewed-by: Eric Engestrom  (v1)
> > >> >> Signed-off-by: Emil Velikov 
> > >> >> ---
> > >> >>  egl/wayland-egl-symbols-check | 10 +-
> > >> >>  1 file changed, 9 insertions(+), 1 deletion(-)
> > >> >>
> > >> >> diff --git a/egl/wayland-egl-symbols-check 
> > >> >> b/egl/wayland-egl-symbols-check
> > >> >> index 6ad28f3..8b3d711 100755
> > >> >> --- a/egl/wayland-egl-symbols-check
> > >> >> +++ b/egl/wayland-egl-symbols-check
> > >> >> @@ -1,6 +1,14 @@
> > >> >>  #!/bin/sh
> > >> >> +set -eu
> > >> >>
> > >> >> -FUNCS=$($NM -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | 
> > >> >> cut -c 3- | while read func; do
> > >> >> +LIB=${WAYLAND_EGL_LIB}
> > >> >> +
> > >> >> +if [ ! -f "$LIB" ]; then
> > >> >> + echo "The test binary \"$LIB\" does no exist"
> > >> >> + exit 1
> > >> >> +fi
> > >> >> +
> > >> >> +FUNCS=$($NM -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | 
> > >> >> while read func; do
> > >> >>  ( grep -q "^$func$" || echo $func )  < > >> >>  wl_egl_window_resize
> > >> >>  wl_egl_window_create
> > >> >
> > >> > Hi Emil,
> > >> >
> > >> > this patch makes distcheck fail with:
> > >> >
> > >> > The test binary "./egl/.libs/libwayland-egl.so" does no exist
> > >> > FAIL egl/wayland-egl-symbols-check (exit status: 1)
> > >> >
> > >> Right - I was aiming to remove the bonkers envvar and forgot about
> > >> this preexisting bug.
> > >> Patch (to be applied before the series) coming in a second.
> > >
> > > Hi Emil,
> > >
> > > because this set of four patches no longer regresses, and I think they
> > > very much need to be in the release, I have pushed them:
> > >f34af17..5f5945b  master -> master
> > >
> > > However, I did a quick test: I renamed wl_egl_window_resize() into
> > > wl_egl_window_resize_uu() to try and trigger several kinds of errors:
> > > symbol not present, extra symbol. The test suite still passes with
> > > success.
> > >
> > > In egl/wayland-egl-symbols-check.log I have:
> > >
> > > ./egl/wayland-egl-symbols-check: line 11: NM: unbound variable
> > >
> > Seems like a 'fun' side-effect of the $NM patch. The series predates
> > the patch by a week.
> > First suggestion that comes to mind - use $(NM-nm}
> 
> yeah that would work, but I'm confused: what's the setup, why is NM
> undefined? Both autotools and meson should be defining NM.

Oh hold on, just as I sent the email I noticed this isn't Mesa, this is
Wayland...

I haven't followed wayland dev for a few months (maybe a year if I'm
honest), I've heard there was a move to meson but I think it hasn't
landed in master?

I guess my question remains though: what's the setup, ie how is the
check script being called? autotools? meson? other?

> 
> > 
> > Eric any suggestions?
> > 
> > Thanks
> > Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland v3 1/3] wayland-egl: fail the symbol check if lib is missing

[Eric Engestrom]

On Monday, 2018-03-19 11:43:08 +, Emil Velikov wrote:
> On 19 March 2018 at 09:56, Pekka Paalanen  wrote:
> > On Fri, 16 Mar 2018 16:18:57 +
> > Emil Velikov  wrote:
> >
> >> On 16 March 2018 at 13:52, Pekka Paalanen  wrote:
> >> > On Thu, 15 Mar 2018 14:30:27 +
> >> > Emil Velikov  wrote:
> >> >
> >> >> From: Emil Velikov 
> >> >>
> >> >> Based on a similar patch (in Mesa) by Eric Engestrom.
> >> >>
> >> >> v2: Rebase on top of $NM patch
> >> >> v3: Rebase
> >> >>
> >> >> Reviewed-by: Eric Engestrom  (v1)
> >> >> Signed-off-by: Emil Velikov 
> >> >> ---
> >> >>  egl/wayland-egl-symbols-check | 10 +-
> >> >>  1 file changed, 9 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/egl/wayland-egl-symbols-check 
> >> >> b/egl/wayland-egl-symbols-check
> >> >> index 6ad28f3..8b3d711 100755
> >> >> --- a/egl/wayland-egl-symbols-check
> >> >> +++ b/egl/wayland-egl-symbols-check
> >> >> @@ -1,6 +1,14 @@
> >> >>  #!/bin/sh
> >> >> +set -eu
> >> >>
> >> >> -FUNCS=$($NM -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | 
> >> >> cut -c 3- | while read func; do
> >> >> +LIB=${WAYLAND_EGL_LIB}
> >> >> +
> >> >> +if [ ! -f "$LIB" ]; then
> >> >> + echo "The test binary \"$LIB\" does no exist"
> >> >> + exit 1
> >> >> +fi
> >> >> +
> >> >> +FUNCS=$($NM -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | 
> >> >> while read func; do
> >> >>  ( grep -q "^$func$" || echo $func )  < >> >>  wl_egl_window_resize
> >> >>  wl_egl_window_create
> >> >
> >> > Hi Emil,
> >> >
> >> > this patch makes distcheck fail with:
> >> >
> >> > The test binary "./egl/.libs/libwayland-egl.so" does no exist
> >> > FAIL egl/wayland-egl-symbols-check (exit status: 1)
> >> >
> >> Right - I was aiming to remove the bonkers envvar and forgot about
> >> this preexisting bug.
> >> Patch (to be applied before the series) coming in a second.
> >
> > Hi Emil,
> >
> > because this set of four patches no longer regresses, and I think they
> > very much need to be in the release, I have pushed them:
> >f34af17..5f5945b  master -> master
> >
> > However, I did a quick test: I renamed wl_egl_window_resize() into
> > wl_egl_window_resize_uu() to try and trigger several kinds of errors:
> > symbol not present, extra symbol. The test suite still passes with
> > success.
> >
> > In egl/wayland-egl-symbols-check.log I have:
> >
> > ./egl/wayland-egl-symbols-check: line 11: NM: unbound variable
> >
> Seems like a 'fun' side-effect of the $NM patch. The series predates
> the patch by a week.
> First suggestion that comes to mind - use $(NM-nm}

yeah that would work, but I'm confused: what's the setup, why is NM
undefined? Both autotools and meson should be defining NM.

> 
> Eric any suggestions?
> 
> Thanks
> Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland v3 1/3] wayland-egl: fail the symbol check if lib is missing

[Emil Velikov]

On 19 March 2018 at 09:56, Pekka Paalanen  wrote:
> On Fri, 16 Mar 2018 16:18:57 +
> Emil Velikov  wrote:
>
>> On 16 March 2018 at 13:52, Pekka Paalanen  wrote:
>> > On Thu, 15 Mar 2018 14:30:27 +
>> > Emil Velikov  wrote:
>> >
>> >> From: Emil Velikov 
>> >>
>> >> Based on a similar patch (in Mesa) by Eric Engestrom.
>> >>
>> >> v2: Rebase on top of $NM patch
>> >> v3: Rebase
>> >>
>> >> Reviewed-by: Eric Engestrom  (v1)
>> >> Signed-off-by: Emil Velikov 
>> >> ---
>> >>  egl/wayland-egl-symbols-check | 10 +-
>> >>  1 file changed, 9 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
>> >> index 6ad28f3..8b3d711 100755
>> >> --- a/egl/wayland-egl-symbols-check
>> >> +++ b/egl/wayland-egl-symbols-check
>> >> @@ -1,6 +1,14 @@
>> >>  #!/bin/sh
>> >> +set -eu
>> >>
>> >> -FUNCS=$($NM -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | cut 
>> >> -c 3- | while read func; do
>> >> +LIB=${WAYLAND_EGL_LIB}
>> >> +
>> >> +if [ ! -f "$LIB" ]; then
>> >> + echo "The test binary \"$LIB\" does no exist"
>> >> + exit 1
>> >> +fi
>> >> +
>> >> +FUNCS=$($NM -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | while 
>> >> read func; do
>> >>  ( grep -q "^$func$" || echo $func )  <> >>  wl_egl_window_resize
>> >>  wl_egl_window_create
>> >
>> > Hi Emil,
>> >
>> > this patch makes distcheck fail with:
>> >
>> > The test binary "./egl/.libs/libwayland-egl.so" does no exist
>> > FAIL egl/wayland-egl-symbols-check (exit status: 1)
>> >
>> Right - I was aiming to remove the bonkers envvar and forgot about
>> this preexisting bug.
>> Patch (to be applied before the series) coming in a second.
>
> Hi Emil,
>
> because this set of four patches no longer regresses, and I think they
> very much need to be in the release, I have pushed them:
>f34af17..5f5945b  master -> master
>
> However, I did a quick test: I renamed wl_egl_window_resize() into
> wl_egl_window_resize_uu() to try and trigger several kinds of errors:
> symbol not present, extra symbol. The test suite still passes with
> success.
>
> In egl/wayland-egl-symbols-check.log I have:
>
> ./egl/wayland-egl-symbols-check: line 11: NM: unbound variable
>
Seems like a 'fun' side-effect of the $NM patch. The series predates
the patch by a week.
First suggestion that comes to mind - use $(NM-nm}

Eric any suggestions?

Thanks
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland v3 1/3] wayland-egl: fail the symbol check if lib is missing

[Pekka Paalanen]

On Fri, 16 Mar 2018 16:18:57 +
Emil Velikov  wrote:

> On 16 March 2018 at 13:52, Pekka Paalanen  wrote:
> > On Thu, 15 Mar 2018 14:30:27 +
> > Emil Velikov  wrote:
> >  
> >> From: Emil Velikov 
> >>
> >> Based on a similar patch (in Mesa) by Eric Engestrom.
> >>
> >> v2: Rebase on top of $NM patch
> >> v3: Rebase
> >>
> >> Reviewed-by: Eric Engestrom  (v1)
> >> Signed-off-by: Emil Velikov 
> >> ---
> >>  egl/wayland-egl-symbols-check | 10 +-
> >>  1 file changed, 9 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
> >> index 6ad28f3..8b3d711 100755
> >> --- a/egl/wayland-egl-symbols-check
> >> +++ b/egl/wayland-egl-symbols-check
> >> @@ -1,6 +1,14 @@
> >>  #!/bin/sh
> >> +set -eu
> >>
> >> -FUNCS=$($NM -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | cut 
> >> -c 3- | while read func; do
> >> +LIB=${WAYLAND_EGL_LIB}
> >> +
> >> +if [ ! -f "$LIB" ]; then
> >> + echo "The test binary \"$LIB\" does no exist"
> >> + exit 1
> >> +fi
> >> +
> >> +FUNCS=$($NM -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | while 
> >> read func; do
> >>  ( grep -q "^$func$" || echo $func )  < >>  wl_egl_window_resize
> >>  wl_egl_window_create  
> >
> > Hi Emil,
> >
> > this patch makes distcheck fail with:
> >
> > The test binary "./egl/.libs/libwayland-egl.so" does no exist
> > FAIL egl/wayland-egl-symbols-check (exit status: 1)
> >  
> Right - I was aiming to remove the bonkers envvar and forgot about
> this preexisting bug.
> Patch (to be applied before the series) coming in a second.

Hi Emil,

because this set of four patches no longer regresses, and I think they
very much need to be in the release, I have pushed them:
   f34af17..5f5945b  master -> master

However, I did a quick test: I renamed wl_egl_window_resize() into
wl_egl_window_resize_uu() to try and trigger several kinds of errors:
symbol not present, extra symbol. The test suite still passes with
success.

In egl/wayland-egl-symbols-check.log I have:

./egl/wayland-egl-symbols-check: line 11: NM: unbound variable

ABI break detected - Required symbol(s) no longer exported!
wl_egl_window_resize
wl_egl_window_create
wl_egl_window_destroy
wl_egl_window_get_attached_size
PASS egl/wayland-egl-symbols-check (exit status: 0)

So it seems 'set -u' is not quite effective, and when ABI breaks are
detected, they don't make the test failed.


Thanks,
pq


pgpa5QQ5FYWV5.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland v3 1/3] wayland-egl: fail the symbol check if lib is missing

[Emil Velikov]

On 16 March 2018 at 13:52, Pekka Paalanen  wrote:
> On Thu, 15 Mar 2018 14:30:27 +
> Emil Velikov  wrote:
>
>> From: Emil Velikov 
>>
>> Based on a similar patch (in Mesa) by Eric Engestrom.
>>
>> v2: Rebase on top of $NM patch
>> v3: Rebase
>>
>> Reviewed-by: Eric Engestrom  (v1)
>> Signed-off-by: Emil Velikov 
>> ---
>>  egl/wayland-egl-symbols-check | 10 +-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
>> index 6ad28f3..8b3d711 100755
>> --- a/egl/wayland-egl-symbols-check
>> +++ b/egl/wayland-egl-symbols-check
>> @@ -1,6 +1,14 @@
>>  #!/bin/sh
>> +set -eu
>>
>> -FUNCS=$($NM -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | cut -c 
>> 3- | while read func; do
>> +LIB=${WAYLAND_EGL_LIB}
>> +
>> +if [ ! -f "$LIB" ]; then
>> + echo "The test binary \"$LIB\" does no exist"
>> + exit 1
>> +fi
>> +
>> +FUNCS=$($NM -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | while 
>> read func; do
>>  ( grep -q "^$func$" || echo $func )  <>  wl_egl_window_resize
>>  wl_egl_window_create
>
> Hi Emil,
>
> this patch makes distcheck fail with:
>
> The test binary "./egl/.libs/libwayland-egl.so" does no exist
> FAIL egl/wayland-egl-symbols-check (exit status: 1)
>
Right - I was aiming to remove the bonkers envvar and forgot about
this preexisting bug.
Patch (to be applied before the series) coming in a second.

-Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland v3 1/3] wayland-egl: fail the symbol check if lib is missing

[Pekka Paalanen]

On Thu, 15 Mar 2018 14:30:27 +
Emil Velikov  wrote:

> From: Emil Velikov 
> 
> Based on a similar patch (in Mesa) by Eric Engestrom.
> 
> v2: Rebase on top of $NM patch
> v3: Rebase
> 
> Reviewed-by: Eric Engestrom  (v1)
> Signed-off-by: Emil Velikov 
> ---
>  egl/wayland-egl-symbols-check | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
> index 6ad28f3..8b3d711 100755
> --- a/egl/wayland-egl-symbols-check
> +++ b/egl/wayland-egl-symbols-check
> @@ -1,6 +1,14 @@
>  #!/bin/sh
> +set -eu
>  
> -FUNCS=$($NM -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | cut -c 
> 3- | while read func; do
> +LIB=${WAYLAND_EGL_LIB}
> +
> +if [ ! -f "$LIB" ]; then
> + echo "The test binary \"$LIB\" does no exist"
> + exit 1
> +fi
> +
> +FUNCS=$($NM -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | while read 
> func; do
>  ( grep -q "^$func$" || echo $func )  <  wl_egl_window_resize
>  wl_egl_window_create

Hi Emil,

this patch makes distcheck fail with:

The test binary "./egl/.libs/libwayland-egl.so" does no exist
FAIL egl/wayland-egl-symbols-check (exit status: 1)


Thanks,
pq


pgpiG3oWA8FsG.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland v3 1/3] wayland-egl: fail the symbol check if lib is missing

[Daniel Stone]

Hi Emil,

On 15 March 2018 at 14:30, Emil Velikov  wrote:
> Based on a similar patch (in Mesa) by Eric Engestrom.
>
> v2: Rebase on top of $NM patch
> v3: Rebase
>
> Reviewed-by: Eric Engestrom  (v1)
> Signed-off-by: Emil Velikov 

The series is:
Reviewed-by: Daniel Stone 

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland v3 1/3] wayland-egl: fail the symbol check if lib is missing

[Emil Velikov]

From: Emil Velikov 

Based on a similar patch (in Mesa) by Eric Engestrom.

v2: Rebase on top of $NM patch
v3: Rebase

Reviewed-by: Eric Engestrom  (v1)
Signed-off-by: Emil Velikov 
---
 egl/wayland-egl-symbols-check | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
index 6ad28f3..8b3d711 100755
--- a/egl/wayland-egl-symbols-check
+++ b/egl/wayland-egl-symbols-check
@@ -1,6 +1,14 @@
 #!/bin/sh
+set -eu
 
-FUNCS=$($NM -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | cut -c 3- 
| while read func; do
+LIB=${WAYLAND_EGL_LIB}
+
+if [ ! -f "$LIB" ]; then
+   echo "The test binary \"$LIB\" does no exist"
+   exit 1
+fi
+
+FUNCS=$($NM -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | while read 
func; do
 ( grep -q "^$func$" || echo $func )