diff mbox

[Oneiric,Natty,Maverick,Lucid,SRU] Link libbfd statically

Message ID 4E4A837F.7060408@canonical.com
State New
Headers show

Commit Message

Stefan Bader Aug. 16, 2011, 2:49 p.m. UTC
On 14.08.2011 18:16, Ben Hutchings wrote:
> On Tue, 2011-08-02 at 11:38 +0200, Stefan Bader wrote:
>> SRU Justification:
>>
>> Impact: By dynamically linking libbfd, it is not possible to have
>> older versions of the perf tool installed (as there can only be one
>> version of this lib). Also, Debian policy actually forbids depending
>> on a certain version of the library).
>>
>> Fix: Change the makefile to statically link libbfd. This is a Ubuntu
>> specific change, though. Which unlikely will make it upstream.
>>
>> Testcase: Check the perf version provided though the builders (it
>> seems that building in chroots can cause builds not linking against
>> libbfd at all as HAVE_CPLUS_DEMANGLE gets set). The ouput of ldd
>> should not show libbfd.
>>
>> Actually, having said this, maybe the better solution is to modify
>> the build dependencies to cause the compile to have
>> HAVE_CPLUS_DEMANGLE set. That way we do not need to carry a
>> modification to the makefile which likely breaks...
> 
> You *must* set HAVE_CPLUS_DEMANGLE; see Debian bug #606050.
> 
> Ben.
> 
> 
Ah ok. Well I saw that setting that option preventing any linkage against
libbfd. Which was what the proposed patch caused by accidentally breaking all
the compile time tests.
What I was not sure of was whether using libiberty only instead of libbfd has
any functional drawback. But if there are actually license problems involved[1],
then there does not seem to be a way around only using libiberty.

So the change would be like attached for Oneiric. The milage for other releases
will vary. The only thing that makes me wonder whether this correct is that ldd
on the resulting binary shows no reference to libiberty.

-Stefan

