linux-kbuild.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Fix kselftest build with sub-directory
@ 2022-07-12  8:29 Guillaume Tucker
  2022-07-12  8:29 ` [PATCH v2 1/4] selftests: drop khdr make target Guillaume Tucker
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Guillaume Tucker @ 2022-07-12  8:29 UTC (permalink / raw)
  To: Masahiro Yamada, Michal Marek, Nick Desaulniers, Shuah Khan, Kees Cook
  Cc: Anders Roxell, Tim.Bird, kernel, linux-kbuild, linux-kernel,
	linux-kselftest

Earlier attempts to get "make O=build kselftest-all" to work were
not successful as they made undesirable changes to some functions
in the top-level Makefile.  This series takes a different
approach by removing the root cause of the problem within
kselftest, which is when the sub-Makefile tries to install kernel
headers "backwards" by calling make with the top-level Makefile.
The actual issue comes from the fact that $(srctree) is ".." when
building in a sub-directory with "O=build" which then obviously
makes "-C $(top_srcdir)" point outside of the real source tree.

With this series, the generic kselftest targets work as expected
from the top level with or without a build directory e.g.:

  $ make kselftest-all

  $ make O=build kselftest-all

Then in order to build using the sub-Makefile explicitly, the
headers have to be installed first.  This is arguably a valid
requirement to have when building a tool from a sub-Makefile.
For example, "make -C tools/testing/nvdimm/" fails in a similar
way until <asm/rwonce.h> has been generated by a kernel build.

v2: replace headers_install with headers

Guillaume Tucker (4):
  selftests: drop khdr make target
  selftests: stop using KSFT_KHDR_INSTALL
  selftests: drop KSFT_KHDR_INSTALL make target
  Makefile: add headers to kselftest targets

 Makefile                                      |  4 +-
 tools/testing/selftests/Makefile              | 28 +-------------
 tools/testing/selftests/arm64/mte/Makefile    |  1 -
 tools/testing/selftests/arm64/signal/Makefile |  1 -
 .../selftests/arm64/signal/test_signals.h     |  4 +-
 .../selftests/drivers/s390x/uvdevice/Makefile |  1 -
 .../selftests/futex/functional/Makefile       |  1 -
 tools/testing/selftests/kvm/Makefile          |  1 -
 tools/testing/selftests/landlock/Makefile     |  1 -
 tools/testing/selftests/lib.mk                | 38 -------------------
 tools/testing/selftests/net/Makefile          |  1 -
 tools/testing/selftests/net/mptcp/Makefile    |  1 -
 tools/testing/selftests/tc-testing/Makefile   |  1 -
 tools/testing/selftests/vm/Makefile           |  1 -
 14 files changed, 5 insertions(+), 79 deletions(-)

--
2.30.2

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/4] selftests: drop khdr make target
  2022-07-12  8:29 [PATCH v2 0/4] Fix kselftest build with sub-directory Guillaume Tucker
@ 2022-07-12  8:29 ` Guillaume Tucker
  2022-07-12  8:29 ` [PATCH v2 2/4] selftests: stop using KSFT_KHDR_INSTALL Guillaume Tucker
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Guillaume Tucker @ 2022-07-12  8:29 UTC (permalink / raw)
  To: Masahiro Yamada, Michal Marek, Nick Desaulniers, Shuah Khan, Kees Cook
  Cc: Anders Roxell, Tim.Bird, kernel, linux-kbuild, linux-kernel,
	linux-kselftest

Drop the "khdr" make target as it fails when the build directory is a
sub-directory of the source tree.  Rely on the "headers_install"
target to have been run first instead.

For example, here's a typical error this patch is addressing:

  $ make O=build -j32 kselftest-gen_tar
  make[1]: Entering directory '/home/kernelci/linux/build'
  make --no-builtin-rules INSTALL_HDR_PATH=/home/kernelci/linux/build/usr \
          ARCH=x86 -C ../../.. headers_install
  make[3]: Entering directory '/home/kernelci/linux'
  Makefile:1022: ../scripts/Makefile.extrawarn: No such file or directory

The source directory is determined in the top-level Makefile as ".."
relatively to the "build" directory, but then the kselftest Makefile
switches to "-C ../../.." so "../scripts" then points one level higher
than the source tree e.g. "linux/../scripts" - which fails obviously.
There is no other use-case in the kernel tree where a sub-directory
Makefile tries to call a top-level make target, and it appears this
isn't really a valid thing to do.

Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
 tools/testing/selftests/Makefile | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index de11992dc577..619451e82863 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -151,30 +151,7 @@ export KHDR_INCLUDES
 # all isn't the first target in the file.
 .DEFAULT_GOAL := all
 
-# Install headers here once for all tests. KSFT_KHDR_INSTALL_DONE
-# is used to avoid running headers_install from lib.mk.
-# Invoke headers install with --no-builtin-rules to avoid circular
-# dependency in "make kselftest" case. In this case, second level
-# make inherits builtin-rules which will use the rule generate
-# Makefile.o and runs into
-# "Circular Makefile.o <- prepare dependency dropped."
-# and headers_install fails and test compile fails.
-#
-# O= KBUILD_OUTPUT cases don't run into this error, since main Makefile
-# invokes them as sub-makes and --no-builtin-rules is not necessary,
-# but doesn't cause any failures. Keep it simple and use the same
-# flags in both cases.
-# Local build cases: "make kselftest", "make -C" - headers are installed
-# in the default INSTALL_HDR_PATH usr/include.
-khdr:
-ifeq (1,$(DEFAULT_INSTALL_HDR_PATH))
-	$(MAKE) --no-builtin-rules ARCH=$(ARCH) -C $(top_srcdir) headers_install
-else
-	$(MAKE) --no-builtin-rules INSTALL_HDR_PATH=$(abs_objtree)/usr \
-		ARCH=$(ARCH) -C $(top_srcdir) headers_install
-endif
-
-all: khdr
+all:
 	@ret=1;							\
 	for TARGET in $(TARGETS); do				\
 		BUILD_TARGET=$$BUILD/$$TARGET;			\
@@ -274,4 +251,4 @@ clean:
 		$(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET clean;\
 	done;
 
-.PHONY: khdr all run_tests hotplug run_hotplug clean_hotplug run_pstore_crash install clean gen_tar
+.PHONY: all run_tests hotplug run_hotplug clean_hotplug run_pstore_crash install clean gen_tar
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/4] selftests: stop using KSFT_KHDR_INSTALL
  2022-07-12  8:29 [PATCH v2 0/4] Fix kselftest build with sub-directory Guillaume Tucker
  2022-07-12  8:29 ` [PATCH v2 1/4] selftests: drop khdr make target Guillaume Tucker
