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

[John Hubbard <jhubbard@nvidia.com>]

[-- Attachment #1: Type: text/plain, Size: 4091 bytes --]

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:

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,
-- 
John Hubbard
NVIDIA

[-- Attachment #2: selftests-makefile.patch --]
[-- Type: text/x-patch, Size: 2076 bytes --]

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