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

[Muhammad Usama Anjum]

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@xxxxxxxxxxxxx>

> 
> 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