netdev - Re: [PATCH bpf-next v3] tools/bpf: generate pkg-config file for libbpf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190320235800.GA13709@rdna-mbp>
Date:   Wed, 20 Mar 2019 23:58:03 +0000
From:   Andrey Ignatov <rdna@...com>
To:     "luca.boccassi@...il.com" <luca.boccassi@...il.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "sdf@...ichev.me" <sdf@...ichev.me>
Subject: Re: [PATCH bpf-next v3] tools/bpf: generate pkg-config file for
 libbpf

luca.boccassi@...il.com <luca.boccassi@...il.com> [Wed, 2019-03-20 06:28 -0700]:
> From: Luca Boccassi <bluca@...ian.org>
> 
> Generate a libbpf.pc file at build time so that users can rely
> on pkg-config to find the library, its CFLAGS and LDFLAGS.
> 
> Signed-off-by: Luca Boccassi <bluca@...ian.org>
> ---
> v2: use QUIET_GEN instead of QUIET_LINK to generate pc file,
>     save kernel version in its own variable instead of calling
>     make inline
> v3: use LIBBPF_VERSION instead of kernel_version
> 
>  tools/lib/bpf/.gitignore         |  1 +
>  tools/lib/bpf/Makefile           | 18 +++++++++++++++---
>  tools/lib/bpf/libbpf.pc.template | 11 +++++++++++
>  3 files changed, 27 insertions(+), 3 deletions(-)
>  create mode 100644 tools/lib/bpf/libbpf.pc.template
> 
> diff --git a/tools/lib/bpf/.gitignore b/tools/lib/bpf/.gitignore
> index 4db74758c674..7d9e182a1f51 100644
> --- a/tools/lib/bpf/.gitignore
> +++ b/tools/lib/bpf/.gitignore
> @@ -1,3 +1,4 @@
>  libbpf_version.h
> +libbpf.pc
>  FEATURE-DUMP.libbpf
>  test_libbpf
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index 61aaacf0cfa1..891fe3da1410 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile

Commenting here rather than on v1/v2 discussion.

Makefile on the github mirror is completely different one:
https://github.com/libbpf/libbpf/blob/master/src/Makefile

It was done like this to avoid the need to patch copied Makefile, mirror
as few files as possible and avoid things like kernel & tools UAPI
comparison, "feature" tests and other kernel-tree-only stuff that is not
applicable to out-of-tree code.

I.e. if somebody uses libbpf mirror and wants to support pkg-config they
would need to make similar changes there.

Just wondering if you plan to create a package using kernel tree?

> @@ -80,6 +80,7 @@ libdir_SQ = $(subst ','\'',$(libdir))
>  libdir_relative_SQ = $(subst ','\'',$(libdir_relative))
>  
>  LIB_FILE = libbpf.a libbpf.so
> +PC_FILE = libbpf.pc
>  
>  VERSION		= $(BPF_VERSION)
>  PATCHLEVEL	= $(BPF_PATCHLEVEL)
> @@ -137,7 +138,7 @@ GLOBAL_SYM_COUNT = $(shell readelf -s --wide $(BPF_IN) | \
>  VERSIONED_SYM_COUNT = $(shell readelf -s --wide $(OUTPUT)libbpf.so | \
>  			      grep -Eo '[^ ]+@...BPF_' | cut -d@ -f1 | sort -u | wc -l)
>  
> -CMD_TARGETS = $(LIB_FILE)
> +CMD_TARGETS = $(LIB_FILE) $(PC_FILE)
>  
>  CXX_TEST_TARGET = $(OUTPUT)test_libbpf
>  
> @@ -180,6 +181,12 @@ $(OUTPUT)libbpf.a: $(BPF_IN)
>  $(OUTPUT)test_libbpf: test_libbpf.cpp $(OUTPUT)libbpf.a
>  	$(QUIET_LINK)$(CXX) $(INCLUDES) $^ -lelf -o $@
>  
> +$(OUTPUT)libbpf.pc:
> +	$(QUIET_GEN)sed -e "s|@...FIX@|$(prefix)|" \
> +		-e "s|@...DIR@|$(libdir_SQ)|" \
> +		-e "s|@...SION@|$(LIBBPF_VERSION)|" \
> +		< $@...mplate > $@
> +
>  check: check_abi
>  
>  check_abi: $(OUTPUT)libbpf.so
> @@ -209,7 +216,12 @@ install_headers:
>  		$(call do_install,libbpf.h,$(prefix)/include/bpf,644);
>  		$(call do_install,btf.h,$(prefix)/include/bpf,644);
>  
> -install: install_lib
> +install_pkgconfig: $(PC_FILE)
> +	$(call QUIET_INSTALL, $(PC_FILE)) \
> +		$(call do_install,$(PC_FILE),$(libdir_SQ)/pkgconfig,644)
> +
> +
> +install: install_lib install_pkgconfig

Should it be part of 'install'? Not everyone needs pc file.  Also pc
template below relies on headers to be installed along with the library
itself, but headers are not installed as part of 'install' target,
they're in a separate 'install_headers' one, so 'install_pkgconfig' and
'install_headers' should probably be consistent in this regard.


>  ### Cleaning rules
>  
> @@ -219,7 +231,7 @@ config-clean:
>  
>  clean:
>  	$(call QUIET_CLEAN, libbpf) $(RM) $(TARGETS) $(CXX_TEST_TARGET) \
> -		*.o *~ *.a *.so .*.d .*.cmd LIBBPF-CFLAGS
> +		*.o *~ *.a *.so .*.d .*.cmd *.pc LIBBPF-CFLAGS
>  	$(call QUIET_CLEAN, core-gen) $(RM) $(OUTPUT)FEATURE-DUMP.libbpf
>  
>  
> diff --git a/tools/lib/bpf/libbpf.pc.template b/tools/lib/bpf/libbpf.pc.template
> new file mode 100644
> index 000000000000..0cac2f5f54a6
> --- /dev/null
> +++ b/tools/lib/bpf/libbpf.pc.template
> @@ -0,0 +1,11 @@
> +prefix=@...FIX@
> +libdir=@...DIR@
> +includedir=${prefix}/include/bpf

I guess it should be includedir=${prefix}/include.

Paths in '#include' are usually provided relative to ${prefix}/include,
e.g. all programs I have access to and that use libbpf do this:

  #include <bpf/bpf.h>
  #include <bpf/libbpf.h>


But `includedir=${prefix}/include/bpf` will force it to be:
  #include <bpf.h>
  #include <libbpf.h>

, what may create inconsistency with already written code.

I also checked .pc files on my devbox and this is what I see:

  % grep -h includedir /usr/share/pkgconfig/*.pc | sort | uniq -c
       28 Cflags: -I${includedir}
        1 Cflags: -I${includedir}/X11/dri
       29 includedir=/usr/include


> +
> +Name: libbpf
> +URL: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> +Description: Linux kernel BPF library
> +Version: @VERSION@
> +Libs: -L${libdir} -lbpf
> +Requires.private: libelf
> +Cflags: -I${includedir}
> -- 
> 2.20.1
> 

-- 
Andrey Ignatov