[PATCH bpf-next] bpf/preload: Clean up .gitignore and "clean-files" target

[PATCH bpf-next] bpf/preload: Clean up .gitignore and "clean-files" target

[Quentin Monnet]

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))
 
 $(obj)/iterators/iterators.o: | libbpf_hdrs
 
-- 
2.30.2

RE: [PATCH bpf-next] bpf/preload: Clean up .gitignore and "clean-files" target

[John Fastabend]

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>

Re: [PATCH bpf-next] bpf/preload: Clean up .gitignore and "clean-files" target

[Andrii Nakryiko]

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
>

Re: [PATCH bpf-next] bpf/preload: Clean up .gitignore and "clean-files" target

[Quentin Monnet]

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