diff mbox series

Revert "selftests: error out if kernel header files are not yet built"

Message ID 20231209020144.244759-1-jhubbard@nvidia.com (mailing list archive)
State Accepted
Commit 43e8832fed08438e2a27afed9bac21acd0ceffe5
Headers show
Series Revert "selftests: error out if kernel header files are not yet built" | expand

Commit Message

John Hubbard Dec. 9, 2023, 2:01 a.m. UTC
This reverts commit 9fc96c7c19df ("selftests: error out if kernel header
files are not yet built").

It turns out that requiring the kernel headers to be built as a
prerequisite to building selftests, does not work in many cases. For
example, Peter Zijlstra writes:

"My biggest beef with the whole thing is that I simply do not want to use
'make headers', it doesn't work for me.

I have a ton of output directories and I don't care to build tools into
the output dirs, in fact some of them flat out refuse to work that way
(bpf comes to mind)." [1]

Therefore, stop erroring out on the selftests build. Additional patches
will be required in order to change over to not requiring the kernel
headers.

[1] https://lore.kernel.org/20231208221007.GO28727@noisy.programming.kicks-ass.net

Cc: Anders Roxell <anders.roxell@linaro.org>
Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 tools/testing/selftests/Makefile | 21 +----------------
 tools/testing/selftests/lib.mk   | 40 +++-----------------------------
 2 files changed, 4 insertions(+), 57 deletions(-)

Comments

John Hubbard Dec. 9, 2023, 2:05 a.m. UTC | #1
...and, somehow I failed to Cc Peter Z. Doing that now.


thanks,
John Hubbard
NVIDIA

On 12/8/23 18:01, John Hubbard wrote:
> This reverts commit 9fc96c7c19df ("selftests: error out if kernel header
> files are not yet built").
> 
> It turns out that requiring the kernel headers to be built as a
> prerequisite to building selftests, does not work in many cases. For
> example, Peter Zijlstra writes:
> 
> "My biggest beef with the whole thing is that I simply do not want to use
> 'make headers', it doesn't work for me.
> 
> I have a ton of output directories and I don't care to build tools into
> the output dirs, in fact some of them flat out refuse to work that way
> (bpf comes to mind)." [1]
> 
> Therefore, stop erroring out on the selftests build. Additional patches
> will be required in order to change over to not requiring the kernel
> headers.
> 
> [1] https://lore.kernel.org/20231208221007.GO28727@noisy.programming.kicks-ass.net
> 
> Cc: Anders Roxell <anders.roxell@linaro.org>
> Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Shuah Khan <shuah@kernel.org>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>   tools/testing/selftests/Makefile | 21 +----------------
>   tools/testing/selftests/lib.mk   | 40 +++-----------------------------
>   2 files changed, 4 insertions(+), 57 deletions(-)
> 
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 3b2061d1c1a5..8247a7c69c36 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -155,12 +155,10 @@ 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
>   
> @@ -174,7 +172,7 @@ export KHDR_INCLUDES
>   # all isn't the first target in the file.
>   .DEFAULT_GOAL := all
>   
> -all: kernel_header_files
> +all:
>   	@ret=1;							\
>   	for TARGET in $(TARGETS); do				\
>   		BUILD_TARGET=$$BUILD/$$TARGET;			\
> @@ -185,23 +183,6 @@ all: kernel_header_files
>   		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;	\
> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index 118e0964bda9..aa646e0661f3 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -44,26 +44,10 @@ endif
>   selfdir = $(realpath $(dir $(filter %/lib.mk,$(MAKEFILE_LIST))))
>   top_srcdir = $(selfdir)/../../..
>   
> -ifeq ("$(origin O)", "command line")
> -  KBUILD_OUTPUT := $(O)
> +ifeq ($(KHDR_INCLUDES),)
> +KHDR_INCLUDES := -isystem $(top_srcdir)/usr/include
>   endif
>   
> -ifneq ($(KBUILD_OUTPUT),)
> -  # Make's built-in functions such as $(abspath ...), $(realpath ...) cannot
> -  # expand a shell special character '~'. We use a somewhat tedious way here.
> -  abs_objtree := $(shell cd $(top_srcdir) && mkdir -p $(KBUILD_OUTPUT) && cd $(KBUILD_OUTPUT) && pwd)
> -  $(if $(abs_objtree),, \
> -    $(error failed to create output directory "$(KBUILD_OUTPUT)"))
> -  # $(realpath ...) resolves symlinks
> -  abs_objtree := $(realpath $(abs_objtree))
> -  KHDR_DIR := ${abs_objtree}/usr/include
> -else
> -  abs_srctree := $(shell cd $(top_srcdir) && pwd)
> -  KHDR_DIR := ${abs_srctree}/usr/include
> -endif
> -
> -KHDR_INCLUDES := -isystem $(KHDR_DIR)
> -
>   # The following are built by lib.mk common compile rules.
>   # TEST_CUSTOM_PROGS should be used by tests that require
>   # custom build rule and prevent common build rule use.
> @@ -74,25 +58,7 @@ TEST_GEN_PROGS := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS))
>   TEST_GEN_PROGS_EXTENDED := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS_EXTENDED))
>   TEST_GEN_FILES := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_FILES))
>   
> -all: kernel_header_files $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) \
> -     $(TEST_GEN_FILES)
> -
> -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
> +all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
>   
>   define RUN_TESTS
>   	BASE_DIR="$(selfdir)";			\
Muhammad Usama Anjum Dec. 11, 2023, 11 a.m. UTC | #2
On 12/9/23 7:01 AM, John Hubbard wrote:
> This reverts commit 9fc96c7c19df ("selftests: error out if kernel header
> files are not yet built").
I don't think whole of this commit needs to be reverted. Lets leave the
warning message as it is and just remove the condition to abort the
compilation.

> 
> It turns out that requiring the kernel headers to be built as a
> prerequisite to building selftests, does not work in many cases. For
> example, Peter Zijlstra writes:
> 
> "My biggest beef with the whole thing is that I simply do not want to use
> 'make headers', it doesn't work for me.
> 
> I have a ton of output directories and I don't care to build tools into
> the output dirs, in fact some of them flat out refuse to work that way
> (bpf comes to mind)." [1]
> 
> Therefore, stop erroring out on the selftests build. Additional patches
> will be required in order to change over to not requiring the kernel
> headers.
> 
> [1] https://lore.kernel.org/20231208221007.GO28727@noisy.programming.kicks-ass.net
> 
> Cc: Anders Roxell <anders.roxell@linaro.org>
> Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Shuah Khan <shuah@kernel.org>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>  tools/testing/selftests/Makefile | 21 +----------------
>  tools/testing/selftests/lib.mk   | 40 +++-----------------------------
>  2 files changed, 4 insertions(+), 57 deletions(-)
> 
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 3b2061d1c1a5..8247a7c69c36 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -155,12 +155,10 @@ 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
>  
> @@ -174,7 +172,7 @@ export KHDR_INCLUDES
>  # all isn't the first target in the file.
>  .DEFAULT_GOAL := all
>  
> -all: kernel_header_files
> +all:
>  	@ret=1;							\
>  	for TARGET in $(TARGETS); do				\
>  		BUILD_TARGET=$$BUILD/$$TARGET;			\
> @@ -185,23 +183,6 @@ all: kernel_header_files
>  		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;	\
> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index 118e0964bda9..aa646e0661f3 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -44,26 +44,10 @@ endif
>  selfdir = $(realpath $(dir $(filter %/lib.mk,$(MAKEFILE_LIST))))
>  top_srcdir = $(selfdir)/../../..
>  
> -ifeq ("$(origin O)", "command line")
> -  KBUILD_OUTPUT := $(O)
> +ifeq ($(KHDR_INCLUDES),)
> +KHDR_INCLUDES := -isystem $(top_srcdir)/usr/include
>  endif
>  
> -ifneq ($(KBUILD_OUTPUT),)
> -  # Make's built-in functions such as $(abspath ...), $(realpath ...) cannot
> -  # expand a shell special character '~'. We use a somewhat tedious way here.
> -  abs_objtree := $(shell cd $(top_srcdir) && mkdir -p $(KBUILD_OUTPUT) && cd $(KBUILD_OUTPUT) && pwd)
> -  $(if $(abs_objtree),, \
> -    $(error failed to create output directory "$(KBUILD_OUTPUT)"))
> -  # $(realpath ...) resolves symlinks
> -  abs_objtree := $(realpath $(abs_objtree))
> -  KHDR_DIR := ${abs_objtree}/usr/include
> -else
> -  abs_srctree := $(shell cd $(top_srcdir) && pwd)
> -  KHDR_DIR := ${abs_srctree}/usr/include
> -endif
> -
> -KHDR_INCLUDES := -isystem $(KHDR_DIR)
> -
>  # The following are built by lib.mk common compile rules.
>  # TEST_CUSTOM_PROGS should be used by tests that require
>  # custom build rule and prevent common build rule use.
> @@ -74,25 +58,7 @@ TEST_GEN_PROGS := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS))
>  TEST_GEN_PROGS_EXTENDED := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS_EXTENDED))
>  TEST_GEN_FILES := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_FILES))
>  
> -all: kernel_header_files $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) \
> -     $(TEST_GEN_FILES)
> -
> -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
> +all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
>  
>  define RUN_TESTS
>  	BASE_DIR="$(selfdir)";			\
Marcos Paulo de Souza Dec. 11, 2023, 12:05 p.m. UTC | #3
On Mon, 2023-12-11 at 16:00 +0500, Muhammad Usama Anjum wrote:
> On 12/9/23 7:01 AM, John Hubbard wrote:
> > This reverts commit 9fc96c7c19df ("selftests: error out if kernel
> > header
> > files are not yet built").
> I don't think whole of this commit needs to be reverted. Lets leave
> the
> warning message as it is and just remove the condition to abort the
> compilation.

Reverting or just printing the warning would work for our testcases, as
long as it doesn't error out.

> 
> > 
> > It turns out that requiring the kernel headers to be built as a
> > prerequisite to building selftests, does not work in many cases.
> > For
> > example, Peter Zijlstra writes:
> > 
> > "My biggest beef with the whole thing is that I simply do not want
> > to use
> > 'make headers', it doesn't work for me.
> > 
> > I have a ton of output directories and I don't care to build tools
> > into
> > the output dirs, in fact some of them flat out refuse to work that
> > way
> > (bpf comes to mind)." [1]
> > 
> > Therefore, stop erroring out on the selftests build. Additional
> > patches
> > will be required in order to change over to not requiring the
> > kernel
> > headers.
> > 
> > [1]
> > https://lore.kernel.org/20231208221007.GO28727@noisy.programming.kicks-ass.net
> > 
> > Cc: Anders Roxell <anders.roxell@linaro.org>
> > Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
> > Cc: David Hildenbrand <david@redhat.com>
> > Cc: Peter Xu <peterx@redhat.com>
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Nathan Chancellor <nathan@kernel.org>
> > Cc: Shuah Khan <shuah@kernel.org>
> > Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> > ---
> >  tools/testing/selftests/Makefile | 21 +----------------
> >  tools/testing/selftests/lib.mk   | 40 +++-------------------------
> > ----
> >  2 files changed, 4 insertions(+), 57 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/Makefile
> > b/tools/testing/selftests/Makefile
> > index 3b2061d1c1a5..8247a7c69c36 100644
> > --- a/tools/testing/selftests/Makefile
> > +++ b/tools/testing/selftests/Makefile
> > @@ -155,12 +155,10 @@ 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
> >  
> > @@ -174,7 +172,7 @@ export KHDR_INCLUDES
> >  # all isn't the first target in the file.
> >  .DEFAULT_GOAL := all
> >  
> > -all: kernel_header_files
> > +all:
> >  	@ret=1;						
> > 	\
> >  	for TARGET in $(TARGETS); do				\
> >  		BUILD_TARGET=$$BUILD/$$TARGET;			\
> > @@ -185,23 +183,6 @@ all: kernel_header_files
> >  		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;	\
> > diff --git a/tools/testing/selftests/lib.mk
> > b/tools/testing/selftests/lib.mk
> > index 118e0964bda9..aa646e0661f3 100644
> > --- a/tools/testing/selftests/lib.mk
> > +++ b/tools/testing/selftests/lib.mk
> > @@ -44,26 +44,10 @@ endif
> >  selfdir = $(realpath $(dir $(filter %/lib.mk,$(MAKEFILE_LIST))))
> >  top_srcdir = $(selfdir)/../../..
> >  
> > -ifeq ("$(origin O)", "command line")
> > -  KBUILD_OUTPUT := $(O)
> > +ifeq ($(KHDR_INCLUDES),)
> > +KHDR_INCLUDES := -isystem $(top_srcdir)/usr/include
> >  endif
> >  
> > -ifneq ($(KBUILD_OUTPUT),)
> > -  # Make's built-in functions such as $(abspath ...), $(realpath
> > ...) cannot
> > -  # expand a shell special character '~'. We use a somewhat
> > tedious way here.
> > -  abs_objtree := $(shell cd $(top_srcdir) && mkdir -p
> > $(KBUILD_OUTPUT) && cd $(KBUILD_OUTPUT) && pwd)
> > -  $(if $(abs_objtree),, \
> > -    $(error failed to create output directory "$(KBUILD_OUTPUT)"))
> > -  # $(realpath ...) resolves symlinks
> > -  abs_objtree := $(realpath $(abs_objtree))
> > -  KHDR_DIR := ${abs_objtree}/usr/include
> > -else
> > -  abs_srctree := $(shell cd $(top_srcdir) && pwd)
> > -  KHDR_DIR := ${abs_srctree}/usr/include
> > -endif
> > -
> > -KHDR_INCLUDES := -isystem $(KHDR_DIR)
> > -
> >  # The following are built by lib.mk common compile rules.
> >  # TEST_CUSTOM_PROGS should be used by tests that require
> >  # custom build rule and prevent common build rule use.
> > @@ -74,25 +58,7 @@ TEST_GEN_PROGS := $(patsubst
> > %,$(OUTPUT)/%,$(TEST_GEN_PROGS))
> >  TEST_GEN_PROGS_EXTENDED := $(patsubst
> > %,$(OUTPUT)/%,$(TEST_GEN_PROGS_EXTENDED))
> >  TEST_GEN_FILES := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_FILES))
> >  
> > -all: kernel_header_files $(TEST_GEN_PROGS)
> > $(TEST_GEN_PROGS_EXTENDED) \
> > -     $(TEST_GEN_FILES)
> > -
> > -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
> > +all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED)
> > $(TEST_GEN_FILES)
> >  
> >  define RUN_TESTS
> >  	BASE_DIR="$(selfdir)";			\
>
John Hubbard Dec. 11, 2023, 7 p.m. UTC | #4
On 12/11/23 03:00, Muhammad Usama Anjum wrote:
> On 12/9/23 7:01 AM, John Hubbard wrote:
>> This reverts commit 9fc96c7c19df ("selftests: error out if kernel header
>> files are not yet built").
> I don't think whole of this commit needs to be reverted. Lets leave the
> warning message as it is and just remove the condition to abort the
> compilation.
> 

Hi Muhammad!

If we do decide that "make headers" or something like it is required,
then yes, this patch should be changed from a revert, to a "warn instead
of failing out" patch.

First, though, I'd like us to choose a design direction. The patch as
written is intended to put us on a design that does not require "make
headers" before building the selftests, because that approach would work
for all the cases I've seen so far.

If we want something else, then David Hildenbrand has listed several
ideas, and I've added a 4th one to the list, in [1].


[1] https://lore.kernel.org/3eadd79c-c02a-495f-92c0-0315046ef59f@nvidia.com


thanks,
Muhammad Usama Anjum Dec. 12, 2023, 6:59 a.m. UTC | #5
On 12/12/23 12:00 AM, John Hubbard wrote:
> On 12/11/23 03:00, Muhammad Usama Anjum wrote:
>> On 12/9/23 7:01 AM, John Hubbard wrote:
>>> This reverts commit 9fc96c7c19df ("selftests: error out if kernel header
>>> files are not yet built").
>> I don't think whole of this commit needs to be reverted. Lets leave the
>> warning message as it is and just remove the condition to abort the
>> compilation.
>>
> 
> Hi Muhammad!
> 
> If we do decide that "make headers" or something like it is required,
> then yes, this patch should be changed from a revert, to a "warn instead
> of failing out" patch.
I support this is as most of the times when the latest headers aren't
installed in the system. Hence the build of all those kselftests would fail
which require the recently added macros. There is no workaround to build
those tests until `make headers` is done or the latest headers are
installed. The former is easier.

If we just turn this into warning, most people reporting issues with `make
headers` would go away. They will be able to build all those kselftest
which don't require latest headers. For example mincore kselftest gets
build without KHDR_INCLUDES. In case people want to build failing tests,
they should add #ifdefs to the tests and submit patches which is idea 4.

> 
> First, though, I'd like us to choose a design direction. The patch as
> written is intended to put us on a design that does not require "make
> headers" before building the selftests, because that approach would work
> for all the cases I've seen so far.
> 
> If we want something else, then David Hildenbrand has listed several
> ideas, and I've added a 4th one to the list, in [1].
> 
> 
> [1] https://lore.kernel.org/3eadd79c-c02a-495f-92c0-0315046ef59f@nvidia.com
> 
> 
> thanks,
diff mbox series

Patch

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 3b2061d1c1a5..8247a7c69c36 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -155,12 +155,10 @@  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
 
@@ -174,7 +172,7 @@  export KHDR_INCLUDES
 # all isn't the first target in the file.
 .DEFAULT_GOAL := all
 
-all: kernel_header_files
+all:
 	@ret=1;							\
 	for TARGET in $(TARGETS); do				\
 		BUILD_TARGET=$$BUILD/$$TARGET;			\
@@ -185,23 +183,6 @@  all: kernel_header_files
 		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;	\
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 118e0964bda9..aa646e0661f3 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -44,26 +44,10 @@  endif
 selfdir = $(realpath $(dir $(filter %/lib.mk,$(MAKEFILE_LIST))))
 top_srcdir = $(selfdir)/../../..
 
-ifeq ("$(origin O)", "command line")
-  KBUILD_OUTPUT := $(O)
+ifeq ($(KHDR_INCLUDES),)
+KHDR_INCLUDES := -isystem $(top_srcdir)/usr/include
 endif
 
-ifneq ($(KBUILD_OUTPUT),)
-  # Make's built-in functions such as $(abspath ...), $(realpath ...) cannot
-  # expand a shell special character '~'. We use a somewhat tedious way here.
-  abs_objtree := $(shell cd $(top_srcdir) && mkdir -p $(KBUILD_OUTPUT) && cd $(KBUILD_OUTPUT) && pwd)
-  $(if $(abs_objtree),, \
-    $(error failed to create output directory "$(KBUILD_OUTPUT)"))
-  # $(realpath ...) resolves symlinks
-  abs_objtree := $(realpath $(abs_objtree))
-  KHDR_DIR := ${abs_objtree}/usr/include
-else
-  abs_srctree := $(shell cd $(top_srcdir) && pwd)
-  KHDR_DIR := ${abs_srctree}/usr/include
-endif
-
-KHDR_INCLUDES := -isystem $(KHDR_DIR)
-
 # The following are built by lib.mk common compile rules.
 # TEST_CUSTOM_PROGS should be used by tests that require
 # custom build rule and prevent common build rule use.
@@ -74,25 +58,7 @@  TEST_GEN_PROGS := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS))
 TEST_GEN_PROGS_EXTENDED := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS_EXTENDED))
 TEST_GEN_FILES := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_FILES))
 
-all: kernel_header_files $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) \
-     $(TEST_GEN_FILES)
-
-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
+all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
 
 define RUN_TESTS
 	BASE_DIR="$(selfdir)";			\