All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] selftests: installation fixes
@ 2015-05-12 21:59 ` tyler.baker at linaro.org
  0 siblings, 0 replies; 22+ messages in thread
From: tyler.baker @ 2015-05-12 21:59 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Andy Lutomirski, Kevin Hilman, John Stultz, Darren Hart,
	Michael Ellerman, David Herrmann, linux-kernel, linux-arm-kernel,
	Tyler Baker

From: Tyler Baker <tyler.baker@linaro.org>

This is a follow up series to address a simple lib.mk installation issue[1]. 
Essentially, install is being called without checking that there exists test 
artifacts to install, which in turn causes 'make install' to fail. By creating 
a simple loop we can avoid if/elif/else blocks to determine if install actually 
needs to be called.

I've tested this series by building and deploying tests on x86[2], arm64[3], and arm[4] 
platforms.

This series is based on next-20150512.

[1] https://lkml.org/lkml/2015/4/20/746
[2] http://lava.kernelci.org/scheduler/job/83448/log_file#L_40
[3] http://lava.kernelci.org/scheduler/job/83665/log_file#L_44  
[4] http://lava.kernelci.org/scheduler/job/83442/log_file#L_65

Tyler Baker (2):
  selftests/lib.mk: fix INSTALL_RULE
  selftests/breakpoints: only set TEST_PROGS when built

 tools/testing/selftests/breakpoints/Makefile | 3 +--
 tools/testing/selftests/lib.mk               | 6 ++++--
 2 files changed, 5 insertions(+), 4 deletions(-)

-- 
2.1.4


^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 0/2] selftests: installation fixes
@ 2015-05-12 21:59 ` tyler.baker at linaro.org
  0 siblings, 0 replies; 22+ messages in thread
From: tyler.baker at linaro.org @ 2015-05-12 21:59 UTC (permalink / raw)
  To: linux-arm-kernel

From: Tyler Baker <tyler.baker@linaro.org>

This is a follow up series to address a simple lib.mk installation issue[1]. 
Essentially, install is being called without checking that there exists test 
artifacts to install, which in turn causes 'make install' to fail. By creating 
a simple loop we can avoid if/elif/else blocks to determine if install actually 
needs to be called.

I've tested this series by building and deploying tests on x86[2], arm64[3], and arm[4] 
platforms.

This series is based on next-20150512.

[1] https://lkml.org/lkml/2015/4/20/746
[2] http://lava.kernelci.org/scheduler/job/83448/log_file#L_40
[3] http://lava.kernelci.org/scheduler/job/83665/log_file#L_44  
[4] http://lava.kernelci.org/scheduler/job/83442/log_file#L_65

Tyler Baker (2):
  selftests/lib.mk: fix INSTALL_RULE
  selftests/breakpoints: only set TEST_PROGS when built

 tools/testing/selftests/breakpoints/Makefile | 3 +--
 tools/testing/selftests/lib.mk               | 6 ++++--
 2 files changed, 5 insertions(+), 4 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 1/2] selftests/lib.mk: fix INSTALL_RULE
  2015-05-12 21:59 ` tyler.baker at linaro.org
@ 2015-05-12 21:59   ` tyler.baker at linaro.org
  -1 siblings, 0 replies; 22+ messages in thread
From: tyler.baker @ 2015-05-12 21:59 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Andy Lutomirski, Kevin Hilman, John Stultz, Darren Hart,
	Michael Ellerman, David Herrmann, linux-kernel, linux-arm-kernel,
	Tyler Baker

From: Tyler Baker <tyler.baker@linaro.org>

This patch fixes the INSTALL_RULE to gracefully handle the case where
TEST_PROGS and TEST_PROGS_EXTENDED and TEST_FILES are not set. In this case,
install is called without any SOURCE arguments causing make install to fail.
The proposed fix is to loop over the items in these variables and only call
install if there is a test artifact present.

Signed-off-by: Tyler Baker <tyler.baker@linaro.org>
---
 tools/testing/selftests/lib.mk | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index ee412ba..89dd785f 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -13,10 +13,12 @@ run_tests: all
 
 define INSTALL_RULE
 	mkdir -p $(INSTALL_PATH)
-	@for TEST_DIR in $(TEST_DIRS); do\
+	@for TEST_DIR in $(TEST_DIRS); do \
 		cp -r $$TEST_DIR $(INSTALL_PATH); \
 	done;
-	install -t $(INSTALL_PATH) $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES)
+	@for ARTIFACT in $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES); do \
+		install -t $(INSTALL_PATH) $$ARTIFACT; \
+	done;
 endef
 
 install: all
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 1/2] selftests/lib.mk: fix INSTALL_RULE
@ 2015-05-12 21:59   ` tyler.baker at linaro.org
  0 siblings, 0 replies; 22+ messages in thread
From: tyler.baker at linaro.org @ 2015-05-12 21:59 UTC (permalink / raw)
  To: linux-arm-kernel

From: Tyler Baker <tyler.baker@linaro.org>

This patch fixes the INSTALL_RULE to gracefully handle the case where
TEST_PROGS and TEST_PROGS_EXTENDED and TEST_FILES are not set. In this case,
install is called without any SOURCE arguments causing make install to fail.
The proposed fix is to loop over the items in these variables and only call
install if there is a test artifact present.

Signed-off-by: Tyler Baker <tyler.baker@linaro.org>
---
 tools/testing/selftests/lib.mk | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index ee412ba..89dd785f 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -13,10 +13,12 @@ run_tests: all
 
 define INSTALL_RULE
 	mkdir -p $(INSTALL_PATH)
-	@for TEST_DIR in $(TEST_DIRS); do\
+	@for TEST_DIR in $(TEST_DIRS); do \
 		cp -r $$TEST_DIR $(INSTALL_PATH); \
 	done;
-	install -t $(INSTALL_PATH) $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES)
+	@for ARTIFACT in $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES); do \
+		install -t $(INSTALL_PATH) $$ARTIFACT; \
+	done;
 endef
 
 install: all
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 2/2] selftests/breakpoints: only set TEST_PROGS when built
  2015-05-12 21:59 ` tyler.baker at linaro.org
