| Message ID | 20190926224014.28910-1-skhan@linuxfoundation.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] selftests: Add kselftest-all and kselftest-install targets | expand |
On Fri, Sep 27, 2019 at 7:40 AM Shuah Khan <skhan@linuxfoundation.org> wrote: > > Add kselftest-all target to build tests from the top level > Makefile. This is to simplify kselftest use-cases for CI and > distributions where build and test systems are different. > > Current kselftest target builds and runs tests on a development > system which is a developer use-case. > > Add kselftest-install target to install tests from the top level > Makefile. This is to simplify kselftest use-cases for CI and > distributions where build and test systems are different. > > This change addresses requests from developers and testers to add > support for installing kselftest from the main Makefile. > > In addition, make the install directory the same when install is > run using "make kselftest-install" or by running kselftest_install.sh. > Also fix the INSTALL_PATH variable conflict between main Makefile and > selftests Makefile. > > Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> For the top Makefile change: Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com> > --- > Changes since v1: > - Collpased two patches that added separate targets to > build and install into one patch using pattern rule to > invoke all, install, and clean targets from main Makefile. > > Makefile | 5 ++--- > tools/testing/selftests/Makefile | 8 ++++++-- > tools/testing/selftests/kselftest_install.sh | 4 ++-- > 3 files changed, 10 insertions(+), 7 deletions(-) > > diff --git a/Makefile b/Makefile > index d456746da347..ec296c60c1af 100644 > --- a/Makefile > +++ b/Makefile > @@ -1237,9 +1237,8 @@ PHONY += kselftest > kselftest: > $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests run_tests > > -PHONY += kselftest-clean > -kselftest-clean: > - $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests clean > +kselftest-%: FORCE > + $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $* > > PHONY += kselftest-merge > kselftest-merge: > diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile > index c3feccb99ff5..bad18145ed1a 100644 > --- a/tools/testing/selftests/Makefile > +++ b/tools/testing/selftests/Makefile > @@ -171,9 +171,12 @@ run_pstore_crash: > # 1. output_dir=kernel_src > # 2. a separate output directory is specified using O= KBUILD_OUTPUT > # 3. a separate output directory is specified using KBUILD_OUTPUT > +# Avoid conflict with INSTALL_PATH set by the main Makefile > # > -INSTALL_PATH ?= $(BUILD)/install > -INSTALL_PATH := $(abspath $(INSTALL_PATH)) > +KSFT_INSTALL_PATH ?= $(BUILD)/kselftest_install > +KSFT_INSTALL_PATH := $(abspath $(KSFT_INSTALL_PATH)) > +# Avoid changing the rest of the logic here and lib.mk. > +INSTALL_PATH := $(KSFT_INSTALL_PATH) > ALL_SCRIPT := $(INSTALL_PATH)/run_kselftest.sh > > install: all > @@ -203,6 +206,7 @@ ifdef INSTALL_PATH > echo "[ -w /dev/kmsg ] && echo \"kselftest: Running tests in $$TARGET\" >> /dev/kmsg" >> $(ALL_SCRIPT); \ > echo "cd $$TARGET" >> $(ALL_SCRIPT); \ > echo -n "run_many" >> $(ALL_SCRIPT); \ > + echo -n "Emit Tests for $$TARGET\n"; \ > $(MAKE) -s --no-print-directory OUTPUT=$$BUILD_TARGET -C $$TARGET emit_tests >> $(ALL_SCRIPT); \ > echo "" >> $(ALL_SCRIPT); \ > echo "cd \$$ROOT" >> $(ALL_SCRIPT); \ > diff --git a/tools/testing/selftests/kselftest_install.sh b/tools/testing/selftests/kselftest_install.sh > index ec304463883c..e2e1911d62d5 100755 > --- a/tools/testing/selftests/kselftest_install.sh > +++ b/tools/testing/selftests/kselftest_install.sh > @@ -24,12 +24,12 @@ main() > echo "$0: Installing in specified location - $install_loc ..." > fi > > - install_dir=$install_loc/kselftest > + install_dir=$install_loc/kselftest_install > > # Create install directory > mkdir -p $install_dir > # Build tests > - INSTALL_PATH=$install_dir make install > + KSFT_INSTALL_PATH=$install_dir make install > } > > main "$@" > -- > 2.20.1 >
Hi Shuah, I know this has been merged already, so this is just FYI and in case it helps anyone else who's tracking down build failures. Sorry I didn't reply before you merged it, I was on leave. Shuah Khan <skhan@linuxfoundation.org> writes: > Add kselftest-all target to build tests from the top level > Makefile. This is to simplify kselftest use-cases for CI and > distributions where build and test systems are different. > > Current kselftest target builds and runs tests on a development > system which is a developer use-case. > > Add kselftest-install target to install tests from the top level > Makefile. This is to simplify kselftest use-cases for CI and > distributions where build and test systems are different. > > This change addresses requests from developers and testers to add > support for installing kselftest from the main Makefile. > > In addition, make the install directory the same when install is > run using "make kselftest-install" or by running kselftest_install.sh. > Also fix the INSTALL_PATH variable conflict between main Makefile and > selftests Makefile. ... > diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile > index c3feccb99ff5..bad18145ed1a 100644 > --- a/tools/testing/selftests/Makefile > +++ b/tools/testing/selftests/Makefile > @@ -171,9 +171,12 @@ run_pstore_crash: > # 1. output_dir=kernel_src > # 2. a separate output directory is specified using O= KBUILD_OUTPUT > # 3. a separate output directory is specified using KBUILD_OUTPUT > +# Avoid conflict with INSTALL_PATH set by the main Makefile > # > -INSTALL_PATH ?= $(BUILD)/install > -INSTALL_PATH := $(abspath $(INSTALL_PATH)) > +KSFT_INSTALL_PATH ?= $(BUILD)/kselftest_install This change broke all my CI, because the tests no longer install in the place it's expecting them :/ I can fix it by explicitly specifying the install path in my CI scripts. > +KSFT_INSTALL_PATH := $(abspath $(KSFT_INSTALL_PATH)) > +# Avoid changing the rest of the logic here and lib.mk. > +INSTALL_PATH := $(KSFT_INSTALL_PATH) But because the over-rideable variable changed from INSTALL_PATH to KSFT_INSTALL_PATH I will need to export both of them in order for my CI to work with old and new kernels. So to emulate the old behaviour for old & new kernels you need to do: # export KSFT_INSTALL_PATH=install # export INSTALL_PATH=install # make -C tools/testing/selftests install cheers
On 10/15/19 8:00 PM, Michael Ellerman wrote: > Hi Shuah, > > I know this has been merged already, so this is just FYI and in case it > helps anyone else who's tracking down build failures. > Thanks for letting me know. I have been sending updates about breakages. Will send an update with this info. > Sorry I didn't reply before you merged it, I was on leave. > > Shuah Khan <skhan@linuxfoundation.org> writes: >> Add kselftest-all target to build tests from the top level >> Makefile. This is to simplify kselftest use-cases for CI and >> distributions where build and test systems are different. >> >> Current kselftest target builds and runs tests on a development >> system which is a developer use-case. >> >> Add kselftest-install target to install tests from the top level >> Makefile. This is to simplify kselftest use-cases for CI and >> distributions where build and test systems are different. >> >> This change addresses requests from developers and testers to add >> support for installing kselftest from the main Makefile. >> >> In addition, make the install directory the same when install is >> run using "make kselftest-install" or by running kselftest_install.sh. >> Also fix the INSTALL_PATH variable conflict between main Makefile and >> selftests Makefile. > ... >> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile >> index c3feccb99ff5..bad18145ed1a 100644 >> --- a/tools/testing/selftests/Makefile >> +++ b/tools/testing/selftests/Makefile >> @@ -171,9 +171,12 @@ run_pstore_crash: >> # 1. output_dir=kernel_src >> # 2. a separate output directory is specified using O= KBUILD_OUTPUT >> # 3. a separate output directory is specified using KBUILD_OUTPUT >> +# Avoid conflict with INSTALL_PATH set by the main Makefile >> # >> -INSTALL_PATH ?= $(BUILD)/install >> -INSTALL_PATH := $(abspath $(INSTALL_PATH)) >> +KSFT_INSTALL_PATH ?= $(BUILD)/kselftest_install > > This change broke all my CI, because the tests no longer install in the > place it's expecting them :/ > Sorry about that. > I can fix it by explicitly specifying the install path in my CI scripts. > >> +KSFT_INSTALL_PATH := $(abspath $(KSFT_INSTALL_PATH)) >> +# Avoid changing the rest of the logic here and lib.mk. >> +INSTALL_PATH := $(KSFT_INSTALL_PATH) > I searched all the selftests Makefiles for it and convinced myself that, the above would take care of it for these cases. I searched powerpc Makefiles so this doesn't break it. Didn't think about the CI. android/Makefile: mkdir -p $(INSTALL_PATH) android/Makefile: install -t $(INSTALL_PATH) $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES) android/Makefile: $(MAKE) OUTPUT=$$BUILD_TARGET -C $$SUBDIR INSTALL_PATH=$(INSTALL_PATH)/$$SUBDIR install; \ futex/Makefile: mkdir -p $(INSTALL_PATH) futex/Makefile: install -t $(INSTALL_PATH) $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES) futex/Makefile: $(MAKE) OUTPUT=$$BUILD_TARGET -C $$SUBDIR INSTALL_PATH=$(INSTALL_PATH)/$$SUBDIR install; \ sparc64/Makefile: mkdir -p $(INSTALL_PATH) sparc64/Makefile: install -t $(INSTALL_PATH) $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES) sparc64/Makefile: $(MAKE) OUTPUT=$$BUILD_TARGET -C $$SUBDIR INSTALL_PATH=$(INSTALL_PATH)/$$SUBDIR install; \ > But because the over-rideable variable changed from INSTALL_PATH to > KSFT_INSTALL_PATH I will need to export both of them in order for my CI > to work with old and new kernels. My mistake. I overlooked that this could be overridden and could be in used in CI scripts. > > So to emulate the old behaviour for old & new kernels you need to do: > > # export KSFT_INSTALL_PATH=install > # export INSTALL_PATH=install Can we avoid exporting both if we updated INSTALL_PATH with KSFT_INSTALL_PATH in all the Makefiles instead of my shortcut? Would that make it easier for you? > # make -C tools/testing/selftests install > > I am looking to simplify the use-case and ran into the INSTALL_PATH variable conflict between main Makefile and selftests Makefile. I think this should have been KSFT_INSTALL_PATH to begin with. thanks, -- Shuah
On 10/16/19 10:08 AM, Shuah Khan wrote: > On 10/15/19 8:00 PM, Michael Ellerman wrote: >> Hi Shuah, >> >> I know this has been merged already, so this is just FYI and in case it >> helps anyone else who's tracking down build failures. >> > > Thanks for letting me know. I have been sending updates about > breakages. Will send an update with this info. > >> Sorry I didn't reply before you merged it, I was on leave. >> >> Shuah Khan <skhan@linuxfoundation.org> writes: >>> Add kselftest-all target to build tests from the top level >>> Makefile. This is to simplify kselftest use-cases for CI and >>> distributions where build and test systems are different. >>> >>> Current kselftest target builds and runs tests on a development >>> system which is a developer use-case. >>> >>> Add kselftest-install target to install tests from the top level >>> Makefile. This is to simplify kselftest use-cases for CI and >>> distributions where build and test systems are different. >>> >>> This change addresses requests from developers and testers to add >>> support for installing kselftest from the main Makefile. >>> >>> In addition, make the install directory the same when install is >>> run using "make kselftest-install" or by running kselftest_install.sh. >>> Also fix the INSTALL_PATH variable conflict between main Makefile and >>> selftests Makefile. >> ... >>> diff --git a/tools/testing/selftests/Makefile >>> b/tools/testing/selftests/Makefile >>> index c3feccb99ff5..bad18145ed1a 100644 >>> --- a/tools/testing/selftests/Makefile >>> +++ b/tools/testing/selftests/Makefile >>> @@ -171,9 +171,12 @@ run_pstore_crash: >>> # 1. output_dir=kernel_src >>> # 2. a separate output directory is specified using O= KBUILD_OUTPUT >>> # 3. a separate output directory is specified using KBUILD_OUTPUT >>> +# Avoid conflict with INSTALL_PATH set by the main Makefile >>> # >>> -INSTALL_PATH ?= $(BUILD)/install >>> -INSTALL_PATH := $(abspath $(INSTALL_PATH)) >>> +KSFT_INSTALL_PATH ?= $(BUILD)/kselftest_install >> >> This change broke all my CI, because the tests no longer install in the >> place it's expecting them :/ >> > > Sorry about that. > >> I can fix it by explicitly specifying the install path in my CI scripts. >> >>> +KSFT_INSTALL_PATH := $(abspath $(KSFT_INSTALL_PATH)) >>> +# Avoid changing the rest of the logic here and lib.mk. >>> +INSTALL_PATH := $(KSFT_INSTALL_PATH) >> > > > I searched all the selftests Makefiles for it and convinced myself that, > the above would take care of it for these cases. I searched powerpc > Makefiles so this doesn't break it. Didn't think about the CI. > > android/Makefile: mkdir -p $(INSTALL_PATH) > android/Makefile: install -t $(INSTALL_PATH) $(TEST_PROGS) > $(TEST_PROGS_EXTENDED) $(TEST_FILES) > android/Makefile: $(MAKE) OUTPUT=$$BUILD_TARGET -C $$SUBDIR > INSTALL_PATH=$(INSTALL_PATH)/$$SUBDIR install; \ > futex/Makefile: mkdir -p $(INSTALL_PATH) > futex/Makefile: install -t $(INSTALL_PATH) $(TEST_PROGS) > $(TEST_PROGS_EXTENDED) $(TEST_FILES) > futex/Makefile: $(MAKE) OUTPUT=$$BUILD_TARGET -C $$SUBDIR > INSTALL_PATH=$(INSTALL_PATH)/$$SUBDIR install; \ > sparc64/Makefile: mkdir -p $(INSTALL_PATH) > sparc64/Makefile: install -t $(INSTALL_PATH) $(TEST_PROGS) > $(TEST_PROGS_EXTENDED) $(TEST_FILES) > sparc64/Makefile: $(MAKE) OUTPUT=$$BUILD_TARGET -C $$SUBDIR > INSTALL_PATH=$(INSTALL_PATH)/$$SUBDIR install; \ > > >> But because the over-rideable variable changed from INSTALL_PATH to >> KSFT_INSTALL_PATH I will need to export both of them in order for my CI >> to work with old and new kernels. > > My mistake. I overlooked that this could be overridden and could > be in used in CI scripts. > >> Can you send me your CI script for testing my changes? I also want to make sure I don't break your CI again. thanks, -- Shuah
diff --git a/Makefile b/Makefile index d456746da347..ec296c60c1af 100644 --- a/Makefile +++ b/Makefile @@ -1237,9 +1237,8 @@ PHONY += kselftest kselftest: $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests run_tests -PHONY += kselftest-clean -kselftest-clean: - $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests clean +kselftest-%: FORCE + $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $* PHONY += kselftest-merge kselftest-merge: diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index c3feccb99ff5..bad18145ed1a 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -171,9 +171,12 @@ run_pstore_crash: # 1. output_dir=kernel_src # 2. a separate output directory is specified using O= KBUILD_OUTPUT # 3. a separate output directory is specified using KBUILD_OUTPUT +# Avoid conflict with INSTALL_PATH set by the main Makefile # -INSTALL_PATH ?= $(BUILD)/install -INSTALL_PATH := $(abspath $(INSTALL_PATH)) +KSFT_INSTALL_PATH ?= $(BUILD)/kselftest_install +KSFT_INSTALL_PATH := $(abspath $(KSFT_INSTALL_PATH)) +# Avoid changing the rest of the logic here and lib.mk. +INSTALL_PATH := $(KSFT_INSTALL_PATH) ALL_SCRIPT := $(INSTALL_PATH)/run_kselftest.sh install: all @@ -203,6 +206,7 @@ ifdef INSTALL_PATH echo "[ -w /dev/kmsg ] && echo \"kselftest: Running tests in $$TARGET\" >> /dev/kmsg" >> $(ALL_SCRIPT); \ echo "cd $$TARGET" >> $(ALL_SCRIPT); \ echo -n "run_many" >> $(ALL_SCRIPT); \ + echo -n "Emit Tests for $$TARGET\n"; \ $(MAKE) -s --no-print-directory OUTPUT=$$BUILD_TARGET -C $$TARGET emit_tests >> $(ALL_SCRIPT); \ echo "" >> $(ALL_SCRIPT); \ echo "cd \$$ROOT" >> $(ALL_SCRIPT); \ diff --git a/tools/testing/selftests/kselftest_install.sh b/tools/testing/selftests/kselftest_install.sh index ec304463883c..e2e1911d62d5 100755 --- a/tools/testing/selftests/kselftest_install.sh +++ b/tools/testing/selftests/kselftest_install.sh @@ -24,12 +24,12 @@ main() echo "$0: Installing in specified location - $install_loc ..." fi - install_dir=$install_loc/kselftest + install_dir=$install_loc/kselftest_install # Create install directory mkdir -p $install_dir # Build tests - INSTALL_PATH=$install_dir make install + KSFT_INSTALL_PATH=$install_dir make install } main "$@"
Add kselftest-all target to build tests from the top level Makefile. This is to simplify kselftest use-cases for CI and distributions where build and test systems are different. Current kselftest target builds and runs tests on a development system which is a developer use-case. Add kselftest-install target to install tests from the top level Makefile. This is to simplify kselftest use-cases for CI and distributions where build and test systems are different. This change addresses requests from developers and testers to add support for installing kselftest from the main Makefile. In addition, make the install directory the same when install is run using "make kselftest-install" or by running kselftest_install.sh. Also fix the INSTALL_PATH variable conflict between main Makefile and selftests Makefile. Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> --- Changes since v1: - Collpased two patches that added separate targets to build and install into one patch using pattern rule to invoke all, install, and clean targets from main Makefile. Makefile | 5 ++--- tools/testing/selftests/Makefile | 8 ++++++-- tools/testing/selftests/kselftest_install.sh | 4 ++-- 3 files changed, 10 insertions(+), 7 deletions(-)