Re: [PATCH 1/5] selftests: Fix selftests build to just build, not run tests

[Shuah Khan]

Hi Michael and Bamovar,

On 03/01/2017 11:43 PM, Michael Ellerman wrote:
> Shuah Khan  writes:
>> Hi Bamovar,
>>
>> Your original series badly broke the selftest build. I can no longer
>> build individual tests. For example:
>>
>> cd breakpoints/
>> shuah@shuah-XPS-13-9350:/lkml/linux_4.11/tools/testing/selftests/breakpoints$
>>  make
>> gcc breakpoint_test.c  -o /breakpoint_test
>> /usr/bin/ld: cannot open output file /breakpoint_test: Permission denied
>> collect2: error: ld returned 1 exit status
>> ../lib.mk:54: recipe for target '/breakpoint_test' failed
>> make: *** [/breakpoint_test] Error 1
> 
> I also got a report of that just yesterday.
> 
> You can do:
> 
> $ cd tools/testing/selftests ; make TARGETS=breakpoints
> 
> But it's not ideal.
> 
>> commit a8ba798bc8ec663cf02e80b0dd770324de9bafd9
>> Author: bamvor.zhangj...@huawei.com 
>> Date:   Tue Nov 29 19:55:52 2016 +0800
>>
>> selftests: enable O and KBUILD_OUTPUT
>>
>> I believe the above patch is one of the suspects. Michael fixed
>> some of the problems in this patch and others he sent.
> 
> It is that patch which caused it yes.
> 
>> At the moment individual tests will not build.
>> tools/testing/selftests/x86$ make
>> Makefile:44: warning: overriding recipe for target 'clean'
>> ../lib.mk:51: warning: ignoring old recipe for target 'clean'
>> gcc -m64 -o /single_step_syscall_64 -O2 -g -std=gnu99 -pthread -Wall  
>> single_step_syscall.c -lrt -ldl
>> /usr/bin/ld: cannot open output file /single_step_syscall_64: Permission 
>> denied
>> collect2: error: ld returned 1 exit status
>> Makefile:50: recipe for target '/single_step_syscall_64' failed
>> make: *** [/single_step_syscall_64] Error 1
>>
>>
>> My guess is OUTPUT doesn't resolve in individual builds from the test 
>> directory.
>> We have to get this fixed for 4.11-rc1
> 
> Yeah, OUTPUT is passed down from the top-level Makefile.
> 
>> Simply Reverting a8ba798bc8ec663cf02e80b0dd770324de9bafd9 doesn't work.
>> Michael's patches depend on this. So anyway, please let me know if you
>> can fix this quickly. I am going to be trying a few things today as well.
> 
> This seems to work, but needs some testing with and without OUTPUT set.
> 
> Basically if OUTPUT is not set, assume the current directory. It should
> only take effect when someone builds from an individual directory,
> because if you build from the top level OUTPUT is already set.
> 
> 
> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index ce96d80ad64f..9087fa949e82 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -2,6 +2,8 @@
>  # Makefile can operate with or without the kbuild infrastructure.
>  CC := $(CROSS_COMPILE)gcc
>  
> +OUTPUT ?= $(PWD)
> +
>  TEST_GEN_PROGS := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS))
>  TEST_GEN_FILES := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_FILES))
>  
> 
> 
> cheers
> 

I sent a fix to lib.mk to set the OUTPUT. Please review. I can request
Linus to merge it in before 4.11-rc1 comes out.

thanks,
-- Shuah

Re: [PATCH 1/5] selftests: Fix selftests build to just build, not run tests

[Michael Ellerman]

Shuah Khan  writes:
> Hi Bamovar,
>
> Your original series badly broke the selftest build. I can no longer
> build individual tests. For example:
>
> cd breakpoints/
> shuah@shuah-XPS-13-9350:/lkml/linux_4.11/tools/testing/selftests/breakpoints$ 
> make
> gcc breakpoint_test.c  -o /breakpoint_test
> /usr/bin/ld: cannot open output file /breakpoint_test: Permission denied
> collect2: error: ld returned 1 exit status
> ../lib.mk:54: recipe for target '/breakpoint_test' failed
> make: *** [/breakpoint_test] Error 1

I also got a report of that just yesterday.

You can do:

$ cd tools/testing/selftests ; make TARGETS=breakpoints

But it's not ideal.

