[4/4] webkitgtk: Ensure that Mesa headers do not include X11 headers on Wayland builds

Message ID 20170712235531.20444-5-aperez@igalia.com
State Changes Requested
Headers show

Commit Message

Adrian Perez de Castro July 12, 2017, 11:55 p.m.
Defining MESA_EGL_NO_X11_HEADERS avoids the situation in which the Mesa
headers try to include the X11 headers as they may not be present when
building for Wayland.

Signed-off-by: Adrian Perez de Castro <aperez@igalia.com>
---
 package/webkitgtk/webkitgtk.mk | 3 +++
 1 file changed, 3 insertions(+)

Comments

Thomas Petazzoni July 21, 2017, 7:09 a.m. | #1
Hello,

On Thu, 13 Jul 2017 02:55:31 +0300, Adrian Perez de Castro wrote:

> diff --git a/package/webkitgtk/webkitgtk.mk b/package/webkitgtk/webkitgtk.mk
> index ef20839f92..09781c2318 100644
> --- a/package/webkitgtk/webkitgtk.mk
> +++ b/package/webkitgtk/webkitgtk.mk
> @@ -84,6 +84,9 @@ endif
>  ifeq ($(BR2_PACKAGE_LIBGTK3_WAYLAND),y)
>  WEBKITGTK_CONF_OPTS += -DENABLE_WAYLAND_TARGET=ON
>  endif
> +WEBKITGTK_CONF_OPTS += \
> +	-DCMAKE_C_FLAGS='$(TARGET_CFLAGS) -DMESA_EGL_NO_X11_HEADERS' \
> +	-DCMAKE_CXX_FLAGS='$(TARGET_CXXFLAGS) -DMESA_EGL_NO_X11_HEADERS'

I am not happy with this: such CFLAGS should be provided by the OpenGL
provider, through its pkg-config file, and consumers of the OpenGL
implementation should get those flags using pkg-config.

If we don't do this, then we need to add -DMESA_EGL_NO_X11_HEADERS
everywhere in all OpenGL consumers, which isn't nice.

We already have a few OpenGL providers that define these flags in
their .pc file:

package/mali-t76x/egl.pc:Cflags: -I${includedir} -DMESA_EGL_NO_X11_HEADERS
package/mali-t76x/glesv2.pc:Cflags: -I${includedir} -DMESA_EGL_NO_X11_HEADERS
package/nvidia-driver/gl.pc:Cflags: -I${includedir}  -DMESA_EGL_NO_X11_HEADERS
package/odroid-mali/odroid-mali.mk:     $(SED) "s/Cflags: /Cflags: -DMESA_EGL_NO_X11_HEADERS /" \

Thanks!

Thomas
Adrian Perez de Castro July 27, 2017, 4:45 p.m. | #2
Hello,

On Fri, 21 Jul 2017 09:09:47 +0200, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:
>
> On Thu, 13 Jul 2017 02:55:31 +0300, Adrian Perez de Castro wrote:
> 
> > diff --git a/package/webkitgtk/webkitgtk.mk b/package/webkitgtk/webkitgtk.mk
> > index ef20839f92..09781c2318 100644
> > --- a/package/webkitgtk/webkitgtk.mk
> > +++ b/package/webkitgtk/webkitgtk.mk
> > @@ -84,6 +84,9 @@ endif
> >  ifeq ($(BR2_PACKAGE_LIBGTK3_WAYLAND),y)
> >  WEBKITGTK_CONF_OPTS += -DENABLE_WAYLAND_TARGET=ON
> >  endif
> > +WEBKITGTK_CONF_OPTS += \
> > +	-DCMAKE_C_FLAGS='$(TARGET_CFLAGS) -DMESA_EGL_NO_X11_HEADERS' \
> > +	-DCMAKE_CXX_FLAGS='$(TARGET_CXXFLAGS) -DMESA_EGL_NO_X11_HEADERS'
> 
> I am not happy with this: such CFLAGS should be provided by the OpenGL
> provider, through its pkg-config file, and consumers of the OpenGL
> implementation should get those flags using pkg-config.
> 
> If we don't do this, then we need to add -DMESA_EGL_NO_X11_HEADERS
> everywhere in all OpenGL consumers, which isn't nice.
> 
> We already have a few OpenGL providers that define these flags in
> their .pc file:
> 
> package/mali-t76x/egl.pc:Cflags: -I${includedir} -DMESA_EGL_NO_X11_HEADERS
> package/mali-t76x/glesv2.pc:Cflags: -I${includedir} -DMESA_EGL_NO_X11_HEADERS
> package/nvidia-driver/gl.pc:Cflags: -I${includedir}  -DMESA_EGL_NO_X11_HEADERS
> package/odroid-mali/odroid-mali.mk:     $(SED) "s/Cflags: /Cflags: -DMESA_EGL_NO_X11_HEADERS /" \

