Re: [PATCH] selftests/arm64: fix build failure during the "emit_tests" step

[John Hubbard]

On 7/10/23 17:56, John Hubbard wrote:
> The build failure reported in [1] occurred because commit 9fc96c7c19df
> ("selftests: error out if kernel header files are not yet built") added
> a new "kernel_header_files" dependency to "all", and that triggered
> another, pre-existing problem. Specifically, the arm64 selftests
> override the emit_tests target, and that override improperly declares
> itself to depend upon the "all" target.

I also have a corresponding fix for selftests/riscv, which has copied the
arm64 selftest approach. But I'll wait to see how reviews go here,
before sending that out.

thanks,
--
John Hubbard
NVIDIA

> This is a problem because the "emit_tests" target in lib.mk was not
> intended to be overridden. emit_tests is a very simple, sequential build
> target that was originally invoked from the "install" target, which in
> turn, depends upon "all".
>
> That approach worked for years. But with 9fc96c7c19df in place,
> emit_tests failed, because it does not set up all of the elaborate
> things that "install" does. And that caused the new
> "kernel_header_files" target (which depends upon $(KBUILD_OUTPUT) being
> correct) to fail.
>
> Some detail: The "all" target is .PHONY. Therefore, each target that
> depends on "all" will cause it to be invoked again, and because
> dependencies are managed quite loosely in the selftests Makefiles, many
> things will run, even "all" is invoked several times in immediate
> succession. So this is not a "real" failure, as far as build steps go:
> everything gets built, but "all" reports a problem when invoked a second
> time from a bad environment.
>
> To fix this, simply remove the unnecessary "all" dependency from the
> overridden emit_tests target. The dependency is still effectively
> honored, because again, invocation is via "install", which also depends
> upon "all".
>
> An alternative approach would be to harden the emit_tests target so that
> it can depend upon "all", but that's a lot more complicated and hard to
> get right, and doesn't seem worth it, especially given that emit_tests
> should probably not be overridden at all.
>
> [1] https://lore.kernel.org/20230710-kselftest-fix-arm64-v1-1-48e872844f25@xxxxxxxxxx
>
> Fixes: 9fc96c7c19df ("selftests: error out if kernel header files are not yet built")
> Reported-by: Mark Brown <broonie@xxxxxxxxxx>
> Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
> ---
>   tools/testing/selftests/arm64/Makefile | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
> index 9460cbe81bcc..ace8b67fb22d 100644
> --- a/tools/testing/selftests/arm64/Makefile
> +++ b/tools/testing/selftests/arm64/Makefile
> @@ -42,7 +42,7 @@ run_tests: all
>   	done
>   
>   # Avoid any output on non arm64 on emit_tests
> -emit_tests: all
> +emit_tests:
>   	@for DIR in $(ARM64_SUBTARGETS); do				\
>   		BUILD_TARGET=$(OUTPUT)/$$DIR;			\
>   		make OUTPUT=$$BUILD_TARGET -C $$DIR $@;		\
>
> base-commit: d5fe758c21f4770763ae4c05580be239be18947d