Re: [PATCH v3 bpf-next 11/14] libbpf: makefile: add C/CXX/LDFLAGS to libbpf.so and test_libpf targets

[Ivan Khoronzhuk]

On Tue, Sep 17, 2019 at 10:19:22PM -0700, Andrii Nakryiko wrote:
> On Mon, Sep 16, 2019 at 4:00 AM Ivan Khoronzhuk
> <ivan.khoronzhuk@xxxxxxxxxx> wrote:
> > In case of LDFLAGS and EXTRA_CC/CXX flags there is no way to pass them
> > correctly to build command, for instance when --sysroot is used or
> > external libraries are used, like -lelf, wich can be absent in
> > toolchain. This can be used for samples/bpf cross-compiling allowing
> > to get elf lib from sysroot.
> >
> > Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@xxxxxxxxxx>
> > ---
> >  tools/lib/bpf/Makefile | 11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> > index c6f94cffe06e..bccfa556ef4e 100644
> > --- a/tools/lib/bpf/Makefile
> > +++ b/tools/lib/bpf/Makefile
> > @@ -94,6 +94,10 @@ else
> >    CFLAGS := -g -Wall
> >  endif
> >
> > +ifdef EXTRA_CXXFLAGS
> > +  CXXFLAGS := $(EXTRA_CXXFLAGS)
> > +endif
> > +
> >  ifeq ($(feature-libelf-mmap), 1)
> >    override CFLAGS += -DHAVE_LIBELF_MMAP_SUPPORT
> >  endif
> > @@ -176,8 +180,9 @@ $(BPF_IN): force elfdep bpfdep
> >  $(OUTPUT)libbpf.so: $(OUTPUT)libbpf.so.$(LIBBPF_VERSION)
> >
> >  $(OUTPUT)libbpf.so.$(LIBBPF_VERSION): $(BPF_IN)
> > -       $(QUIET_LINK)$(CC) --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
> > -                                   -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
> > +       $(QUIET_LINK)$(CC) $(LDFLAGS) \
> > +               --shared -Wl,-soname,libbpf.so.$(LIBBPF_MAJOR_VERSION) \
> > +               -Wl,--version-script=$(VERSION_SCRIPT) $^ -lelf -o $@
> >         @ln -sf $(@F) $(OUTPUT)libbpf.so
> >         @ln -sf $(@F) $(OUTPUT)libbpf.so.$(LIBBPF_MAJOR_VERSION)
> >
> > @@ -185,7 +190,7 @@ $(OUTPUT)libbpf.a: $(BPF_IN)
> >         $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
> >
> >  $(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a
> > -       $(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@
> > +       $(QUIET_LINK)$(CXX) $(CXXFLAGS) $(LDFLAGS) $(INCLUDES) $^ -lelf -o $@
>
> Instead of doing ifdef EXTRA_CXXFLAGS bit above, you can just include
> both $(CXXFLAGS) and $(EXTRA_CXXFLAGS), which will do the right thing
> (and is actually recommended my make documentation way to do this).
It's good practice to follow existent style, I've done similar way as for
CFLAGS + EXTRACFLAGS here, didn't want to verify it can impact on
smth else. And my goal is not to correct everything but embed my
functionality, series tool large w/o it.

> But actually, there is no need to use C++ compiler here,
> test_libbpf.cpp can just be plain C. Do you mind renaming it to .c and
> using C compiler instead?
Seems like, will try in next v.

> >  $(OUTPUT)libbpf.pc:
> >         $(QUIET_GEN)sed -e "s|@PREFIX@|$(prefix)|" \
> > --
> > 2.17.1

--
Regards,
Ivan Khoronzhuk