Re: [PATCH] selftests: default to host arch for LLVM builds

From: Shuah Khan
Date: Fri May 03 2024 - 15:55:13 EST


On 4/28/24 06:08, Valentin Obst wrote:
Align the behavior for gcc and clang builds by interpreting unset
`ARCH` and `CROSS_COMPILE` variables in `LLVM` builds as a sign that the
user wants to build for the host architecture.

This patch preserves the properties that setting the `ARCH` variable to an
unknown value will trigger an error that complains about insufficient
information, and that a set `CROSS_COMPILE` variable will override the
target triple that is determined based on presence/absence of `ARCH`.

When compiling with clang, i.e., `LLVM` is set, an unset `ARCH` variable in
combination with an unset `CROSS_COMPILE` variable, i.e., compiling for
the host architecture, leads to compilation failures since `lib.mk` can
not determine the clang target triple. In this case, the following error
message is displayed for each subsystem that does not set `ARCH` in its
own Makefile before including `lib.mk` (lines wrapped at 75 chrs):

make[1]: Entering directory '/mnt/build/linux/tools/testing/selftests/
sysctl'
../lib.mk:33: *** Specify CROSS_COMPILE or add '--target=' option to
lib.mk. Stop.
make[1]: Leaving directory '/mnt/build/linux/tools/testing/selftests/
sysctl'

Thanks for fixing this.

And yes, the selftests "normal" (non-cross-compile) build is *broken*
right now, for clang. I didn't realize from the patch title that this is
actually a significant fix. Maybe we should change the subject line (patch
title) to something like:

[PATCH] selftests: fix the clang build: default to host arch for LLVM builds

Yes, I agree that the title should contain the word 'fix' somewhere. For
me its okay if maintainers reword the title when applying the patch,
alternatively I can send a v2. (Is it still a v2 if I change the title, or
rather a new patch?).

Any thoughts on whether this also needs a 'Cc stable'? Its not quite
clear to me if this fix meets the requirements. As above, no objections if
maintainers should decide to add it.


?

Just a thought. The "Fixes:" tag covers it already, I realize.

Anyway, this looks correct, and fixes that aspect of the build for me, so
either way, please feel free to add:

Reviewed-by: John Hubbard <jhubbard@xxxxxxxxxx>


Thanks for the patch. Applied to linux-kselftest next for Linux 6.10-rc1

thanks,
-- Shuah