Re: [PATCH bpf-next] libbpf: make libbpf.map source of truth for libbpf version

[Andrey Ignatov <rdna@fb.com>]

Andrii Nakryiko <andriin@fb.com> [Tue, 2019-08-13 16:24 -0700]:
> Currently libbpf version is specified in 2 places: libbpf.map and
> Makefile. They easily get out of sync and it's very easy to update one,
> but forget to update another one. In addition, Github projection of
> libbpf has to maintain its own version which has to be remembered to be
> kept in sync manually, which is very error-prone approach.
> 
> This patch makes libbpf.map a source of truth for libbpf version and
> uses shell invocation to parse out correct full and major libbpf version
> to use during build. Now we need to make sure that once new release
> cycle starts, we need to add (initially) empty section to libbpf.map
> with correct latest version.
> 
> This also will make it possible to keep Github projection consistent
> with kernel sources version of libbpf by adopting similar parsing of
> version from libbpf.map.

Thanks for taking care of this!


> Cc: Andrey Ignatov <rdna@fb.com>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  tools/lib/bpf/Makefile   | 12 +++++-------
>  tools/lib/bpf/libbpf.map |  3 +++
>  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index 9312066a1ae3..d9afc8509725 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -1,9 +1,10 @@
>  # SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
>  # Most of this file is copied from tools/lib/traceevent/Makefile
>  
> -BPF_VERSION = 0
> -BPF_PATCHLEVEL = 0
> -BPF_EXTRAVERSION = 4
> +BPF_FULL_VERSION = $(shell \

Nit: Should it be LIBBPF_VERSION? IMO it's more descriptive name.

> +	grep -E 'LIBBPF_([0-9]+)\.([0-9]+)\.([0-9]+) \{' libbpf.map | \
> +	tail -n1 | cut -d'_' -f2 | cut -d' ' -f1)

It can be done simpler and IMO versions should be sorted before taking
the last one (just in case), something like:

grep -oE '^LIBBPF_[0-9.]+' libbpf.map | cut -d_ -f 2 | sort -nr | head -n 1


> +BPF_VERSION = $(firstword $(subst ., ,$(BPF_FULL_VERSION)))
>  
>  MAKEFLAGS += --no-print-directory
>  
> @@ -79,15 +80,12 @@ export prefix libdir src obj
>  libdir_SQ = $(subst ','\'',$(libdir))
>  libdir_relative_SQ = $(subst ','\'',$(libdir_relative))
>  
> +LIBBPF_VERSION	= $(BPF_FULL_VERSION)
>  VERSION		= $(BPF_VERSION)
> -PATCHLEVEL	= $(BPF_PATCHLEVEL)
> -EXTRAVERSION	= $(BPF_EXTRAVERSION)
>  
>  OBJ		= $@
>  N		=
>  
> -LIBBPF_VERSION	= $(BPF_VERSION).$(BPF_PATCHLEVEL).$(BPF_EXTRAVERSION)
> -
>  LIB_TARGET	= libbpf.a libbpf.so.$(LIBBPF_VERSION)
>  LIB_FILE	= libbpf.a libbpf.so*
>  PC_FILE		= libbpf.pc
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index f9d316e873d8..4e72df8e98ba 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -184,3 +184,6 @@ LIBBPF_0.0.4 {
>  		perf_buffer__new_raw;
>  		perf_buffer__poll;
>  } LIBBPF_0.0.3;
> +
> +LIBBPF_0.0.5 {
> +} LIBBPF_0.0.4;

I'm not sure version should be bumped in this patch since this patch is
about keeping the version in one place, not about bumping it, right?


> -- 
> 2.17.1
> 

-- 
Andrey Ignatov