diff mbox

[4/6] kbuild: add a new kselftest_install make target to install selftests

Message ID 1420794375-31881-4-git-send-email-mpe@ellerman.id.au (mailing list archive)
State New, archived
Headers show

Commit Message

Michael Ellerman Jan. 9, 2015, 9:06 a.m. UTC 
Add a new make target to install kernel selftests. This new target will
build and install selftests.

The default is just $(objtree)/selftests. This is preferable to
something based on $(INSTALL_MOD_PATH) (which defaults to /), as it
allows a normal user to install the tests. This is similar to the
default behaviour of make headers_install.

Therefore the most basic usage is:

$ make kselftests_install
$ ./selftests/all.sh

To install elsewhere use:

$ make kselftests_install INSTALL_SELFTESTS_PATH=/some/where
$ /some/where/all.sh

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 Makefile                         | 13 ++++++++++++-
 tools/testing/selftests/Makefile |  1 +
 2 files changed, 13 insertions(+), 1 deletion(-)

Comments

Shuah Khan Jan. 16, 2015, 4:34 p.m. UTC | #1 
On 01/09/2015 02:06 AM, Michael Ellerman wrote:
> Add a new make target to install kernel selftests. This new target will
> build and install selftests.
> 
> The default is just $(objtree)/selftests. This is preferable to
> something based on $(INSTALL_MOD_PATH) (which defaults to /), as it
> allows a normal user to install the tests. This is similar to the
> default behaviour of make headers_install.

A normal user can install tests at any location they choose by
overriding the default path. For example:

INSTALL_MOD_PATH=/tmp make kselftest_install

will install under tmp.

The approach I used also ties test installs to kernel release.
This addresses an important use-case for kernel developers
that want to compare results from release to release.

The use-case for any user to be able to install tests at
any location is addressed by the above example.

I would like these two above use-cases continued to be supported,
especially the one that tries the test installs to kernel release.
Another goal is to keep changes to the main Makefile minimal and
the rest of the install support belongs under selftests/Makefile
and any other include file (like the one you proposed).

The patch I have in patch v4 addresses the use-cases mentioned above.
I do like the lib.mk approach in general and I am going to review that
patch and give you feedback.

thanks,
-- Shuah
Michael Ellerman Jan. 19, 2015, 12:35 a.m. UTC | #2 
On Fri, 2015-01-16 at 09:34 -0700, Shuah Khan wrote:
> On 01/09/2015 02:06 AM, Michael Ellerman wrote:
> > Add a new make target to install kernel selftests. This new target will
> > build and install selftests.
> > 
> > The default is just $(objtree)/selftests. This is preferable to
> > something based on $(INSTALL_MOD_PATH) (which defaults to /), as it
> > allows a normal user to install the tests. This is similar to the
> > default behaviour of make headers_install.
> 
> A normal user can install tests at any location they choose by
> overriding the default path. For example:
> 
> INSTALL_MOD_PATH=/tmp make kselftest_install
> 
> will install under tmp.

Why default to a directory that most users can't write to? That's not helpful.

Users who are root can override the path, for example:

INSTALL_MOD_PATH=/ make kselftest_install

> The approach I used also ties test installs to kernel release.
> This addresses an important use-case for kernel developers
> that want to compare results from release to release.

Sure, I'm happy to add the kernel release, so the default would be
$(objtree)/selftests/$(kernel-release)/.

> The use-case for any user to be able to install tests at
> any location is addressed by the above example.

The default should work for most users most of the time, / does not achieve
that.

> I would like these two above use-cases continued to be supported,
> especially the one that tries the test installs to kernel release.

That's fine, I'm happy to update this to use kernel release. But defaulting to
/ doesn't make sense.

> Another goal is to keep changes to the main Makefile minimal and
> the rest of the install support belongs under selftests/Makefile
> and any other include file (like the one you proposed).

Yes, this patch does just that.

cheers


