[Spice-devel] [PATCH] Add missing libX11 reference to build system

[Marc-André Lureau]

----- Original Message -----
> On 18/08/16 10:14 AM, Ian Stakenvicius wrote:
> > On 18/08/16 04:15 AM, Marc-André Lureau wrote:
> >> Hi
> >>
> >> ----- Original Message -----
> >>> GTK on its own is not enough to ensure libX11 is properly linked with
> >>> libspice-client-gtk.  This patch adds X11_LIBS to SPICE_GTK_LIBADD_COMMON
> >>> (and X11_CFLAGS to a relevant section as well) in src/Makefile.am, and
> >>> performs an approriate pkg-config based check to determine the correct
> >>> values in configure.ac when not building for win32 or quartz.
> >>>
> >>
> >> I would check it whenever the header is present (since the code will
> >> compile with x11 in this case)
> >>
> >> AC_CHECK_HEADERS([X11/XKBlib.h], [PKG_CHECK_MODULES(X11, x11)])
> >>
> > 
> > I didn't go into the code itself -- is libX11 only needed at link time
> > for functions/symbols coming from XKBlib.h ?
> > 
> > ./configure should be fatal about libX11 not being found for any case
> > were libX11 is needed, and the AC_CHECK_HEADERS() check won't make it
> > be fatal just because the header is missing.  I guess I'll go through
> > the code and confirm that if XKBLib.h is missing then libX11 isn't a
> > link-time dep.
> > 
> > 
> 
> Confirmed -- the AC_CHECK_HEADERS check would not be sufficient.  The
> following symbols are what is reported missing when linking with
> ld.gold due to the lack of libX11:
> 
> XGetKeyboardControl
> XGetPointerControl
> XChangePointerControl
> XkbGetMap
> XkbGetNames
> XkbFreeKeyboard
> XListExtensions
> XFreeExtensionList
> 
> While the usage of some of these are wrapped by "#ifdef
> HAVE_X11_XKBLIB_H", most are called within blocks wrapped by "#ifdef
> GDK_WINDOWING_X11".  So the pkg-config check needs to remain global so
> it fails on non-windows,non-quartz when libX11 isn't found.
> 
> To be honest I don't like the AS_IF's that much either, but outside of
> restructuring the previous checks I think this patch is still the best
> route forward.

ok, at least it would be nice to have the AS_IF as a single line (same level condition)

AS_IF([test "x$have_quartz" != "xyes" && test "x$have_win32" != "xyes"]..

If that's ok for you, I can commit with this change.

thanks