Bug 23162

Summary:	[patch for dbus-1.4.x branch] Move $(DBUS_TEST_LIBS) as first in Makefile.am for -Wl,--as-needed compability
Product:	dbus	Reporter:	Samuli Suominen <ssuominen>
Component:	core	Assignee:	Thiago Macieira <thiago>
Status:	RESOLVED FIXED	QA Contact:	John (J5) Palmieri <johnp>
Severity:	normal
Priority:	medium	CC:	freedesktop-bugs, reavertm, xake
Version:	1.4.x	Keywords:	NEEDINFO, patch
Hardware:	All
OS:	Linux (All)
Whiteboard:
i915 platform:		i915 features:
Attachments:	Fix issue git 0001-Fix-ordering-and-missing-DBUS_TEST_LIBS-for-LDFLAGS-.patch Fix building of test suite with -Wl,--as-needed

Description Samuli Suominen 2009-08-05 10:39:59 UTC

Created attachment 28379 [details] [review]
Fix issue

dbus fails at linking phase when creating test suite because $(DBUS_TEST_LIBS) is either in wrong place (importance of order in static linking) or missing

The problem is following, e.g.:

[ .. snip .. ]

libtool: link: x86_64-pc-linux-gnu-gcc -ffunction-sections -fdata-sections -march=core2 -msse4.1 -O2 -pipe -rdynamic -Wall -Wchar-subscripts -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wcast-align -Wdeclaration-after-statement -fno-common -Wno-unused -Wno-sign-compare -Wno-pointer-sign -Wno-format -fno-strict-aliasing -Wl,-O1 -Wl,--hash-style=gnu -o dbus-test dbus-test-main.o -Wl,--export-dynamic  -Wl,--as-needed -lpthread -lrt ./.libs/libdbus-convenience.a
./.libs/libdbus-convenience.a(dbus-sysdeps-pthread.o): In function `_dbus_pthread_condvar_wait_timeout':
dbus-sysdeps-pthread.c:(.text._dbus_pthread_condvar_wait_timeout+0x89): undefined reference to `clock_gettime'
./.libs/libdbus-convenience.a(dbus-sysdeps-pthread.o): In function `_dbus_pthread_condvar_new':
dbus-sysdeps-pthread.c:(.text._dbus_pthread_condvar_new+0x81): undefined reference to `pthread_condattr_setclock'
./.libs/libdbus-convenience.a(dbus-sysdeps-pthread.o): In function `_dbus_threads_init_platform_specific':
dbus-sysdeps-pthread.c:(.text._dbus_threads_init_platform_specific+0xd): undefined reference to `clock_getres'
collect2: ld returned 1 exit status

[ .. snip .. ]

Reproducing:

export LDFLAGS="-Wl,--as-needed"
./configure --enable-tests --enable-asserts
make
make check

Patch follows...

Downstream bug, http://bugs.gentoo.org/show_bug.cgi?id=280170

Comment 1 Thiago Macieira 2009-08-05 14:08:47 UTC

Patch looks good.

Could you make this a Git commit, please?

Comment 2 Samuli Suominen 2009-08-05 15:03:42 UTC

Created attachment 28383 [details] [review]
git

I take it you meant this? I'm not that familiar with git since none of the projects i'm involved use it yet heh :-)

Comment 3 Thiago Macieira 2009-08-05 22:57:18 UTC

Yes, that's what I meant. A patch like this allows us to keep the attribution to you.

I'll apply it.

But if you happen to see this email before I do, can you make the commit message more suitable? The first line in the commit subject, then you have a longer body explaining the change.

Comment 4 Samuli Suominen 2009-08-05 23:20:00 UTC

Created attachment 28390 [details] [review]
0001-Fix-ordering-and-missing-DBUS_TEST_LIBS-for-LDFLAGS-.patch

Comment 5 Peter Hjalmarsson 2009-10-11 04:01:50 UTC

