| Message ID | 20220703212551.1114923-1-andres@anarazel.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | tools: fix compilation failure caused by init_disassemble_info API changes | expand |
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
Hi, On 2022-07-04 11:13:33 +0200, Jiri Olsa wrote: > 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 Makes sense - I was wondering why disassembler-four-args is displayed, but though it better to mirror the existing behaviour. Does "hiding" disassembler-four-args need to be its own set of commits? > 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 This I don't understand - why do we want these to run under NO_LIBBFD etc? Greetings, Andres Freund
On Mon, Jul 04, 2022 at 01:19:22PM -0700, Andres Freund wrote: > Hi, > > On 2022-07-04 11:13:33 +0200, Jiri Olsa wrote: > > 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 > > Makes sense - I was wondering why disassembler-four-args is displayed, but > though it better to mirror the existing behaviour. Does "hiding" > disassembler-four-args need to be its own set of commits? I guess first hide the disassembler-four-args and add the new the same way > > > > 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 > > This I don't understand - why do we want these to run under NO_LIBBFD etc? when I was quickly testing that I did not have any of them detected and got compile fail.. so I moved it to safe place ;-) it might be placed in smarter place thanks, jirka > > Greetings, > > Andres Freund
On Sun, Jul 3, 2022 at 11:25 PM Andres Freund <andres@anarazel.de> 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 > HI, what was the base for this patchset? I tried with Linux v5.19-rc5 and it doesn not apply cleanly. Regards, -Sedat- > 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 > > tools/bpf/Makefile | 7 ++- > tools/bpf/bpf_jit_disasm.c | 5 +- > tools/bpf/bpftool/Makefile | 7 ++- > tools/bpf/bpftool/jit_disasm.c | 42 ++++++++++++--- > tools/build/Makefile.feature | 4 +- > tools/build/feature/Makefile | 4 ++ > tools/build/feature/test-all.c | 4 ++ > .../feature/test-disassembler-init-styled.c | 13 +++++ > tools/include/tools/dis-asm-compat.h | 53 +++++++++++++++++++ > tools/perf/Makefile.config | 8 +++ > tools/perf/util/annotate.c | 7 +-- > 11 files changed, 137 insertions(+), 17 deletions(-) > create mode 100644 tools/build/feature/test-disassembler-init-styled.c > create mode 100644 tools/include/tools/dis-asm-compat.h > > -- > 2.37.0.3.g30cc8d0f14 >
On Sun, Jul 10, 2022 at 1:43 PM Sedat Dilek <sedat.dilek@gmail.com> wrote: > > On Sun, Jul 3, 2022 at 11:25 PM Andres Freund <andres@anarazel.de> 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 > > > > HI, > > what was the base for this patchset? > I tried with Linux v5.19-rc5 and it doesn not apply cleanly. > More coffee. $ egrep 'Auto-detecting|disassembler' make-log_perf-python3.10-install_bin.txt 15:Auto-detecting system features: 36:... disassembler-four-args: [ on ] 37:... disassembler-init-styled: [ on ] Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM-14 (x86-64) -Sedat- > > > 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 > > > > tools/bpf/Makefile | 7 ++- > > tools/bpf/bpf_jit_disasm.c | 5 +- > > tools/bpf/bpftool/Makefile | 7 ++- > > tools/bpf/bpftool/jit_disasm.c | 42 ++++++++++++--- > > tools/build/Makefile.feature | 4 +- > > tools/build/feature/Makefile | 4 ++ > > tools/build/feature/test-all.c | 4 ++ > > .../feature/test-disassembler-init-styled.c | 13 +++++ > > tools/include/tools/dis-asm-compat.h | 53 +++++++++++++++++++ > > tools/perf/Makefile.config | 8 +++ > > tools/perf/util/annotate.c | 7 +-- > > 11 files changed, 137 insertions(+), 17 deletions(-) > > create mode 100644 tools/build/feature/test-disassembler-init-styled.c > > create mode 100644 tools/include/tools/dis-asm-compat.h > > > > -- > > 2.37.0.3.g30cc8d0f14 > >
On Sun, Jul 3, 2022 at 11:25 PM Andres Freund <andres@anarazel.de> 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 Ben ] The Debian kernel-team has integrated your patchset v2. In case you build without libbfd support there is [1]. So, feel free to take this for v3. -Sedat- [1] https://salsa.debian.org/kernel-team/linux/-/blob/sid/debian/patches/bugfix/all/tools-perf-fix-build-without-libbfd.patch > 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 > > tools/bpf/Makefile | 7 ++- > tools/bpf/bpf_jit_disasm.c | 5 +- > tools/bpf/bpftool/Makefile | 7 ++- > tools/bpf/bpftool/jit_disasm.c | 42 ++++++++++++--- > tools/build/Makefile.feature | 4 +- > tools/build/feature/Makefile | 4 ++ > tools/build/feature/test-all.c | 4 ++ > .../feature/test-disassembler-init-styled.c | 13 +++++ > tools/include/tools/dis-asm-compat.h | 53 +++++++++++++++++++ > tools/perf/Makefile.config | 8 +++ > tools/perf/util/annotate.c | 7 +-- > 11 files changed, 137 insertions(+), 17 deletions(-) > create mode 100644 tools/build/feature/test-disassembler-init-styled.c > create mode 100644 tools/include/tools/dis-asm-compat.h > > -- > 2.37.0.3.g30cc8d0f14 >
On Thu, 2022-07-14 at 11:16 +0200, Sedat Dilek wrote: > On Sun, Jul 3, 2022 at 11:25 PM Andres Freund <andres@anarazel.de> 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 Ben ] > > The Debian kernel-team has integrated your patchset v2. > > In case you build without libbfd support there is [1]. > So, feel free to take this for v3. > > -Sedat- > > [1] https://salsa.debian.org/kernel-team/linux/-/blob/sid/debian/patches/bugfix/all/tools-perf-fix-build-without-libbfd.patch [...] Thanks, I meant to send that fix upstream but got distracted. It should really be folded into "tools perf: Fix compilation error with new binutils". Ben. >
Hi, On 2022-07-14 15:25:44 +0200, Ben Hutchings wrote: > Thanks, I meant to send that fix upstream but got distracted. It > should really be folded into "tools perf: Fix compilation error with > new binutils". I'll try to send a new version out soon. I think the right process is to add Signed-off-by: Ben Hutchings <benh@debian.org> to the patch I squash it with? Greetings, Andres Freund
On Fri, 2022-07-15 at 12:16 -0700, Andres Freund wrote: > Hi, > > On 2022-07-14 15:25:44 +0200, Ben Hutchings wrote: > > Thanks, I meant to send that fix upstream but got distracted. It > > should really be folded into "tools perf: Fix compilation error with > > new binutils". > > I'll try to send a new version out soon. I think the right process is to add > Signed-off-by: Ben Hutchings <benh@debian.org> > to the patch I squash it with? Yes, OK. Ben.
Em Fri, Jul 15, 2022 at 12:16:41PM -0700, Andres Freund escreveu: > Hi, > > On 2022-07-14 15:25:44 +0200, Ben Hutchings wrote: > > Thanks, I meant to send that fix upstream but got distracted. It > > should really be folded into "tools perf: Fix compilation error with > > new binutils". > > I'll try to send a new version out soon. I think the right process is to add > Signed-off-by: Ben Hutchings <benh@debian.org> > to the patch I squash it with? Hi, How is this going? Any new patch coming soon? :-) - Arnaldo
Hi, On 2022-07-27 12:47:16 -0300, Arnaldo Carvalho de Melo wrote: > Em Fri, Jul 15, 2022 at 12:16:41PM -0700, Andres Freund escreveu: > > On 2022-07-14 15:25:44 +0200, Ben Hutchings wrote: > > > Thanks, I meant to send that fix upstream but got distracted. It > > > should really be folded into "tools perf: Fix compilation error with > > > new binutils". > > > > I'll try to send a new version out soon. I think the right process is to add > > Signed-off-by: Ben Hutchings <benh@debian.org> > > to the patch I squash it with? > > Hi, > > How is this going? Any new patch coming soon? :-) Sorry - had hoped to finish sending it out before my vacation (and then on the flight, but wifi didn't work...). Now back, will work on it asap. Greetings, Andres Freund
Hi, On 2022-07-05 00:12:37 +0200, Jiri Olsa wrote: > On Mon, Jul 04, 2022 at 01:19:22PM -0700, Andres Freund wrote: > > > 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 > > > > This I don't understand - why do we want these to run under NO_LIBBFD etc? > > when I was quickly testing that I did not have any of them detected > and got compile fail.. so I moved it to safe place ;-) it might be > placed in smarter place I think that's because you'd removed them from FEATURE_TESTS_BASIC in Makefile.feature. In v3 I just sent out I just removed them from FEATURE_DISPLAY, without any more "structural" changes in tools/perf/Makefile.config. Greetings, Andres Freund
Em Fri, Jul 15, 2022 at 09:18:26PM +0200, Ben Hutchings escreveu: > On Fri, 2022-07-15 at 12:16 -0700, Andres Freund wrote: > > Hi, > > > > On 2022-07-14 15:25:44 +0200, Ben Hutchings wrote: > > > Thanks, I meant to send that fix upstream but got distracted. It > > > should really be folded into "tools perf: Fix compilation error with > > > new binutils". > > > > I'll try to send a new version out soon. I think the right process is to add > > Signed-off-by: Ben Hutchings <benh@debian.org> > > to the patch I squash it with? > > Yes, OK. Ok, so you agreed on this, I'm adding Ben's Signed-off-by tag then, Thanks, - Arnaldo
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 tools/bpf/Makefile | 7 ++- tools/bpf/bpf_jit_disasm.c | 5 +- tools/bpf/bpftool/Makefile | 7 ++- tools/bpf/bpftool/jit_disasm.c | 42 ++++++++++++--- tools/build/Makefile.feature | 4 +- tools/build/feature/Makefile | 4 ++ tools/build/feature/test-all.c | 4 ++ .../feature/test-disassembler-init-styled.c | 13 +++++ tools/include/tools/dis-asm-compat.h | 53 +++++++++++++++++++ tools/perf/Makefile.config | 8 +++ tools/perf/util/annotate.c | 7 +-- 11 files changed, 137 insertions(+), 17 deletions(-) create mode 100644 tools/build/feature/test-disassembler-init-styled.c create mode 100644 tools/include/tools/dis-asm-compat.h