diff mbox series

selftests/riscv: fix potential build failure during the "emit_tests" step

Message ID 20230712193514.740033-1-jhubbard@nvidia.com (mailing list archive)
State Accepted
Commit c77896b143d3c9c3e84c4ed0662b807ccbd8730b
Headers show
Series selftests/riscv: fix potential build failure during the "emit_tests" step | expand

Commit Message

John Hubbard July 12, 2023, 7:35 p.m. UTC
The riscv selftests (which were modeled after the arm64 selftests) are
improperly declaring the "emit_tests" target to depend upon the "all"
target. This approach, when combined with commit 9fc96c7c19df
("selftests: error out if kernel header files are not yet built"), has
caused build failures [1] on arm64, and is likely to cause similar
failures for riscv.

To fix this, simply remove the unnecessary "all" dependency from the
emit_tests target. The dependency is still effectively honored, because
again, invocation is via "install", which also depends upon "all".

An alternative approach would be to harden the emit_tests target so that
it can depend upon "all", but that's a lot more complicated and hard to
get right, and doesn't seem worth it, especially given that emit_tests
should probably not be overridden at all.

[1] https://lore.kernel.org/20230710-kselftest-fix-arm64-v1-1-48e872844f25@kernel.org

Fixes: 9fc96c7c19df ("selftests: error out if kernel header files are not yet built")
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---

Andrew,

With this, and with my arm64 fix [2] that you've already put into
mm-unstable, you should be able to safely drop commit 819187ab8741
("selftests: fix arm64 test installation").

[2] https://lore.kernel.org/20230711005629.2547838-1-jhubbard@nvidia.com

thanks,
John Hubbard


 tools/testing/selftests/riscv/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: 3f01e9fed8454dcd89727016c3e5b2fbb8f8e50c
prerequisite-patch-id: 37c92f7425689ff069fb83996a25cd98e78d7242

Comments

Alexandre Ghiti July 13, 2023, 11:51 a.m. UTC | #1
Hi John,


On 12/07/2023 21:35, John Hubbard wrote:
> The riscv selftests (which were modeled after the arm64 selftests) are
> improperly declaring the "emit_tests" target to depend upon the "all"
> target. This approach, when combined with commit 9fc96c7c19df
> ("selftests: error out if kernel header files are not yet built"), has
> caused build failures [1] on arm64, and is likely to cause similar
> failures for riscv.
>
> To fix this, simply remove the unnecessary "all" dependency from the
> emit_tests target. The dependency is still effectively honored, because
> again, invocation is via "install", which also depends upon "all".
>
> An alternative approach would be to harden the emit_tests target so that
> it can depend upon "all", but that's a lot more complicated and hard to
> get right, and doesn't seem worth it, especially given that emit_tests
> should probably not be overridden at all.
>
> [1] https://lore.kernel.org/20230710-kselftest-fix-arm64-v1-1-48e872844f25@kernel.org
>
> Fixes: 9fc96c7c19df ("selftests: error out if kernel header files are not yet built")
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>
> Andrew,
>
> With this, and with my arm64 fix [2] that you've already put into
> mm-unstable, you should be able to safely drop commit 819187ab8741
> ("selftests: fix arm64 test installation").
>
> [2] https://lore.kernel.org/20230711005629.2547838-1-jhubbard@nvidia.com
>
> thanks,
> John Hubbard
>
>
>   tools/testing/selftests/riscv/Makefile | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/riscv/Makefile b/tools/testing/selftests/riscv/Makefile
> index 9dd629cc86aa..f4b3d5c9af5b 100644
> --- a/tools/testing/selftests/riscv/Makefile
> +++ b/tools/testing/selftests/riscv/Makefile
> @@ -43,7 +43,7 @@ run_tests: all
>   	done
>   
>   # Avoid any output on non riscv on emit_tests
> -emit_tests: all
> +emit_tests:
>   	@for DIR in $(RISCV_SUBTARGETS); do				\
>   		BUILD_TARGET=$(OUTPUT)/$$DIR;			\
>   		$(MAKE) OUTPUT=$$BUILD_TARGET -C $$DIR $@;		\
>
> base-commit: 3f01e9fed8454dcd89727016c3e5b2fbb8f8e50c
> prerequisite-patch-id: 37c92f7425689ff069fb83996a25cd98e78d7242