@ 2015-05-12 21:59   ` tyler.baker at linaro.org
  -1 siblings, 0 replies; 22+ messages in thread
From: tyler.baker @ 2015-05-12 21:59 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Andy Lutomirski, Kevin Hilman, John Stultz, Darren Hart,
	Michael Ellerman, David Herrmann, linux-kernel, linux-arm-kernel,
	Tyler Baker

From: Tyler Baker <tyler.baker@linaro.org>

Set TEST_PROGS only when a build has occurred.

Signed-off-by: Tyler Baker <tyler.baker@linaro.org>
---
 tools/testing/selftests/breakpoints/Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/testing/selftests/breakpoints/Makefile b/tools/testing/selftests/breakpoints/Makefile
index 1822356..54cc3e7 100644
--- a/tools/testing/selftests/breakpoints/Makefile
+++ b/tools/testing/selftests/breakpoints/Makefile
@@ -12,12 +12,11 @@ endif
 all:
 ifeq ($(ARCH),x86)
 	gcc breakpoint_test.c -o breakpoint_test
+	TEST_PROGS := breakpoint_test
 else
 	echo "Not an x86 target, can't build breakpoints selftests"
 endif
 
-TEST_PROGS := breakpoint_test
-
 include ../lib.mk
 
 clean:
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 2/2] selftests/breakpoints: only set TEST_PROGS when built
@ 2015-05-12 21:59   ` tyler.baker at linaro.org
  0 siblings, 0 replies; 22+ messages in thread
From: tyler.baker at linaro.org @ 2015-05-12 21:59 UTC (permalink / raw)
  To: linux-arm-kernel

From: Tyler Baker <tyler.baker@linaro.org>

Set TEST_PROGS only when a build has occurred.

Signed-off-by: Tyler Baker <tyler.baker@linaro.org>
---
 tools/testing/selftests/breakpoints/Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/testing/selftests/breakpoints/Makefile b/tools/testing/selftests/breakpoints/Makefile
index 1822356..54cc3e7 100644
--- a/tools/testing/selftests/breakpoints/Makefile
+++ b/tools/testing/selftests/breakpoints/Makefile
@@ -12,12 +12,11 @@ endif
 all:
 ifeq ($(ARCH),x86)
 	gcc breakpoint_test.c -o breakpoint_test
+	TEST_PROGS := breakpoint_test
 else
 	echo "Not an x86 target, can't build breakpoints selftests"
 endif
 
-TEST_PROGS := breakpoint_test
-
 include ../lib.mk
 
 clean:
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] selftests/lib.mk: fix INSTALL_RULE
  2015-05-12 21:59   ` tyler.baker at linaro.org
@ 2015-05-12 22:02     ` Shuah Khan
  -1 siblings, 0 replies; 22+ messages in thread
From: Shuah Khan @ 2015-05-12 22:02 UTC (permalink / raw)
  To: tyler.baker
  Cc: Andy Lutomirski, Kevin Hilman, John Stultz, Darren Hart,
	Michael Ellerman, David Herrmann, linux-kernel, linux-arm-kernel

On 05/12/2015 03:59 PM, tyler.baker@linaro.org wrote:
> From: Tyler Baker <tyler.baker@linaro.org>

This is odd. Did you use git send-email to send the patches?

-- Shuah
> 
> This patch fixes the INSTALL_RULE to gracefully handle the case where
> TEST_PROGS and TEST_PROGS_EXTENDED and TEST_FILES are not set. In this case,
> install is called without any SOURCE arguments causing make install to fail.
> The proposed fix is to loop over the items in these variables and only call
> install if there is a test artifact present.
> 
> Signed-off-by: Tyler Baker <tyler.baker@linaro.org>
> ---
>  tools/testing/selftests/lib.mk | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index ee412ba..89dd785f 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -13,10 +13,12 @@ run_tests: all
>  
>  define INSTALL_RULE
>  	mkdir -p $(INSTALL_PATH)
> -	@for TEST_DIR in $(TEST_DIRS); do\
> +	@for TEST_DIR in $(TEST_DIRS); do \
>  		cp -r $$TEST_DIR $(INSTALL_PATH); \
>  	done;
> -	install -t $(INSTALL_PATH) $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES)
> +	@for ARTIFACT in $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES); do \
> +		install -t $(INSTALL_PATH) $$ARTIFACT; \
> +	done;
>  endef
>  
>  install: all
> 


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 1/2] selftests/lib.mk: fix INSTALL_RULE
@ 2015-05-12 22:02     ` Shuah Khan
  0 siblings, 0 replies; 22+ messages in thread
From: Shuah Khan @ 2015-05-12 22:02 UTC (permalink / raw)
  To: linux-arm-kernel

On 05/12/2015 03:59 PM, tyler.baker at linaro.org wrote:
> From: Tyler Baker <tyler.baker@linaro.org>

This is odd. Did you use git send-email to send the patches?

-- Shuah
> 
> This patch fixes the INSTALL_RULE to gracefully handle the case where
> TEST_PROGS and TEST_PROGS_EXTENDED and TEST_FILES are not set. In this case,
> install is called without any SOURCE arguments causing make install to fail.
> The proposed fix is to loop over the items in these variables and only call
> install if there is a test artifact present.
> 
> Signed-off-by: Tyler Baker <tyler.baker@linaro.org>
> ---
>  tools/testing/selftests/lib.mk | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
> index ee412ba..89dd785f 100644
> --- a/tools/testing/selftests/lib.mk
> +++ b/tools/testing/selftests/lib.mk
> @@ -13,10 +13,12 @@ run_tests: all
>  
>  define INSTALL_RULE
>  	mkdir -p $(INSTALL_PATH)
> -	@for TEST_DIR in $(TEST_DIRS); do\
> +	@for TEST_DIR in $(TEST_DIRS); do \
>  		cp -r $$TEST_DIR $(INSTALL_PATH); \
>  	done;
> -	install -t $(INSTALL_PATH) $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES)
> +	@for ARTIFACT in $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES); do \
> +		install -t $(INSTALL_PATH) $$ARTIFACT; \
> +	done;
>  endef
>  
>  install: all
> 


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh at osg.samsung.com | (970) 217-8978

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] selftests/lib.mk: fix INSTALL_RULE
  2015-05-12 22:02     ` Shuah Khan
