[PATCH v2] mesa-demos: Fix building demos which require GLU.

[PATCH v2] mesa-demos: Fix building demos which require GLU.

[Drew Moseley]

The existing test for GLU support is backwards.  Reverse the
sense of the conditional when built with "--enable-glu".

Signed-off-by: Drew Moseley <drew_moseley@mentor.com>
---
 ...figure-Allow-to-disable-demos-which-require-GLEW-.patch | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/meta/recipes-graphics/mesa/mesa-demos/0003-configure-Allow-to-disable-demos-which-require-GLEW-.patch b/meta/recipes-graphics/mesa/mesa-demos/0003-configure-Allow-to-disable-demos-which-require-GLEW-.patch
index 4b07193..b25f5ce 100644
--- a/meta/recipes-graphics/mesa/mesa-demos/0003-configure-Allow-to-disable-demos-which-require-GLEW-.patch
+++ b/meta/recipes-graphics/mesa/mesa-demos/0003-configure-Allow-to-disable-demos-which-require-GLEW-.patch
@@ -10,20 +10,21 @@ Subject: [PATCH 3/9] configure: Allow to disable demos which require GLEW or
 Upstream-Status: Pending
 
 Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
+Signed-off-by: Drew Moseley <drew_moseley@mentor.com>
 ---
- configure.ac                  | 49 ++++++++++++++++++++---------
- src/Makefile.am               | 14 ++++++---
+ configure.ac                  | 50 ++++++++++++++++++++---------
+ src/Makefile.am               | 18 ++++++++---
  src/demos/Makefile.am         | 73 ++++++++++++++++++++++++-------------------
  src/egl/Makefile.am           |  8 +++--
  src/egl/opengles1/Makefile.am | 44 +++++++++++++++-----------
  src/egl/opengles2/Makefile.am | 33 ++++++++++---------
- 6 files changed, 135 insertions(+), 86 deletions(-)
+ 6 files changed, 140 insertions(+), 86 deletions(-)
 
 diff --git a/configure.ac b/configure.ac
 index 9445424..bc4c8d1 100644
 --- a/configure.ac
 +++ b/configure.ac
-@@ -93,25 +93,44 @@ AC_EGREP_HEADER([glutInitContextProfile],
+@@ -89,25 +89,45 @@ AC_EGREP_HEADER([glutInitContextProfile],
  		[AC_DEFINE(HAVE_FREEGLUT)],
  		[])
  
@@ -66,7 +67,7 @@ index 9445424..bc4c8d1 100644
 -DEMO_CFLAGS="$DEMO_CFLAGS $GLU_CFLAGS"
 -DEMO_LIBS="$DEMO_LIBS $GLU_LIBS"
 +if test "x$enable_glu" = xyes; then
-+    PKG_CHECK_MODULES(GLU, [glu], [],
++    PKG_CHECK_MODULES(GLU, [glu],
 +                     [AC_CHECK_HEADER([GL/glu.h],
 +                                      [],
 +                                      AC_MSG_ERROR([GLU not found]))
@@ -74,7 +75,8 @@ index 9445424..bc4c8d1 100644
 +                                   [gluBeginCurve],
 +                                   [GLU_LIBS=-lGLU
 +				    glu_enabled=yes],
-+                                   AC_MSG_ERROR([GLU required])) ])
++                                   AC_MSG_ERROR([GLU required])) ],
++                      [])
 +
 +    DEMO_CFLAGS="$DEMO_CFLAGS $GLU_CFLAGS"
 +    DEMO_LIBS="$DEMO_LIBS $GLU_LIBS"
-- 
1.9.1

Re: [PATCH v2] mesa-demos: Fix building demos which require GLU.

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 3522 bytes --]

