| Message ID | 20211010002400.9339-1-quentin@isovalent.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] bpf/preload: Clean up .gitignore and "clean-files" target | expand |
Quentin Monnet wrote: > kernel/bpf/preload/Makefile was recently updated to have it install > libbpf's headers locally instead of pulling them from tools/lib/bpf. But > two items still need to be addressed. > > First, the local .gitignore file was not adjusted to ignore the files > generated in the new kernel/bpf/preload/libbpf output directory. > > Second, the "clean-files" target is now incorrect. The old artefacts > names were not removed from the target, while the new ones were added > incorrectly. This is because "clean-files" expects names relative to > $(obj), but we passed the absolute path instead. This results in the > output and header-destination directories for libbpf (and their > contents) not being removed from kernel/bpf/preload on "make clean" from > the root of the repository. > > This commit fixes both issues. Note that $(userprogs) needs not be added > to "clean-files", because the cleaning infrastructure already accounts > for it. > > Cleaning the files properly also prevents make from printing the > following message, for builds coming after a "make clean": > "make[4]: Nothing to be done for 'install_headers'." > > Fixes: bf60791741d4 ("bpf: preload: Install libbpf headers when building") > Signed-off-by: Quentin Monnet <quentin@isovalent.com> > --- [...] Acked-by: John Fastabend <john.fastabend@gmail.com>
On Sat, Oct 9, 2021 at 5:24 PM Quentin Monnet <quentin@isovalent.com> wrote: > > kernel/bpf/preload/Makefile was recently updated to have it install > libbpf's headers locally instead of pulling them from tools/lib/bpf. But > two items still need to be addressed. > > First, the local .gitignore file was not adjusted to ignore the files > generated in the new kernel/bpf/preload/libbpf output directory. > > Second, the "clean-files" target is now incorrect. The old artefacts > names were not removed from the target, while the new ones were added > incorrectly. This is because "clean-files" expects names relative to > $(obj), but we passed the absolute path instead. This results in the > output and header-destination directories for libbpf (and their > contents) not being removed from kernel/bpf/preload on "make clean" from > the root of the repository. > > This commit fixes both issues. Note that $(userprogs) needs not be added > to "clean-files", because the cleaning infrastructure already accounts > for it. > > Cleaning the files properly also prevents make from printing the > following message, for builds coming after a "make clean": > "make[4]: Nothing to be done for 'install_headers'." > > Fixes: bf60791741d4 ("bpf: preload: Install libbpf headers when building") > Signed-off-by: Quentin Monnet <quentin@isovalent.com> > --- > kernel/bpf/preload/.gitignore | 4 +--- > kernel/bpf/preload/Makefile | 3 +-- > 2 files changed, 2 insertions(+), 5 deletions(-) > > diff --git a/kernel/bpf/preload/.gitignore b/kernel/bpf/preload/.gitignore > index 856a4c5ad0dd..9452322902a5 100644 > --- a/kernel/bpf/preload/.gitignore > +++ b/kernel/bpf/preload/.gitignore > @@ -1,4 +1,2 @@ > -/FEATURE-DUMP.libbpf > -/bpf_helper_defs.h > -/feature > +/libbpf > /bpf_preload_umd > diff --git a/kernel/bpf/preload/Makefile b/kernel/bpf/preload/Makefile > index 469d35e890eb..d8379af88161 100644 > --- a/kernel/bpf/preload/Makefile > +++ b/kernel/bpf/preload/Makefile > @@ -27,8 +27,7 @@ userccflags += -I $(srctree)/tools/include/ -I $(srctree)/tools/include/uapi \ > > userprogs := bpf_preload_umd > > -clean-files := $(userprogs) bpf_helper_defs.h FEATURE-DUMP.libbpf staticobjs/ feature/ > -clean-files += $(LIBBPF_OUT) $(LIBBPF_DESTDIR) > +clean-files := $(subst $(abspath $(obj))/,,$(LIBBPF_OUT) $(LIBBPF_DESTDIR)) why so complicated? also isn't LIBBPF_OUT and LIBBPF_DESTDIR the same? Wouldn't just this work and be super clear: clean-files: libbpf/ ? > > $(obj)/iterators/iterators.o: | libbpf_hdrs > > -- > 2.30.2 >
2021-10-19 16:50 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com> > On Sat, Oct 9, 2021 at 5:24 PM Quentin Monnet <quentin@isovalent.com> wrote: >> >> kernel/bpf/preload/Makefile was recently updated to have it install >> libbpf's headers locally instead of pulling them from tools/lib/bpf. But >> two items still need to be addressed. >> >> First, the local .gitignore file was not adjusted to ignore the files >> generated in the new kernel/bpf/preload/libbpf output directory. >> >> Second, the "clean-files" target is now incorrect. The old artefacts >> names were not removed from the target, while the new ones were added >> incorrectly. This is because "clean-files" expects names relative to >> $(obj), but we passed the absolute path instead. This results in the >> output and header-destination directories for libbpf (and their >> contents) not being removed from kernel/bpf/preload on "make clean" from >> the root of the repository. >> >> This commit fixes both issues. Note that $(userprogs) needs not be added >> to "clean-files", because the cleaning infrastructure already accounts >> for it. >> >> Cleaning the files properly also prevents make from printing the >> following message, for builds coming after a "make clean": >> "make[4]: Nothing to be done for 'install_headers'." >> >> Fixes: bf60791741d4 ("bpf: preload: Install libbpf headers when building") >> Signed-off-by: Quentin Monnet <quentin@isovalent.com> >> --- >> kernel/bpf/preload/.gitignore | 4 +--- >> kernel/bpf/preload/Makefile | 3 +-- >> 2 files changed, 2 insertions(+), 5 deletions(-) >> >> diff --git a/kernel/bpf/preload/.gitignore b/kernel/bpf/preload/.gitignore >> index 856a4c5ad0dd..9452322902a5 100644 >> --- a/kernel/bpf/preload/.gitignore >> +++ b/kernel/bpf/preload/.gitignore >> @@ -1,4 +1,2 @@ >> -/FEATURE-DUMP.libbpf >> -/bpf_helper_defs.h >> -/feature >> +/libbpf >> /bpf_preload_umd >> diff --git a/kernel/bpf/preload/Makefile b/kernel/bpf/preload/Makefile >> index 469d35e890eb..d8379af88161 100644 >> --- a/kernel/bpf/preload/Makefile >> +++ b/kernel/bpf/preload/Makefile >> @@ -27,8 +27,7 @@ userccflags += -I $(srctree)/tools/include/ -I $(srctree)/tools/include/uapi \ >> >> userprogs := bpf_preload_umd >> >> -clean-files := $(userprogs) bpf_helper_defs.h FEATURE-DUMP.libbpf staticobjs/ feature/ >> -clean-files += $(LIBBPF_OUT) $(LIBBPF_DESTDIR) >> +clean-files := $(subst $(abspath $(obj))/,,$(LIBBPF_OUT) $(LIBBPF_DESTDIR)) > > why so complicated? also isn't LIBBPF_OUT and LIBBPF_DESTDIR the same? They're the same. My reasoning was that since we had these two variables for output directories, it felt logical somehow to add both, in case one changes in the future. > Wouldn't just this work and be super clear: > > clean-files: libbpf/ It does look simpler. OK I'll change and submit a new version. Thanks, Quentin
diff --git a/kernel/bpf/preload/.gitignore b/kernel/bpf/preload/.gitignore index 856a4c5ad0dd..9452322902a5 100644 --- a/kernel/bpf/preload/.gitignore +++ b/kernel/bpf/preload/.gitignore @@ -1,4 +1,2 @@ -/FEATURE-DUMP.libbpf -/bpf_helper_defs.h -/feature +/libbpf /bpf_preload_umd diff --git a/kernel/bpf/preload/Makefile b/kernel/bpf/preload/Makefile index 469d35e890eb..d8379af88161 100644 --- a/kernel/bpf/preload/Makefile +++ b/kernel/bpf/preload/Makefile @@ -27,8 +27,7 @@ userccflags += -I $(srctree)/tools/include/ -I $(srctree)/tools/include/uapi \ userprogs := bpf_preload_umd -clean-files := $(userprogs) bpf_helper_defs.h FEATURE-DUMP.libbpf staticobjs/ feature/ -clean-files += $(LIBBPF_OUT) $(LIBBPF_DESTDIR) +clean-files := $(subst $(abspath $(obj))/,,$(LIBBPF_OUT) $(LIBBPF_DESTDIR)) $(obj)/iterators/iterators.o: | libbpf_hdrs
kernel/bpf/preload/Makefile was recently updated to have it install libbpf's headers locally instead of pulling them from tools/lib/bpf. But two items still need to be addressed. First, the local .gitignore file was not adjusted to ignore the files generated in the new kernel/bpf/preload/libbpf output directory. Second, the "clean-files" target is now incorrect. The old artefacts names were not removed from the target, while the new ones were added incorrectly. This is because "clean-files" expects names relative to $(obj), but we passed the absolute path instead. This results in the output and header-destination directories for libbpf (and their contents) not being removed from kernel/bpf/preload on "make clean" from the root of the repository. This commit fixes both issues. Note that $(userprogs) needs not be added to "clean-files", because the cleaning infrastructure already accounts for it. Cleaning the files properly also prevents make from printing the following message, for builds coming after a "make clean": "make[4]: Nothing to be done for 'install_headers'." Fixes: bf60791741d4 ("bpf: preload: Install libbpf headers when building") Signed-off-by: Quentin Monnet <quentin@isovalent.com> --- kernel/bpf/preload/.gitignore | 4 +--- kernel/bpf/preload/Makefile | 3 +-- 2 files changed, 2 insertions(+), 5 deletions(-)