Messages in this thread | | | Date | Fri, 3 May 2024 13:55:00 -0600 | Subject | Re: [PATCH] selftests: default to host arch for LLVM builds | From | Shuah Khan <> |
| |
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@nvidia.com> >
Thanks for the patch. Applied to linux-kselftest next for Linux 6.10-rc1
thanks, -- Shuah
| |