During [MAINTAINERS SUMMIT] & [TECH TOPIC] Improve regression tracking
discussion, it was brought to my attention that kselftest lacks support
for make O=dir use-case which is used by several developers to relocate
objects and keep the source tree clean.
I mentioned in thread that I would take a look at what it takes to support
it and here is the patch series that does that.
This 11 patch series consists of fixes to get "make O=dir kselftest"
use-case working, extending the existing KBUILD_OUTPUT support.
Majority of the changes are made to kselftest common infrastructure.
Some test make files are changed as needed to address the custom build
and run_tests.
-- futex has sub-directories which require custom build and run_tests.
-- sync test needed a few changes to make use of lib.mk as much as possible
and still be able to run its custom build sequence.
With this series the following ways to build and run kselftest is possible:
-- Build all and Relocate objects to /tmp/kselftest and run tests:
make O=/tmp/kselftest kselftest
or
make KBUILD_OUTPUT=/tmp/kselftest kselftest
-- Build TARGETS and Relocate objects to /tmp/kselftest and run tests:
make O=/tmp/kselftest TARGETS="futex sync size" kselftest
or
make KBUILD_OUTPUT=/tmp/kselftest TARGETS="futex sync size" kselftest
-- Clean tests:
make O=/tmp/kselftest kselftest-clean
or
make KBUILD_OUTPUT=/tmp/kselftest kselftest-clean
All existing use-cases documented in Documentation/dev-tools/kselftest.rst
are still supported.
Shuah Khan (11):
Makefile: kselftest and kselftest-clean fail for make O=dir case
selftests: lib.mk: kselftest and kselftest-clean fail for make O=dir
case
selftests: Makefile: clear LDFLAGS for make O=dir use-case
selftests: lib.mk: fix test executable status check to use full path
selftests: watchdog: fix to use TEST_GEN_PROGS and remove clean
selftests: lib.mk: add TEST_CUSTOM_PROGS to allow custom test
run/install
selftests: sync: use TEST_CUSTOM_PROGS instead of TEST_PROGS
selftests: sync: kselftest and kselftest-clean fail for make O=dir
case
selftests: lib.mk: copy test scripts and test files for make O=dir run
selftests: futex: copy sub-dir test scripts for make O=dir run
selftests: mqueue: Use full path to run tests from Makefile
Makefile | 13 ++++++---
tools/testing/selftests/Makefile | 4 +++
tools/testing/selftests/futex/Makefile | 5 +++-
tools/testing/selftests/lib.mk | 44 ++++++++++++++++++++++++++-----
tools/testing/selftests/mqueue/Makefile | 4 +--
tools/testing/selftests/sync/Makefile | 24 +++++++++++++----
tools/testing/selftests/watchdog/Makefile | 7 +----
7 files changed, 77 insertions(+), 24 deletions(-)
--
2.11.0
kselftest and kselftest-clean targets fail when object directory is
specified to relocate objects. Fix it so it can find the source tree
to build from.
make O=/tmp/kselftest_top kselftest
make[1]: Entering directory '/tmp/kselftest_top'
make[2]: Entering directory '/tmp/kselftest_top'
make[2]: *** tools/testing/selftests: No such file or directory. Stop.
make[2]: Leaving directory '/tmp/kselftest_top'
./linux-kselftest/Makefile:1185: recipe for target
'kselftest' failed
make[1]: *** [kselftest] Error 2
make[1]: Leaving directory '/tmp/kselftest_top'
Makefile:145: recipe for target 'sub-make' failed
make: *** [sub-make] Error 2
Signed-off-by: Shuah Khan <[email protected]>
---
Makefile | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index eccb8d704c23..6a85322d0b3e 100644
--- a/Makefile
+++ b/Makefile
@@ -1180,13 +1180,18 @@ headers_check: headers_install
# ---------------------------------------------------------------------------
# Kernel selftest
+PHONY += __kselftest
+ kselftest_src := tools/testing/selftests
+ ifneq ($(KBUILD_SRC),)
+ kselftest_src := $(KBUILD_SRC)/tools/testing/selftests
+ endif
PHONY += kselftest
-kselftest:
- $(Q)$(MAKE) -C tools/testing/selftests run_tests
+kselftest: __kselftest
+ $(Q)$(MAKE) -C $(kselftest_src) run_tests
PHONY += kselftest-clean
-kselftest-clean:
- $(Q)$(MAKE) -C tools/testing/selftests clean
+kselftest-clean: __kselftest
+ $(Q)$(MAKE) -C $(kselftest_src) clean
PHONY += kselftest-merge
kselftest-merge:
--
2.11.0
For make O=dir run_tests to work, test scripts from sub-directories
need to be copied over to the object directory. Running tests from the
object directory is necessary to avoid making the source tree dirty.
Signed-off-by: Shuah Khan <[email protected]>
---
tools/testing/selftests/futex/Makefile | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/futex/Makefile b/tools/testing/selftests/futex/Makefile
index 7c647f619d63..9358cb210fd5 100644
--- a/tools/testing/selftests/futex/Makefile
+++ b/tools/testing/selftests/futex/Makefile
@@ -11,10 +11,13 @@ all:
BUILD_TARGET=$(OUTPUT)/$$DIR; \
mkdir $$BUILD_TARGET -p; \
make OUTPUT=$$BUILD_TARGET -C $$DIR $@;\
+ if [ -e $$DIR/$(TEST_PROGS) ]; then
+ rsync -a $$DIR/$(TEST_PROGS) $$BUILD_TARGET/;
+ fi
done
override define RUN_TESTS
- $(OUTPUT)/run.sh
+ cd $(OUTPUT); ./run.sh
endef
override define INSTALL_RULE
--
2.11.0
sync test fails to build when object directory is specified to relocate
object files. Fix it to specify the correct path. Fix clean target to
remove objects. Also include simplified logic to use TEST_CUSTOM_PROGS
in build and clean targets instead of hard-coding the test name each
time.
Signed-off-by: Shuah Khan <[email protected]>
---
tools/testing/selftests/sync/Makefile | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/sync/Makefile b/tools/testing/selftests/sync/Makefile
index 43db80b71e80..8e04d0afcbd7 100644
--- a/tools/testing/selftests/sync/Makefile
+++ b/tools/testing/selftests/sync/Makefile
@@ -2,14 +2,16 @@ CFLAGS += -O2 -g -std=gnu89 -pthread -Wall -Wextra
CFLAGS += -I../../../../usr/include/
LDFLAGS += -pthread
-# lib.mk TEST_CUSTOM_PROGS var is for custome tests that need special
+.PHONY: all clean
+
+include ../lib.mk
+
+# lib.mk TEST_CUSTOM_PROGS var is for custom tests that need special
# build rules. lib.mk will run and install them.
-TEST_CUSTOM_PROGS = sync_test
+TEST_CUSTOM_PROGS := $(OUTPUT)/sync_test
all: $(TEST_CUSTOM_PROGS)
-include ../lib.mk
-
OBJS = sync_test.o sync.o
TESTS += sync_alloc.o
@@ -20,6 +22,16 @@ TESTS += sync_stress_parallelism.o
TESTS += sync_stress_consumer.o
TESTS += sync_stress_merge.o
-sync_test: $(OBJS) $(TESTS)
+OBJS := $(patsubst %,$(OUTPUT)/%,$(OBJS))
+TESTS := $(patsubst %,$(OUTPUT)/%,$(TESTS))
+
+$(TEST_CUSTOM_PROGS): $(TESTS) $(OBJS)
+ $(CC) -o $(TEST_CUSTOM_PROGS) $(OBJS) $(TESTS) $(CFLAGS) $(LDFLAGS)
+
+$(OBJS): $(OUTPUT)/%.o: %.c
+ $(CC) -c $^ -o $@
+
+$(TESTS): $(OUTPUT)/%.o: %.c
+ $(CC) -c $^ -o $@
-EXTRA_CLEAN := sync_test $(OBJS) $(TESTS)
+EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(OBJS) $(TESTS)
--
2.11.0
Use full path including $(OUTPUT) to run tests from Makefile for
normal case when objects reside in the source tree as well as when
objects are relocated with make O=dir. In both cases $(OUTPUT) will
be set correctly by lib.mk.
Signed-off-by: Shuah Khan <[email protected]>
---
tools/testing/selftests/mqueue/Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/mqueue/Makefile b/tools/testing/selftests/mqueue/Makefile
index 79a664aeb8d7..0f5e347b068d 100644
--- a/tools/testing/selftests/mqueue/Makefile
+++ b/tools/testing/selftests/mqueue/Makefile
@@ -5,8 +5,8 @@ TEST_GEN_PROGS := mq_open_tests mq_perf_tests
include ../lib.mk
override define RUN_TESTS
- @./mq_open_tests /test1 || echo "selftests: mq_open_tests [FAIL]"
- @./mq_perf_tests || echo "selftests: mq_perf_tests [FAIL]"
+ $(OUTPUT)/mq_open_tests /test1 || echo "selftests: mq_open_tests [FAIL]"
+ $(OUTPUT)//mq_perf_tests || echo "selftests: mq_perf_tests [FAIL]"
endef
override define EMIT_TESTS
--
2.11.0
Some tests such as sync can't use generic build rules in lib.mk and require
custom rules. Currently there is no provision to allow custom builds and
test such as sync use TEST_PROGS which is reserved for test shell scripts.
Add TEST_CUSTOM_PROGS variable to lib.mk to run and install custom tests
built by individual test make files.
Signed-off-by: Shuah Khan <[email protected]>
---
tools/testing/selftests/lib.mk | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 22032ba802ba..2e13cabb8007 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -6,6 +6,12 @@ ifeq (0,$(MAKELEVEL))
OUTPUT := $(shell pwd)
endif
+# 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.
+# TEST_PROGS are for test shell scripts.
+# TEST_CUSTOM_PROGS and TEST_PROGS will be run by common run_tests
+# and install targets. Common clean doesn't touch them.
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))
@@ -31,7 +37,7 @@ define RUN_TESTS
endef
run_tests: all
- $(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_PROGS))
+ $(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS))
define INSTALL_RULE
@if [ "X$(TEST_PROGS)$(TEST_PROGS_EXTENDED)$(TEST_FILES)" != "X" ]; then \
@@ -39,10 +45,10 @@ define INSTALL_RULE
echo "rsync -a $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES) $(INSTALL_PATH)/"; \
rsync -a $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES) $(INSTALL_PATH)/; \
fi
- @if [ "X$(TEST_GEN_PROGS)$(TEST_GEN_PROGS_EXTENDED)$(TEST_GEN_FILES)" != "X" ]; then \
+ @if [ "X$(TEST_GEN_PROGS)$(TEST_CUSTOM_PROGS)$(TEST_GEN_PROGS_EXTENDED)$(TEST_GEN_FILES)" != "X" ]; then \
mkdir -p ${INSTALL_PATH}; \
- echo "rsync -a $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) $(INSTALL_PATH)/"; \
- rsync -a $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) $(INSTALL_PATH)/; \
+ echo "rsync -a $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) $(INSTALL_PATH)/"; \
+ rsync -a $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) $(INSTALL_PATH)/; \
fi
endef
@@ -54,7 +60,7 @@ else
endif
define EMIT_TESTS
- @for TEST in $(TEST_GEN_PROGS) $(TEST_PROGS); do \
+ @for TEST in $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS); do \
BASENAME_TEST=`basename $$TEST`; \
echo "(./$$BASENAME_TEST && echo \"selftests: $$BASENAME_TEST [PASS]\") || echo \"selftests: $$BASENAME_TEST [FAIL]\""; \
done;
--
2.11.0
For make O=dir run_tests to work, test scripts, test files, and other
dependencies need to be copied over to the object directory. Running
tests from the object directory is necessary to avoid making the source
tree dirty.
Signed-off-by: Shuah Khan <[email protected]>
---
tools/testing/selftests/lib.mk | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 2e13cabb8007..9aa820d35c0c 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -37,7 +37,18 @@ define RUN_TESTS
endef
run_tests: all
+ifneq ($(KBUILD_SRC),)
+ @if [ "X$(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES)" != "X" ]; then
+ @rsync -aq $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES) $(OUTPUT)
+ fi
+ @if [ "X$(TEST_PROGS)" != "X" ]; then
+ $(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(OUTPUT)/$(TEST_PROGS))
+ else
+ $(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS))
+ fi
+else
$(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS))
+endif
define INSTALL_RULE
@if [ "X$(TEST_PROGS)$(TEST_PROGS_EXTENDED)$(TEST_FILES)" != "X" ]; then \
--
2.11.0
TEST_PROGS should be used for test scripts that don't ned to be built.
Use TEST_GEN_PROGS instead which is intended for test executables.
Remove clean target and let the common clean take care of cleaning.
Signed-off-by: Shuah Khan <[email protected]>
---
tools/testing/selftests/watchdog/Makefile | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/tools/testing/selftests/watchdog/Makefile b/tools/testing/selftests/watchdog/Makefile
index f863c664e3d1..ee068511fd0b 100644
--- a/tools/testing/selftests/watchdog/Makefile
+++ b/tools/testing/selftests/watchdog/Makefile
@@ -1,8 +1,3 @@
-TEST_PROGS := watchdog-test
-
-all: $(TEST_PROGS)
+TEST_GEN_PROGS := watchdog-test
include ../lib.mk
-
-clean:
- rm -fr $(TEST_PROGS)
--
2.11.0
lib.mk var TEST_CUSTOM_PROGS is for tests that need custom build
rules. TEST_PROGS is used for test shell scripts. Fix it to use
TEST_CUSTOM_PROGS. lib.mk will run and install them.
Signed-off-by: Shuah Khan <[email protected]>
---
tools/testing/selftests/sync/Makefile | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/sync/Makefile b/tools/testing/selftests/sync/Makefile
index 4981c6b6d050..43db80b71e80 100644
--- a/tools/testing/selftests/sync/Makefile
+++ b/tools/testing/selftests/sync/Makefile
@@ -2,9 +2,11 @@ CFLAGS += -O2 -g -std=gnu89 -pthread -Wall -Wextra
CFLAGS += -I../../../../usr/include/
LDFLAGS += -pthread
-TEST_PROGS = sync_test
+# lib.mk TEST_CUSTOM_PROGS var is for custome tests that need special
+# build rules. lib.mk will run and install them.
+TEST_CUSTOM_PROGS = sync_test
-all: $(TEST_PROGS)
+all: $(TEST_CUSTOM_PROGS)
include ../lib.mk
--
2.11.0
Fix test executable status check to use full path for make O=dir case,m
when tests are relocated to user specified object directory. Without the
full path, this check fails to find the file and fails the test.
Signed-off-by: Shuah Khan <[email protected]>
---
tools/testing/selftests/lib.mk | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index e779c8758e15..22032ba802ba 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -21,7 +21,7 @@ define RUN_TESTS
test_num=`echo $$test_num+1 | bc`; \
echo "selftests: $$BASENAME_TEST"; \
echo "========================================"; \
- if [ ! -x $$BASENAME_TEST ]; then \
+ if [ ! -x $$TEST ]; then \
echo "selftests: Warning: file $$BASENAME_TEST is not executable, correct this.";\
echo "not ok 1..$$test_num selftests: $$BASENAME_TEST [FAIL]"; \
else \
--
2.11.0
kselftest and kselftest-clean targets fail when object directory is
specified to relocate objects. Main Makefile make O= path clears the
built-in defines LINK.c, COMPILE.S, LINK.S, and RM that are used in
lib.mk to build and clean targets. Define them.
Signed-off-by: Shuah Khan <[email protected]>
---
tools/testing/selftests/lib.mk | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 693616651da5..e779c8758e15 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -7,6 +7,7 @@ OUTPUT := $(shell pwd)
endif
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: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
@@ -62,6 +63,11 @@ endef
emit_tests:
$(EMIT_TESTS)
+# define if isn't already. It is undefined in make O= case.
+ifeq ($(RM),)
+RM := rm -f
+endif
+
define CLEAN
$(RM) -r $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) $(EXTRA_CLEAN)
endef
@@ -69,6 +75,15 @@ endef
clean:
$(CLEAN)
+# When make O= with kselftest target from main level
+# the following aren't defined.
+#
+ifneq ($(KBUILD_SRC),)
+LINK.c = $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_ARCH)
+COMPILE.S = $(CC) $(ASFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c
+LINK.S = $(CC) $(ASFLAGS) $(CPPFLAGS) $(LDFLAGS) $(TARGET_ARCH)
+endif
+
$(OUTPUT)/%:%.c
$(LINK.c) $^ $(LDLIBS) -o $@
--
2.11.0
kselftest target fails when object directory is specified to relocate
objects. Inherited "LDFLAGS = -m" fails the test builds. Clear it.
Signed-off-by: Shuah Khan <[email protected]>
---
tools/testing/selftests/Makefile | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 26ce4f7168be..f4368db011ea 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -52,6 +52,10 @@ override LDFLAGS =
override MAKEFLAGS =
endif
+ifneq ($(KBUILD_SRC),)
+override LDFLAGS =
+endif
+
BUILD := $(O)
ifndef BUILD
BUILD := $(KBUILD_OUTPUT)
--
2.11.0
On Tue, Sep 12, 2017 at 05:52:53PM -0600, Shuah Khan wrote:
> During [MAINTAINERS SUMMIT] & [TECH TOPIC] Improve regression tracking
> discussion, it was brought to my attention that kselftest lacks support
> for make O=dir use-case which is used by several developers to relocate
> objects and keep the source tree clean.
>
> I mentioned in thread that I would take a look at what it takes to support
> it and here is the patch series that does that.
>
> This 11 patch series consists of fixes to get "make O=dir kselftest"
> use-case working, extending the existing KBUILD_OUTPUT support.
> Majority of the changes are made to kselftest common infrastructure.
> Some test make files are changed as needed to address the custom build
> and run_tests.
>
> -- futex has sub-directories which require custom build and run_tests.
> -- sync test needed a few changes to make use of lib.mk as much as possible
> and still be able to run its custom build sequence.
>
> With this series the following ways to build and run kselftest is possible:
>
> -- Build all and Relocate objects to /tmp/kselftest and run tests:
> make O=/tmp/kselftest kselftest
> or
> make KBUILD_OUTPUT=/tmp/kselftest kselftest
>
> -- Build TARGETS and Relocate objects to /tmp/kselftest and run tests:
> make O=/tmp/kselftest TARGETS="futex sync size" kselftest
> or
> make KBUILD_OUTPUT=/tmp/kselftest TARGETS="futex sync size" kselftest
>
> -- Clean tests:
> make O=/tmp/kselftest kselftest-clean
> or
> make KBUILD_OUTPUT=/tmp/kselftest kselftest-clean
>
> All existing use-cases documented in Documentation/dev-tools/kselftest.rst
> are still supported.
Yeah! Nice work.
greg k-h
> -----Original Message-----
> From: Shuah Khan on Tuesday, September 12, 2017 4:53 PM
>
> For make O=dir run_tests to work, test scripts, test files, and other
> dependencies need to be copied over to the object directory. Running
> tests from the object directory is necessary to avoid making the source
> tree dirty.
>
> Signed-off-by: Shuah Khan <[email protected]>
> ---
> tools/testing/selftests/lib.mk | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index 2e13cabb8007..9aa820d35c0c 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -37,7 +37,18 @@ define RUN_TESTS
> endef
>
> run_tests: all
> +ifneq ($(KBUILD_SRC),)
> + @if [ "X$(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES)" !=
> "X" ]; then
> + @rsync -aq $(TEST_PROGS) $(TEST_PROGS_EXTENDED)
> $(TEST_FILES) $(OUTPUT)
rsync seems a bit heavy-handed for this. Is there a reason to use
rsync vs. just a regular cp? Does the existing kbuild system already
have a dependency on rsync? If not, I don't think we should introduce one
here.
BTW - great work!
> + fi
> + @if [ "X$(TEST_PROGS)" != "X" ]; then
> + $(call RUN_TESTS, $(TEST_GEN_PROGS)
> $(TEST_CUSTOM_PROGS) $(OUTPUT)/$(TEST_PROGS))
> + else
> + $(call RUN_TESTS, $(TEST_GEN_PROGS)
> $(TEST_CUSTOM_PROGS))
> + fi
> +else
> $(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS)
> $(TEST_PROGS))
> +endif
>
> define INSTALL_RULE
> @if [ "X$(TEST_PROGS)$(TEST_PROGS_EXTENDED)$(TEST_FILES)" !=
> "X" ]; then \
> --
> 2.11.0
>
> -----Original Message-----
> From: Shuah on Tuesday, September 12, 2017 4:53 PM
>
> For make O=dir run_tests to work, test scripts from sub-directories
> need to be copied over to the object directory. Running tests from the
> object directory is necessary to avoid making the source tree dirty.
>
> Signed-off-by: Shuah Khan <[email protected]>
> ---
> tools/testing/selftests/futex/Makefile | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/futex/Makefile
> b/tools/testing/selftests/futex/Makefile
> index 7c647f619d63..9358cb210fd5 100644
> --- a/tools/testing/selftests/futex/Makefile
> +++ b/tools/testing/selftests/futex/Makefile
> @@ -11,10 +11,13 @@ all:
> BUILD_TARGET=$(OUTPUT)/$$DIR; \
> mkdir $$BUILD_TARGET -p; \
> make OUTPUT=$$BUILD_TARGET -C $$DIR $@;\
> + if [ -e $$DIR/$(TEST_PROGS) ]; then
> + rsync -a $$DIR/$(TEST_PROGS) $$BUILD_TARGET/;
Same issue with rsync here. This could be a 'cp -a', if I'm not mistaken.
> + fi
> done
>
> override define RUN_TESTS
> - $(OUTPUT)/run.sh
> + cd $(OUTPUT); ./run.sh
> endef
>
> override define INSTALL_RULE
> --
> 2.11.0
>
On 09/13/2017 12:14 PM, Bird, Timothy wrote:
>
>
>> -----Original Message-----
>> From: Shuah Khan on Tuesday, September 12, 2017 4:53 PM
>>
>> For make O=dir run_tests to work, test scripts, test files, and other
>> dependencies need to be copied over to the object directory. Running
>> tests from the object directory is necessary to avoid making the source
>> tree dirty.
>>
>> Signed-off-by: Shuah Khan <[email protected]>
>> ---
>> tools/testing/selftests/lib.mk | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
>> index 2e13cabb8007..9aa820d35c0c 100644
>> --- a/tools/testing/selftests/lib.mk
>> +++ b/tools/testing/selftests/lib.mk
>> @@ -37,7 +37,18 @@ define RUN_TESTS
>> endef
>>
>> run_tests: all
>> +ifneq ($(KBUILD_SRC),)
>> + @if [ "X$(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES)" !=
>> "X" ]; then
>> + @rsync -aq $(TEST_PROGS) $(TEST_PROGS_EXTENDED)
>> $(TEST_FILES) $(OUTPUT)
>
> rsync seems a bit heavy-handed for this. Is there a reason to use
> rsync vs. just a regular cp? Does the existing kbuild system already
> have a dependency on rsync? If not, I don't think we should introduce one
> here.
>
rsync is used now in kselftest make files for install. The reason
I picked rsync in this case is that these files might exist in the
object directory, if the same object directory is used for multiple
kselftest builds. Using rsync helps avoid copying them again. These
files are not generated each time build is run. These are shell scripts
and other dependencies that won't change from build to build.
Are you concerned about rsync availability on some environments?
> BTW - great work!
Thanks.
>
>> + fi
>> + @if [ "X$(TEST_PROGS)" != "X" ]; then
>> + $(call RUN_TESTS, $(TEST_GEN_PROGS)
>> $(TEST_CUSTOM_PROGS) $(OUTPUT)/$(TEST_PROGS))
>> + else
>> + $(call RUN_TESTS, $(TEST_GEN_PROGS)
>> $(TEST_CUSTOM_PROGS))
>> + fi
>> +else
>> $(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS)
>> $(TEST_PROGS))
>> +endif
>>
>> define INSTALL_RULE
>> @if [ "X$(TEST_PROGS)$(TEST_PROGS_EXTENDED)$(TEST_FILES)" !=
>> "X" ]; then \
>> --
>> 2.11.0
>>
>
thanks,
-- Shuah
On 09/12/2017 05:52 PM, Shuah Khan wrote:
> kselftest and kselftest-clean targets fail when object directory is
> specified to relocate objects. Fix it so it can find the source tree
> to build from.
>
> make O=/tmp/kselftest_top kselftest
>
> make[1]: Entering directory '/tmp/kselftest_top'
> make[2]: Entering directory '/tmp/kselftest_top'
> make[2]: *** tools/testing/selftests: No such file or directory. Stop.
> make[2]: Leaving directory '/tmp/kselftest_top'
> ./linux-kselftest/Makefile:1185: recipe for target
> 'kselftest' failed
> make[1]: *** [kselftest] Error 2
> make[1]: Leaving directory '/tmp/kselftest_top'
> Makefile:145: recipe for target 'sub-make' failed
> make: *** [sub-make] Error 2
>
> Signed-off-by: Shuah Khan <[email protected]>
Hi Masahiro/Michal,
Is it okay to take this patch via linux-kselftest git? If you are okay
with that, please Ack it and I will plan to include this in my update.
thanks,
-- Shuah
> ---
> Makefile | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index eccb8d704c23..6a85322d0b3e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1180,13 +1180,18 @@ headers_check: headers_install
> # ---------------------------------------------------------------------------
> # Kernel selftest
>
> +PHONY += __kselftest
> + kselftest_src := tools/testing/selftests
> + ifneq ($(KBUILD_SRC),)
> + kselftest_src := $(KBUILD_SRC)/tools/testing/selftests
> + endif
> PHONY += kselftest
> -kselftest:
> - $(Q)$(MAKE) -C tools/testing/selftests run_tests
> +kselftest: __kselftest
> + $(Q)$(MAKE) -C $(kselftest_src) run_tests
>
> PHONY += kselftest-clean
> -kselftest-clean:
> - $(Q)$(MAKE) -C tools/testing/selftests clean
> +kselftest-clean: __kselftest
> + $(Q)$(MAKE) -C $(kselftest_src) clean
>
> PHONY += kselftest-merge
> kselftest-merge:
>
2017-09-19 8:14 GMT+09:00 Shuah Khan <[email protected]>:
> On 09/12/2017 05:52 PM, Shuah Khan wrote:
>> kselftest and kselftest-clean targets fail when object directory is
>> specified to relocate objects. Fix it so it can find the source tree
>> to build from.
>>
>> make O=/tmp/kselftest_top kselftest
>>
>> make[1]: Entering directory '/tmp/kselftest_top'
>> make[2]: Entering directory '/tmp/kselftest_top'
>> make[2]: *** tools/testing/selftests: No such file or directory. Stop.
>> make[2]: Leaving directory '/tmp/kselftest_top'
>> ./linux-kselftest/Makefile:1185: recipe for target
>> 'kselftest' failed
>> make[1]: *** [kselftest] Error 2
>> make[1]: Leaving directory '/tmp/kselftest_top'
>> Makefile:145: recipe for target 'sub-make' failed
>> make: *** [sub-make] Error 2
>>
>> Signed-off-by: Shuah Khan <[email protected]>
>
> Hi Masahiro/Michal,
>
> Is it okay to take this patch via linux-kselftest git? If you are okay
> with that, please Ack it and I will plan to include this in my update.
>
> thanks,
> -- Shuah
>
>
>> ---
>> Makefile | 13 +++++++++----
>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index eccb8d704c23..6a85322d0b3e 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1180,13 +1180,18 @@ headers_check: headers_install
>> # ---------------------------------------------------------------------------
>> # Kernel selftest
>>
>> +PHONY += __kselftest
>> + kselftest_src := tools/testing/selftests
>> + ifneq ($(KBUILD_SRC),)
>> + kselftest_src := $(KBUILD_SRC)/tools/testing/selftests
>> + endif
>> PHONY += kselftest
>> -kselftest:
>> - $(Q)$(MAKE) -C tools/testing/selftests run_tests
>> +kselftest: __kselftest
>> + $(Q)$(MAKE) -C $(kselftest_src) run_tests
>>
>> PHONY += kselftest-clean
>> -kselftest-clean:
>> - $(Q)$(MAKE) -C tools/testing/selftests clean
>> +kselftest-clean: __kselftest
>> + $(Q)$(MAKE) -C $(kselftest_src) clean
>>
>> PHONY += kselftest-merge
>> kselftest-merge:
>>
>
Why don't you simply add $(srctree)/ to tools/testing/selftests?
Like,
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
--
Best Regards
Masahiro Yamada
On 09/19/2017 09:39 PM, Masahiro Yamada wrote:
> 2017-09-19 8:14 GMT+09:00 Shuah Khan <[email protected]>:
>> On 09/12/2017 05:52 PM, Shuah Khan wrote:
>>> kselftest and kselftest-clean targets fail when object directory is
>>> specified to relocate objects. Fix it so it can find the source tree
>>> to build from.
>>>
>>> make O=/tmp/kselftest_top kselftest
>>>
>>> make[1]: Entering directory '/tmp/kselftest_top'
>>> make[2]: Entering directory '/tmp/kselftest_top'
>>> make[2]: *** tools/testing/selftests: No such file or directory. Stop.
>>> make[2]: Leaving directory '/tmp/kselftest_top'
>>> ./linux-kselftest/Makefile:1185: recipe for target
>>> 'kselftest' failed
>>> make[1]: *** [kselftest] Error 2
>>> make[1]: Leaving directory '/tmp/kselftest_top'
>>> Makefile:145: recipe for target 'sub-make' failed
>>> make: *** [sub-make] Error 2
>>>
>>> Signed-off-by: Shuah Khan <[email protected]>
>>
>> Hi Masahiro/Michal,
>>
>> Is it okay to take this patch via linux-kselftest git? If you are okay
>> with that, please Ack it and I will plan to include this in my update.
>>
>> thanks,
>> -- Shuah
>>
>>
>>> ---
>>> Makefile | 13 +++++++++----
>>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index eccb8d704c23..6a85322d0b3e 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -1180,13 +1180,18 @@ headers_check: headers_install
>>> # ---------------------------------------------------------------------------
>>> # Kernel selftest
>>>
>>> +PHONY += __kselftest
>>> + kselftest_src := tools/testing/selftests
>>> + ifneq ($(KBUILD_SRC),)
>>> + kselftest_src := $(KBUILD_SRC)/tools/testing/selftests
>>> + endif
>>> PHONY += kselftest
>>> -kselftest:
>>> - $(Q)$(MAKE) -C tools/testing/selftests run_tests
>>> +kselftest: __kselftest
>>> + $(Q)$(MAKE) -C $(kselftest_src) run_tests
>>>
>>> PHONY += kselftest-clean
>>> -kselftest-clean:
>>> - $(Q)$(MAKE) -C tools/testing/selftests clean
>>> +kselftest-clean: __kselftest
>>> + $(Q)$(MAKE) -C $(kselftest_src) clean
>>>
>>> PHONY += kselftest-merge
>>> kselftest-merge:
>>>
>>
>
> Why don't you simply add $(srctree)/ to tools/testing/selftests?
>
> Like,
>
> 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
>
>
>
I started with the simpler logic and decided to add explicit check
for KBUILD_SRC due to isolate some error I was seeing from sub-make.
Forgot to go back and revisit the simpler logic. Thanks for catching
this. I am not seeing any issues with the simplified. Sending v2.
Thanks for the review.
-- Shuah
On Tue, Sep 12, 2017 at 05:53:03PM -0600, Shuah Khan wrote:
> For make O=dir run_tests to work, test scripts from sub-directories
> need to be copied over to the object directory. Running tests from the
> object directory is necessary to avoid making the source tree dirty.
>
> Signed-off-by: Shuah Khan <[email protected]>
> ---
> tools/testing/selftests/futex/Makefile | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/futex/Makefile b/tools/testing/selftests/futex/Makefile
> index 7c647f619d63..9358cb210fd5 100644
> --- a/tools/testing/selftests/futex/Makefile
> +++ b/tools/testing/selftests/futex/Makefile
> @@ -11,10 +11,13 @@ all:
> BUILD_TARGET=$(OUTPUT)/$$DIR; \
> mkdir $$BUILD_TARGET -p; \
> make OUTPUT=$$BUILD_TARGET -C $$DIR $@;\
> + if [ -e $$DIR/$(TEST_PROGS) ]; then
> + rsync -a $$DIR/$(TEST_PROGS) $$BUILD_TARGET/;
Hrm, I was going to raise a concern with adding an rsync dependency
here, but it is already used several times by lib.mk, so that isn't new.
Reviewed-by: Darren Hart (VMware) <[email protected]>
--
Darren Hart
VMware Open Source Technology Center