@ 2022-07-12  8:29 ` Guillaume Tucker
  2022-07-12  8:29 ` [PATCH v2 3/4] selftests: drop KSFT_KHDR_INSTALL make target Guillaume Tucker
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Guillaume Tucker @ 2022-07-12  8:29 UTC (permalink / raw)
  To: Masahiro Yamada, Michal Marek, Nick Desaulniers, Shuah Khan, Kees Cook
  Cc: Anders Roxell, Tim.Bird, kernel, linux-kbuild, linux-kernel,
	linux-kselftest

Stop using the KSFT_KHDR_INSTALL flag as installing the kernel headers
from the kselftest Makefile is causing some issues.  Instead, rely on
the headers to be installed directly by the top-level Makefile
"headers_install" make target prior to building kselftest.

Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
 tools/testing/selftests/arm64/mte/Makefile              | 1 -
 tools/testing/selftests/arm64/signal/Makefile           | 1 -
 tools/testing/selftests/arm64/signal/test_signals.h     | 4 +---
 tools/testing/selftests/drivers/s390x/uvdevice/Makefile | 1 -
 tools/testing/selftests/futex/functional/Makefile       | 1 -
 tools/testing/selftests/kvm/Makefile                    | 1 -
 tools/testing/selftests/landlock/Makefile               | 1 -
 tools/testing/selftests/net/Makefile                    | 1 -
 tools/testing/selftests/net/mptcp/Makefile              | 1 -
 tools/testing/selftests/tc-testing/Makefile             | 1 -
 tools/testing/selftests/vm/Makefile                     | 1 -
 11 files changed, 1 insertion(+), 13 deletions(-)

diff --git a/tools/testing/selftests/arm64/mte/Makefile b/tools/testing/selftests/arm64/mte/Makefile
index 409e3e53d00a..a5a0744423d8 100644
--- a/tools/testing/selftests/arm64/mte/Makefile
+++ b/tools/testing/selftests/arm64/mte/Makefile
@@ -22,7 +22,6 @@ ifeq ($(mte_cc_support),1)
 TEST_GEN_PROGS := $(PROGS)
 
 # Get Kernel headers installed and use them.
-KSFT_KHDR_INSTALL := 1
 else
     $(warning compiler "$(CC)" does not support the ARMv8.5 MTE extension.)
     $(warning test program "mte" will not be created.)
