Re: [PATCH] libbpf: Fix build on read-only filesystems

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andriin@fb.com>,
	bpf <bpf@vger.kernel.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Jiri Olsa <jolsa@redhat.com>, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] libbpf: Fix build on read-only filesystems
Date: Sat, 21 Dec 2019 08:22:00 -0800	[thread overview]
Message-ID: <20191221162158.rw6xqqktubozg6fg@ast-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <CAM9d7cg0A0+Oq5uDS6ZJNzAgFsWc-Pd30GYC0+PxEXdcxAxBKg@mail.gmail.com>

On Sat, Dec 21, 2019 at 05:25:51PM +0900, Namhyung Kim wrote:
> Hello,
> 
> On Sat, Dec 21, 2019 at 5:29 AM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Thu, Dec 19, 2019 at 7:26 PM Namhyung Kim <namhyung@kernel.org> wrote:
> > >
> > > I got the following error when I tried to build perf on a read-only
> > > filesystem with O=dir option.
> > >
> > >   $ cd /some/where/ro/linux/tools/perf
> > >   $ make O=$HOME/build/perf
> > >   ...
> > >     CC       /home/namhyung/build/perf/lib.o
> > >   /bin/sh: bpf_helper_defs.h: Read-only file system
> > >   make[3]: *** [Makefile:184: bpf_helper_defs.h] Error 1
> > >   make[2]: *** [Makefile.perf:778: /home/namhyung/build/perf/libbpf.a] Error 2
> > >   make[2]: *** Waiting for unfinished jobs....
> > >     LD       /home/namhyung/build/perf/libperf-in.o
> > >     AR       /home/namhyung/build/perf/libperf.a
> > >     PERF_VERSION = 5.4.0
> > >   make[1]: *** [Makefile.perf:225: sub-make] Error 2
> > >   make: *** [Makefile:70: all] Error 2
> > >
> > > It was becaused bpf_helper_defs.h was generated in current directory.
> > > Move it to OUTPUT directory.
> > >
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > ---
> >
> > Overall nothing is obviously broken, except you need to fix up
> > selftests/bpf's Makefile as well.
> 
> Thanks for pointing this out.  It's because bpf selftest also needs the
> bpf_helper_defs.h right?  But I'm currently having a problem with LLVM
> when building the selftests.  Can you help me testing the patch below?
> (It should be applied after this patch.  Are you ok with it?)
> 
> 
> >
> > BTW, this patch doesn't apply cleanly to latest bpf-next, so please rebase.
> >
> > Also subject prefix should look like [PATCH bpf-next] if it's meant to
> > be applied against bpf-next.
> 
> Will do.
> 
> Thanks
> Namhyung
> 
> -----------8<-------------
> diff --git a/tools/testing/selftests/bpf/Makefile
> b/tools/testing/selftests/bpf/Makefile
> index 866fc1cadd7c..897877f7849b 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -151,9 +151,9 @@ $(DEFAULT_BPFTOOL): force
>  $(BPFOBJ): force
>         $(MAKE) -C $(BPFDIR) OUTPUT=$(OUTPUT)/
> 
> -BPF_HELPERS := $(BPFDIR)/bpf_helper_defs.h $(wildcard $(BPFDIR)/bpf_*.h)
> -$(BPFDIR)/bpf_helper_defs.h:
> -       $(MAKE) -C $(BPFDIR) OUTPUT=$(OUTPUT)/ bpf_helper_defs.h
> +BPF_HELPERS := $(OUTPUT)/bpf_helper_defs.h $(wildcard $(BPFDIR)/bpf_*.h)
> +$(OUTPUT)/bpf_helper_defs.h:
> +       $(MAKE) -C $(BPFDIR) OUTPUT=$(OUTPUT)/ $(OUTPUT)/bpf_helper_defs.h

The fix makes sense, but you cannot break it down into two patches.
The selftests/bpf are absolutely essential for everyone working on bpf.
For both developers and maintainers. You cannot break them in one patch
and then try to fix in another.
Please resubmit as one patch and tag the subject as [PATCH bpf].