Bug 23162 - [patch for dbus-1.4.x branch] Move $(DBUS_TEST_LIBS) as first in Makefile.am for -Wl,--as-needed compability
Summary: [patch for dbus-1.4.x branch] Move $(DBUS_TEST_LIBS) as first in Makefile.am ...
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: 1.4.x
Hardware: All Linux (All)
: medium normal
Assignee: Thiago Macieira
QA Contact: John (J5) Palmieri
URL:
Whiteboard:
Keywords: NEEDINFO, patch
Depends on:
Blocks:
 
Reported: 2009-08-05 10:39 UTC by Samuli Suominen
Modified: 2012-06-05 03:41 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:

Attachments
Fix issue (3.57 KB, patch)
2009-08-05 10:39 UTC, Samuli Suominen 		Details | Splinter Review
git (4.09 KB, patch)
2009-08-05 15:03 UTC, Samuli Suominen 		Details | Splinter Review
0001-Fix-ordering-and-missing-DBUS_TEST_LIBS-for-LDFLAGS-.patch (4.09 KB, patch)
2009-08-05 23:20 UTC, Samuli Suominen 		Details | Splinter Review
Fix building of test suite with -Wl,--as-needed (4.01 KB, patch)
2010-09-18 08:56 UTC, Samuli Suominen 		Details | Splinter Review
Show Obsolete (3) View All

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.