> commit a8ba798bc8ec663cf02e80b0dd770324de9bafd9
> Author: bamvor.zhangj...@huawei.com 
> Date:   Tue Nov 29 19:55:52 2016 +0800
>
> selftests: enable O and KBUILD_OUTPUT
>
> I believe the above patch is one of the suspects. Michael fixed
> some of the problems in this patch and others he sent.

It is that patch which caused it yes.

> At the moment individual tests will not build.
> tools/testing/selftests/x86$ make
> Makefile:44: warning: overriding recipe for target 'clean'
> ../lib.mk:51: warning: ignoring old recipe for target 'clean'
> gcc -m64 -o /single_step_syscall_64 -O2 -g -std=gnu99 -pthread -Wall  
> single_step_syscall.c -lrt -ldl
> /usr/bin/ld: cannot open output file /single_step_syscall_64: Permission 
> denied
> collect2: error: ld returned 1 exit status
> Makefile:50: recipe for target '/single_step_syscall_64' failed
> make: *** [/single_step_syscall_64] Error 1
>
>
> My guess is OUTPUT doesn't resolve in individual builds from the test 
> directory.
> We have to get this fixed for 4.11-rc1

Yeah, OUTPUT is passed down from the top-level Makefile.

> Simply Reverting a8ba798bc8ec663cf02e80b0dd770324de9bafd9 doesn't work.
> Michael's patches depend on this. So anyway, please let me know if you
> can fix this quickly. I am going to be trying a few things today as well.

This seems to work, but needs some testing with and without OUTPUT set.

Basically if OUTPUT is not set, assume the current directory. It should
only take effect when someone builds from an individual directory,
because if you build from the top level OUTPUT is already set.


diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index ce96d80ad64f..9087fa949e82 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -2,6 +2,8 @@
 # Makefile can operate with or without the kbuild infrastructure.
 CC := $(CROSS_COMPILE)gcc
 
+OUTPUT ?= $(PWD)
+
 TEST_GEN_PROGS := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS))
 TEST_GEN_FILES := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_FILES))
 


cheers

Re: [PATCH 1/5] selftests: Fix selftests build to just build, not run tests

[Shuah Khan]