Was this ever commited?

Comment 6 Thiago Macieira 2009-10-11 06:26:22 UTC

No, sorry.

Need to run the tests to confirm this is ok.

Comment 7 Samuli Suominen 2009-10-11 14:48:18 UTC

(In reply to comment #6)
> Need to run the tests to confirm this is ok.

It's been applied in Gentoo's Portage since I've posted it here,
and there has been no bug reports about failing tests since...

If that counts for anything :)

Comment 8 Samuli Suominen 2010-09-18 08:56:59 UTC

Created attachment 38786 [details] [review]
Fix building of test suite with -Wl,--as-needed

Still a problem in released 1.4.0, fails test suite with our default LDFLAGS which include -Wl,--as-needed

Maciej Mrozowski <reavertm@gentoo.org> forwardported the patch from 1.3.0 to 1.4.0

Comment 9 Samuli Suominen 2010-12-21 10:14:48 UTC

Sorry but ping, 1.4.1 got released and it's still failing to build. The patch here still applies.

Comment 10 Simon McVittie 2011-01-06 09:30:13 UTC

On Debian unstable with gcc 4.4.5-10, ld 2.20.1-system.20100303 and libtool 2.2.6b-2, both of these configurations seem to work fine:

./configure --enable-maintainer-mode LDFLAGS=-Wl,--as-needed

./configure --enable-maintainer-mode LDFLAGS=-Wl,--as-needed,--no-add-needed

The link line you pasted comes out like:

libtool: link: gcc -g -O2 -Werror -Wall -Wchar-subscripts -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wcast-align -Wdeclaration-after-statement -fno-common -Wno-unused -Wno-sign-compare -Wno-pointer-sign -fno-strict-aliasing -Wl,--as-needed -o dbus-test dbus-test-main.o -Wl,--export-dynamic  ./.libs/libdbus-internal.a -lpthread -lrt

and libdbus-internal.la correctly contains:

# Libraries that this one depends upon.
dependency_libs=' -lpthread -lrt'

What do you get in dependency_libs on Gentoo? libtool is meant to sort this out automatically when there are .la files present.

Comment 11 Samuli Suominen 2011-01-11 04:28:24 UTC

(In reply to comment #10)
> On Debian unstable with gcc 4.4.5-10, ld 2.20.1-system.20100303 and libtool
> 2.2.6b-2, both of these configurations seem to work fine:
> ./configure --enable-maintainer-mode LDFLAGS=-Wl,--as-needed
> ./configure --enable-maintainer-mode LDFLAGS=-Wl,--as-needed,--no-add-needed

It might work outside of Debian's and Gentoo's package manager because by default, upstream libtool is ignoring -Wl,--as-needed:

http://bugs.debian.org/347650
http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/eclass/ELT-patches/as-needed/2.2.6?revision=1.1&view=markup

You'd need to run this with Debian patch to libtool's ltmain.sh, or from inside Gentoo's Portage to get properly patched libtool to not ignore asneeded...

Comment 12 Samuli Suominen 2011-02-26 03:55:50 UTC

still fails with 1.4.6

Comment 13 Samuli Suominen 2012-05-15 14:25:13 UTC

Some test results from today:

This patch is not required anymore for dbus-1.5.12 and git. The Makefile.am's no longer have the problematic DBUS_TEST_LIBS at all, hence the problem is gone as well.

So, this patch is only required for dbus-1.4 branch.

Comment 14 Simon McVittie 2012-06-05 03:41:50 UTC

(In reply to comment #13)
> Some test results from today:
> 
> This patch is not required anymore for dbus-1.5.12 and git.

Thanks for re-testing.

I'm going to release a new-stable dbus-1.6 branch shortly (1.5.x fixes some threading bugs in a way that's too intrusive for 1.4), at which point I'll stop caring about minor issues in 1.4, so I'm going to consider this fixed.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.