diff --git a/tools/testing/selftests/arm64/signal/Makefile b/tools/testing/selftests/arm64/signal/Makefile
index ac4ad0005715..be7520a863b0 100644
--- a/tools/testing/selftests/arm64/signal/Makefile
+++ b/tools/testing/selftests/arm64/signal/Makefile
@@ -11,7 +11,6 @@ PROGS := $(patsubst %.c,%,$(SRCS))
 TEST_GEN_PROGS := $(notdir $(PROGS))
 
 # Get Kernel headers installed and use them.
-KSFT_KHDR_INSTALL := 1
 
 # Including KSFT lib.mk here will also mangle the TEST_GEN_PROGS list
 # to account for any OUTPUT target-dirs optionally provided by
diff --git a/tools/testing/selftests/arm64/signal/test_signals.h b/tools/testing/selftests/arm64/signal/test_signals.h
index c70fdec7d7c4..0c645834ddc3 100644
--- a/tools/testing/selftests/arm64/signal/test_signals.h
+++ b/tools/testing/selftests/arm64/signal/test_signals.h
@@ -9,9 +9,7 @@
 #include <ucontext.h>
 
 /*
- * Using ARCH specific and sanitized Kernel headers installed by KSFT
- * framework since we asked for it by setting flag KSFT_KHDR_INSTALL
- * in our Makefile.
+ * Using ARCH specific and sanitized Kernel headers from the tree.
  */
 #include <asm/ptrace.h>
 #include <asm/hwcap.h>
diff --git a/tools/testing/selftests/drivers/s390x/uvdevice/Makefile b/tools/testing/selftests/drivers/s390x/uvdevice/Makefile
index 5e701d2708d4..891215a7dc8a 100644
--- a/tools/testing/selftests/drivers/s390x/uvdevice/Makefile
+++ b/tools/testing/selftests/drivers/s390x/uvdevice/Makefile
@@ -11,7 +11,6 @@ else
 TEST_GEN_PROGS := test_uvdevice
 
 top_srcdir ?= ../../../../../..
-KSFT_KHDR_INSTALL := 1
 khdr_dir = $(top_srcdir)/usr/include
 LINUX_TOOL_ARCH_INCLUDE = $(top_srcdir)/tools/arch/$(ARCH)/include
 
diff --git a/tools/testing/selftests/futex/functional/Makefile b/tools/testing/selftests/futex/functional/Makefile
index b8152c573e8a..732149011692 100644
--- a/tools/testing/selftests/futex/functional/Makefile
+++ b/tools/testing/selftests/futex/functional/Makefile
@@ -22,7 +22,6 @@ TEST_GEN_FILES := \
 TEST_PROGS := run.sh
 
 top_srcdir = ../../../../..
-KSFT_KHDR_INSTALL := 1
 DEFAULT_INSTALL_HDR_PATH := 1
 include ../../lib.mk
 
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 81470a99ed1c..e15bb9693922 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -4,7 +4,6 @@ include ../../../build/Build.include
 all:
 
 top_srcdir = ../../../..
-KSFT_KHDR_INSTALL := 1
 
 # For cross-builds to work, UNAME_M has to map to ARCH and arch specific
 # directories and targets in this Makefile. "uname -m" doesn't map to
diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
index 0b0049e133bb..1313e44e8fb9 100644
--- a/tools/testing/selftests/landlock/Makefile
+++ b/tools/testing/selftests/landlock/Makefile
@@ -8,7 +8,6 @@ TEST_GEN_PROGS := $(src_test:.c=)
 
 TEST_GEN_PROGS_EXTENDED := true
 
-KSFT_KHDR_INSTALL := 1
 OVERRIDE_TARGETS := 1
 include ../lib.mk
 
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 464df13831f2..eaf3bb457576 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -63,7 +63,6 @@ TEST_GEN_FILES += bind_bhash_test
 
 TEST_FILES := settings
 
-KSFT_KHDR_INSTALL := 1
 include ../lib.mk
 
 include bpf/Makefile
diff --git a/tools/testing/selftests/net/mptcp/Makefile b/tools/testing/selftests/net/mptcp/Makefile
index f905d5358e68..1af2f66fb59a 100644
--- a/tools/testing/selftests/net/mptcp/Makefile
+++ b/tools/testing/selftests/net/mptcp/Makefile
@@ -1,7 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
 top_srcdir = ../../../../..
-KSFT_KHDR_INSTALL := 1
 
 CFLAGS =  -Wall -Wl,--no-as-needed -O2 -g -I$(top_srcdir)/usr/include $(KHDR_INCLUDES)
 
