| Message ID | 20190115195953.238574-1-sdf@google.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf] tools/bpf: properly account for libbfd variations | expand |
On Tue, 15 Jan 2019 11:59:53 -0800, Stanislav Fomichev wrote: > On some platforms, in order to link against libbfd, we need to > link against liberty and even possibly libz. Account for that > in the bpftool Makefile. We now have proper feature detection > for each case, so handle each one separately. > > See recent commit 14541b1e7e72 ("perf build: Don't unconditionally link the > libbfd feature test to -liberty and -lz") where I fixed feature > detection. > > Fixes: 29a9c10e4110 ("bpftool: make libbfd optional") > Signed-off-by: Stanislav Fomichev <sdf@google.com> Minor nits below, in any case: Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com> Thanks for making bpftool build! :) > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile > index 492f0f24e2d3..af9a25bf480d 100644 > --- a/tools/bpf/bpftool/Makefile > +++ b/tools/bpf/bpftool/Makefile > @@ -92,10 +92,21 @@ BFD_SRCS = jit_disasm.c > > SRCS = $(filter-out $(BFD_SRCS),$(wildcard *.c)) > > -ifeq ($(feature-libbfd),1) > +ifeq ($(feature-libbfd), 1) nit: no space there is more common $ git grep 'ifeq' | grep ',[^ ]' | wc -l 482 $ git grep 'ifeq' | grep ', ' | wc -l 136 > +LIBS += -lbfd -ldl -lopcodes nit: should this be indented? > +else > + ifeq ($(feature-libbfd-liberty), 1) > + LIBS += -lbfd -ldl -lopcodes -liberty > + else > + ifeq ($(feature-libbfd-liberty-z), 1) > + LIBS += -lbfd -ldl -lopcodes -liberty -lz > + endif Would this syntax: ifeq ($(feature-libbfd),1) LIBS += .. else ifeq ($(feature-libbfd-liberty),1) LIBS += .. else ifeq ($(feature-libbfd-liberty-z),1) LIBS += .. endif Not work? https://www.gnu.org/software/make/manual/html_node/Conditional-Syntax.html I don't think I've ever tried, but looks more concise.. > + endif > +endif > + > +ifneq ($(filter -lbfd,$(EXTLIBS)),) > CFLAGS += -DHAVE_LIBBFD_SUPPORT > SRCS += $(BFD_SRCS) > -LIBS += -lbfd -lopcodes > endif > > OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
On 01/15, Jakub Kicinski wrote: > On Tue, 15 Jan 2019 11:59:53 -0800, Stanislav Fomichev wrote: > > On some platforms, in order to link against libbfd, we need to > > link against liberty and even possibly libz. Account for that > > in the bpftool Makefile. We now have proper feature detection > > for each case, so handle each one separately. > > > > See recent commit 14541b1e7e72 ("perf build: Don't unconditionally link the > > libbfd feature test to -liberty and -lz") where I fixed feature > > detection. > > > > Fixes: 29a9c10e4110 ("bpftool: make libbfd optional") > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > > Minor nits below, in any case: > > Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com> > > Thanks for making bpftool build! :) > > > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile > > index 492f0f24e2d3..af9a25bf480d 100644 > > --- a/tools/bpf/bpftool/Makefile > > +++ b/tools/bpf/bpftool/Makefile > > @@ -92,10 +92,21 @@ BFD_SRCS = jit_disasm.c > > > > SRCS = $(filter-out $(BFD_SRCS),$(wildcard *.c)) > > > > -ifeq ($(feature-libbfd),1) > > +ifeq ($(feature-libbfd), 1) > > nit: no space there is more common > > $ git grep 'ifeq' | grep ',[^ ]' | wc -l > 482 > $ git grep 'ifeq' | grep ', ' | wc -l > 136 > > > > +LIBS += -lbfd -ldl -lopcodes > > nit: should this be indented? > > > +else > > + ifeq ($(feature-libbfd-liberty), 1) > > + LIBS += -lbfd -ldl -lopcodes -liberty > > + else > > + ifeq ($(feature-libbfd-liberty-z), 1) > > + LIBS += -lbfd -ldl -lopcodes -liberty -lz > > + endif > > Would this syntax: > > ifeq ($(feature-libbfd),1) > LIBS += .. > else ifeq ($(feature-libbfd-liberty),1) > LIBS += .. > else ifeq ($(feature-libbfd-liberty-z),1) > LIBS += .. > endif > > Not work? I does seem to work :-) This is the first time I see it being written that way. (but your link indeed shows that this syntax is supported) I'll post v2 shortly with all the nits addressed. Thanks for a quick review! > > https://www.gnu.org/software/make/manual/html_node/Conditional-Syntax.html > > I don't think I've ever tried, but looks more concise.. > > > + endif > > +endif > > + > > +ifneq ($(filter -lbfd,$(EXTLIBS)),) > > CFLAGS += -DHAVE_LIBBFD_SUPPORT > > SRCS += $(BFD_SRCS) > > -LIBS += -lbfd -lopcodes > > endif > > > > OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o >
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile index 492f0f24e2d3..af9a25bf480d 100644 --- a/tools/bpf/bpftool/Makefile +++ b/tools/bpf/bpftool/Makefile @@ -92,10 +92,21 @@ BFD_SRCS = jit_disasm.c SRCS = $(filter-out $(BFD_SRCS),$(wildcard *.c)) -ifeq ($(feature-libbfd),1) +ifeq ($(feature-libbfd), 1) +LIBS += -lbfd -ldl -lopcodes +else + ifeq ($(feature-libbfd-liberty), 1) + LIBS += -lbfd -ldl -lopcodes -liberty + else + ifeq ($(feature-libbfd-liberty-z), 1) + LIBS += -lbfd -ldl -lopcodes -liberty -lz + endif + endif +endif + +ifneq ($(filter -lbfd,$(EXTLIBS)),) CFLAGS += -DHAVE_LIBBFD_SUPPORT SRCS += $(BFD_SRCS) -LIBS += -lbfd -lopcodes endif OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
On some platforms, in order to link against libbfd, we need to link against liberty and even possibly libz. Account for that in the bpftool Makefile. We now have proper feature detection for each case, so handle each one separately. See recent commit 14541b1e7e72 ("perf build: Don't unconditionally link the libbfd feature test to -liberty and -lz") where I fixed feature detection. Fixes: 29a9c10e4110 ("bpftool: make libbfd optional") Signed-off-by: Stanislav Fomichev <sdf@google.com> --- tools/bpf/bpftool/Makefile | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)