All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Olsa <olsajiri@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
	Alexei Starovoitov <ast@kernel.org>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Sedat Dilek <sedat.dilek@gmail.com>,
	Quentin Monnet <quentin@isovalent.com>
Subject: Re: [PATCH v2 0/5] tools: fix compilation failure caused by init_disassemble_info API changes
Date: Mon, 4 Jul 2022 11:13:33 +0200	[thread overview]
Message-ID: <YsKvPW+1RkVvq8aX@krava> (raw)
In-Reply-To: <20220703212551.1114923-1-andres@anarazel.de>

On Sun, Jul 03, 2022 at 02:25:46PM -0700, Andres Freund wrote:
> binutils changed the signature of init_disassemble_info(), which now causes
> compilation failures for tools/{perf,bpf} on e.g. debian unstable. Relevant
> binutils commit:
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=60a3da00bd5407f07
> 
> I first fixed this without introducing the compat header, as suggested by
> Quentin, but I thought the amount of repeated boilerplate was a bit too
> much. So instead I introduced a compat header to wrap the API changes. Even
> tools/bpf/bpftool/jit_disasm.c, which needs its own callbacks for json, imo
> looks nicer this way.
> 
> I'm not regular contributor, so it very well might be my procedures are a
> bit off...
> 
> I am not sure I added the right [number of] people to CC?
> 
> WRT the feature test: Not sure what the point of the -DPACKAGE='"perf"' is,
> nor why tools/perf/Makefile.config sets some LDFLAGS/CFLAGS that are also
> in feature/Makefile and why -ldl isn't needed in the other places. But...
> 
> V2:
> - split patches further, so that tools/bpf and tools/perf part are entirely
>   separate
> - included a bit more information about tests I did in commit messages
> - add a maybe_unused to fprintf_json_styled's style argument
> 
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Sedat Dilek <sedat.dilek@gmail.com>
> Cc: Quentin Monnet <quentin@isovalent.com>
> To: bpf@vger.kernel.org
> To: linux-kernel@vger.kernel.org
> Link: https://lore.kernel.org/lkml/20220622181918.ykrs5rsnmx3og4sv@alap3.anarazel.de
> Link: https://lore.kernel.org/lkml/CA+icZUVpr8ZeOKCj4zMMqbFT013KJz2T1csvXg+VSkdvJH1Ubw@mail.gmail.com
> 
> Andres Freund (5):
>   tools build: add feature test for init_disassemble_info API changes
>   tools include: add dis-asm-compat.h to handle version differences
>   tools perf: Fix compilation error with new binutils
>   tools bpf_jit_disasm: Fix compilation error with new binutils
>   tools bpftool: Fix compilation error with new binutils

I think the disassembler checks should not be displayed by default,
with your change I can see all the time:

...        disassembler-four-args: [ on  ]
...      disassembler-init-styled: [ OFF ]


could you please squash something like below in? moving disassembler
checks out of sight and do manual detection

thanks,
jirka


---
diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 339686b99a6e..bce9a9b52b2c 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -69,8 +69,6 @@ FEATURE_TESTS_BASIC :=                  \
         setns				\
         libaio				\
         libzstd				\
-        disassembler-four-args		\
-        disassembler-init-styled	\
         file-handle
 
 # FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list
@@ -106,7 +104,9 @@ FEATURE_TESTS_EXTRA :=                  \
          libbpf-bpf_create_map		\
          libpfm4                        \
          libdebuginfod			\
-         clang-bpf-co-re
+         clang-bpf-co-re		\
+         disassembler-four-args		\
+         disassembler-init-styled
 
 
 FEATURE_TESTS ?= $(FEATURE_TESTS_BASIC)
@@ -135,9 +135,7 @@ FEATURE_DISPLAY ?=              \
          get_cpuid              \
          bpf			\
          libaio			\
-         libzstd		\
-         disassembler-four-args	\
-         disassembler-init-styled
+         libzstd
 
 # Set FEATURE_CHECK_(C|LD)FLAGS-all for all FEATURE_TESTS features.
 # If in the future we need per-feature checks/flags for features not
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index ee417c321adb..2aa0bad11f05 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -914,8 +914,6 @@ ifndef NO_LIBBFD
         FEATURE_CHECK_LDFLAGS-disassembler-init-styled += -liberty -lz -ldl
       endif
     endif
-    $(call feature_check,disassembler-four-args)
-    $(call feature_check,disassembler-init-styled)
   endif
 
   ifeq ($(feature-libbfd-buildid), 1)
@@ -1025,6 +1023,9 @@ ifdef HAVE_KVM_STAT_SUPPORT
     CFLAGS += -DHAVE_KVM_STAT_SUPPORT
 endif
 