@ 2015-05-12 22:04       ` Tyler Baker
  -1 siblings, 0 replies; 22+ messages in thread
From: Tyler Baker @ 2015-05-12 22:04 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Andy Lutomirski, Kevin Hilman, John Stultz, Darren Hart,
	Michael Ellerman, David Herrmann, linux-kernel, linux-arm-kernel

On 12 May 2015 at 15:02, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> On 05/12/2015 03:59 PM, tyler.baker@linaro.org wrote:
>> From: Tyler Baker <tyler.baker@linaro.org>
>
> This is odd. Did you use git send-email to send the patches?

Yes I did.

>
> -- Shuah
>>
>> This patch fixes the INSTALL_RULE to gracefully handle the case where
>> TEST_PROGS and TEST_PROGS_EXTENDED and TEST_FILES are not set. In this case,
>> install is called without any SOURCE arguments causing make install to fail.
>> The proposed fix is to loop over the items in these variables and only call
>> install if there is a test artifact present.
>>
>> Signed-off-by: Tyler Baker <tyler.baker@linaro.org>
>> ---
>>  tools/testing/selftests/lib.mk | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
>> index ee412ba..89dd785f 100644
>> --- a/tools/testing/selftests/lib.mk
>> +++ b/tools/testing/selftests/lib.mk
>> @@ -13,10 +13,12 @@ run_tests: all
>>
>>  define INSTALL_RULE
>>       mkdir -p $(INSTALL_PATH)
>> -     @for TEST_DIR in $(TEST_DIRS); do\
>> +     @for TEST_DIR in $(TEST_DIRS); do \
>>               cp -r $$TEST_DIR $(INSTALL_PATH); \
>>       done;
>> -     install -t $(INSTALL_PATH) $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES)
>> +     @for ARTIFACT in $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES); do \
>> +             install -t $(INSTALL_PATH) $$ARTIFACT; \
>> +     done;
>>  endef
>>
>>  install: all
>>
>
>
> --
> Shuah Khan
> Sr. Linux Kernel Developer
> Open Source Innovation Group
> Samsung Research America (Silicon Valley)
> shuahkh@osg.samsung.com | (970) 217-8978



-- 
Tyler Baker
Tech Lead, LAVA
Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 1/2] selftests/lib.mk: fix INSTALL_RULE
@ 2015-05-12 22:04       ` Tyler Baker
  0 siblings, 0 replies; 22+ messages in thread
From: Tyler Baker @ 2015-05-12 22:04 UTC (permalink / raw)
  To: linux-arm-kernel

On 12 May 2015 at 15:02, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> On 05/12/2015 03:59 PM, tyler.baker at linaro.org wrote:
>> From: Tyler Baker <tyler.baker@linaro.org>
>
> This is odd. Did you use git send-email to send the patches?

Yes I did.

>
> -- Shuah
>>
>> This patch fixes the INSTALL_RULE to gracefully handle the case where
>> TEST_PROGS and TEST_PROGS_EXTENDED and TEST_FILES are not set. In this case,
>> install is called without any SOURCE arguments causing make install to fail.
>> The proposed fix is to loop over the items in these variables and only call
>> install if there is a test artifact present.
>>
>> Signed-off-by: Tyler Baker <tyler.baker@linaro.org>
>> ---
>>  tools/testing/selftests/lib.mk | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
>> index ee412ba..89dd785f 100644
>> --- a/tools/testing/selftests/lib.mk
>> +++ b/tools/testing/selftests/lib.mk
>> @@ -13,10 +13,12 @@ run_tests: all
>>
>>  define INSTALL_RULE
>>       mkdir -p $(INSTALL_PATH)
>> -     @for TEST_DIR in $(TEST_DIRS); do\
>> +     @for TEST_DIR in $(TEST_DIRS); do \
>>               cp -r $$TEST_DIR $(INSTALL_PATH); \
>>       done;
>> -     install -t $(INSTALL_PATH) $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES)
>> +     @for ARTIFACT in $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES); do \
>> +             install -t $(INSTALL_PATH) $$ARTIFACT; \
>> +     done;
>>  endef
>>
>>  install: all
>>
>
>
> --
> Shuah Khan
> Sr. Linux Kernel Developer
> Open Source Innovation Group
> Samsung Research America (Silicon Valley)
> shuahkh at osg.samsung.com | (970) 217-8978



-- 
Tyler Baker
Tech Lead, LAVA
Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] selftests/lib.mk: fix INSTALL_RULE
  2015-05-12 22:04       ` Tyler Baker
@ 2015-05-12 22:07         ` Shuah Khan
  -1 siblings, 0 replies; 22+ messages in thread
From: Shuah Khan @ 2015-05-12 22:07 UTC (permalink / raw)
  To: Tyler Baker
  Cc: Andy Lutomirski, Kevin Hilman, John Stultz, Darren Hart,
	Michael Ellerman, David Herrmann, linux-kernel, linux-arm-kernel

