[gtk] ./configure doesn't check for x toolkit availability

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Dominik Röttsches (drott)]

In r33439 building fails for files including the X Toolkit's Intrinsic.h header if the library headers are not available. 
(e.g. WebCore/plugins/gtk/gtk2xtbin.c:#include <X11/Intrinsic.h>, WebCore/plugins/gtk/gtk2xtbin.h:#include <X11/Intrinsic.h>)

These files were added with r32287, as far as I can see.

I suggest to add a check for the Xt library in the top level configure.ac. This way the library dependency can be resolved without having to wait for the compilation to fail.

I will attach a small patch to configure.ac. I'm not sure about the exact version that's required. My ubuntu says 1.0.5 is installed, so I figured 1.0 might be a good guess.

[Dominik Röttsches (drott)]

Created attachment 21125 [details]
Suggested change to configure.ac

[Jan Alonzo]

Hi Dominik

Is XT needed for DirectFB or Win32 builds as well? Also, if you want your patch to be reviewed, please set the flag to '?' so people will notice.

[Dominik Röttsches (drott)]

Hi Jan,
for the win32 port, I would assume yes. For the GTK-DirectFB build, I can't say, I'm afraid. When I checked this last, the files required for plugin support on GTK did not seem to be conditionalised by platform or target environment with regard to including them in the build or not. Neither was the #include <X11/Intrinsic.h> line. 

[Jan Alonzo]

Dominik,

Hi. Is your patch for review? If so can you please set the Flags column to "review: ?"? Thanks.

[Dominik Röttsches (drott)]

Created attachment 22029 [details]
Updated: Suggested change to configure.ac

I updated the patch for r34917 and set it for review.

[Holger Freyther]

Comment on attachment 22029 [details]
Updated: Suggested change to configure.ac

There are some issues with this patch:
  - xt is a X11 library and we only need it if we have TARGET_X11 set. So a directFB build should not fail with missing xt..

  - gtk2xtbin.c is using xt, we should link to libxt directly. So please change WebCore/GNUmakefile.am to link to xt?

[Mike Hommey]

*** Bug 19909 has been marked as a duplicate of this bug. ***

[Marco Barisione]

*** Bug 20275 has been marked as a duplicate of this bug. ***

[Marco Barisione]

Created attachment 22653 [details]
Check for Xt

[Alp Toker]

Comment on attachment 22653 [details]
Check for Xt

Please rename XTDEPS to just XT though, eg. XT_CFLAGS. Looks good otherwise.

I think this change should be fine, even for users of non-modular Xorg and XFree86 since pkg-config allows the XT_CFLAGS and XT_LIBS values to be passed by configure manually.

[Marco Barisione]

Created attachment 22657 [details]
Check for Xt

I'm marking again for review as the patch was changed

[Alp Toker]

Landed in r35570.

[Mark Rowe (bdash)]

This broke the Gtk buildbot.

[Marco Barisione]

(In reply to comment #13)
> This broke the Gtk buildbot.

I wrote an email to Holger about this. I wonder if there are working Xt versions that do not provide a xt.pc file, in this case what is the right fix: just fail or use AC_TRY_COMPILE to se if there is an installed Xt?

[Marco Barisione]

Created attachment 22693 [details]
Fallback to -lXt is xt.pc is not installed

If xt.pc is not available check for the existence of libXt using AC_CHECK_LIB and if it works just use -lXt.

Holger said that versions of Xt without xt.pc just still be quite common, so this workaround should be needed.

[Jan Alonzo]

(In reply to comment #15)
> Created an attachment (id=22693) [edit]
> Fallback to -lXt is xt.pc is not installed
> 
> If xt.pc is not available check for the existence of libXt using AC_CHECK_LIB
> and if it works just use -lXt.
> 
> Holger said that versions of Xt without xt.pc just still be quite common, so
> this workaround should be needed.

Hi! The autoconf manual doesn't seem to recommend AC_CHECK_LIB so you might want to use AC_TRY_CPP or a similar macro to see if the installed XT/Xlib libraries are actually usable. 

[Marco Barisione]

(In reply to comment #16)
> Hi! The autoconf manual doesn't seem to recommend AC_CHECK_LIB so you might
> want to use AC_TRY_CPP or a similar macro to see if the installed XT/Xlib
> libraries are actually usable. 

From what I understand the manual warns about cases when the same symbol can be defined by more libraries. In this case we don't care about the symbol but only about *that* library, and just *that* as we are hard coding -lXt.

[Marco Barisione]

Created attachment 22696 [details]
Fallback to -lXt is xt.pc is not installed

As discussed on IRC using AC_CHECK_LIB is ok in this case.

In this patch I added a comment about AC_CHECK_LIB vs. AC_SEARCH_LIBS and fixed a style issue.

[Jan Alonzo]

(In reply to comment #18)
> Created an attachment (id=22696) [edit]
> Fallback to -lXt is xt.pc is not installed
> 
> As discussed on IRC using AC_CHECK_LIB is ok in this case.
> 
> In this patch I added a comment about AC_CHECK_LIB vs. AC_SEARCH_LIBS and fixed
> a style issue.

Thanks for the update. Looks good and should be OK in my opinion.

[Alp Toker]

Landed in r35656.

[Daniel Macks]

OS X 10.4 apple x11 has no xt.pc but its libXt.dylib does have XtOpenDisplay. However, building SVN r39007 --with-target=x11 fails:

>checking for XT... no
>checking for XtOpenDisplay in -lXt... no
>configure: error: X Toolkit Intrinsics library (libXt) not found

The x11 libs are in /usr/X11R6/lib, which is not in the standard gcc search paths. Looks like the AC_CHECK_LIB([Xt],[XtOpenDisplay]) is relying on libXt being in the default search path. The XtOpenDisplay test works if I pass LDFLAGS=-L/usr/X11R6/lib during autogen.sh. Should configure.ac do some checks for the x11 paths and pass/propagate some -L here? Or is this test too rarely used and easily overridden to bother fixing this corner case?

[Holger Freyther]

(In reply to comment #21)
> OS X 10.4 apple x11 has no xt.pc but its libXt.dylib does have XtOpenDisplay.
> However, building SVN r39007 --with-target=x11 fails:
> 
> >checking for XT... no
> >checking for XtOpenDisplay in -lXt... no
> >configure: error: X Toolkit Intrinsics library (libXt) not found
> 
> The x11 libs are in /usr/X11R6/lib, which is not in the standard gcc search
> paths. Looks like the AC_CHECK_LIB([Xt],[XtOpenDisplay]) is relying on libXt
> being in the default search path. The XtOpenDisplay test works if I pass
> LDFLAGS=-L/usr/X11R6/lib during autogen.sh. Should configure.ac do some checks
> for the x11 paths and pass/propagate some -L here? Or is this test too rarely
> used and easily overridden to bother fixing this corner case?

I argue that setting LDFLAGS is correct. When cross compiling it should not even attempt to link against libs in /usr/X11R6/lib... or paths we come up with.

[Daniel Macks]

(In reply to comment #22)
>
> I argue that setting LDFLAGS is correct. When cross compiling it should not
> even attempt to link against libs in /usr/X11R6/lib... or paths we come up
> with.

For the record, --with-target=x11 is not cross-compiling in any way, merely declaring an option for which type of GUI to use.