Perfect timing, I have just encountered this failure this morning:

make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1

that your patch fixed :) So you can add:

Tested-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thank you!

Alex
Palmer Dabbelt Aug. 3, 2023, 2:45 p.m. UTC | #2
On Wed, 12 Jul 2023 12:35:14 -0700, John Hubbard wrote:
> The riscv selftests (which were modeled after the arm64 selftests) are
> improperly declaring the "emit_tests" target to depend upon the "all"
> target. This approach, when combined with commit 9fc96c7c19df
> ("selftests: error out if kernel header files are not yet built"), has
> caused build failures [1] on arm64, and is likely to cause similar
> failures for riscv.
> 
> [...]

Applied, thanks!

[1/1] selftests/riscv: fix potential build failure during the "emit_tests" step
      https://git.kernel.org/palmer/c/8c82d2bf5944

Best regards,
patchwork-bot+linux-riscv@kernel.org Aug. 3, 2023, 3:10 p.m. UTC | #3
Hello:

This patch was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Wed, 12 Jul 2023 12:35:14 -0700 you wrote:
> The riscv selftests (which were modeled after the arm64 selftests) are
> improperly declaring the "emit_tests" target to depend upon the "all"
> target. This approach, when combined with commit 9fc96c7c19df
> ("selftests: error out if kernel header files are not yet built"), has
> caused build failures [1] on arm64, and is likely to cause similar
> failures for riscv.
> 
> [...]

Here is the summary with links:
  - selftests/riscv: fix potential build failure during the "emit_tests" step
    https://git.kernel.org/riscv/c/8c82d2bf5944

You are awesome, thank you!
Shuah Khan Aug. 3, 2023, 4:48 p.m. UTC | #4
On 8/3/23 08:45, Palmer Dabbelt wrote:
> 
> On Wed, 12 Jul 2023 12:35:14 -0700, John Hubbard wrote:
>> The riscv selftests (which were modeled after the arm64 selftests) are
>> improperly declaring the "emit_tests" target to depend upon the "all"
>> target. This approach, when combined with commit 9fc96c7c19df
>> ("selftests: error out if kernel header files are not yet built"), has
>> caused build failures [1] on arm64, and is likely to cause similar
>> failures for riscv.
>>
>> [...]
> 
> Applied, thanks!
> 
> [1/1] selftests/riscv: fix potential build failure during the "emit_tests" step
>        https://git.kernel.org/palmer/c/8c82d2bf5944
> 
> Best regards,

This is already in Linus's tree. I sent this in for Linux 6.5-rc3

c77896b143d3c9c3e84c4ed0662b807ccbd8730b
selftests/riscv: fix potential build failure during the "emit_tests" step

thanks,
-- Shuah
diff mbox series

Patch

diff --git a/tools/testing/selftests/riscv/Makefile b/tools/testing/selftests/riscv/Makefile
index 9dd629cc86aa..f4b3d5c9af5b 100644
--- a/tools/testing/selftests/riscv/Makefile
+++ b/tools/testing/selftests/riscv/Makefile
@@ -43,7 +43,7 @@  run_tests: all
 	done
 
 # Avoid any output on non riscv on emit_tests
-emit_tests: all
+emit_tests:
 	@for DIR in $(RISCV_SUBTARGETS); do				\
 		BUILD_TARGET=$(OUTPUT)/$$DIR;			\
 		$(MAKE) OUTPUT=$$BUILD_TARGET -C $$DIR $@;		\