Re: [PATCH v3 11/11] selftests: error out if kernel header files are not yet built

Muhammad Usama Anjum
2023-06-07T05:38:07Z

On 6/7/23 1:10 AM, John Hubbard wrote:
> On 6/6/23 00:38, Muhammad Usama Anjum wrote:
> ...
>>> +kernel_header_files:
>>> + @ls $(KHDR_DIR)/linux/*.h >/dev/null 2>/dev/null; \
>>> + if [ $$? -ne 0 ]; then \
>>> + RED='\033[1;31m'; \
>>> + NOCOLOR='\033[0m'; \
>>> + echo; \
>>> + echo -e "$${RED}error$${NOCOLOR}: missing kernel header files."; \
>>> + echo "Please run this and try again:"; \
>>> + echo; \
>>> + echo " cd $(top_srcdir)"; \
>>> + echo " make headers"; \
>>> + echo; \
>>> + exit 1; \
>>> + fi
>> Thank you for adding this. This is outputting error for every selftest
>> directory. We should try to make it even better by just aborting the
>> Make-ing process the first time headers aren't detected. We can do this now
>> or later, fine by me.
>>
> OK, I see. Yes, this can be improved by adding the same mechanism to the
> selftests/Makefile, that is in selftests/mm/Makefile.
>
> I'd like to keep both, because as I mentioned earlier, mm folks like to
> run just that one Makefile, sometimes, and selftests/mm/Makefile is not
> invoking the top level Makefile. Rather, it includes lib.mk--which the
> top level Makefile does *not* include.
>
> Arguably, using includes instead of recursive Make, would improve this
> framework: reduce duplication such as the above. But that's a larger
> project and just food for thought at this point.
>
> Anyway, this works nicely on my system, and I'll attach it as a patch
> also in case you want to try it out. What do you think of this:
Nice patch. Thanks. Lets add this patch as well. Please add the tag for
this new patch:

Tested-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

>
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 90a62cf75008..bdca160063d8 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -144,10 +144,12 @@ ifneq ($(KBUILD_OUTPUT),)
> abs_objtree := $(realpath $(abs_objtree))
> BUILD := $(abs_objtree)/kselftest
> KHDR_INCLUDES := -isystem ${abs_objtree}/usr/include
> + KHDR_DIR := ${abs_objtree}/usr/include
> else
> BUILD := $(CURDIR)
> abs_srctree := $(shell cd $(top_srcdir) && pwd)
> KHDR_INCLUDES := -isystem ${abs_srctree}/usr/include
> + KHDR_DIR := ${abs_srctree}/usr/include
> DEFAULT_INSTALL_HDR_PATH := 1
> endif
>
> @@ -161,7 +163,7 @@ export KHDR_INCLUDES
> # all isn't the first target in the file.
> .DEFAULT_GOAL := all
>
> -all:
> +all: kernel_header_files
> @ret=1; \
> for TARGET in $(TARGETS); do \
> BUILD_TARGET=$$BUILD/$$TARGET; \
> @@ -172,6 +174,23 @@ all:
> ret=$$((ret * $$?)); \
> done; exit $$ret;
>
> +kernel_header_files:
> + @ls $(KHDR_DIR)/linux/*.h >/dev/null 2>/dev/null; \
> + if [ $$? -ne 0 ]; then \
> + RED='\033[1;31m'; \
> + NOCOLOR='\033[0m'; \
> + echo; \
> + echo -e "$${RED}error$${NOCOLOR}: missing kernel header files."; \
> + echo "Please run this and try again:"; \
> + echo; \
> + echo " cd $(top_srcdir)"; \
> + echo " make headers"; \
> + echo; \
> + exit 1; \
> + fi
> +
> +.PHONY: kernel_header_files
> +
> run_tests: all
> @for TARGET in $(TARGETS); do \
> BUILD_TARGET=$$BUILD/$$TARGET; \
>
>
>
> thanks,

--
BR,
Muhammad Usama Anjum