RESOLVED FIXED 19045
[gtk] ./configure doesn't check for x toolkit availability
https://bugs.webkit.org/show_bug.cgi?id=19045
Summary [gtk] ./configure doesn't check for x toolkit availability
Dominik Röttsches (drott)
Reported 2008-05-14 06:09:30 PDT
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.
Attachments
Suggested change to configure.ac (769 bytes, patch)
2008-05-14 06:11 PDT, Dominik Röttsches (drott)
no flags
Updated: Suggested change to configure.ac (762 bytes, patch)
2008-07-01 07:47 PDT, Dominik Röttsches (drott)
zecke: review-
Check for Xt (2.00 KB, patch)
2008-08-05 05:30 PDT, Marco Barisione
alp: review+
Check for Xt (1.97 KB, patch)
2008-08-05 10:57 PDT, Marco Barisione
alp: review+
Fallback to -lXt is xt.pc is not installed (1.45 KB, patch)
2008-08-07 03:18 PDT, Marco Barisione
no flags
Fallback to -lXt is xt.pc is not installed (1.62 KB, patch)
2008-08-07 07:27 PDT, Marco Barisione
alp: review+
Dominik Röttsches (drott)
Comment 1 2008-05-14 06:11:48 PDT
Created attachment 21125 [details] Suggested change to configure.ac
Jan Alonzo
Comment 2 2008-05-26 05:06:50 PDT
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)
Comment 3 2008-05-27 07:46:22 PDT
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
Comment 4 2008-06-26 04:48:58 PDT
Dominik, Hi. Is your patch for review? If so can you please set the Flags column to "review: ?"? Thanks.
Dominik Röttsches (drott)
Comment 5 2008-07-01 07:47:17 PDT
Created attachment 22029 [details] Updated: Suggested change to configure.ac I updated the patch for r34917 and set it for review.
Holger Freyther
Comment 6 2008-07-16 10:55:52 PDT
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
Comment 7 2008-07-22 04:28:05 PDT
*** Bug 19909 has been marked as a duplicate of this bug. ***
Marco Barisione
Comment 8 2008-08-05 04:28:42 PDT
*** Bug 20275 has been marked as a duplicate of this bug. ***
Marco Barisione
Comment 9 2008-08-05 05:30:10 PDT
Created attachment 22653 [details] Check for Xt
Alp Toker
Comment 10 2008-08-05 09:50:31 PDT
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
Comment 11 2008-08-05 10:57:03 PDT
Created attachment 22657 [details] Check for Xt I'm marking again for review as the patch was changed
Alp Toker
Comment 12 2008-08-05 14:34:13 PDT
Landed in r35570.
Mark Rowe (bdash)
Comment 13 2008-08-05 20:58:05 PDT
This broke the Gtk buildbot.
Marco Barisione
Comment 14 2008-08-06 02:19:34 PDT
(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
Comment 15 2008-08-07 03:18:14 PDT
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
Comment 16 2008-08-07 06:41:09 PDT
(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
Comment 17 2008-08-07 07:07:19 PDT
(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
Comment 18 2008-08-07 07:27:30 PDT
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
Comment 19 2008-08-07 07:48:45 PDT
(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
Comment 20 2008-08-09 20:11:56 PDT
Landed in r35656.
Daniel Macks
Comment 21 2008-12-06 23:39:05 PST
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
Comment 22 2008-12-07 11:04:50 PST
(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
Comment 23 2008-12-09 22:48:48 PST
(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.
Note You need to log in before you can comment on or make changes to this bug.