[build-selftests] Clean selftests before building them

[eddyz87]

Cleaning selftests is useful in case if cached kernel build artifacts contain build results for selftests.

For the context:

I'm currently working on a BPI CI job that executes veristat tool for both push and pull_request events:

• for push in order to obtain baseline performance data;
• for pull_request in order to compare branch performance data with baseline performance data.

The veristat is built with selftests and uses some selftest .bpf.o binary files as an input. Currently selftests are not built on push, but this would have to change. Which means that cached incremental kernel build artifacts might now contain selftest binary files. I'd like to add the clean action to selftests compilation to avoid any potential issues with stale selftest binary files when pull_request event is processed.

[eddyz87]

I still think we shouldn't do a clean. If incremental builds are broken they should be fixed. While I understand that they may be less frequently used for selftests than for the kernel itself, in my opinion a bug is a bug. But I guess I don't feel strongly about it.

This does not have to be a bug to be an annoyance, though. For example, here is a portion of the selftests makefile:

ifeq ($($(TRUNNER_TESTS_DIR)-tests-hdr),)
$(TRUNNER_TESTS_DIR)-tests-hdr := y
$(TRUNNER_TESTS_HDR): $(TRUNNER_TESTS_DIR)/*.c
    $$(call msg,TEST-HDR,$(TRUNNER_BINARY),$$@)
    $$(shell (echo '/* Generated header, do not edit */';                   \
          sed -n -E 's/^void (serial_)?test_([a-zA-Z0-9_]+)\((void)?\).*/DEFINE_TEST(\2)/p' \
            $(TRUNNER_TESTS_DIR)/*.c | sort ;   \
         ) > $$@)
endif

This code is responsible for generation of ./prog_tests/tests.h header file and it scans the test_progs/ directory to generate it. If some C file would be removed from test_progs/ by a merge request, the content of the tests.h might be no longer valid, however make has no way to tell this.

So, what I'm saying is that we trade marginal gain in CI performance (if any), for some potential not so obvious to debug or reproduce CI failures.

[danielocfb]

Not so much worried about the performance, mostly just about complexity creep, which all this logic suffers from badly already.

Well, to be honest, what your describe sounds very much like an incremental compilation bug that should be fixed to me :-) But sure, point taken. Thanks for the example.

[danielocfb]

@chantra Do you want to go over Eduard's changes as well? Otherwise I will merge this tomorrow, or once https://github.com/libbpf/ci/pull/98 has been updated.

[eddyz87]

Not so much worried about the performance, mostly just about complexity creep, which all this logic suffers from badly already.

Well, to be honest, what your describe sounds very much like an incremental compilation bug that should be fixed to me :-) But sure, point taken. Thanks for the example.

Well, in some sense it is a bug, as makefile abstraction breaks apart. But any fixes that come to my mind only make already complicated makefile more complicated, unfortunately.

[chantra]

Thanks @eddyz87 !