On 05/12/2015 04:04 PM, Tyler Baker wrote:
> On 12 May 2015 at 15:02, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>> On 05/12/2015 03:59 PM, tyler.baker@linaro.org wrote:
>>> From: Tyler Baker <tyler.baker@linaro.org>
>>
>> This is odd. Did you use git send-email to send the patches?
> 
> Yes I did.
> 

No need to resend. I will try to apply them. Check your .gitconfig.
The extra From is odd.

-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 1/2] selftests/lib.mk: fix INSTALL_RULE
@ 2015-05-12 22:07         ` Shuah Khan
  0 siblings, 0 replies; 22+ messages in thread
From: Shuah Khan @ 2015-05-12 22:07 UTC (permalink / raw)
  To: linux-arm-kernel

On 05/12/2015 04:04 PM, Tyler Baker wrote:
> On 12 May 2015 at 15:02, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>> On 05/12/2015 03:59 PM, tyler.baker at linaro.org wrote:
>>> From: Tyler Baker <tyler.baker@linaro.org>
>>
>> This is odd. Did you use git send-email to send the patches?
> 
> Yes I did.
> 

No need to resend. I will try to apply them. Check your .gitconfig.
The extra From is odd.

-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh at osg.samsung.com | (970) 217-8978

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 1/2] selftests/lib.mk: fix INSTALL_RULE
  2015-05-12 22:07         ` Shuah Khan
@ 2015-05-12 22:41           ` Tyler Baker
  -1 siblings, 0 replies; 22+ messages in thread
From: Tyler Baker @ 2015-05-12 22:41 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Andy Lutomirski, Kevin Hilman, John Stultz, Darren Hart,
	Michael Ellerman, David Herrmann, linux-kernel, linux-arm-kernel

On 12 May 2015 at 15:07, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> On 05/12/2015 04:04 PM, Tyler Baker wrote:
>> On 12 May 2015 at 15:02, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>> On 05/12/2015 03:59 PM, tyler.baker@linaro.org wrote:
>>>> From: Tyler Baker <tyler.baker@linaro.org>
>>>
>>> This is odd. Did you use git send-email to send the patches?
>>
>> Yes I did.
>>
>
> No need to resend. I will try to apply them. Check your .gitconfig.
> The extra From is odd.

Sorry about that, not sure why this has happened. I'll investigate on
my end. Thanks.

>
> -- Shuah
>
> --
> Shuah Khan
> Sr. Linux Kernel Developer
> Open Source Innovation Group
> Samsung Research America (Silicon Valley)
> shuahkh@osg.samsung.com | (970) 217-8978



-- 
Tyler Baker
Tech Lead, LAVA
Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 1/2] selftests/lib.mk: fix INSTALL_RULE
@ 2015-05-12 22:41           ` Tyler Baker
  0 siblings, 0 replies; 22+ messages in thread
From: Tyler Baker @ 2015-05-12 22:41 UTC (permalink / raw)
  To: linux-arm-kernel

On 12 May 2015 at 15:07, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> On 05/12/2015 04:04 PM, Tyler Baker wrote:
>> On 12 May 2015 at 15:02, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>> On 05/12/2015 03:59 PM, tyler.baker at linaro.org wrote:
>>>> From: Tyler Baker <tyler.baker@linaro.org>
>>>
>>> This is odd. Did you use git send-email to send the patches?
>>
>> Yes I did.
>>
>
> No need to resend. I will try to apply them. Check your .gitconfig.
> The extra From is odd.

Sorry about that, not sure why this has happened. I'll investigate on
my end. Thanks.

>
> -- Shuah
>
> --
> Shuah Khan
> Sr. Linux Kernel Developer
> Open Source Innovation Group
> Samsung Research America (Silicon Valley)
> shuahkh at osg.samsung.com | (970) 217-8978



-- 
Tyler Baker
Tech Lead, LAVA
Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] selftests/breakpoints: only set TEST_PROGS when built
  2015-05-12 21:59   ` tyler.baker at linaro.org
@ 2015-05-13 21:41     ` Shuah Khan
  -1 siblings, 0 replies; 22+ messages in thread
From: Shuah Khan @ 2015-05-13 21:41 UTC (permalink / raw)
  To: tyler.baker
  Cc: Andy Lutomirski, Kevin Hilman, John Stultz, Darren Hart,
	Michael Ellerman, David Herrmann, linux-kernel, linux-arm-kernel,
	Shuah Khan

On 05/12/2015 03:59 PM, tyler.baker@linaro.org wrote:
> From: Tyler Baker <tyler.baker@linaro.org>
> 
> Set TEST_PROGS only when a build has occurred.
> 
> Signed-off-by: Tyler Baker <tyler.baker@linaro.org>
> ---
>  tools/testing/selftests/breakpoints/Makefile | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/breakpoints/Makefile b/tools/testing/selftests/breakpoints/Makefile
> index 1822356..54cc3e7 100644
> --- a/tools/testing/selftests/breakpoints/Makefile
> +++ b/tools/testing/selftests/breakpoints/Makefile
> @@ -12,12 +12,11 @@ endif
>  all:
>  ifeq ($(ARCH),x86)
>  	gcc breakpoint_test.c -o breakpoint_test
> +	TEST_PROGS := breakpoint_test
>  else
>  	echo "Not an x86 target, can't build breakpoints selftests"
>  endif
>  
> -TEST_PROGS := breakpoint_test
> -
>  include ../lib.mk
>  
>  clean:
> 

Hmm. With this change install fails to copy breakpoint_test all
together. Remember setting TEST_PROGS in compile step makes it
not stick around when install target is called. A better approach
would be the following:

if [ -f breakpoint_test ]
    TEST_PROGS := breakpoint_test
fi

include ../lib.mk

