[PATCH] selftests: find echo binary to use -ne options

[PATCH] selftests: find echo binary to use -ne options

[Guillaume Tucker]

Find the actual echo binary using $(which echo) and use it for
formatted output with -ne.  On some systems, the default echo command
doesn't handle the -e option and the output looks like this (arm64
build):

-ne Emit Tests for alsa

-ne Emit Tests for amd-pstate

-ne Emit Tests for arm64

This is for example the case with the KernelCI Docker images
e.g. kernelci/gcc-10:x86-kselftest-kernelci.  With the actual echo
binary (e.g. in /bin/echo), the output is formatted as expected (x86
build this time):

Emit Tests for alsa
Emit Tests for amd-pstate
Skipping non-existent dir: arm64

Only the install target is using "echo -ne" so keep the $ECHO variable
local to it.

Reported-by: "kernelci.org bot" <bot@kernelci.org>
Fixes: 3297a4df805d ("kselftests: Enable the echo command to print newlines in Makefile")
Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
 tools/testing/selftests/Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 41b649452560..9619d0f3b2ff 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -234,10 +234,11 @@ ifdef INSTALL_PATH
 	@# While building kselftest-list.text skip also non-existent TARGET dirs:
 	@# they could be the result of a build failure and should NOT be
 	@# included in the generated runlist.
+	ECHO=`which echo`; \
 	for TARGET in $(TARGETS); do \
 		BUILD_TARGET=$$BUILD/$$TARGET;	\
-		[ ! -d $(INSTALL_PATH)/$$TARGET ] && echo "Skipping non-existent dir: $$TARGET" && continue; \
-		echo -ne "Emit Tests for $$TARGET\n"; \
+		[ ! -d $(INSTALL_PATH)/$$TARGET ] && $$ECHO "Skipping non-existent dir: $$TARGET" && continue; \
+		$$ECHO -ne "Emit Tests for $$TARGET\n"; \
 		$(MAKE) -s --no-print-directory OUTPUT=$$BUILD_TARGET COLLECTION=$$TARGET \
 			-C $$TARGET emit_tests >> $(TEST_LIST); \
 	done;
-- 
2.30.2

Re: [PATCH] selftests: find echo binary to use -ne options

[Shuah Khan]

On 2/3/23 08:26, Guillaume Tucker wrote:
> Find the actual echo binary using $(which echo) and use it for
> formatted output with -ne.  On some systems, the default echo command
> doesn't handle the -e option and the output looks like this (arm64
> build):
> 
> -ne Emit Tests for alsa
> 
> -ne Emit Tests for amd-pstate
> 
> -ne Emit Tests for arm64
> 
> This is for example the case with the KernelCI Docker images
> e.g. kernelci/gcc-10:x86-kselftest-kernelci.  With the actual echo
> binary (e.g. in /bin/echo), the output is formatted as expected (x86
> build this time):
> 
> Emit Tests for alsa
> Emit Tests for amd-pstate
> Skipping non-existent dir: arm64
> 
> Only the install target is using "echo -ne" so keep the $ECHO variable
> local to it.
> 
> Reported-by: "kernelci.org bot" <bot@kernelci.org>
> Fixes: 3297a4df805d ("kselftests: Enable the echo command to print newlines in Makefile")
> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> ---

Thank you - will appear shortly in linuxk-selftest next.

thanks,
-- Shuah

RE: [PATCH] selftests: find echo binary to use -ne options

[David Laight]

From: Guillaume Tucker
> Sent: 03 February 2023 15:26
> 
> Find the actual echo binary using $(which echo) and use it for
> formatted output with -ne.  On some systems, the default echo command
> doesn't handle the -e option and the output looks like this (arm64
> build):
> 
> -ne Emit Tests for alsa
> 
> -ne Emit Tests for amd-pstate
> 
> -ne Emit Tests for arm64

Nack.
There is no reason to suppose that /bin/echo is any different from
the version of echo builtin to the shell that make uses.

Not only that 'which' is a horrid shell script that is trying to
emulate csh builtin.
The bourne shell equivalent is 'type' and the posix one 'command'.

In any case the portable way to fix this is to use printf.
This is a well defined program and is bultin to all modern shells.

	David