On Thu, Jul 16, 2015 at 11:12:04AM -0400, Drew Moseley wrote:
> The existing test for GLU support is backwards.  Reverse the
> sense of the conditional when built with "--enable-glu".
> 
> Signed-off-by: Drew Moseley <drew_moseley@mentor.com>
> ---
>  ...figure-Allow-to-disable-demos-which-require-GLEW-.patch | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/meta/recipes-graphics/mesa/mesa-demos/0003-configure-Allow-to-disable-demos-which-require-GLEW-.patch b/meta/recipes-graphics/mesa/mesa-demos/0003-configure-Allow-to-disable-demos-which-require-GLEW-.patch
> index 4b07193..b25f5ce 100644
> --- a/meta/recipes-graphics/mesa/mesa-demos/0003-configure-Allow-to-disable-demos-which-require-GLEW-.patch
> +++ b/meta/recipes-graphics/mesa/mesa-demos/0003-configure-Allow-to-disable-demos-which-require-GLEW-.patch
> @@ -10,20 +10,21 @@ Subject: [PATCH 3/9] configure: Allow to disable demos which require GLEW or
>  Upstream-Status: Pending
>  
>  Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> +Signed-off-by: Drew Moseley <drew_moseley@mentor.com>
>  ---
> - configure.ac                  | 49 ++++++++++++++++++++---------
> - src/Makefile.am               | 14 ++++++---
> + configure.ac                  | 50 ++++++++++++++++++++---------
> + src/Makefile.am               | 18 ++++++++---
>   src/demos/Makefile.am         | 73 ++++++++++++++++++++++++-------------------
>   src/egl/Makefile.am           |  8 +++--
>   src/egl/opengles1/Makefile.am | 44 +++++++++++++++-----------
>   src/egl/opengles2/Makefile.am | 33 ++++++++++---------
> - 6 files changed, 135 insertions(+), 86 deletions(-)
> + 6 files changed, 140 insertions(+), 86 deletions(-)
>  
>  diff --git a/configure.ac b/configure.ac
>  index 9445424..bc4c8d1 100644
>  --- a/configure.ac
>  +++ b/configure.ac
> -@@ -93,25 +93,44 @@ AC_EGREP_HEADER([glutInitContextProfile],
> +@@ -89,25 +89,45 @@ AC_EGREP_HEADER([glutInitContextProfile],
>   		[AC_DEFINE(HAVE_FREEGLUT)],
>   		[])
>   
> @@ -66,7 +67,7 @@ index 9445424..bc4c8d1 100644
>  -DEMO_CFLAGS="$DEMO_CFLAGS $GLU_CFLAGS"
>  -DEMO_LIBS="$DEMO_LIBS $GLU_LIBS"
>  +if test "x$enable_glu" = xyes; then
> -+    PKG_CHECK_MODULES(GLU, [glu], [],
> ++    PKG_CHECK_MODULES(GLU, [glu],

Are you sure? It was like this in original PKG_CHECK_MODULES and I think
it was correct. 4th parameter is used as fallback to find it manually
when pkg-config doesn't find it, with your change it would use
AC_CHECK_HEADER and ignore the values set returned by pkg-config.

>  +                     [AC_CHECK_HEADER([GL/glu.h],
>  +                                      [],
>  +                                      AC_MSG_ERROR([GLU not found]))
> @@ -74,7 +75,8 @@ index 9445424..bc4c8d1 100644
>  +                                   [gluBeginCurve],
>  +                                   [GLU_LIBS=-lGLU
>  +				    glu_enabled=yes],
> -+                                   AC_MSG_ERROR([GLU required])) ])
> ++                                   AC_MSG_ERROR([GLU required])) ],
> ++                      [])
>  +
>  +    DEMO_CFLAGS="$DEMO_CFLAGS $GLU_CFLAGS"
>  +    DEMO_LIBS="$DEMO_LIBS $GLU_LIBS"
> -- 
> 1.9.1
> 
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH v2] mesa-demos: Fix building demos which require GLU.

[Moseley, Drew]

