On Tue, Aug 13, 2019 at 5:28 PM Andrey Ignatov <rdna@xxxxxx> wrote:
>
> Andrii Nakryiko <andriin@xxxxxx> [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@xxxxxx>
> > Signed-off-by: Andrii Nakryiko <andriin@xxxxxx>
> > ---
> > 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.
LIBBPF_VERSION is used below, but combining your suggestion with
Jakub's eager evaluation, I can use just LIBBPF_VERSION and drop
BPF_FULL_VERSION altogether.
>
> > + 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
Ah, you mean making regex simpler? Yeah, I originally intended to
extract major, patch, and extra version, but ralized patch and extra
are not used for anything. I'll simplify regex. But second `cut -d' '
-f1` is still needed to drop " {".
Regarding sorting. I don't think it's necessary, as I can't imagine
having non-ordered libbpf.map. Even more so, sort -nr doesn't sort
versions like these correctly anyway:
0.1.2
0.1.12
So this will just give us false sense of correctness, while being a "time bomb".
>
>
> > +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?
This is actually fixing a version. Current libbpf version in bpf-next
is 0.0.5, it just was never updated in Makefile.
>
>
> > --
> > 2.17.1
> >
>
> --
> Andrey Ignatov