--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shuah Khan Jan. 19, 2015, 4:34 p.m. UTC | #3 
On 01/18/2015 05:35 PM, Michael Ellerman wrote:
> On Fri, 2015-01-16 at 09:34 -0700, Shuah Khan wrote:
>> On 01/09/2015 02:06 AM, Michael Ellerman wrote:
>>> Add a new make target to install kernel selftests. This new target will
>>> build and install selftests.
>>>
>>> The default is just $(objtree)/selftests. This is preferable to
>>> something based on $(INSTALL_MOD_PATH) (which defaults to /), as it
>>> allows a normal user to install the tests. This is similar to the
>>> default behaviour of make headers_install.
>>
>> A normal user can install tests at any location they choose by
>> overriding the default path. For example:
>>
>> INSTALL_MOD_PATH=/tmp make kselftest_install
>>
>> will install under tmp.
> 
> Why default to a directory that most users can't write to? That's not helpful.
> 
> Users who are root can override the path, for example:
> 
> INSTALL_MOD_PATH=/ make kselftest_install
> 
>> The approach I used also ties test installs to kernel release.
>> This addresses an important use-case for kernel developers
>> that want to compare results from release to release.
> 
> Sure, I'm happy to add the kernel release, so the default would be
> $(objtree)/selftests/$(kernel-release)/.
> 
>> The use-case for any user to be able to install tests at
>> any location is addressed by the above example.
> 
> The default should work for most users most of the time, / does not achieve
> that.
> 
>> I would like these two above use-cases continued to be supported,
>> especially the one that tries the test installs to kernel release.
> 
> That's fine, I'm happy to update this to use kernel release. But defaulting to
> / doesn't make sense.
> 

I want to keep the kselftest installs to default to the location
other kernel installs such as firmware and modules default to.
This keep the use-case the same as other kernel installs. As user
can override the location, I don't any problems with this.

For most kernel developers, INSTALL_MOD_PATH is the familiar place
to cleanup kernels and I don't see any reason to change that.

You are welcome to send patches to simply the install process for
the individual tests for the next release. I want individual patches
for each test and a separate patch for the shared logic patch.

thanks,
-- Shuah
diff mbox

Patch

diff --git a/Makefile b/Makefile
index eb4eca56843a..c186a928d8be 100644
--- a/Makefile
+++ b/Makefile
@@ -1072,12 +1072,20 @@  headers_check: headers_install
 	$(Q)$(MAKE) $(hdr-inst)=arch/$(hdr-arch)/include/uapi/asm $(hdr-dst) HDRCHECK=1
 
 # ---------------------------------------------------------------------------
-# Kernel selftest
+# Kernel selftest targets
+
+# Default base path for kselftest install
+INSTALL_SELFTESTS_PATH = $(abspath $(objtree)/selftests)
 
 PHONY += kselftest
 kselftest:
 	$(Q)$(MAKE) -C tools/testing/selftests run_tests
 
+# Kernel selftest install
+PHONY += kselftest_install
+kselftest_install:
+	$(Q)$(MAKE) -C tools/testing/selftests INSTALL_PATH=$(INSTALL_SELFTESTS_PATH) install
+
 # ---------------------------------------------------------------------------
 # Modules
 
@@ -1286,6 +1294,9 @@  help:
 	@echo  '                    Build, install, and boot kernel before'
 	@echo  '                    running kselftest on it'
 	@echo  ''
+	@echo  '  kselftest_install - Install selftests to INSTALL_SELFTESTS_PATH'
+	@echo  '                      default: $(INSTALL_SELFTESTS_PATH)'
+	@echo  ''
 	@echo  'Kernel packaging:'
 	@$(MAKE) $(build)=$(package-dir) help
 	@echo  ''
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 4f2849b5ff77..a2345f4512bb 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -62,6 +62,7 @@  ifdef INSTALL_PATH
 
 	@# Ask all targets to emit their test scripts
 	echo "#!/bin/bash\n\n" > $(ALL_SCRIPT)
+	echo "cd \$$(dirname \$$0)" >> $(ALL_SCRIPT)
 	echo "ROOT=\$$PWD\n" >> $(ALL_SCRIPT)
 
 	for TARGET in $(TARGETS); do \