diff --git a/tools/testing/selftests/tc-testing/Makefile b/tools/testing/selftests/tc-testing/Makefile
index 4d639279f41e..cb553eac9f41 100644
--- a/tools/testing/selftests/tc-testing/Makefile
+++ b/tools/testing/selftests/tc-testing/Makefile
@@ -5,7 +5,6 @@ top_srcdir = $(abspath ../../../..)
 APIDIR := $(top_scrdir)/include/uapi
 TEST_GEN_FILES = action.o
 
-KSFT_KHDR_INSTALL := 1
 include ../lib.mk
 
 PROBE := $(shell $(LLC) -march=bpf -mcpu=probe -filetype=null /dev/null 2>&1)
diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
index 44f25acfbeca..108587cb327a 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -94,7 +94,6 @@ TEST_PROGS := run_vmtests.sh
 TEST_FILES := test_vmalloc.sh
 TEST_FILES += test_hmm.sh
 
-KSFT_KHDR_INSTALL := 1
 include ../lib.mk
 
 $(OUTPUT)/madv_populate: vm_util.c
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/4] selftests: drop KSFT_KHDR_INSTALL make target
  2022-07-12  8:29 [PATCH v2 0/4] Fix kselftest build with sub-directory Guillaume Tucker
  2022-07-12  8:29 ` [PATCH v2 1/4] selftests: drop khdr make target Guillaume Tucker
  2022-07-12  8:29 ` [PATCH v2 2/4] selftests: stop using KSFT_KHDR_INSTALL Guillaume Tucker
@ 2022-07-12  8:29 ` Guillaume Tucker
  2022-07-12  9:59   ` Anders Roxell
  2022-07-12  8:29 ` [PATCH v2 4/4] Makefile: add headers to kselftest targets Guillaume Tucker
  2022-07-13  2:27 ` [PATCH v2 0/4] Fix kselftest build with sub-directory Shuah Khan
  4 siblings, 1 reply; 10+ messages in thread
From: Guillaume Tucker @ 2022-07-12  8:29 UTC (permalink / raw)
  To: Masahiro Yamada, Michal Marek, Nick Desaulniers, Shuah Khan, Kees Cook
  Cc: Anders Roxell, Tim.Bird, kernel, linux-kbuild, linux-kernel,
	linux-kselftest

Drop the KSFT_KHDR_INSTALL make target now that all use-cases have
been removed from the other kselftest Makefiles.

Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
 tools/testing/selftests/Makefile |  1 -
 tools/testing/selftests/lib.mk   | 38 --------------------------------
 2 files changed, 39 deletions(-)

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 619451e82863..e060777239a4 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -143,7 +143,6 @@ endif
 # Prepare for headers install
 include $(top_srcdir)/scripts/subarch.include
 ARCH           ?= $(SUBARCH)
-export KSFT_KHDR_INSTALL_DONE := 1
 export BUILD
 export KHDR_INCLUDES
 
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 2a2d240cdc1b..df5f853951f2 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -30,45 +30,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))
 
-ifdef KSFT_KHDR_INSTALL
-top_srcdir ?= ../../../..
-include $(top_srcdir)/scripts/subarch.include
-ARCH		?= $(SUBARCH)
-
-# set default goal to all, so make without a target runs all, even when
-# all isn't the first target in the file.
-.DEFAULT_GOAL := all
-
-# Invoke headers install with --no-builtin-rules to avoid circular
-# dependency in "make kselftest" case. In this case, second level
-# make inherits builtin-rules which will use the rule generate
-# Makefile.o and runs into
-# "Circular Makefile.o <- prepare dependency dropped."
-# and headers_install fails and test compile fails.
-# O= KBUILD_OUTPUT cases don't run into this error, since main Makefile
-# invokes them as sub-makes and --no-builtin-rules is not necessary,
-# but doesn't cause any failures. Keep it simple and use the same
-# flags in both cases.
-# Note that the support to install headers from lib.mk is necessary
-# when test Makefile is run directly with "make -C".
-# When local build is done, headers are installed in the default
-# INSTALL_HDR_PATH usr/include.
-.PHONY: khdr
-.NOTPARALLEL:
-khdr:
-ifndef KSFT_KHDR_INSTALL_DONE
-ifeq (1,$(DEFAULT_INSTALL_HDR_PATH))
-	$(MAKE) --no-builtin-rules ARCH=$(ARCH) -C $(top_srcdir) headers_install
-else
-	$(MAKE) --no-builtin-rules INSTALL_HDR_PATH=$$OUTPUT/usr \
-		ARCH=$(ARCH) -C $(top_srcdir) headers_install
-endif
-endif
-
-all: khdr $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
-else
 all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
