diff mbox series

[bpf-next,v4,01/12] libbpf: skip re-installing headers file if source is older than target

Message ID 20211007194438.34443-2-quentin@isovalent.com (mailing list archive)
State Accepted
Delegated to: BPF
Headers show
Series install libbpf headers when using the library | expand

Checks

Context Check Description
netdev/cover_letter success Series has a cover letter
netdev/fixes_present success Fixes tag not required for -next series
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for bpf-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 5 maintainers not CCed: kafai@fb.com yhs@fb.com john.fastabend@gmail.com songliubraving@fb.com kpsingh@kernel.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success No Fixes tag
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 32 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success No static functions without inline keyword in header files
bpf/vmtest-bpf-next success VM_Test
bpf/vmtest-bpf-next-PR success PR summary

Commit Message

Quentin Monnet Oct. 7, 2021, 7:44 p.m. UTC
The "install_headers" target in libbpf's Makefile would unconditionally
export all API headers to the target directory. When those headers are
installed to compile another application, this means that make always
finds newer dependencies for the source files relying on those headers,
and deduces that the targets should be rebuilt.

Avoid that by making "install_headers" depend on the source header
files, and (re-)install them only when necessary.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
 tools/lib/bpf/Makefile | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Comments

Andrii Nakryiko Oct. 8, 2021, 7:13 p.m. UTC | #1
On Thu, Oct 7, 2021 at 12:44 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> The "install_headers" target in libbpf's Makefile would unconditionally
> export all API headers to the target directory. When those headers are
> installed to compile another application, this means that make always
> finds newer dependencies for the source files relying on those headers,
> and deduces that the targets should be rebuilt.
>
> Avoid that by making "install_headers" depend on the source header
> files, and (re-)install them only when necessary.
>
> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
> ---
>  tools/lib/bpf/Makefile | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index 41e4f78dbad5..a92d3b9692a8 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -241,15 +241,23 @@ install_lib: all_cmd
>                 $(call do_install_mkdir,$(libdir_SQ)); \
>                 cp -fpR $(LIB_FILE) $(DESTDIR)$(libdir_SQ)
>
> -INSTALL_HEADERS = bpf.h libbpf.h btf.h libbpf_common.h libbpf_legacy.h xsk.h \
> -                 bpf_helpers.h $(BPF_GENERATED) bpf_tracing.h               \
> -                 bpf_endian.h bpf_core_read.h skel_internal.h               \
> -                 libbpf_version.h
> +SRC_HDRS := bpf.h libbpf.h btf.h libbpf_common.h libbpf_legacy.h xsk.h        \
> +           bpf_helpers.h bpf_tracing.h bpf_endian.h bpf_core_read.h           \
> +           skel_internal.h libbpf_version.h
> +GEN_HDRS := $(BPF_GENERATED)
> +INSTALL_SRC_HDRS := $(addprefix $(DESTDIR)$(prefix)/include/bpf/,$(SRC_HDRS))
> +INSTALL_GEN_HDRS := $(addprefix $(DESTDIR)$(prefix)/include/bpf/, \
> +                               $(notdir $(GEN_HDRS)))

I've added INSTALL_PFX := $(DESTDIR)$(prefix)/include/bpf and used it
where possible

> +$(INSTALL_SRC_HDRS): $(DESTDIR)$(prefix)/include/bpf/%.h: %.h
> +       $(call QUIET_INSTALL, $@) \
> +               $(call do_install,$<,$(prefix)/include/bpf,644)
> +$(INSTALL_GEN_HDRS): $(DESTDIR)$(prefix)/include/bpf/%.h: $(OUTPUT)%.h
> +       $(call QUIET_INSTALL, $@) \
> +               $(call do_install,$<,$(prefix)/include/bpf,644)
>
> -install_headers: $(BPF_GENERATED)
> -       $(call QUIET_INSTALL, headers)                                       \
> -               $(foreach hdr,$(INSTALL_HEADERS),                            \
> -                       $(call do_install,$(hdr),$(prefix)/include/bpf,644);)
> +INSTALL_HEADERS := $(INSTALL_SRC_HDRS) $(INSTALL_GEN_HDRS)

I felt like INSTALL_HEADERS just adds one more indirection and
otherwise is not useful, so I dropped it and inlined
$(INSTALL_SRC_HDRS) $(INSTALL_GEN_HDRS) below


> +
> +install_headers: $(BPF_GENERATED) $(INSTALL_HEADERS)
>
>  install_pkgconfig: $(PC_FILE)
>         $(call QUIET_INSTALL, $(PC_FILE)) \
> --
> 2.30.2
>
diff mbox series

Patch

diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index 41e4f78dbad5..a92d3b9692a8 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -241,15 +241,23 @@  install_lib: all_cmd
 		$(call do_install_mkdir,$(libdir_SQ)); \
 		cp -fpR $(LIB_FILE) $(DESTDIR)$(libdir_SQ)
 
-INSTALL_HEADERS = bpf.h libbpf.h btf.h libbpf_common.h libbpf_legacy.h xsk.h \
-		  bpf_helpers.h $(BPF_GENERATED) bpf_tracing.h		     \
-		  bpf_endian.h bpf_core_read.h skel_internal.h		     \
-		  libbpf_version.h
+SRC_HDRS := bpf.h libbpf.h btf.h libbpf_common.h libbpf_legacy.h xsk.h	       \
+	    bpf_helpers.h bpf_tracing.h bpf_endian.h bpf_core_read.h	       \
+	    skel_internal.h libbpf_version.h
+GEN_HDRS := $(BPF_GENERATED)
+INSTALL_SRC_HDRS := $(addprefix $(DESTDIR)$(prefix)/include/bpf/,$(SRC_HDRS))
+INSTALL_GEN_HDRS := $(addprefix $(DESTDIR)$(prefix)/include/bpf/, \
+				$(notdir $(GEN_HDRS)))
+$(INSTALL_SRC_HDRS): $(DESTDIR)$(prefix)/include/bpf/%.h: %.h
+	$(call QUIET_INSTALL, $@) \
+		$(call do_install,$<,$(prefix)/include/bpf,644)
+$(INSTALL_GEN_HDRS): $(DESTDIR)$(prefix)/include/bpf/%.h: $(OUTPUT)%.h
+	$(call QUIET_INSTALL, $@) \
+		$(call do_install,$<,$(prefix)/include/bpf,644)
 
-install_headers: $(BPF_GENERATED)
-	$(call QUIET_INSTALL, headers)					     \
-		$(foreach hdr,$(INSTALL_HEADERS),			     \
-			$(call do_install,$(hdr),$(prefix)/include/bpf,644);)
+INSTALL_HEADERS := $(INSTALL_SRC_HDRS) $(INSTALL_GEN_HDRS)
+
+install_headers: $(BPF_GENERATED) $(INSTALL_HEADERS)
 
 install_pkgconfig: $(PC_FILE)
 	$(call QUIET_INSTALL, $(PC_FILE)) \