ebpf: update Makefile to add header dependencies

[korniltsev]

I noticed when I change a header, the ebpf program is not recompiled and requires a manual clean.

This change should force recompilation if any dependent header changed

[rockdaboot]

Suggestions for discussion: A simpler "better be on the safe side of things" would be

diff --git a/support/ebpf/Makefile b/support/ebpf/Makefile
index 0acb416..5596461 100644
--- a/support/ebpf/Makefile
+++ b/support/ebpf/Makefile
@@ -47,6 +47,7 @@ FLAGS=$(TARGET_FLAGS) -g \
        -Xclang -fmacro-prefix-map=$(CURDIR)=$(REPRODUCIBLE_PREFIX)
 
 SRCS := $(wildcard *.ebpf.c)
+HDRS := $(wildcard *.h) errors.h
 OBJS := $(SRCS:.c=.$(TARGET_ARCH).o)
 
 .DEFAULT_GOAL := all
@@ -62,9 +63,9 @@ arm64:
 errors.h: ../../tools/errors-codegen/errors.json
        go run ../../tools/errors-codegen/main.go bpf $@
 
-%.ebpf.c: errors.h ;
+%.ebpf.c: $(HDRS) ;
 
-%.ebpf.$(TARGET_ARCH).o: %.ebpf.c
+%.ebpf.$(TARGET_ARCH).o: %.ebpf.c $(HDRS)
        $(BPF_CLANG) $(FLAGS) -o $@
 
 $(TRACER_NAME): $(OBJS)

[korniltsev]

Suggestions for discussion: A simpler "better be on the safe side of things" would be

I am fine with the suggested solution as well. Couple of nits on the proposed:

1. I think it does not work, and requires a slight change to be %.ebpf.$(TARGET_ARCH).o: %.ebpf.c $(HDRS) instead of the proposed %.ebpf.c: $(HDRS) ;
2. It adds all headers as the dependencies even if some header is not used, for example tsd.h and some others. Which is not a big deal to be fair, just wanted to note that.

[rockdaboot]

Yeah, feel free to amend, just quickly tested it here.
Re 2: That's what I meant with "better be on the safe side of things": header files and dependencies can change in the future, and this rule still applies and doesn't have to be changed. You are right that a bit too much might be re-compiled - but IMO it doesn't matter (compiling everything here takes <1s, linking takes more).

We could additionally remove errors.h with make clean.

[korniltsev]

👍

I will wait for some more votes in favor of the wildcard implementation and then implement it

[christos68k]

Re 2: That's what I meant with "better be on the safe side of things": header files and dependencies can change in the future, and this rule still applies and doesn't have to be changed. You are right that a bit too much might be re-compiled - but IMO it doesn't matter (compiling everything here takes <1s, linking takes mor

What's the issue with the original proposal? It's using the dependencies that the compiler dumps, which will always be accurate. Also it's the standard way we capture C dependencies in Makefiles. I'd rather avoid inefficiencies, even if they're now small.

[fabled]

I think I'd prefer the generated dependencies as well.

[korniltsev]

should we remove error.h in clean?

[christos68k]

should we remove error.h in clean?

It's committed into the repository so probably not. What would removing it gain us, if the json changes it will be automatically re-generated (and then changes should be committed to repo). Otherwise, no need to spend time generating it.

[rockdaboot]

should we remove error.h in clean?

It's committed into the repository so probably not. What would removing it gain us, if the json changes it will be automatically re-generated (and then changes should be committed to repo). Otherwise, no need to spend time generating it.

It didn't work in the past. rm errors.h && make didn't nothing here. Didn't test yet with the changes in this PR.

[rockdaboot]

Re 2: That's what I meant with "better be on the safe side of things": header files and dependencies can change in the future, and this rule still applies and doesn't have to be changed. You are right that a bit too much might be re-compiled - but IMO it doesn't matter (compiling everything here takes <1s, linking takes mor

What's the issue with the original proposal? It's using the dependencies that the compiler dumps, which will always be accurate. Also it's the standard way we capture C dependencies in Makefiles. I'd rather avoid inefficiencies, even if they're now small.

Introducing *.d files is something I try to avoid. They clutter the ls output, need to be taken into account, e.g. for make clean etc. IMO, noise that doesn't pay out in this project, maybe in larger C projects.
But not a blocker for me.

[florianl]

thanks for the update!