-endif
 
 define RUN_TESTS
 	BASE_DIR="$(selfdir)";			\
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 4/4] Makefile: add headers to kselftest targets
  2022-07-12  8:29 [PATCH v2 0/4] Fix kselftest build with sub-directory Guillaume Tucker
                   ` (2 preceding siblings ...)
  2022-07-12  8:29 ` [PATCH v2 3/4] selftests: drop KSFT_KHDR_INSTALL make target Guillaume Tucker
@ 2022-07-12  8:29 ` Guillaume Tucker
  2022-07-13  9:09   ` Nicolas Schier
  2022-07-13  2:27 ` [PATCH v2 0/4] Fix kselftest build with sub-directory Shuah Khan
  4 siblings, 1 reply; 10+ messages in thread
From: Guillaume Tucker @ 2022-07-12  8:29 UTC (permalink / raw)
  To: Masahiro Yamada, Michal Marek, Nick Desaulniers, Shuah Khan, Kees Cook
  Cc: Anders Roxell, Tim.Bird, kernel, linux-kbuild, linux-kernel,
	linux-kselftest

Add headers as a dependency to kselftest targets so that they can be
run directly from the top of the tree.  The kselftest Makefile used to
try to call headers_install "backwards" but failed due to the relative
path not being consistent.

Now we can either run this directly:

  $ make O=build kselftest-all

or this:

  $ make O=build headers
  $ make O=build -C tools/testing/selftests all

The same commands work as well when building directly in the source
tree (no O=) or any arbitrary path (relative or absolute).

Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---

Notes:
    v2: replace headers_install with headers

 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 1a6678d817bd..02502cc4ced5 100644
--- a/Makefile
+++ b/Makefile
@@ -1347,10 +1347,10 @@ tools/%: FORCE
 # Kernel selftest
 
 PHONY += kselftest
-kselftest:
+kselftest: headers
 	$(Q)$(MAKE) -C $(srctree)/tools/testing/selftests run_tests
 
-kselftest-%: FORCE
+kselftest-%: headers FORCE
 	$(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $*
 
 PHONY += kselftest-merge
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/4] selftests: drop KSFT_KHDR_INSTALL make target
  2022-07-12  8:29 ` [PATCH v2 3/4] selftests: drop KSFT_KHDR_INSTALL make target Guillaume Tucker
@ 2022-07-12  9:59   ` Anders Roxell
  2022-07-12 14:57     ` Guillaume Tucker
  0 siblings, 1 reply; 10+ messages in thread
From: Anders Roxell @ 2022-07-12  9:59 UTC (permalink / raw)
  To: Guillaume Tucker
  Cc: Masahiro Yamada, Michal Marek, Nick Desaulniers, Shuah Khan,
	Kees Cook, Tim.Bird, kernel, linux-kbuild, linux-kernel,
	linux-kselftest

On Tue, 12 Jul 2022 at 10:29, Guillaume Tucker
<guillaume.tucker@collabora.com> wrote:
>
> Drop the KSFT_KHDR_INSTALL make target now that all use-cases have
> been removed from the other kselftest Makefiles.
>
> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> ---
>  tools/testing/selftests/Makefile |  1 -
>  tools/testing/selftests/lib.mk   | 38 --------------------------------
>  2 files changed, 39 deletions(-)
>
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 619451e82863..e060777239a4 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -143,7 +143,6 @@ endif
>  # Prepare for headers install
>  include $(top_srcdir)/scripts/subarch.include
>  ARCH           ?= $(SUBARCH)
> -export KSFT_KHDR_INSTALL_DONE := 1
>  export BUILD
>  export KHDR_INCLUDES
>
> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index 2a2d240cdc1b..df5f853951f2 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -30,45 +30,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))
>
> -ifdef KSFT_KHDR_INSTALL
> -top_srcdir ?= ../../../..
> -include $(top_srcdir)/scripts/subarch.include
> -ARCH           ?= $(SUBARCH)
> -
> -# set default goal to all, so make without a target runs all, even when
> -# all isn't the first target in the file.
> -.DEFAULT_GOAL := all
> -
> -# Invoke headers install with --no-builtin-rules to avoid circular
> -# dependency in "make kselftest" case. In this case, second level
> -# make inherits builtin-rules which will use the rule generate
> -# Makefile.o and runs into
> -# "Circular Makefile.o <- prepare dependency dropped."
> -# and headers_install fails and test compile fails.
> -# O= KBUILD_OUTPUT cases don't run into this error, since main Makefile
> -# invokes them as sub-makes and --no-builtin-rules is not necessary,
> -# but doesn't cause any failures. Keep it simple and use the same
> -# flags in both cases.
> -# Note that the support to install headers from lib.mk is necessary
> -# when test Makefile is run directly with "make -C".
> -# When local build is done, headers are installed in the default
> -# INSTALL_HDR_PATH usr/include.
> -.PHONY: khdr
> -.NOTPARALLEL:
> -khdr:
> -ifndef KSFT_KHDR_INSTALL_DONE
> -ifeq (1,$(DEFAULT_INSTALL_HDR_PATH))
> -       $(MAKE) --no-builtin-rules ARCH=$(ARCH) -C $(top_srcdir) headers_install
> -else
> -       $(MAKE) --no-builtin-rules INSTALL_HDR_PATH=$$OUTPUT/usr \
> -               ARCH=$(ARCH) -C $(top_srcdir) headers_install
> -endif
> -endif
> -
> -all: khdr $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
> -else
>  all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
> -endif
>
>  define RUN_TESTS
>         BASE_DIR="$(selfdir)";                  \

