Re: [RFC PATCH 4/6] tools: build kernel selftests in tools_build

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] build: static pattern rule fails on targets containing slashes
- [Medium] selftests: kexec tests excluded on supported architectures
--

commit 52a33182ba4feee21276ad02270b7c707e405a85
Author: Sasha Levin <sashal@kernel.org>

tools: build kernel selftests in tools_build

This commit adds a "Kernel selftests" sub-menu to tools_build. It allows
building selected targets under tools/testing/selftests in parallel without
running them.

> diff --git a/tools/Kconfig b/tools/Kconfig
> index 488f439592150..d713741782e22 100644
> --- a/tools/Kconfig
> +++ b/tools/Kconfig
[ ... ]
> +config TOOLS_BUILD_SELFTESTS_KEXEC
> +	bool "kexec"
> +	depends on X86_64

[Severity: Medium]
Does restricting this to X86_64 exclude kexec tests on other supported
architectures?

Looking at tools/testing/selftests/kexec/Makefile:

ifeq ($(ARCH_PROCESSED),$(filter $(ARCH_PROCESSED),x86 ppc64le))
TEST_PROGS := test_kexec_load.sh test_kexec_file_load.sh
TEST_FILES := kexec_common_lib.sh

The selftest natively supports building and installing scripts for both
x86 (including 32-bit) and ppc64le. Setting the Kconfig dependency
exclusively to X86_64 prevents these tests from being built or selected via
the new tools build infrastructure on architectures that fully support them,
which reduces test coverage.

> diff --git a/tools/Makefile.kbuild b/tools/Makefile.kbuild
> index 62fd494253eca..c31347258d0a9 100644
> --- a/tools/Makefile.kbuild
> +++ b/tools/Makefile.kbuild
[ ... ]
> +selftests-$(CONFIG_TOOLS_BUILD_SELFTESTS_FILESYSTEMS_MOUNT_CYCLE)	+= filesystems/mount_cycle

[ ... ]

> +selftest-targets := $(addprefix selftest-,$(selftests-y))
> +
> +__tools_build: $(selftest-targets)
> +
> +PHONY += $(selftest-targets)
> +$(selftest-targets): selftest-%: $(selftest-shared) FORCE
> +	$(call build,$(outdir)/selftests/$*,-C $(toolsdir)/testing/selftests/$* OUTPUT=$(outdir)/selftests/$* $(selftest-flags),$(selftest-shared) $(toolsdir)/testing/selftests/$* $(selftest-extra-$*),TOOLS_BUILD_SELFTESTS_$(shell echo $* | tr a-z/- A-Z__))

[Severity: High]
Does this static pattern rule fail and abort the build when the target
contains a slash?

When matching a target containing a slash, such as
selftest-filesystems/mount_cycle when
CONFIG_TOOLS_BUILD_SELFTESTS_FILESYSTEMS_MOUNT_CYCLE is enabled, against a
slash-less pattern like selftest-%, GNU Make semantics state that the
directory part is temporarily stripped.

Make then compares only the base name (mount_cycle) against the target
pattern (selftest-%). Since mount_cycle does not start with selftest-, the
match fails, and Make throws a fatal syntax error indicating the target does
not match the target pattern.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011233647.2454648-1-sashal@kernel.org?part=4