perf build: fix -lbfd feature check
diff mbox series

Message ID 4dfc634cfcfb236883971b5107cf3c28ec8a31be.1542328222.git.sdf@google.com
State In Next
Commit 14541b1e7e723859ff2c75c6fc10cdbbec6b8c34
Headers show
Series
  • perf build: fix -lbfd feature check
Related show

Commit Message

Stanislav Fomichev Nov. 16, 2018, 12:32 a.m. UTC
Current libbfd feature test unconditionally links against -liberty and -lz.
While it's required on some systems (e.g. opensuse), it's completely
unnecessary on the others, where only -lbdf is sufficient (debian).
This patch streamlines (and renames) the following feature checks:

feature-libbfd           - only link against -lbfd (debian),
                           see commit 2cf9040714f3 ("perf tools: Fix bfd
			   dependency libraries detection")
feature-libbfd-liberty   - link against -lbfd and -liberty
feature-libbfd-liberty-z - link against -lbfd, -liberty and -lz (opensuse),
                           see commit 280e7c48c3b8 ("perf tools: fix BFD
			   detection on opensuse")

(feature-liberty{,-z} were renamed to feature-libbfd-liberty{,z}
for clarity)

The main motivation is to fix this feature test for bpftool which is
currently broken on debian (libbfd feature shows OFF, but we still
unconditionally link against -lbfd and it works).

Tested on debian with only -lbfd installed (without -liberty); I'd
appreciate if somebody on the other systems can test this new detection
method.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 tools/build/Makefile.feature |  4 ++--
 tools/build/feature/Makefile | 10 ++++----
 tools/perf/Makefile.config   | 44 +++++++++++++++++++-----------------
 3 files changed, 30 insertions(+), 28 deletions(-)

Comments

Jiri Olsa Nov. 16, 2018, 1:34 p.m. UTC | #1
On Thu, Nov 15, 2018 at 04:32:01PM -0800, Stanislav Fomichev wrote:
> Current libbfd feature test unconditionally links against -liberty and -lz.
> While it's required on some systems (e.g. opensuse), it's completely
> unnecessary on the others, where only -lbdf is sufficient (debian).
> This patch streamlines (and renames) the following feature checks:
> 
> feature-libbfd           - only link against -lbfd (debian),
>                            see commit 2cf9040714f3 ("perf tools: Fix bfd
> 			   dependency libraries detection")
> feature-libbfd-liberty   - link against -lbfd and -liberty
> feature-libbfd-liberty-z - link against -lbfd, -liberty and -lz (opensuse),
>                            see commit 280e7c48c3b8 ("perf tools: fix BFD
> 			   detection on opensuse")
> 
> (feature-liberty{,-z} were renamed to feature-libbfd-liberty{,z}
> for clarity)
> 
> The main motivation is to fix this feature test for bpftool which is
> currently broken on debian (libbfd feature shows OFF, but we still
> unconditionally link against -lbfd and it works).
> 
> Tested on debian with only -lbfd installed (without -liberty); I'd
> appreciate if somebody on the other systems can test this new detection
> method.
> 
> Signed-off-by: Stanislav Fomichev <sdf@google.com>

looks good, Fedora goes with standalone libbfd

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka
Stanislav Fomichev Dec. 19, 2018, 2:08 a.m. UTC | #2
On Fri, Nov 16, 2018 at 5:34 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Thu, Nov 15, 2018 at 04:32:01PM -0800, Stanislav Fomichev wrote:
> > Current libbfd feature test unconditionally links against -liberty and -lz.
> > While it's required on some systems (e.g. opensuse), it's completely
> > unnecessary on the others, where only -lbdf is sufficient (debian).
> > This patch streamlines (and renames) the following feature checks:
> >
> > feature-libbfd           - only link against -lbfd (debian),
> >                            see commit 2cf9040714f3 ("perf tools: Fix bfd
> >                          dependency libraries detection")
> > feature-libbfd-liberty   - link against -lbfd and -liberty
> > feature-libbfd-liberty-z - link against -lbfd, -liberty and -lz (opensuse),
> >                            see commit 280e7c48c3b8 ("perf tools: fix BFD
> >                          detection on opensuse")
> >
> > (feature-liberty{,-z} were renamed to feature-libbfd-liberty{,z}
> > for clarity)
> >
> > The main motivation is to fix this feature test for bpftool which is
> > currently broken on debian (libbfd feature shows OFF, but we still
> > unconditionally link against -lbfd and it works).
> >
> > Tested on debian with only -lbfd installed (without -liberty); I'd
> > appreciate if somebody on the other systems can test this new detection
> > method.
> >
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
>
> looks good, Fedora goes with standalone libbfd
>
> Acked-by: Jiri Olsa <jolsa@kernel.org>
>
> thanks,
> jirka

Will this be included in the next merge window? Or this patch was
somehow missed out? (I didn't get any notification about it being
pulled).
We need this to get -lbfd working with bpftool as well...

Patch
diff mbox series

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index f216b2f5c3d7..42a787856cd8 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -79,8 +79,8 @@  FEATURE_TESTS_EXTRA :=                  \
          cplus-demangle                 \
          hello                          \
          libbabeltrace                  \
-         liberty                        \
-         liberty-z                      \
+         libbfd-liberty                 \
+         libbfd-liberty-z               \
          libunwind-debug-frame          \
          libunwind-debug-frame-arm      \
          libunwind-debug-frame-aarch64  \
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index 0516259be70f..bf8a8ebcca1e 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -15,8 +15,8 @@  FILES=                                          \
          test-libbfd.bin                        \
          test-disassembler-four-args.bin        \
          test-reallocarray.bin			\
-         test-liberty.bin                       \
-         test-liberty-z.bin                     \
+         test-libbfd-liberty.bin                \
+         test-libbfd-liberty-z.bin              \
          test-cplus-demangle.bin                \
          test-libelf.bin                        \
          test-libelf-getphdrnum.bin             \
@@ -200,7 +200,7 @@  FLAGS_PERL_EMBED=$(PERL_EMBED_CCOPTS) $(PERL_EMBED_LDOPTS)
 	$(BUILD)
 
 $(OUTPUT)test-libbfd.bin:
-	$(BUILD) -DPACKAGE='"perf"' -lbfd -lz -liberty -ldl
+	$(BUILD) -DPACKAGE='"perf"' -lbfd -ldl
 
 $(OUTPUT)test-disassembler-four-args.bin:
 	$(BUILD) -DPACKAGE='"perf"' -lbfd -lopcodes
@@ -208,10 +208,10 @@  FLAGS_PERL_EMBED=$(PERL_EMBED_CCOPTS) $(PERL_EMBED_LDOPTS)
 $(OUTPUT)test-reallocarray.bin:
 	$(BUILD)
 
-$(OUTPUT)test-liberty.bin:
+$(OUTPUT)test-libbfd-liberty.bin:
 	$(CC) $(CFLAGS) -Wall -Werror -o $@ test-libbfd.c -DPACKAGE='"perf"' $(LDFLAGS) -lbfd -ldl -liberty
 
-$(OUTPUT)test-liberty-z.bin:
+$(OUTPUT)test-libbfd-liberty-z.bin:
 	$(CC) $(CFLAGS) -Wall -Werror -o $@ test-libbfd.c -DPACKAGE='"perf"' $(LDFLAGS) -lbfd -ldl -liberty -lz
 
 $(OUTPUT)test-cplus-demangle.bin:
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index e30d20fb482d..6287fa0ebd1d 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -686,18 +686,20 @@  endif
 
 ifeq ($(feature-libbfd), 1)
   EXTLIBS += -lbfd
+else
+  # we are on a system that requires -liberty and (maybe) -lz
+  # to link against -lbfd; test each case individually here
 
   # call all detections now so we get correct
   # status in VF output
-  $(call feature_check,liberty)
-  $(call feature_check,liberty-z)
-  $(call feature_check,cplus-demangle)
+  $(call feature_check,libbfd-liberty)
+  $(call feature_check,libbfd-liberty-z)
 
-  ifeq ($(feature-liberty), 1)
-    EXTLIBS += -liberty
+  ifeq ($(feature-libbfd-liberty), 1)
+    EXTLIBS += -lbfd -liberty
   else
-    ifeq ($(feature-liberty-z), 1)
-      EXTLIBS += -liberty -lz
+    ifeq ($(feature-libbfd-liberty-z), 1)
+      EXTLIBS += -lbfd -liberty -lz
     endif
   endif
 endif
@@ -707,24 +709,24 @@  ifdef NO_DEMANGLE
 else
   ifdef HAVE_CPLUS_DEMANGLE_SUPPORT
     EXTLIBS += -liberty
-    CFLAGS += -DHAVE_CPLUS_DEMANGLE_SUPPORT
   else
-    ifneq ($(feature-libbfd), 1)
-      ifneq ($(feature-liberty), 1)
-        ifneq ($(feature-liberty-z), 1)
-          # we dont have neither HAVE_CPLUS_DEMANGLE_SUPPORT
-          # or any of 'bfd iberty z' trinity
-          ifeq ($(feature-cplus-demangle), 1)
-            EXTLIBS += -liberty
-            CFLAGS += -DHAVE_CPLUS_DEMANGLE_SUPPORT
-          else
-            msg := $(warning No bfd.h/libbfd found, please install binutils-dev[el]/zlib-static/libiberty-dev to gain symbol demangling)
-            CFLAGS += -DNO_DEMANGLE
-          endif
-        endif
+    ifeq ($(filter -liberty,$(EXTLIBS)),)
+      $(call feature_check,cplus-demangle)
+
+      # we dont have neither HAVE_CPLUS_DEMANGLE_SUPPORT
+      # or any of 'bfd iberty z' trinity
+      ifeq ($(feature-cplus-demangle), 1)
+        EXTLIBS += -liberty
+      else
+        msg := $(warning No bfd.h/libbfd found, please install binutils-dev[el]/zlib-static/libiberty-dev to gain symbol demangling)
+        CFLAGS += -DNO_DEMANGLE
       endif
     endif
   endif
+
+  ifneq ($(filter -liberty,$(EXTLIBS)),)
+    CFLAGS += -DHAVE_CPLUS_DEMANGLE_SUPPORT
+  endif
 endif
 
 ifneq ($(filter -lbfd,$(EXTLIBS)),)