Re: [PATCH] perf build: Fix build error when NO_LIBBPF=1

From: Ian Rogers <irogers@google.com>
To: Mike Leach <mike.leach@linaro.org>
Cc: linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	bpf@vger.kernel.org, acme@kernel.org, peterz@infradead.org,
	mingo@redhat.com, mark.rutland@arm.com,
	alexander.shishkin@linux.intel.com, jolsa@kernel.org,
	namhyung@kernel.org
Subject: Re: [PATCH] perf build: Fix build error when NO_LIBBPF=1
Date: Thu, 5 Jan 2023 11:02:48 -0800	[thread overview]
Message-ID: <CAP-5=fVbPVE4rxJ2s8phhJ5RRH4EnKaWrF2kaT0oCmK6kvhP2g@mail.gmail.com> (raw)
In-Reply-To: <20230105172243.7238-1-mike.leach@linaro.org>

On Thu, Jan 5, 2023 at 9:22 AM Mike Leach <mike.leach@linaro.org> wrote:
>
> Recent updates to perf build result in the following output when cross
> compiling to aarch64, with libelf unavailable, and therefore
> NO_LIBBPF=1 set.
>
> ```
>   $make -C tools/perf
>
>   <cut>
>
>   Makefile.config:428: No libelf found. Disables 'probe' tool, jvmti
>   and BPF support in 'perf record'. Please install libelf-dev,
>   libelf-devel or elfutils-libelf-devel
>
>   <cut>
>
>   libbpf.c:46:10: fatal error: libelf.h: No such file or directory
>       46 | #include <libelf.h>
>          |          ^~~~~~~~~~
>   compilation terminated.
>
>   ./tools/build/Makefile.build:96: recipe for target
>   '.tools/perf/libbpf/staticobjs/libbpf.o' failed
>
> ```
>
> plus one other include error for <gelf.h>

Ouch, apologies for the breakage. You wouldn't happen to have
something like a way with say a docker image to repro the problem? The
make line above is somewhat minimal.

> The issue is that the commit noted below adds libbpf to the prepare:
> target but no longer accounts for the NO_LIBBPF define. Additionally
> changing the include directories means that even if the libbpf target
> build is prevented, bpf headers are missing in other parts of the build.
>
> This patch ensures that in the case of NO_LIBBPF=1, the build target is
> changed to a header only target, and the headers are installed, without
> attempting to build the libbpf.a target.
>
> Applies to perf/core
>
> Fixes: 746bd29e348f ("perf build: Use tools/lib headers from install path")
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> ---
>  tools/perf/Makefile.perf | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 13e7d26e77f0..ee08ecf469f6 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -305,7 +305,11 @@ else
>  endif
>  LIBBPF_DESTDIR = $(LIBBPF_OUTPUT)
>  LIBBPF_INCLUDE = $(LIBBPF_DESTDIR)/include
> +ifndef NO_LIBBPF
>  LIBBPF = $(LIBBPF_OUTPUT)/libbpf.a
> +else
> +LIBBPF = $(LIBBPF_INCLUDE)/bpf/bpf.h

This seems strange, don't we want to avoid libbpf targets?

> +endif
>  CFLAGS += -I$(LIBBPF_OUTPUT)/include
>
>  ifneq ($(OUTPUT),)
> @@ -826,10 +830,16 @@ $(LIBAPI)-clean:
>         $(call QUIET_CLEAN, libapi)
>         $(Q)$(RM) -r -- $(LIBAPI_OUTPUT)
>
> +ifndef NO_LIBBPF
>  $(LIBBPF): FORCE | $(LIBBPF_OUTPUT)
>         $(Q)$(MAKE) -C $(LIBBPF_DIR) FEATURES_DUMP=$(FEATURE_DUMP_EXPORT) \
>                 O= OUTPUT=$(LIBBPF_OUTPUT)/ DESTDIR=$(LIBBPF_DESTDIR) prefix= \
>                 $@ install_headers
> +else
> +$(LIBBPF): FORCE | $(LIBBPF_OUTPUT)
> +       $(Q)$(MAKE) -C $(LIBBPF_DIR) OUTPUT=$(LIBBPF_OUTPUT)/ \
> +               DESTDIR=$(LIBBPF_DESTDIR) prefix= install_headers
> +endif

Shouldn't we just be able to conditionalize having $(LIBBPF) as a
dependency for the perf binary? If there is no dependency then the
targets won't be built and we shouldn't need to conditionalize here.

Thanks!
Ian

>  $(LIBBPF)-clean:
>         $(call QUIET_CLEAN, libbpf)
> --
> 2.17.1
>