On 02/13/2017 07:09 PM, Michael Ellerman wrote:
> Michael Ellerman  writes:
> 
>> In commit 88baa78d1f31 ("selftests: remove duplicated all and clean
>> target"), the "all" target was removed from individual Makefiles and
>> added to lib.mk.
>>
>> However the "all" target was added to lib.mk *after* the existing
>> "runtests" target. This means "runtests" becomes the first (default)
>> target for most of our Makefiles.
> ...
>>
>> Fix it by moving the "all" target to the start of lib.mk, making it the
>> default target.
>>
>> Fixes: 88baa78d1f31 ("selftests: remove duplicated all and clean target")
>> Signed-off-by: Michael Ellerman 
> 
> Hi Shuah,
> 
> Can you please merge this series into linux-next?
> 
> The selftests are badly broken otherwise.
> 
> cheers
> 

Hi Bamovar,

Your original series badly broke the selftest build. I can no longer
build individual tests. For example:

cd breakpoints/
shuah@shuah-XPS-13-9350:/lkml/linux_4.11/tools/testing/selftests/breakpoints$ 
make
gcc breakpoint_test.c  -o /breakpoint_test
/usr/bin/ld: cannot open output file /breakpoint_test: Permission denied
collect2: error: ld returned 1 exit status
../lib.mk:54: recipe for target '/breakpoint_test' failed
make: *** [/breakpoint_test] Error 1

commit a8ba798bc8ec663cf02e80b0dd770324de9bafd9
Author: bamvor.zhangj...@huawei.com 
Date:   Tue Nov 29 19:55:52 2016 +0800

selftests: enable O and KBUILD_OUTPUT

I believe the above patch is one of the suspects. Michael fixed
some of the problems in this patch and others he sent.

At the moment individual tests will not build.
tools/testing/selftests/x86$ make
Makefile:44: warning: overriding recipe for target 'clean'
../lib.mk:51: warning: ignoring old recipe for target 'clean'
gcc -m64 -o /single_step_syscall_64 -O2 -g -std=gnu99 -pthread -Wall  
single_step_syscall.c -lrt -ldl
/usr/bin/ld: cannot open output file /single_step_syscall_64: Permission denied
collect2: error: ld returned 1 exit status
Makefile:50: recipe for target '/single_step_syscall_64' failed
make: *** [/single_step_syscall_64] Error 1


My guess is OUTPUT doesn't resolve in individual builds from the test directory.
We have to get this fixed for 4.11-rc1

Simply Reverting a8ba798bc8ec663cf02e80b0dd770324de9bafd9 doesn't work.
Michael's patches depend on this. So anyway, please let me know if you
can fix this quickly. I am going to be trying a few things today as well.

thanks,
-- Shuah

Re: [PATCH 1/5] selftests: Fix selftests build to just build, not run tests

[Michael Ellerman]

On 15 February 2017 03:14:24 GMT+11:00, Shuah Khan  
wrote:
>On 02/13/2017 07:09 PM, Michael Ellerman wrote:
>> Michael Ellerman  writes:
>> 
>>> In commit 88baa78d1f31 ("selftests: remove duplicated all and clean
>>> target"), the "all" target was removed from individual Makefiles and
>>> added to lib.mk.
>>>
>>> However the "all" target was added to lib.mk *after* the existing
>>> "runtests" target. This means "runtests" becomes the first (default)
>>> target for most of our Makefiles.
>> ...
>>>
>>> Fix it by moving the "all" target to the start of lib.mk, making it
>the
>>> default target.
>>>
>>> Fixes: 88baa78d1f31 ("selftests: remove duplicated all and clean
>target")
>>> Signed-off-by: Michael Ellerman 
>> 
>> Hi Shuah,
>> 
>> Can you please merge this series into linux-next?
>> 
>> The selftests are badly broken otherwise.
>> 
>> cheers
>> 
>
>Hi Michael,
>
>Thanks. All 5 patches are now in linux-kselftest next with Tested-by
>tag from Bamvor for 1,2,3.

Thanks!
-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

Re: [PATCH 1/5] selftests: Fix selftests build to just build, not run tests

[Shuah Khan]

On 02/13/2017 07:09 PM, Michael Ellerman wrote:
> Michael Ellerman  writes:
> 
>> In commit 88baa78d1f31 ("selftests: remove duplicated all and clean
>> target"), the "all" target was removed from individual Makefiles and
>> added to lib.mk.
>>
>> However the "all" target was added to lib.mk *after* the existing
>> "runtests" target. This means "runtests" becomes the first (default)
>> target for most of our Makefiles.
> ...
>>
>> Fix it by moving the "all" target to the start of lib.mk, making it the
>> default target.
>>
>> Fixes: 88baa78d1f31 ("selftests: remove duplicated all and clean target")
>> Signed-off-by: Michael Ellerman 
> 
> Hi Shuah,
> 
> Can you please merge this series into linux-next?
> 
> The selftests are badly broken otherwise.
> 
> cheers
> 

Hi Michael,

Thanks. All 5 patches are now in linux-kselftest next with Tested-by
tag from Bamvor for 1,2,3.

thank you both,
-- Shuah

Re: [PATCH 1/5] selftests: Fix selftests build to just build, not run tests

[Bamvor Zhang Jian]

Tested by: Bamvor Jian Zhang 

On 9 February 2017 at 16:56, Michael Ellerman  wrote:
> In commit 88baa78d1f31 ("selftests: remove duplicated all and clean
> target"), the "all" target was removed from individual Makefiles and
> added to lib.mk.
>
> However the "all" target was added to lib.mk *after* the existing
> "runtests" target. This means "runtests" becomes the first (default)
> target for most of our Makefiles.
>
> This has the effect of causing a plain "make" to build *and run* the
> tests. Which is at best rude, but depending on which tests are run could
> oops someone's build machine.
>
>   $ make -C tools/testing/selftests/
>   ...
>   make[1]: Entering directory 'tools/testing/selftests/bpf'
>   gcc -Wall -O2 -I../../../../usr/include   test_verifier.c -o 
> tools/testing/selftests/bpf/test_verifier
>   gcc -Wall -O2 -I../../../../usr/include   test_maps.c -o 
> tools/testing/selftests/bpf/test_maps
>   gcc -Wall -O2 -I../../../../usr/include   test_lru_map.c -o 
> tools/testing/selftests/bpf/test_lru_map
>   #0 add+sub+mul FAIL
>   Failed to load prog 'Function not implemented'!
>   #1 unreachable FAIL
>   Unexpected error message!
>   #2 unreachable2 FAIL
>   ...
>
> Fix it by moving the "all" target to the start of lib.mk, making it the
> default target.
>
> Fixes: 88baa78d1f31 ("selftests: remove duplicated all and clean target")
> Signed-off-by: Michael Ellerman 
> ---
>  tools/testing/selftests/lib.mk | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index 01bb7782a35e..17ed4bbe3963 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -2,6 +2,11 @@
>  # Makefile can operate with or without the kbuild infrastructure.
>  CC := $(CROSS_COMPILE)gcc
>
> +TEST_GEN_PROGS := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS))
> +TEST_GEN_FILES := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_FILES))
> +
> +all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
> +
>  define RUN_TESTS
> @for TEST in $(TEST_GEN_PROGS) $(TEST_PROGS); do \
> BASENAME_TEST=`basename $$TEST`;\
> @@ -42,11 +47,6 @@ endef
>  emit_tests:
> $(EMIT_TESTS)
>
> -TEST_GEN_PROGS := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS))
> -TEST_GEN_FILES := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_FILES))
> -
> -all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
> -
>  clean:
> $(RM) -r $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) 
> $(TEST_GEN_FILES) $(EXTRA_CLEAN)
>
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-api" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] selftests: Fix selftests build to just build, not run tests

