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

[Martin Jansa <martin.jansa@gmail.com>]

[-- 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 --]