GNOME Bugzilla – Bug 723877
configure: Don't link to both Qt and gdk-pixbuf if both are autodetected
Last modified: 2014-02-10 11:23:02 UTC
If both qt and gdk-pixbuf are found at configure time the backend is set to gdk-pixbuf but the library is linked with both.
Created attachment 268466 [details] [review] proposed fix
Created attachment 268478 [details] [review] Additional linkage fix Left to its devices automake+libtool use CXXLINK for the final library link despite not building any .cpp files. This leads to extra unnecessary linkage to libstdc++.
Created attachment 268479 [details] [review] Additional linkage fix - remove one more excplicit -lm remove one more excplicit -lm
Comment on attachment 268466 [details] [review] proposed fix Superb patch, thanks for noticing. Please commit. If you need me to commit, I can of course :)
Comment on attachment 268479 [details] [review] Additional linkage fix - remove one more excplicit -lm >diff --git a/configure.ac b/configure.ac >index ddba936..792cfd3 100644 >--- a/configure.ac >+++ b/configure.ac >@@ -151,8 +151,6 @@ PKG_CHECK_MODULES(LIBMEDIAART, [$LIBMEDIAART_REQUIRED]) > GLIB_GENMARSHAL=`$PKG_CONFIG glib-2.0 --variable=glib_genmarshal` > AC_SUBST(GLIB_GENMARSHAL) > >-LIBMEDIAART_LIBS="$LIBMEDIAART_LIBS -lz -lm" Nice. I think the linking with libm is really just left over linking foo from Tracker. I didn't actually know if it was needed or not so I kept it for now, but thanks for checking. >diff --git a/libmediaart/Makefile.am b/libmediaart/Makefile.am >index 6d370f2..2057fe3 100644 >--- a/libmediaart/Makefile.am >+++ b/libmediaart/Makefile.am >@@ -34,11 +34,14 @@ libmediaart_@LIBMEDIAART_API_VERSION@_la_SOURCES = \ > > if HAVE_GDKPIXBUF > libmediaart_@LIBMEDIAART_API_VERSION@_la_SOURCES += extractpixbuf.c >+libmediaart_@LIBMEDIAART_API_VERSION@_la_LINK = $(LINK) $(libmediaart_1.0_la_LDFLAGS) > else > if HAVE_QT > libmediaart_@LIBMEDIAART_API_VERSION@_la_SOURCES += extractqt.cpp >+libmediaart_@LIBMEDIAART_API_VERSION@_la_LINK = $(CXXLINK) $(libmediaart_1.0_la_LDFLAGS) > else > libmediaart_@LIBMEDIAART_API_VERSION@_la_SOURCES += extractdummy.c >+libmediaart_@LIBMEDIAART_API_VERSION@_la_LINK = $(LINK) $(libmediaart_1.0_la_LDFLAGS) > endif > endif Hmm, is the extra _LINK line necessary if we already have the libmediaart_@LIBMEDIAART_API_VERSION@_la_LIBADD line in the Makefile.am? I also wonder what $(LINK) is set to. Perhaps you did this for completeness? It worked before so ... >@@ -46,7 +49,6 @@ libmediaart_@LIBMEDIAART_API_VERSION@_la_LDFLAGS = \ > -version-info $(LT_CURRENT):$(LT_REVISION):$(LT_AGE) > > libmediaart_@LIBMEDIAART_API_VERSION@_la_LIBADD = \ >- -lm \ > $(BUILD_LIBS) \ > $(LIBMEDIAART_LIBS) The rest looks good. Thanks :)
Comment on attachment 268466 [details] [review] proposed fix 53381ad
I probably shouldn't have mixed the explict -lm/-lz removal with the CXXLINK/LINK stuff. Both are indenpendent issues. Automake chooses the linker to use by examining the _SOURCES. http://www.gnu.org/software/automake/manual/html_node/How-the-Linker-is-Chosen.html#How-the-Linker-is-Chosen Unfortunately this includes all potential sources, even the ones excluded by CONDITIONALS, so it picks CXXLINK because of extractqt.cpp. Linking the library with CXXLINK brings with it a soname linkage with libstdc++. overriding <TARGET>_LINK on target by target basis is the way to force automake to use the specific linker, C(LINK) or C++(CXXLINK)
Created attachment 268648 [details] [review] build: Remove leftover explicit -lm -lz linkage The -lm -lz part separately
Created attachment 268649 [details] [review] build: Force automake C linkage when building C only The LINK/CXXLINK part with additional comments in the patch
Comment on attachment 268648 [details] [review] build: Remove leftover explicit -lm -lz linkage Thanks for separating the patch :)
Comment on attachment 268649 [details] [review] build: Force automake C linkage when building C only Thanks for clarifying why we're using _LINK here... Please be sure to include the bug report in the commit of this patch so people can find out why we made this change! :) Thanks
Comment on attachment 268648 [details] [review] build: Remove leftover explicit -lm -lz linkage pushed to master as 4f07de7
Comment on attachment 268649 [details] [review] build: Force automake C linkage when building C only pushed to master as 4ae83f6
Thanks. Added bug reference in the commits. Closing