-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 2/2] selftests/breakpoints: only set TEST_PROGS when built
@ 2015-05-13 21:41     ` Shuah Khan
  0 siblings, 0 replies; 22+ messages in thread
From: Shuah Khan @ 2015-05-13 21:41 UTC (permalink / raw)
  To: linux-arm-kernel

On 05/12/2015 03:59 PM, tyler.baker at linaro.org wrote:
> From: Tyler Baker <tyler.baker@linaro.org>
> 
> Set TEST_PROGS only when a build has occurred.
> 
> Signed-off-by: Tyler Baker <tyler.baker@linaro.org>
> ---
>  tools/testing/selftests/breakpoints/Makefile | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/breakpoints/Makefile b/tools/testing/selftests/breakpoints/Makefile
> index 1822356..54cc3e7 100644
> --- a/tools/testing/selftests/breakpoints/Makefile
> +++ b/tools/testing/selftests/breakpoints/Makefile
> @@ -12,12 +12,11 @@ endif
>  all:
>  ifeq ($(ARCH),x86)
>  	gcc breakpoint_test.c -o breakpoint_test
> +	TEST_PROGS := breakpoint_test
>  else
>  	echo "Not an x86 target, can't build breakpoints selftests"
>  endif
>  
> -TEST_PROGS := breakpoint_test
> -
>  include ../lib.mk
>  
>  clean:
> 

Hmm. With this change install fails to copy breakpoint_test all
together. Remember setting TEST_PROGS in compile step makes it
not stick around when install target is called. A better approach
would be the following:

if [ -f breakpoint_test ]
    TEST_PROGS := breakpoint_test
fi

include ../lib.mk

-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh at osg.samsung.com | (970) 217-8978

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] selftests/breakpoints: only set TEST_PROGS when built
  2015-05-13 21:41     ` Shuah Khan
@ 2015-05-14 14:15       ` Tyler Baker
  -1 siblings, 0 replies; 22+ messages in thread
From: Tyler Baker @ 2015-05-14 14:15 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Andy Lutomirski, Kevin Hilman, John Stultz, Darren Hart,
	Michael Ellerman, David Herrmann, linux-kernel, linux-arm-kernel

On 13 May 2015 at 14:41, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> On 05/12/2015 03:59 PM, tyler.baker@linaro.org wrote:
>> From: Tyler Baker <tyler.baker@linaro.org>
>>
>> Set TEST_PROGS only when a build has occurred.
>>
>> Signed-off-by: Tyler Baker <tyler.baker@linaro.org>
>> ---
>>  tools/testing/selftests/breakpoints/Makefile | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/breakpoints/Makefile b/tools/testing/selftests/breakpoints/Makefile
>> index 1822356..54cc3e7 100644
>> --- a/tools/testing/selftests/breakpoints/Makefile
>> +++ b/tools/testing/selftests/breakpoints/Makefile
>> @@ -12,12 +12,11 @@ endif
>>  all:
>>  ifeq ($(ARCH),x86)
>>       gcc breakpoint_test.c -o breakpoint_test
>> +     TEST_PROGS := breakpoint_test
>>  else
>>       echo "Not an x86 target, can't build breakpoints selftests"
>>  endif
>>
>> -TEST_PROGS := breakpoint_test
>> -
>>  include ../lib.mk
>>
>>  clean:
>>
>
> Hmm. With this change install fails to copy breakpoint_test all
> together. Remember setting TEST_PROGS in compile step makes it
> not stick around when install target is called. A better approach
> would be the following:
>
> if [ -f breakpoint_test ]
>     TEST_PROGS := breakpoint_test
> fi

Thanks for pointing this out, this is a good catch. We will also need
to do this for the x86 tests IIRC. Would it make more sense to have
this check performed in the INSTALL_RULE so that we don't have to have
a bunch of IF statements in the various Makefiles?

Something like...

       @for ARTIFACT in $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES); do \
               if [ -f $$ARTIFACT ]; then \
                    install -t $(INSTALL_PATH) $$ARTIFACT; \
               fi; \
       done;

>
> include ../lib.mk
>
> -- Shuah
>
> --
> Shuah Khan
> Sr. Linux Kernel Developer
> Open Source Innovation Group
> Samsung Research America (Silicon Valley)
> shuahkh@osg.samsung.com | (970) 217-8978

Cheers,

Tyler

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 2/2] selftests/breakpoints: only set TEST_PROGS when built
@ 2015-05-14 14:15       ` Tyler Baker
  0 siblings, 0 replies; 22+ messages in thread
From: Tyler Baker @ 2015-05-14 14:15 UTC (permalink / raw)
  To: linux-arm-kernel

On 13 May 2015 at 14:41, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> On 05/12/2015 03:59 PM, tyler.baker at linaro.org wrote:
>> From: Tyler Baker <tyler.baker@linaro.org>
>>
>> Set TEST_PROGS only when a build has occurred.
>>
>> Signed-off-by: Tyler Baker <tyler.baker@linaro.org>
>> ---
>>  tools/testing/selftests/breakpoints/Makefile | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/breakpoints/Makefile b/tools/testing/selftests/breakpoints/Makefile
>> index 1822356..54cc3e7 100644
>> --- a/tools/testing/selftests/breakpoints/Makefile
>> +++ b/tools/testing/selftests/breakpoints/Makefile
>> @@ -12,12 +12,11 @@ endif
>>  all:
>>  ifeq ($(ARCH),x86)
>>       gcc breakpoint_test.c -o breakpoint_test
>> +     TEST_PROGS := breakpoint_test
>>  else
>>       echo "Not an x86 target, can't build breakpoints selftests"
>>  endif
>>
>> -TEST_PROGS := breakpoint_test
>> -
>>  include ../lib.mk
>>
>>  clean:
>>
>
> Hmm. With this change install fails to copy breakpoint_test all
> together. Remember setting TEST_PROGS in compile step makes it
> not stick around when install target is called. A better approach
> would be the following:
>
> if [ -f breakpoint_test ]
>     TEST_PROGS := breakpoint_test
> fi