Should this be removed as well, since 'khdr' gets droped from file the lib.mk ?

diff --git a/tools/testing/selftests/landlock/Makefile
b/tools/testing/selftests/landlock/Makefile
index 1313e44e8fb9..99f88c52d61a 100644
--- a/tools/testing/selftests/landlock/Makefile
+++ b/tools/testing/selftests/landlock/Makefile
@@ -13,9 +13,6 @@ include ../lib.mk

 khdr_dir = $(top_srcdir)/usr/include

-$(khdr_dir)/linux/landlock.h: khdr
-        @:
-
 $(OUTPUT)/true: true.c
  $(LINK.c) $< $(LDLIBS) -o $@ -static


Cheers,
Anders

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/4] selftests: drop KSFT_KHDR_INSTALL make target
  2022-07-12  9:59   ` Anders Roxell
@ 2022-07-12 14:57     ` Guillaume Tucker
  0 siblings, 0 replies; 10+ messages in thread
From: Guillaume Tucker @ 2022-07-12 14:57 UTC (permalink / raw)
  To: Anders Roxell
  Cc: Masahiro Yamada, Michal Marek, Nick Desaulniers, Shuah Khan,
	Kees Cook, Tim.Bird, kernel, linux-kbuild, linux-kernel,
	linux-kselftest

On 12/07/2022 10:59, Anders Roxell wrote:
> On Tue, 12 Jul 2022 at 10:29, Guillaume Tucker
> <guillaume.tucker@collabora.com> wrote:
>>
>> Drop the KSFT_KHDR_INSTALL make target now that all use-cases have
>> been removed from the other kselftest Makefiles.
>>
>> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
>> ---
>>  tools/testing/selftests/Makefile |  1 -
>>  tools/testing/selftests/lib.mk   | 38 --------------------------------
>>  2 files changed, 39 deletions(-)
>>
>> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
>> index 619451e82863..e060777239a4 100644
>> --- a/tools/testing/selftests/Makefile
>> +++ b/tools/testing/selftests/Makefile
>> @@ -143,7 +143,6 @@ endif
>>  # Prepare for headers install
>>  include $(top_srcdir)/scripts/subarch.include
>>  ARCH           ?= $(SUBARCH)
>> -export KSFT_KHDR_INSTALL_DONE := 1
>>  export BUILD
>>  export KHDR_INCLUDES
>>
>> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
>> index 2a2d240cdc1b..df5f853951f2 100644
>> --- a/tools/testing/selftests/lib.mk
>> +++ b/tools/testing/selftests/lib.mk
>> @@ -30,45 +30,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))
>>
>> -ifdef KSFT_KHDR_INSTALL
>> -top_srcdir ?= ../../../..
>> -include $(top_srcdir)/scripts/subarch.include
>> -ARCH           ?= $(SUBARCH)
>> -
>> -# set default goal to all, so make without a target runs all, even when
>> -# all isn't the first target in the file.
>> -.DEFAULT_GOAL := all
>> -
>> -# Invoke headers install with --no-builtin-rules to avoid circular
>> -# dependency in "make kselftest" case. In this case, second level
>> -# make inherits builtin-rules which will use the rule generate
>> -# Makefile.o and runs into
>> -# "Circular Makefile.o <- prepare dependency dropped."
>> -# and headers_install fails and test compile fails.
>> -# O= KBUILD_OUTPUT cases don't run into this error, since main Makefile
>> -# invokes them as sub-makes and --no-builtin-rules is not necessary,
>> -# but doesn't cause any failures. Keep it simple and use the same
>> -# flags in both cases.
>> -# Note that the support to install headers from lib.mk is necessary
>> -# when test Makefile is run directly with "make -C".
>> -# When local build is done, headers are installed in the default
>> -# INSTALL_HDR_PATH usr/include.
>> -.PHONY: khdr
>> -.NOTPARALLEL:
>> -khdr:
>> -ifndef KSFT_KHDR_INSTALL_DONE
>> -ifeq (1,$(DEFAULT_INSTALL_HDR_PATH))
>> -       $(MAKE) --no-builtin-rules ARCH=$(ARCH) -C $(top_srcdir) headers_install
>> -else
>> -       $(MAKE) --no-builtin-rules INSTALL_HDR_PATH=$$OUTPUT/usr \
>> -               ARCH=$(ARCH) -C $(top_srcdir) headers_install
>> -endif
>> -endif
>> -
>> -all: khdr $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
>> -else
>>  all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
>> -endif
>>
>>  define RUN_TESTS
>>         BASE_DIR="$(selfdir)";                  \
> 
> Should this be removed as well, since 'khdr' gets droped from file the lib.mk ?
> 
> diff --git a/tools/testing/selftests/landlock/Makefile
> b/tools/testing/selftests/landlock/Makefile
> index 1313e44e8fb9..99f88c52d61a 100644
> --- a/tools/testing/selftests/landlock/Makefile
> +++ b/tools/testing/selftests/landlock/Makefile
> @@ -13,9 +13,6 @@ include ../lib.mk
> 
>  khdr_dir = $(top_srcdir)/usr/include
> 
> -$(khdr_dir)/linux/landlock.h: khdr
> -        @:
> -
>  $(OUTPUT)/true: true.c
>   $(LINK.c) $< $(LDLIBS) -o $@ -static