[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=606050

Comments

Ben Hutchings Aug. 16, 2011, 3:58 p.m. UTC | #1
On Tue, Aug 16, 2011 at 04:49:35PM +0200, Stefan Bader wrote:
> On 14.08.2011 18:16, Ben Hutchings wrote:
> > On Tue, 2011-08-02 at 11:38 +0200, Stefan Bader wrote:
[...]
> >> Actually, having said this, maybe the better solution is to modify
> >> the build dependencies to cause the compile to have
> >> HAVE_CPLUS_DEMANGLE set. That way we do not need to carry a
> >> modification to the makefile which likely breaks...
> > 
> > You *must* set HAVE_CPLUS_DEMANGLE; see Debian bug #606050.
> > 
> > Ben.
> > 
> > 
> Ah ok. Well I saw that setting that option preventing any linkage against
> libbfd. Which was what the proposed patch caused by accidentally breaking all
> the compile time tests.
> What I was not sure of was whether using libiberty only instead of libbfd has
> any functional drawback. But if there are actually license problems involved[1],
> then there does not seem to be a way around only using libiberty.
 
AFAIK, perf only needs a C++ symbol demangling function from one
or other of these libraries, and either version works just as well as
the other.

> So the change would be like attached for Oneiric. The milage for other releases
> will vary. The only thing that makes me wonder whether this correct is that ldd
> on the resulting binary shows no reference to libiberty.
[...]

I'm not sure about Ubuntu but in Debian binutils-dev only provides a
static libiberty.

Ben.
Stefan Bader Aug. 16, 2011, 6:22 p.m. UTC | #2
On 16.08.2011 17:58, Ben Hutchings wrote:
> On Tue, Aug 16, 2011 at 04:49:35PM +0200, Stefan Bader wrote:
>> On 14.08.2011 18:16, Ben Hutchings wrote:
>>> On Tue, 2011-08-02 at 11:38 +0200, Stefan Bader wrote:
> [...]
>>>> Actually, having said this, maybe the better solution is to modify
>>>> the build dependencies to cause the compile to have
>>>> HAVE_CPLUS_DEMANGLE set. That way we do not need to carry a
>>>> modification to the makefile which likely breaks...
>>>
>>> You *must* set HAVE_CPLUS_DEMANGLE; see Debian bug #606050.
>>>
>>> Ben.
>>>
>>>
>> Ah ok. Well I saw that setting that option preventing any linkage against
>> libbfd. Which was what the proposed patch caused by accidentally breaking all
>> the compile time tests.
>> What I was not sure of was whether using libiberty only instead of libbfd has
>> any functional drawback. But if there are actually license problems involved[1],
>> then there does not seem to be a way around only using libiberty.
>  
> AFAIK, perf only needs a C++ symbol demangling function from one
> or other of these libraries, and either version works just as well as
> the other.
> 
Ah, ok.

>> So the change would be like attached for Oneiric. The milage for other releases
>> will vary. The only thing that makes me wonder whether this correct is that ldd
>> on the resulting binary shows no reference to libiberty.
> [...]
> 
> I'm not sure about Ubuntu but in Debian binutils-dev only provides a
> static libiberty.
>
Gah, not seeing the obvious... So does ours...

> Ben.
>
Tim Gardner Aug. 17, 2011, 1:31 p.m. UTC | #3
On 08/16/2011 08:49 AM, Stefan Bader wrote:
> On 14.08.2011 18:16, Ben Hutchings wrote:
>> On Tue, 2011-08-02 at 11:38 +0200, Stefan Bader wrote:
>>> SRU Justification:
>>>
>>> Impact: By dynamically linking libbfd, it is not possible to have
>>> older versions of the perf tool installed (as there can only be one
>>> version of this lib). Also, Debian policy actually forbids depending
>>> on a certain version of the library).
>>>
>>> Fix: Change the makefile to statically link libbfd. This is a Ubuntu
>>> specific change, though. Which unlikely will make it upstream.
>>>
>>> Testcase: Check the perf version provided though the builders (it
>>> seems that building in chroots can cause builds not linking against
>>> libbfd at all as HAVE_CPLUS_DEMANGLE gets set). The ouput of ldd
>>> should not show libbfd.
>>>
>>> Actually, having said this, maybe the better solution is to modify
>>> the build dependencies to cause the compile to have
>>> HAVE_CPLUS_DEMANGLE set. That way we do not need to carry a
>>> modification to the makefile which likely breaks...
>>
>> You *must* set HAVE_CPLUS_DEMANGLE; see Debian bug #606050.
>>
>> Ben.
>>
>>
> Ah ok. Well I saw that setting that option preventing any linkage against
> libbfd. Which was what the proposed patch caused by accidentally breaking all
> the compile time tests.
> What I was not sure of was whether using libiberty only instead of libbfd has
> any functional drawback. But if there are actually license problems involved[1],
> then there does not seem to be a way around only using libiberty.
>
> So the change would be like attached for Oneiric. The milage for other releases
> will vary. The only thing that makes me wonder whether this correct is that ldd
> on the resulting binary shows no reference to libiberty.
>
> -Stefan
>
> [1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=606050
>
>
diff mbox

Patch

From 19c6b33d1b1ff39df7943716d9b8cee30f71adc8 Mon Sep 17 00:00:00 2001
From: Stefan Bader <stefan.bader@canonical.com>
Date: Tue, 16 Aug 2011 16:14:22 +0200
Subject: [PATCH] UBUNTU: (build) Force perf to use libiberty for demangling

Because libbfd is GPLv3 only and perf is GPLv2 only. Also this avoids
statically linking against libbfd to allow multiple versions of perf
being installed in parallel.

See: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=606050

BugLink: http://bugs.launchpad.net/bugs/783660

Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
---
 debian/rules.d/2-binary-arch.mk |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk
index 29788fc..466c8a4 100644
--- a/debian/rules.d/2-binary-arch.mk
+++ b/debian/rules.d/2-binary-arch.mk
@@ -365,7 +365,8 @@  endif
 
 $(stampdir)/stamp-build-perarch: prepare-perarch
 ifeq ($(do_tools),true)
-	cd $(builddir)/tools/tools/perf && make $(CROSS_COMPILE)
+	cd $(builddir)/tools/tools/perf && \
+		make HAVE_CPLUS_DEMANGLE=1 $(CROSS_COMPILE)
 	if [ "$(arch)" = "amd64" ] || [ "$(arch)" = "i386" ]; then \
 		cd $(builddir)/tools/tools/power/x86/x86_energy_perf_policy && make $(CROSS_COMPILE); \
 		cd $(builddir)/tools/tools/power/x86/turbostat && make $(CROSS_COMPILE); \
-- 
1.7.4.1