Thanks for pointing this out, this is a good catch. We will also need
to do this for the x86 tests IIRC. Would it make more sense to have
this check performed in the INSTALL_RULE so that we don't have to have
a bunch of IF statements in the various Makefiles?

Something like...

       @for ARTIFACT in $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES); do \
               if [ -f $$ARTIFACT ]; then \
                    install -t $(INSTALL_PATH) $$ARTIFACT; \
               fi; \
       done;

>
> include ../lib.mk
>
> -- Shuah
>
> --
> Shuah Khan
> Sr. Linux Kernel Developer
> Open Source Innovation Group
> Samsung Research America (Silicon Valley)
> shuahkh at osg.samsung.com | (970) 217-8978

Cheers,

Tyler

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] selftests/breakpoints: only set TEST_PROGS when built
  2015-05-14 14:15       ` Tyler Baker
@ 2015-05-14 15:27         ` Shuah Khan
  -1 siblings, 0 replies; 22+ messages in thread
From: Shuah Khan @ 2015-05-14 15:27 UTC (permalink / raw)
  To: Tyler Baker
  Cc: Andy Lutomirski, Kevin Hilman, John Stultz, Darren Hart,
	Michael Ellerman, David Herrmann, linux-kernel, linux-arm-kernel,
	Shuah Khan

On 05/14/2015 08:15 AM, Tyler Baker wrote:
> On 13 May 2015 at 14:41, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>> On 05/12/2015 03:59 PM, tyler.baker@linaro.org wrote:
>>> From: Tyler Baker <tyler.baker@linaro.org>
>>>
>>> Set TEST_PROGS only when a build has occurred.
>>>
>>> Signed-off-by: Tyler Baker <tyler.baker@linaro.org>
>>> ---
>>>  tools/testing/selftests/breakpoints/Makefile | 3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/breakpoints/Makefile b/tools/testing/selftests/breakpoints/Makefile
>>> index 1822356..54cc3e7 100644
>>> --- a/tools/testing/selftests/breakpoints/Makefile
>>> +++ b/tools/testing/selftests/breakpoints/Makefile
>>> @@ -12,12 +12,11 @@ endif
>>>  all:
>>>  ifeq ($(ARCH),x86)
>>>       gcc breakpoint_test.c -o breakpoint_test
>>> +     TEST_PROGS := breakpoint_test
>>>  else
>>>       echo "Not an x86 target, can't build breakpoints selftests"
>>>  endif
>>>
>>> -TEST_PROGS := breakpoint_test
>>> -
>>>  include ../lib.mk
>>>
>>>  clean:
>>>
>>
>> Hmm. With this change install fails to copy breakpoint_test all
>> together. Remember setting TEST_PROGS in compile step makes it
>> not stick around when install target is called. A better approach
>> would be the following:
>>
>> if [ -f breakpoint_test ]
>>     TEST_PROGS := breakpoint_test
>> fi
> 
> Thanks for pointing this out, this is a good catch. We will also need
> to do this for the x86 tests IIRC. Would it make more sense to have
> this check performed in the INSTALL_RULE so that we don't have to have
> a bunch of IF statements in the various Makefiles?

Right. x86 will need this type of logic for 32-bit execs when they
aren't not built on a 64-bit system, and for 64-bit execs when they
aren't built on a 32-bit system.

> 
> Something like...
> 
>        @for ARTIFACT in $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES); do \
>                if [ -f $$ARTIFACT ]; then \
>                     install -t $(INSTALL_PATH) $$ARTIFACT; \
>                fi; \
>        done;
> 

I think it makes perfect sense to do this in INSTALL_RULE.
As you said, this will avoid changes to test individual
Makefiles and new test writers don't have to worry about
adding this.

Would you like make the necessary changes?

thanks,
-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

^ permalink raw reply	[flat|nested] 22+ messages in thread
* [PATCH 2/2] selftests/breakpoints: only set TEST_PROGS when built
@ 2015-05-14 15:27         ` Shuah Khan
  0 siblings, 0 replies; 22+ messages in thread
From: Shuah Khan @ 2015-05-14 15:27 UTC (permalink / raw)
  To: linux-arm-kernel

On 05/14/2015 08:15 AM, Tyler Baker wrote:
> On 13 May 2015 at 14:41, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>> On 05/12/2015 03:59 PM, tyler.baker at linaro.org wrote:
>>> From: Tyler Baker <tyler.baker@linaro.org>
>>>
>>> Set TEST_PROGS only when a build has occurred.
>>>
>>> Signed-off-by: Tyler Baker <tyler.baker@linaro.org>
>>> ---
>>>  tools/testing/selftests/breakpoints/Makefile | 3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/breakpoints/Makefile b/tools/testing/selftests/breakpoints/Makefile
>>> index 1822356..54cc3e7 100644
>>> --- a/tools/testing/selftests/breakpoints/Makefile
>>> +++ b/tools/testing/selftests/breakpoints/Makefile
>>> @@ -12,12 +12,11 @@ endif
>>>  all:
>>>  ifeq ($(ARCH),x86)
>>>       gcc breakpoint_test.c -o breakpoint_test
>>> +     TEST_PROGS := breakpoint_test
>>>  else
>>>       echo "Not an x86 target, can't build breakpoints selftests"
>>>  endif
>>>
>>> -TEST_PROGS := breakpoint_test
>>> -
>>>  include ../lib.mk
>>>
>>>  clean:
>>>
>>
>> Hmm. With this change install fails to copy breakpoint_test all
>> together. Remember setting TEST_PROGS in compile step makes it
>> not stick around when install target is called. A better approach
>> would be the following:
>>
>> if [ -f breakpoint_test ]
>>     TEST_PROGS := breakpoint_test
>> fi
> 
> Thanks for pointing this out, this is a good catch. We will also need
> to do this for the x86 tests IIRC. Would it make more sense to have
> this check performed in the INSTALL_RULE so that we don't have to have
> a bunch of IF statements in the various Makefiles?

Right. x86 will need this type of logic for 32-bit execs when they
aren't not built on a 64-bit system, and for 64-bit execs when they
aren't built on a 32-bit system.

> 
> Something like...
> 
>        @for ARTIFACT in $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES); do \
>                if [ -f $$ARTIFACT ]; then \
>                     install -t $(INSTALL_PATH) $$ARTIFACT; \
>                fi; \
>        done;
> 

I think it makes perfect sense to do this in INSTALL_RULE.
As you said, this will avoid changes to test individual
Makefiles and new test writers don't have to worry about
adding this.

Would you like make the necessary changes?

thanks,
-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh at osg.samsung.com | (970) 217-8978

^ permalink raw reply	[flat|nested] 22+ messages in thread
* Re: [PATCH 2/2] selftests/breakpoints: only set TEST_PROGS when built
  2015-05-14 15:27         ` Shuah Khan
