Messages in this thread | | | Date | Mon, 10 Jul 2023 17:58:30 -0700 | Subject | Re: [PATCH] selftests/arm64: fix build failure during the "emit_tests" step | From | 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@kernel.org > > Fixes: 9fc96c7c19df ("selftests: error out if kernel header files are not yet built") > Reported-by: Mark Brown <broonie@kernel.org> > Signed-off-by: John Hubbard <jhubbard@nvidia.com> > --- > 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
| |