[-- Attachment #1.1: Type: text/plain, Size: 3613 bytes --]


> On Jul 16, 2015, at 11:45 AM, Martin Jansa <martin.jansa@gmail.com> wrote:
> 
> On Thu, Jul 16, 2015 at 11:12:04AM -0400, Drew Moseley wrote:
>> The existing test for GLU support is backwards.  Reverse the
>> sense of the conditional when built with "--enable-glu".
>> 
>> Signed-off-by: Drew Moseley <drew_moseley@mentor.com <mailto:drew_moseley@mentor.com>>
>> ---
>> ...figure-Allow-to-disable-demos-which-require-GLEW-.patch | 14 ++++++++------
>> 1 file changed, 8 insertions(+), 6 deletions(-)
>> 
>> diff --git a/meta/recipes-graphics/mesa/mesa-demos/0003-configure-Allow-to-disable-demos-which-require-GLEW-.patch b/meta/recipes-graphics/mesa/mesa-demos/0003-configure-Allow-to-disable-demos-which-require-GLEW-.patch
>> index 4b07193..b25f5ce 100644
>> --- a/meta/recipes-graphics/mesa/mesa-demos/0003-configure-Allow-to-disable-demos-which-require-GLEW-.patch
>> +++ b/meta/recipes-graphics/mesa/mesa-demos/0003-configure-Allow-to-disable-demos-which-require-GLEW-.patch
>> @@ -10,20 +10,21 @@ Subject: [PATCH 3/9] configure: Allow to disable demos which require GLEW or
>> Upstream-Status: Pending
>> 
>> Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com <mailto:Martin.Jansa@gmail.com>>
>> +Signed-off-by: Drew Moseley <drew_moseley@mentor.com <mailto:drew_moseley@mentor.com>>
>> ---
>> - configure.ac                  | 49 ++++++++++++++++++++---------
>> - src/Makefile.am               | 14 ++++++---
>> + configure.ac                  | 50 ++++++++++++++++++++---------
>> + src/Makefile.am               | 18 ++++++++---
>>  src/demos/Makefile.am         | 73 ++++++++++++++++++++++++-------------------
>>  src/egl/Makefile.am           |  8 +++--
>>  src/egl/opengles1/Makefile.am | 44 +++++++++++++++-----------
>>  src/egl/opengles2/Makefile.am | 33 ++++++++++---------
>> - 6 files changed, 135 insertions(+), 86 deletions(-)
>> + 6 files changed, 140 insertions(+), 86 deletions(-)
>> 
>> diff --git a/configure.ac b/configure.ac
>> index 9445424..bc4c8d1 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> -@@ -93,25 +93,44 @@ AC_EGREP_HEADER([glutInitContextProfile],
>> +@@ -89,25 +89,45 @@ AC_EGREP_HEADER([glutInitContextProfile],
>>  		[AC_DEFINE(HAVE_FREEGLUT)],
>>  		[])
>> 
>> @@ -66,7 +67,7 @@ index 9445424..bc4c8d1 100644
>> -DEMO_CFLAGS="$DEMO_CFLAGS $GLU_CFLAGS"
>> -DEMO_LIBS="$DEMO_LIBS $GLU_LIBS"
>> +if test "x$enable_glu" = xyes; then
>> -+    PKG_CHECK_MODULES(GLU, [glu], [],
>> ++    PKG_CHECK_MODULES(GLU, [glu],
> 
> Are you sure? It was like this in original PKG_CHECK_MODULES and I think
> it was correct. 4th parameter is used as fallback to find it manually
> when pkg-config doesn't find it, with your change it would use
> AC_CHECK_HEADER and ignore the values set returned by pkg-config.


Hi Martin,

I’m fairly certain this is needed however I’m not terribly familiar with autotools so it’s possible I missed something.  I know that without this change a number of the demo apps I expect do not get built.  It’s been a while since I looked but I seem to recall that the “geartrain” demo among others will not be available without this change.

From my reading of the PKG_CHECK_MODULES docs at https://autotools.io/pkgconfig/pkg_check_modules.html <https://autotools.io/pkgconfig/pkg_check_modules.html>, it seems the 3rd parameter is the action-if-found and my read of the logic here is that if glu is found it then verifies the presence of the header files and libs.

I’ll kick off a fresh build and dig around a bit just in case I missed something.

Regards,

Drew

[-- Attachment #1.2: Type: text/html, Size: 9718 bytes --]

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 202 bytes --]

Re: [PATCH v2] mesa-demos: Fix building demos which require GLU.

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 4338 bytes --]

On Thu, Jul 16, 2015 at 04:38:42PM +0000, Moseley, Drew wrote:
> 
> > On Jul 16, 2015, at 11:45 AM, Martin Jansa <martin.jansa@gmail.com> wrote:
> > 
> > On Thu, Jul 16, 2015 at 11:12:04AM -0400, Drew Moseley wrote:
> >> The existing test for GLU support is backwards.  Reverse the
> >> sense of the conditional when built with "--enable-glu".
> >> 
> >> Signed-off-by: Drew Moseley <drew_moseley@mentor.com <mailto:drew_moseley@mentor.com>>
> >> ---
> >> ...figure-Allow-to-disable-demos-which-require-GLEW-.patch | 14 ++++++++------
> >> 1 file changed, 8 insertions(+), 6 deletions(-)
> >> 
> >> diff --git a/meta/recipes-graphics/mesa/mesa-demos/0003-configure-Allow-to-disable-demos-which-require-GLEW-.patch b/meta/recipes-graphics/mesa/mesa-demos/0003-configure-Allow-to-disable-demos-which-require-GLEW-.patch
> >> index 4b07193..b25f5ce 100644
> >> --- a/meta/recipes-graphics/mesa/mesa-demos/0003-configure-Allow-to-disable-demos-which-require-GLEW-.patch
> >> +++ b/meta/recipes-graphics/mesa/mesa-demos/0003-configure-Allow-to-disable-demos-which-require-GLEW-.patch
> >> @@ -10,20 +10,21 @@ Subject: [PATCH 3/9] configure: Allow to disable demos which require GLEW or
> >> Upstream-Status: Pending
> >> 
> >> Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com <mailto:Martin.Jansa@gmail.com>>
> >> +Signed-off-by: Drew Moseley <drew_moseley@mentor.com <mailto:drew_moseley@mentor.com>>
> >> ---
> >> - configure.ac                  | 49 ++++++++++++++++++++---------
> >> - src/Makefile.am               | 14 ++++++---
> >> + configure.ac                  | 50 ++++++++++++++++++++---------
> >> + src/Makefile.am               | 18 ++++++++---
> >>  src/demos/Makefile.am         | 73 ++++++++++++++++++++++++-------------------
> >>  src/egl/Makefile.am           |  8 +++--
> >>  src/egl/opengles1/Makefile.am | 44 +++++++++++++++-----------
> >>  src/egl/opengles2/Makefile.am | 33 ++++++++++---------
> >> - 6 files changed, 135 insertions(+), 86 deletions(-)
> >> + 6 files changed, 140 insertions(+), 86 deletions(-)
> >> 
> >> diff --git a/configure.ac b/configure.ac
> >> index 9445424..bc4c8d1 100644
> >> --- a/configure.ac
> >> +++ b/configure.ac
> >> -@@ -93,25 +93,44 @@ AC_EGREP_HEADER([glutInitContextProfile],
> >> +@@ -89,25 +89,45 @@ AC_EGREP_HEADER([glutInitContextProfile],
> >>  		[AC_DEFINE(HAVE_FREEGLUT)],
> >>  		[])
> >> 
> >> @@ -66,7 +67,7 @@ index 9445424..bc4c8d1 100644
> >> -DEMO_CFLAGS="$DEMO_CFLAGS $GLU_CFLAGS"
> >> -DEMO_LIBS="$DEMO_LIBS $GLU_LIBS"
> >> +if test "x$enable_glu" = xyes; then
> >> -+    PKG_CHECK_MODULES(GLU, [glu], [],
> >> ++    PKG_CHECK_MODULES(GLU, [glu],
> > 
> > Are you sure? It was like this in original PKG_CHECK_MODULES and I think
> > it was correct. 4th parameter is used as fallback to find it manually
> > when pkg-config doesn't find it, with your change it would use
> > AC_CHECK_HEADER and ignore the values set returned by pkg-config.
> 
> 
> Hi Martin,
> 
> I’m fairly certain this is needed however I’m not terribly familiar with autotools so it’s possible I missed something.  I know that without this change a number of the demo apps I expect do not get built.  It’s been a while since I looked but I seem to recall that the “geartrain” demo among others will not be available without this change.
> 
> From my reading of the PKG_CHECK_MODULES docs at https://autotools.io/pkgconfig/pkg_check_modules.html <https://autotools.io/pkgconfig/pkg_check_modules.html>, it seems the 3rd parameter is the action-if-found and my read of the logic here is that if glu is found it then verifies the presence of the header files and libs.

Yes, but if-found (by pkg-config) then the macro itself sets GLU_LIBS
and GLU_CFLAGS, so it doesn't make sense to call AC_CHECK_LIB to change
GLU_LIBS value.

action-if-not-found on the other hand is for cases where your GLU
implementation doesn't provide pkg-config and you want to try to find it
with AC_CHECK_LIB.

I believe you that there could be an issue with building glu demos, but
I guess it's issue in your GLU implementation (whatever that is) and
this change is not correct fix for it - it just happen to help in your
case and breaks other cases.

Regards,

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH v2] mesa-demos: Fix building demos which require GLU.

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 778 bytes --]

On 16 July 2015 at 17:51, Martin Jansa <martin.jansa@gmail.com> wrote:

> Yes, but if-found (by pkg-config) then the macro itself sets GLU_LIBS
> and GLU_CFLAGS, so it doesn't make sense to call AC_CHECK_LIB to change
> GLU_LIBS value.
>
> action-if-not-found on the other hand is for cases where your GLU
> implementation doesn't provide pkg-config and you want to try to find it
> with AC_CHECK_LIB.
>
> I believe you that there could be an issue with building glu demos, but
> I guess it's issue in your GLU implementation (whatever that is) and
> this change is not correct fix for it - it just happen to help in your
> case and breaks other cases.
>

I agree with Martin here.  Can you replicate the problem using a qemu
machine and just oe-core?

Ross

[-- Attachment #2: Type: text/html, Size: 1219 bytes --]

Re: [PATCH v2] mesa-demos: Fix building demos which require GLU.

[Moseley, Drew]

[-- Attachment #1: Type: text/plain, Size: 1286 bytes --]


On Jul 16, 2015, at 6:01 PM, Burton, Ross <ross.burton@intel.com<mailto:ross.burton@intel.com>> wrote:


On 16 July 2015 at 17:51, Martin Jansa <martin.jansa@gmail.com<mailto:martin.jansa@gmail.com>> wrote:
Yes, but if-found (by pkg-config) then the macro itself sets GLU_LIBS
and GLU_CFLAGS, so it doesn't make sense to call AC_CHECK_LIB to change
GLU_LIBS value.

action-if-not-found on the other hand is for cases where your GLU
implementation doesn't provide pkg-config and you want to try to find it
with AC_CHECK_LIB.

I believe you that there could be an issue with building glu demos, but
I guess it's issue in your GLU implementation (whatever that is) and
this change is not correct fix for it - it just happen to help in your
case and breaks other cases.

I agree with Martin here.  Can you replicate the problem using a qemu machine and just oe-core?

Ross


I took a deeper look at this today and what I think needs to happen is that in the “if-found” block we need to set glu_enabled=yes.  That way the HAVE_GLU AM_CONDITIONAL is set to yes later in the configure process.  Does that seem right?  I’ll test a patch against my platform.

I have not figured out how to enable GLUT on the QEMU platform so I cannot yet test it there.

Drew

[-- Attachment #2: Type: text/html, Size: 2510 bytes --]