@ 2015-05-14 17:11           ` Tyler Baker
  -1 siblings, 0 replies; 22+ messages in thread
From: Tyler Baker @ 2015-05-14 17:11 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Andy Lutomirski, Kevin Hilman, John Stultz, Darren Hart,
	Michael Ellerman, David Herrmann, linux-kernel, linux-arm-kernel

On 14 May 2015 at 08:27, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> On 05/14/2015 08:15 AM, Tyler Baker wrote:
>> On 13 May 2015 at 14:41, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>> On 05/12/2015 03:59 PM, tyler.baker@linaro.org wrote:
>>>> From: Tyler Baker <tyler.baker@linaro.org>
>>>>
>>>> Set TEST_PROGS only when a build has occurred.
>>>>
>>>> Signed-off-by: Tyler Baker <tyler.baker@linaro.org>
>>>> ---
>>>>  tools/testing/selftests/breakpoints/Makefile | 3 +--
>>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/breakpoints/Makefile b/tools/testing/selftests/breakpoints/Makefile
>>>> index 1822356..54cc3e7 100644
>>>> --- a/tools/testing/selftests/breakpoints/Makefile
>>>> +++ b/tools/testing/selftests/breakpoints/Makefile
>>>> @@ -12,12 +12,11 @@ endif
>>>>  all:
>>>>  ifeq ($(ARCH),x86)
>>>>       gcc breakpoint_test.c -o breakpoint_test
>>>> +     TEST_PROGS := breakpoint_test
>>>>  else
>>>>       echo "Not an x86 target, can't build breakpoints selftests"
>>>>  endif
>>>>
>>>> -TEST_PROGS := breakpoint_test
>>>> -
>>>>  include ../lib.mk
>>>>
>>>>  clean:
>>>>
>>>
>>> Hmm. With this change install fails to copy breakpoint_test all
>>> together. Remember setting TEST_PROGS in compile step makes it
>>> not stick around when install target is called. A better approach
>>> would be the following:
>>>
>>> if [ -f breakpoint_test ]
>>>     TEST_PROGS := breakpoint_test
>>> fi
>>
>> Thanks for pointing this out, this is a good catch. We will also need
>> to do this for the x86 tests IIRC. Would it make more sense to have
>> this check performed in the INSTALL_RULE so that we don't have to have
>> a bunch of IF statements in the various Makefiles?
>
> Right. x86 will need this type of logic for 32-bit execs when they
> aren't not built on a 64-bit system, and for 64-bit execs when they
> aren't built on a 32-bit system.

Considering the change below we can now simplify this case for x86 to:

diff --git a/tools/testing/selftests/x86/Makefile
b/tools/testing/selftests/x86/Makefile
index 12d8e76..3e238d0 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -14,13 +14,9 @@ UNAME_M := $(shell uname -m)
 ifeq ($(CROSS_COMPILE),)
 # Always build 32-bit tests
 all: all_32
-# Install 32-bit tests
-TEST_PROGS += $(BINARIES_32) run_x86_tests.sh
 # If we're on a 64-bit host, build 64-bit tests as well
 ifeq ($(UNAME_M),x86_64)
 all: all_64
-# Install 64-bit tests
-TEST_PROGS += $(BINARIES_64)
 endif
 endif

@@ -28,6 +24,9 @@ all_32: check_build32 $(BINARIES_32)

 all_64: $(BINARIES_64)

+# Install the tests
+TEST_PROGS := $(BINARIES_32) $(BINARIES_64) run_x86_tests.sh
+
 include ../lib.mk

 clean:

If the binaries do not exist, they will be not be installed. If you
and Andy are ok with this, I'll add a patch to this series.

>
>>
>> Something like...
>>
>>        @for ARTIFACT in $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES); do \
>>                if [ -f $$ARTIFACT ]; then \
>>                     install -t $(INSTALL_PATH) $$ARTIFACT; \
>>                fi; \
>>        done;
>>
>
> I think it makes perfect sense to do this in INSTALL_RULE.
> As you said, this will avoid changes to test individual
> Makefiles and new test writers don't have to worry about
> adding this.
>
> Would you like make the necessary changes?

Sure, I'll add this in the next revision.

>
> thanks,
> -- Shuah
>
>
> --
> Shuah Khan
> Sr. Linux Kernel Developer
> Open Source Innovation Group
> Samsung Research America (Silicon Valley)
> shuahkh@osg.samsung.com | (970) 217-8978

Cheers,

Tyler

^ permalink raw reply related	[flat|nested] 22+ messages in thread
* [PATCH 2/2] selftests/breakpoints: only set TEST_PROGS when built
@ 2015-05-14 17:11           ` Tyler Baker
  0 siblings, 0 replies; 22+ messages in thread