> 
> This is for example the case with the KernelCI Docker images
> e.g. kernelci/gcc-10:x86-kselftest-kernelci.  With the actual echo
> binary (e.g. in /bin/echo), the output is formatted as expected (x86
> build this time):
> 
> Emit Tests for alsa
> Emit Tests for amd-pstate
> Skipping non-existent dir: arm64
> 
> Only the install target is using "echo -ne" so keep the $ECHO variable
> local to it.
> 
> Reported-by: "kernelci.org bot" <bot@kernelci.org>
> Fixes: 3297a4df805d ("kselftests: Enable the echo command to print newlines in Makefile")
> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> ---
>  tools/testing/selftests/Makefile | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 41b649452560..9619d0f3b2ff 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -234,10 +234,11 @@ ifdef INSTALL_PATH
>  	@# While building kselftest-list.text skip also non-existent TARGET dirs:
>  	@# they could be the result of a build failure and should NOT be
>  	@# included in the generated runlist.
> +	ECHO=`which echo`; \
>  	for TARGET in $(TARGETS); do \
>  		BUILD_TARGET=$$BUILD/$$TARGET;	\
> -		[ ! -d $(INSTALL_PATH)/$$TARGET ] && echo "Skipping non-existent dir: $$TARGET" &&
> continue; \
> -		echo -ne "Emit Tests for $$TARGET\n"; \
> +		[ ! -d $(INSTALL_PATH)/$$TARGET ] && $$ECHO "Skipping non-existent dir: $$TARGET" &&
> continue; \
> +		$$ECHO -ne "Emit Tests for $$TARGET\n"; \
>  		$(MAKE) -s --no-print-directory OUTPUT=$$BUILD_TARGET COLLECTION=$$TARGET \
>  			-C $$TARGET emit_tests >> $(TEST_LIST); \
>  	done;
> --
> 2.30.2

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] selftests: find echo binary to use -ne options

[Guillaume Tucker]

On 04/02/2023 14:04, David Laight wrote:
> From: Guillaume Tucker
>> Sent: 03 February 2023 15:26
>>
>> Find the actual echo binary using $(which echo) and use it for
>> formatted output with -ne.  On some systems, the default echo command
>> doesn't handle the -e option and the output looks like this (arm64
>> build):
>>
>> -ne Emit Tests for alsa
>>
>> -ne Emit Tests for amd-pstate
>>
>> -ne Emit Tests for arm64
> 
> Nack.
> There is no reason to suppose that /bin/echo is any different from
> the version of echo builtin to the shell that make uses.
> 
> Not only that 'which' is a horrid shell script that is trying to
> emulate csh builtin.
> The bourne shell equivalent is 'type' and the posix one 'command'.
> 
> In any case the portable way to fix this is to use printf.
> This is a well defined program and is bultin to all modern shells.

Ah great, thanks for the review.  Will send a v2 with printf.

Guillaume

>> This is for example the case with the KernelCI Docker images
>> e.g. kernelci/gcc-10:x86-kselftest-kernelci.  With the actual echo
>> binary (e.g. in /bin/echo), the output is formatted as expected (x86
>> build this time):
>>
>> Emit Tests for alsa
>> Emit Tests for amd-pstate
>> Skipping non-existent dir: arm64
>>
>> Only the install target is using "echo -ne" so keep the $ECHO variable
>> local to it.
>>
>> Reported-by: "kernelci.org bot" <bot@kernelci.org>
>> Fixes: 3297a4df805d ("kselftests: Enable the echo command to print newlines in Makefile")
>> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
>> ---
>>  tools/testing/selftests/Makefile | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
>> index 41b649452560..9619d0f3b2ff 100644
>> --- a/tools/testing/selftests/Makefile
>> +++ b/tools/testing/selftests/Makefile
>> @@ -234,10 +234,11 @@ ifdef INSTALL_PATH
>>  	@# While building kselftest-list.text skip also non-existent TARGET dirs:
>>  	@# they could be the result of a build failure and should NOT be
>>  	@# included in the generated runlist.
>> +	ECHO=`which echo`; \
>>  	for TARGET in $(TARGETS); do \
>>  		BUILD_TARGET=$$BUILD/$$TARGET;	\
>> -		[ ! -d $(INSTALL_PATH)/$$TARGET ] && echo "Skipping non-existent dir: $$TARGET" &&
>> continue; \
>> -		echo -ne "Emit Tests for $$TARGET\n"; \
>> +		[ ! -d $(INSTALL_PATH)/$$TARGET ] && $$ECHO "Skipping non-existent dir: $$TARGET" &&
>> continue; \
>> +		$$ECHO -ne "Emit Tests for $$TARGET\n"; \
>>  		$(MAKE) -s --no-print-directory OUTPUT=$$BUILD_TARGET COLLECTION=$$TARGET \
>>  			-C $$TARGET emit_tests >> $(TEST_LIST); \
>>  	done;
>> --
>> 2.30.2
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
> 
>