+$(call feature_check,disassembler-four-args)
+$(call feature_check,disassembler-init-styled)
+
 ifeq ($(feature-disassembler-four-args), 1)
     CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
 endif

  parent reply	other threads:[~2022-07-04  9:13 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-22 18:19 init_disassemble_info() signature changes causes compile failures Andres Freund
2022-06-22 22:53 ` Quentin Monnet
2022-06-22 23:16   ` Andres Freund
2022-06-23  9:49     ` Andrew Burgess
2022-07-03  4:48     ` [PATCH v1 0/3] tools: fix compilation failure caused by init_disassemble_info API changes Andres Freund
2022-07-03  4:48       ` [PATCH v1 1/3] tools build: add feature test for " Andres Freund
2022-07-03  4:48       ` [PATCH v1 2/3] tools: add dis-asm-compat.h to centralize handling of version differences Andres Freund
2022-07-03  4:48       ` [PATCH v1 2/3] tools: introduce dis-asm.h wrapper to hide " Andres Freund
2022-07-03  4:54         ` Andres Freund
2022-07-03  4:48       ` [PATCH v1 3/3] tools: Use tools/dis-asm-compat.h to fix compilation errors with new binutils Andres Freund
2022-07-03 21:25     ` [PATCH v2 0/5] tools: fix compilation failure caused by init_disassemble_info API changes Andres Freund
2022-07-03 21:25       ` [PATCH v2 1/5] tools build: add feature test for " Andres Freund
2022-07-03 21:25       ` [PATCH v2 2/5] tools include: add dis-asm-compat.h to handle version differences Andres Freund
2022-07-05 13:44         ` Quentin Monnet
2022-07-15 19:39           ` Andres Freund
2022-07-15 19:46             ` Andres Freund
2022-07-18  8:58             ` Quentin Monnet
2022-07-03 21:25       ` [PATCH v2 3/5] tools perf: Fix compilation error with new binutils Andres Freund
2022-07-03 21:25       ` [PATCH v2 4/5] tools bpf_jit_disasm: " Andres Freund
2022-07-03 21:25       ` [PATCH v2 5/5] tools bpftool: " Andres Freund
2022-07-04  9:13       ` Jiri Olsa [this message]
2022-07-04 20:19         ` [PATCH v2 0/5] tools: fix compilation failure caused by init_disassemble_info API changes Andres Freund
2022-07-04 22:12           ` Jiri Olsa
2022-08-01  1:40             ` Andres Freund
2022-07-10 11:43       ` Sedat Dilek
2022-07-10 17:52         ` Sedat Dilek
2022-07-14  9:16       ` Sedat Dilek
2022-07-14 13:25         ` Ben Hutchings
2022-07-15 19:16           ` Andres Freund
2022-07-15 19:18             ` Ben Hutchings
2022-08-01 18:08               ` Arnaldo Carvalho de Melo
2022-07-27 15:47             ` Arnaldo Carvalho de Melo
2022-07-30 21:45               ` Andres Freund
2022-08-01  1:38     ` [PATCH v3 0/8] " Andres Freund
2022-08-01  1:38       ` [PATCH v3 1/8] tools build: Add feature test for " Andres Freund
2022-08-01  1:38       ` [PATCH v3 2/8] tools build: Don't display disassembler-four-args feature test Andres Freund
2022-08-01 18:10         ` Arnaldo Carvalho de Melo
2022-08-01  1:38       ` [PATCH v3 3/8] tools include: add dis-asm-compat.h to handle version differences Andres Freund
2022-08-01 18:05         ` Arnaldo Carvalho de Melo
2022-08-01 18:10           ` Andres Freund
2022-08-01  1:38       ` [PATCH v3 4/8] tools perf: Fix compilation error with new binutils Andres Freund
2022-08-01  1:38       ` [PATCH v3 5/8] tools bpf_jit_disasm: " Andres Freund
2022-08-01  1:38       ` [PATCH v3 6/8] tools bpf_jit_disasm: Don't display disassembler-four-args feature test Andres Freund
2022-08-01 18:27         ` Arnaldo Carvalho de Melo
2022-08-01 18:41           ` Andres Freund
2022-08-01  1:38       ` [PATCH v3 7/8] tools bpftool: Fix compilation error with new binutils Andres Freund
2022-08-01  1:38       ` [PATCH v3 8/8] tools bpftool: Don't display disassembler-four-args feature test Andres Freund
2022-08-01 18:28         ` Arnaldo Carvalho de Melo
2022-08-01 12:45       ` [PATCH v3 0/8] tools: fix compilation failure caused by init_disassemble_info API changes Arnaldo Carvalho de Melo
2022-08-01 15:15         ` Quentin Monnet
2022-08-01 18:02           ` Arnaldo Carvalho de Melo
2022-08-08 13:35             ` Daniel Borkmann
2022-08-01 19:53         ` Jiri Olsa
2022-08-01 19:12       ` Sedat Dilek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YsKvPW+1RkVvq8aX@krava \
    --to=olsajiri@gmail.com \
    --cc=acme@redhat.com \
    --cc=andres@anarazel.de \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=quentin@isovalent.com \
    --cc=sedat.dilek@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.