From: Tyler Baker @ 2015-05-14 17:11 UTC (permalink / raw)
  To: linux-arm-kernel

On 14 May 2015 at 08:27, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> On 05/14/2015 08:15 AM, Tyler Baker wrote:
>> On 13 May 2015 at 14:41, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>> On 05/12/2015 03:59 PM, tyler.baker at linaro.org wrote:
>>>> From: Tyler Baker <tyler.baker@linaro.org>
>>>>
>>>> Set TEST_PROGS only when a build has occurred.
>>>>
>>>> Signed-off-by: Tyler Baker <tyler.baker@linaro.org>
>>>> ---
>>>>  tools/testing/selftests/breakpoints/Makefile | 3 +--
>>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/breakpoints/Makefile b/tools/testing/selftests/breakpoints/Makefile
>>>> index 1822356..54cc3e7 100644
>>>> --- a/tools/testing/selftests/breakpoints/Makefile
>>>> +++ b/tools/testing/selftests/breakpoints/Makefile
>>>> @@ -12,12 +12,11 @@ endif
>>>>  all:
>>>>  ifeq ($(ARCH),x86)
>>>>       gcc breakpoint_test.c -o breakpoint_test
>>>> +     TEST_PROGS := breakpoint_test
>>>>  else
>>>>       echo "Not an x86 target, can't build breakpoints selftests"
>>>>  endif
>>>>
>>>> -TEST_PROGS := breakpoint_test
>>>> -
>>>>  include ../lib.mk
>>>>
>>>>  clean:
>>>>
>>>
>>> Hmm. With this change install fails to copy breakpoint_test all
>>> together. Remember setting TEST_PROGS in compile step makes it
>>> not stick around when install target is called. A better approach
>>> would be the following:
>>>
>>> if [ -f breakpoint_test ]
>>>     TEST_PROGS := breakpoint_test
>>> fi
>>
>> Thanks for pointing this out, this is a good catch. We will also need
>> to do this for the x86 tests IIRC. Would it make more sense to have
>> this check performed in the INSTALL_RULE so that we don't have to have
>> a bunch of IF statements in the various Makefiles?
>
> Right. x86 will need this type of logic for 32-bit execs when they
> aren't not built on a 64-bit system, and for 64-bit execs when they
> aren't built on a 32-bit system.

Considering the change below we can now simplify this case for x86 to:

diff --git a/tools/testing/selftests/x86/Makefile
b/tools/testing/selftests/x86/Makefile
index 12d8e76..3e238d0 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -14,13 +14,9 @@ UNAME_M := $(shell uname -m)
 ifeq ($(CROSS_COMPILE),)
 # Always build 32-bit tests
 all: all_32
-# Install 32-bit tests
-TEST_PROGS += $(BINARIES_32) run_x86_tests.sh
 # If we're on a 64-bit host, build 64-bit tests as well
 ifeq ($(UNAME_M),x86_64)
 all: all_64
-# Install 64-bit tests
-TEST_PROGS += $(BINARIES_64)
 endif
 endif

@@ -28,6 +24,9 @@ all_32: check_build32 $(BINARIES_32)

 all_64: $(BINARIES_64)

+# Install the tests
+TEST_PROGS := $(BINARIES_32) $(BINARIES_64) run_x86_tests.sh
+
 include ../lib.mk

 clean:

If the binaries do not exist, they will be not be installed. If you
and Andy are ok with this, I'll add a patch to this series.

>
>>
>> Something like...
>>
>>        @for ARTIFACT in $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES); do \
>>                if [ -f $$ARTIFACT ]; then \
>>                     install -t $(INSTALL_PATH) $$ARTIFACT; \
>>                fi; \
>>        done;
>>
>
> I think it makes perfect sense to do this in INSTALL_RULE.
> As you said, this will avoid changes to test individual
> Makefiles and new test writers don't have to worry about
> adding this.
>
> Would you like make the necessary changes?

Sure, I'll add this in the next revision.

>
> thanks,
> -- Shuah
>
>
> --
> Shuah Khan
> Sr. Linux Kernel Developer
> Open Source Innovation Group
> Samsung Research America (Silicon Valley)
> shuahkh at osg.samsung.com | (970) 217-8978

Cheers,

Tyler

^ permalink raw reply related	[flat|nested] 22+ messages in thread
end of thread, other threads:[~2015-05-14 17:11 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-12 21:59 [PATCH 0/2] selftests: installation fixes tyler.baker
2015-05-12 21:59 ` tyler.baker at linaro.org
2015-05-12 21:59 ` [PATCH 1/2] selftests/lib.mk: fix INSTALL_RULE tyler.baker
2015-05-12 21:59   ` tyler.baker at linaro.org
2015-05-12 22:02   ` Shuah Khan
2015-05-12 22:02     ` Shuah Khan
2015-05-12 22:04     ` Tyler Baker
2015-05-12 22:04       ` Tyler Baker
2015-05-12 22:07       ` Shuah Khan
2015-05-12 22:07         ` Shuah Khan
2015-05-12 22:41         ` Tyler Baker
2015-05-12 22:41           ` Tyler Baker
2015-05-12 21:59 ` [PATCH 2/2] selftests/breakpoints: only set TEST_PROGS when built tyler.baker
2015-05-12 21:59   ` tyler.baker at linaro.org
2015-05-13 21:41   ` Shuah Khan
2015-05-13 21:41     ` Shuah Khan
2015-05-14 14:15     ` Tyler Baker
2015-05-14 14:15       ` Tyler Baker
2015-05-14 15:27       ` Shuah Khan
2015-05-14 15:27         ` Shuah Khan
2015-05-14 17:11         ` Tyler Baker
2015-05-14 17:11           ` Tyler Baker

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.