Good point, however I think I'll drop it in PATCH 1/4 "selftests:
drop khdr make target" as it's already dropped there.  Ideally,
the khdr dependency mentioned in this PATCH 3/4 should probably
also be removed in PATCH 1/4.  I'll send a v3 with this.

Thanks,
Guillaume

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 0/4] Fix kselftest build with sub-directory
  2022-07-12  8:29 [PATCH v2 0/4] Fix kselftest build with sub-directory Guillaume Tucker
                   ` (3 preceding siblings ...)
  2022-07-12  8:29 ` [PATCH v2 4/4] Makefile: add headers to kselftest targets Guillaume Tucker
@ 2022-07-13  2:27 ` Shuah Khan
  4 siblings, 0 replies; 10+ messages in thread
From: Shuah Khan @ 2022-07-13  2:27 UTC (permalink / raw)
  To: Guillaume Tucker, Masahiro Yamada, Michal Marek,
	Nick Desaulniers, Shuah Khan, Kees Cook
  Cc: Anders Roxell, Tim.Bird, kernel, linux-kbuild, linux-kernel,
	linux-kselftest, Shuah Khan

On 7/12/22 2:29 AM, Guillaume Tucker wrote:
> Earlier attempts to get "make O=build kselftest-all" to work were
> not successful as they made undesirable changes to some functions
> in the top-level Makefile.  This series takes a different
> approach by removing the root cause of the problem within
> kselftest, which is when the sub-Makefile tries to install kernel
> headers "backwards" by calling make with the top-level Makefile.
> The actual issue comes from the fact that $(srctree) is ".." when
> building in a sub-directory with "O=build" which then obviously
> makes "-C $(top_srcdir)" point outside of the real source tree.
> 
> With this series, the generic kselftest targets work as expected
> from the top level with or without a build directory e.g.:
> 
>    $ make kselftest-all
> 
>    $ make O=build kselftest-all
> 
> Then in order to build using the sub-Makefile explicitly, the
> headers have to be installed first.  This is arguably a valid
> requirement to have when building a tool from a sub-Makefile.
> For example, "make -C tools/testing/nvdimm/" fails in a similar
> way until <asm/rwonce.h> has been generated by a kernel build.
> 
> v2: replace headers_install with headers
> 

I already applied the series. Please send me patches I can apply
on top of the ones in linux-kselftest next branch.

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 4/4] Makefile: add headers to kselftest targets
  2022-07-12  8:29 ` [PATCH v2 4/4] Makefile: add headers to kselftest targets Guillaume Tucker
@ 2022-07-13  9:09   ` Nicolas Schier
  2022-07-13 13:04     ` Guillaume Tucker
  0 siblings, 1 reply; 10+ messages in thread
From: Nicolas Schier @ 2022-07-13  9:09 UTC (permalink / raw)
  To: Guillaume Tucker
  Cc: Masahiro Yamada, Michal Marek, Nick Desaulniers, Shuah Khan,
	Kees Cook, Anders Roxell, Tim.Bird, kernel, linux-kbuild,
	linux-kernel, linux-kselftest