[Michael Ellerman]

Michael Ellerman  writes:

> In commit 88baa78d1f31 ("selftests: remove duplicated all and clean
> target"), the "all" target was removed from individual Makefiles and
> added to lib.mk.
>
> However the "all" target was added to lib.mk *after* the existing
> "runtests" target. This means "runtests" becomes the first (default)
> target for most of our Makefiles.
...
>
> Fix it by moving the "all" target to the start of lib.mk, making it the
> default target.
>
> Fixes: 88baa78d1f31 ("selftests: remove duplicated all and clean target")
> Signed-off-by: Michael Ellerman 

Hi Shuah,

Can you please merge this series into linux-next?

The selftests are badly broken otherwise.

cheers

[PATCH 1/5] selftests: Fix selftests build to just build, not run tests

[Michael Ellerman]

In commit 88baa78d1f31 ("selftests: remove duplicated all and clean
target"), the "all" target was removed from individual Makefiles and
added to lib.mk.

However the "all" target was added to lib.mk *after* the existing
"runtests" target. This means "runtests" becomes the first (default)
target for most of our Makefiles.

This has the effect of causing a plain "make" to build *and run* the
tests. Which is at best rude, but depending on which tests are run could
oops someone's build machine.

  $ make -C tools/testing/selftests/
  ...
  make[1]: Entering directory 'tools/testing/selftests/bpf'
  gcc -Wall -O2 -I../../../../usr/include   test_verifier.c -o 
tools/testing/selftests/bpf/test_verifier
  gcc -Wall -O2 -I../../../../usr/include   test_maps.c -o 
tools/testing/selftests/bpf/test_maps
  gcc -Wall -O2 -I../../../../usr/include   test_lru_map.c -o 
tools/testing/selftests/bpf/test_lru_map
  #0 add+sub+mul FAIL
  Failed to load prog 'Function not implemented'!
  #1 unreachable FAIL
  Unexpected error message!
  #2 unreachable2 FAIL
  ...

Fix it by moving the "all" target to the start of lib.mk, making it the
default target.

Fixes: 88baa78d1f31 ("selftests: remove duplicated all and clean target")
Signed-off-by: Michael Ellerman 
---
 tools/testing/selftests/lib.mk | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 01bb7782a35e..17ed4bbe3963 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -2,6 +2,11 @@
 # Makefile can operate with or without the kbuild infrastructure.
 CC := $(CROSS_COMPILE)gcc
 
+TEST_GEN_PROGS := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS))
+TEST_GEN_FILES := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_FILES))
+
+all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
+
 define RUN_TESTS
@for TEST in $(TEST_GEN_PROGS) $(TEST_PROGS); do \
BASENAME_TEST=`basename $$TEST`;\
@@ -42,11 +47,6 @@ endef
 emit_tests:
$(EMIT_TESTS)
 
-TEST_GEN_PROGS := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS))
-TEST_GEN_FILES := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_FILES))
-
-all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
-
 clean:
$(RM) -r $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) 
$(EXTRA_CLEAN)
 
-- 
2.7.4