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.
Guillaume Tucker (4): selftests: drop khdr make target selftests: stop using KSFT_KHDR_INSTALL selftests: drop KSFT_KHDR_INSTALL make target Makefile: add headers_install 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
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
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
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)"; \
Add headers_install 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_install $ make O=build -C tools/testing/selftest 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 --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile index 1a6678d817bd..afc9d739ba44 100644 --- a/Makefile +++ b/Makefile @@ -1347,10 +1347,10 @@ tools/%: FORCE # Kernel selftest
PHONY += kselftest -kselftest: +kselftest: headers_install $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests run_tests
-kselftest-%: FORCE +kselftest-%: headers_install FORCE $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $*
PHONY += kselftest-merge
On Sat, Jul 9, 2022 at 1:23 AM Guillaume Tucker guillaume.tucker@collabora.com wrote:
Add headers_install 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_install $ make O=build -C tools/testing/selftest 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
Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile index 1a6678d817bd..afc9d739ba44 100644 --- a/Makefile +++ b/Makefile @@ -1347,10 +1347,10 @@ tools/%: FORCE # Kernel selftest
PHONY += kselftest -kselftest: +kselftest: headers_install $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests run_tests
Nit. Please use 'headers' for in-kernel use of exportedI headers.
kselftest: headers
-kselftest-%: FORCE +kselftest-%: headers_install FORCE $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $*
Ditto.
kselftest-%: headers FORCE
PHONY += kselftest-merge
2.30.2
On 12/07/2022 02:35, Masahiro Yamada wrote:
On Sat, Jul 9, 2022 at 1:23 AM Guillaume Tucker guillaume.tucker@collabora.com wrote:
Add headers_install 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_install $ make O=build -C tools/testing/selftest 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
Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile index 1a6678d817bd..afc9d739ba44 100644 --- a/Makefile +++ b/Makefile @@ -1347,10 +1347,10 @@ tools/%: FORCE # Kernel selftest
PHONY += kselftest -kselftest: +kselftest: headers_install $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests run_tests
Nit. Please use 'headers' for in-kernel use of exportedI headers.
kselftest: headers
-kselftest-%: FORCE +kselftest-%: headers_install FORCE $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $*
Ditto.
kselftest-%: headers FORCE
Thank you all for the reviews.
I've sent a v2 with this tweak and double-checked that the kselftest build produced exactly the same result.
Best wishes, Guillaume
On 08/07/2022 18:23, Guillaume Tucker wrote:
$ make O=build headers_install $ make O=build -C tools/testing/selftest all
Typo, it should be selftests:
$ make O=build -C tools/testing/selftests all
Thanks, Guillaume
On 7/8/22 10:23 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.
Guillaume Tucker (4): selftests: drop khdr make target selftests: stop using KSFT_KHDR_INSTALL selftests: drop KSFT_KHDR_INSTALL make target Makefile: add headers_install to kselftest targets
This takes us to back to the state before b2d35fa5fc80 added khdr support. I reluctantly agreed to the change and it has proven to be a problematic change. I would rather have had the dependency stated that headers should be installed prior to building tests - test build depends on kernel build anyway and having dependency on headers having build isn't a huge deal.
So I am in favor of getting rid of khdr support. However, this khdr support was a change originated from Linaro test ring. Undoing this might have implication on their workflow.
I will pull them into the discussion so they are aware of it and be prepared for this change.
thanks, -- Shuah
-----Original Message----- From: Shuah Khan skhan@linuxfoundation.org
On 7/8/22 10:23 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.
Guillaume Tucker (4): selftests: drop khdr make target selftests: stop using KSFT_KHDR_INSTALL selftests: drop KSFT_KHDR_INSTALL make target Makefile: add headers_install to kselftest targets
This takes us to back to the state before b2d35fa5fc80 added khdr support. I reluctantly agreed to the change and it has proven to be a problematic change. I would rather have had the dependency stated that headers should be installed prior to building tests - test build depends on kernel build anyway and having dependency on headers having build isn't a huge deal.
So I am in favor of getting rid of khdr support. However, this khdr support was a change originated from Linaro test ring. Undoing this might have implication on their workflow.
I will pull them into the discussion so they are aware of it and be prepared for this change.
I ran into this bug quite a while ago. I reported it here: https://lore.kernel.org/all/ECADFF3FD767C149AD96A924E7EA6EAF977BD214@USCULXM... it was fixed here: https://lore.kernel.org/all/20191015014505.14259-1-skhan@linuxfoundation.org... but apparently reverting it was discussed, based on this: https://lore.kernel.org/all/8d34a9b9-f8f3-0e37-00bf-c342cf3d4074@arm.com/
I'm not sure what happened after that.
I was able to work around it by avoiding putting the build directory inside the source tree.
I strongly support getting rid of the khdr stuff, as it's quite hard to follow. I think my workflow would not be affected (but I should run off and test it.)
Thanks for working on this Guillaume! -- Tim
On Fri, 8 Jul 2022 at 19:14, Shuah Khan skhan@linuxfoundation.org wrote:
On 7/8/22 10:23 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.
Guillaume Tucker (4): selftests: drop khdr make target selftests: stop using KSFT_KHDR_INSTALL selftests: drop KSFT_KHDR_INSTALL make target Makefile: add headers_install to kselftest targets
This takes us to back to the state before b2d35fa5fc80 added khdr support. I reluctantly agreed to the change and it has proven to be a problematic change. I would rather have had the dependency stated that headers should be installed prior to building tests - test build depends on kernel build anyway and having dependency on headers having build isn't a huge deal.
I agree that it's not a huge deal.
So I am in favor of getting rid of khdr support. However, this khdr support was a change originated from Linaro test ring. Undoing this might have implication on their workflow.
It shouldn't be a problem. I've been running these patches through a smoke test and it looks good.
Tested-by: Anders Roxell anders.roxell@linaro.org
Cheers, Anders
I will pull them into the discussion so they are aware of it and be prepared for this change.
thanks, -- Shuah
On 7/11/22 6:13 AM, Anders Roxell wrote:
On Fri, 8 Jul 2022 at 19:14, Shuah Khan skhan@linuxfoundation.org wrote:
On 7/8/22 10:23 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-allThen 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.
Guillaume Tucker (4): selftests: drop khdr make target selftests: stop using KSFT_KHDR_INSTALL selftests: drop KSFT_KHDR_INSTALL make target Makefile: add headers_install to kselftest targets
This takes us to back to the state before b2d35fa5fc80 added khdr support. I reluctantly agreed to the change and it has proven to be a problematic change. I would rather have had the dependency stated that headers should be installed prior to building tests - test build depends on kernel build anyway and having dependency on headers having build isn't a huge deal.
I agree that it's not a huge deal.
So I am in favor of getting rid of khdr support. However, this khdr support was a change originated from Linaro test ring. Undoing this might have implication on their workflow.
It shouldn't be a problem. I've been running these patches through a smoke test and it looks good.
Tested-by: Anders Roxell anders.roxell@linaro.org
Thank you Anders for confirming this isn't a problem for Linaro workflow and testing.
Than you Guillaume for fixing the problem. I will apply these for 5.20-rc1.
thanks, -- Shuah
linux-kselftest-mirror@lists.linaro.org
-
Anders Roxell -
Bird, Tim -
Guillaume Tucker -
Masahiro Yamada -
Shuah Khan