Good point. I will make instead a patch for having Mesa's “egl.pc” include
the “-DMESA_EGL_NO_X11_HEADERS” when X11 support is disabled.

Cheers,


--
 Adrián 🎩
Adrian Perez de Castro July 31, 2017, 1:21 p.m. | #3
Hi all,

On Thu, 27 Jul 2017 19:45:13 +0300, Adrian Perez de Castro <aperez@igalia.com> wrote:

> On Fri, 21 Jul 2017 09:09:47 +0200, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:
> >
> > On Thu, 13 Jul 2017 02:55:31 +0300, Adrian Perez de Castro wrote:
> > 
> > > diff --git a/package/webkitgtk/webkitgtk.mk b/package/webkitgtk/webkitgtk.mk
> > > index ef20839f92..09781c2318 100644
> > > --- a/package/webkitgtk/webkitgtk.mk
> > > +++ b/package/webkitgtk/webkitgtk.mk
> > > @@ -84,6 +84,9 @@ endif
> > >  ifeq ($(BR2_PACKAGE_LIBGTK3_WAYLAND),y)
> > >  WEBKITGTK_CONF_OPTS += -DENABLE_WAYLAND_TARGET=ON
> > >  endif
> > > +WEBKITGTK_CONF_OPTS += \
> > > +	-DCMAKE_C_FLAGS='$(TARGET_CFLAGS) -DMESA_EGL_NO_X11_HEADERS' \
> > > +	-DCMAKE_CXX_FLAGS='$(TARGET_CXXFLAGS) -DMESA_EGL_NO_X11_HEADERS'
> > 
> > I am not happy with this: such CFLAGS should be provided by the OpenGL
> > provider, through its pkg-config file, and consumers of the OpenGL
> > implementation should get those flags using pkg-config.
> > 
> > If we don't do this, then we need to add -DMESA_EGL_NO_X11_HEADERS
> > everywhere in all OpenGL consumers, which isn't nice.
> > 
> > We already have a few OpenGL providers that define these flags in
> > their .pc file:
> > 
> > package/mali-t76x/egl.pc:Cflags: -I${includedir} -DMESA_EGL_NO_X11_HEADERS
> > package/mali-t76x/glesv2.pc:Cflags: -I${includedir} -DMESA_EGL_NO_X11_HEADERS
> > package/nvidia-driver/gl.pc:Cflags: -I${includedir}  -DMESA_EGL_NO_X11_HEADERS
> > package/odroid-mali/odroid-mali.mk:     $(SED) "s/Cflags: /Cflags: -DMESA_EGL_NO_X11_HEADERS /" \
> 
> Good point. I will make instead a patch for having Mesa's “egl.pc” include
> the “-DMESA_EGL_NO_X11_HEADERS” when X11 support is disabled.

Well, it turns out that the Mesa build system is smart enough to figure out by
itself that the X11 support has been disabled:

   buildroot % grep Cflags output/staging/lib/pkgconfig/egl.pc
   Cflags: -I${includedir}  -DMESA_EGL_NO_X11_HEADERS
   buildroot %

It seems that the WebKitGTK+ CMake files are not picking the “Cflags” value
from the “.pc” file. I'll try to fix this in WebKit, and that way it will not
be needed to pass this from “webkitgtk.mk” in Buildroot.

Cheers,


--
 Adrián 🎩

Patch

diff --git a/package/webkitgtk/webkitgtk.mk b/package/webkitgtk/webkitgtk.mk
index ef20839f92..09781c2318 100644
--- a/package/webkitgtk/webkitgtk.mk
+++ b/package/webkitgtk/webkitgtk.mk
@@ -84,6 +84,9 @@  endif
 ifeq ($(BR2_PACKAGE_LIBGTK3_WAYLAND),y)
 WEBKITGTK_CONF_OPTS += -DENABLE_WAYLAND_TARGET=ON
 endif
+WEBKITGTK_CONF_OPTS += \
+	-DCMAKE_C_FLAGS='$(TARGET_CFLAGS) -DMESA_EGL_NO_X11_HEADERS' \
+	-DCMAKE_CXX_FLAGS='$(TARGET_CXXFLAGS) -DMESA_EGL_NO_X11_HEADERS'
 endif
 
 $(eval $(cmake-package))