On Tue, 12 Jul 2022 09:29 +0100 Guillaume Tucker wrote:
> Add headers as a dependency to kselftest targets so that they can be
> run directly from the top of the tree.  The kselftest Makefile used to
> try to call headers_install "backwards" but failed due to the relative
> path not being consistent.
> 
> Now we can either run this directly:
> 
>   $ make O=build kselftest-all
> 
> or this:
> 
>   $ make O=build headers
>   $ make O=build -C tools/testing/selftests all
> 
> The same commands work as well when building directly in the source
> tree (no O=) or any arbitrary path (relative or absolute).
> 
> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> ---

You might want to add the 'Reported-by: as you did in 
https://lore.kernel.org/linux-kbuild/a7af58feaa6ae6d3b0c8c55972a470cec62341e5.1657693952.git.guillaume.tucker@collabora.com/ 
?

Tested-by: Nicolas Schier <nicolas@fjasle.eu>

> 
> Notes:
>     v2: replace headers_install with headers
> 
>  Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 1a6678d817bd..02502cc4ced5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1347,10 +1347,10 @@ tools/%: FORCE
>  # Kernel selftest
>  
>  PHONY += kselftest
> -kselftest:
> +kselftest: headers
>  	$(Q)$(MAKE) -C $(srctree)/tools/testing/selftests run_tests
>  
> -kselftest-%: FORCE
> +kselftest-%: headers FORCE
>  	$(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $*
>  
>  PHONY += kselftest-merge
> -- 
> 2.30.2

-- 
epost|xmpp: nicolas@fjasle.eu          irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb  c82b 7d97 0932 55a0 ce7f
     -- frykten for herren er opphav til kunnskap --

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 4/4] Makefile: add headers to kselftest targets
  2022-07-13  9:09   ` Nicolas Schier
@ 2022-07-13 13:04     ` Guillaume Tucker
  0 siblings, 0 replies; 10+ messages in thread
From: Guillaume Tucker @ 2022-07-13 13:04 UTC (permalink / raw)
  To: Nicolas Schier
  Cc: Masahiro Yamada, Michal Marek, Nick Desaulniers, Shuah Khan,
	Kees Cook, Anders Roxell, Tim.Bird, kernel, linux-kbuild,
	linux-kernel, linux-kselftest

On 13/07/2022 11:09, Nicolas Schier wrote:
> On Tue, 12 Jul 2022 09:29 +0100 Guillaume Tucker wrote:
>> Add headers as a dependency to kselftest targets so that they can be
>> run directly from the top of the tree.  The kselftest Makefile used to
>> try to call headers_install "backwards" but failed due to the relative
>> path not being consistent.
>>
>> Now we can either run this directly:
>>
>>   $ make O=build kselftest-all
>>
>> or this:
>>
>>   $ make O=build headers
>>   $ make O=build -C tools/testing/selftests all
>>
>> The same commands work as well when building directly in the source
>> tree (no O=) or any arbitrary path (relative or absolute).
>>
>> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
>> ---
> You might want to add the 'Reported-by: as you did in 
> https://lore.kernel.org/linux-kbuild/a7af58feaa6ae6d3b0c8c55972a470cec62341e5.1657693952.git.guillaume.tucker@collabora.com/ 

Except I don't know who reported the issue, I think it was just
very well known.  KernelCI builds have been working around it for
a couple of years.

The Reported-by in the other patch was about using "headers"
rather than "headers_install", as a follow-up improvement on top
of this patch.

> Tested-by: Nicolas Schier <nicolas@fjasle.eu>

Thank you!

Guillaume

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-07-13 13:04 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-12  8:29 [PATCH v2 0/4] Fix kselftest build with sub-directory Guillaume Tucker
2022-07-12  8:29 ` [PATCH v2 1/4] selftests: drop khdr make target Guillaume Tucker
2022-07-12  8:29 ` [PATCH v2 2/4] selftests: stop using KSFT_KHDR_INSTALL Guillaume Tucker
2022-07-12  8:29 ` [PATCH v2 3/4] selftests: drop KSFT_KHDR_INSTALL make target Guillaume Tucker
2022-07-12  9:59   ` Anders Roxell
2022-07-12 14:57     ` Guillaume Tucker
2022-07-12  8:29 ` [PATCH v2 4/4] Makefile: add headers to kselftest targets Guillaume Tucker
2022-07-13  9:09   ` Nicolas Schier
2022-07-13 13:04     ` Guillaume Tucker
2022-07-13  2:27 ` [PATCH v2 0/4] Fix kselftest build with sub-directory Shuah Khan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).