-
-
Notifications
You must be signed in to change notification settings - Fork 272
driver: Pick a better default C cross compiler on non-Apple Posix #4983
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
25265fc to
57a6d72
Compare
|
I have no idea how this fails with |
57a6d72 to
9ac7a1d
Compare
LLVM 16 and onwards is compiled with |
|
Oh, I guess that makes sense. |
5859087 to
74a4062
Compare
| #ifdef _WIN32 | ||
| choices = { | ||
| { "clang-cl.exe" }, | ||
| { "cl.exe" }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, this should actually work, because of the preceding windows::MsvcEnvironmentScope in runCPreprocessor() - cl.exe should be in PATH by then, so that the later findProgramByName() succeeds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I wasn't sure if I wanted to move it inside findCCFallback because it looked like that setup included environment needed for compilation & preprocessing and, at the end of the function, they would be rollback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine as-is in that regard. But what's more problematic is that native Windows builds would now have -v output and potential errors mentioning a 'C cross-compiler', while that's not the case in this #ifdef _WIN32 block. So analogous to the Posix-native-build case, we'd need an early-return / common helper function / or simply using choices for the Posix-native-build case too, and then adapting the strings based on isNativeBuild.
321f9c4 to
0326b59
Compare
|
Looks pretty ready to me, just 2 nits. - If this lands after #4978, you could remove the FIXME comment for the Android x86[_64] .confs, no need to inject |
Try to pick a better default instead of `cc` when cross-compiling. When targeting Linux on a non-Linux build machine, for example, try to pick `cc` from: - `<triple>-gcc` - `<triple>-clang` - `clang --target=<triple>` This should be a better UX when cross-compiling with simpler build systems like `dub` or even without one at all, offering a out-of-the-box functional setup, on most systems. The advantage to performing this check in the compiler code as opposed to hardcoding a default value in the config file is that: 1. Multiple programs can be searched, instead of just one 2. The `$CC` value continues to be respected, which keeps more advanced cross-compiling setups working (for example Meson) Signed-off-by: Andrei Horodniceanu <a.horodniceanu@proton.me>
0326b59 to
ee08071
Compare
|
Cheers, LGTM, modulo error strings starting in uppercase. Let me fix the changelog merge conflict and convert to lowercase. |
When cross-compiling on a non-Apple Posix build machine pick the C cross compiler (when not specified through
$CCor-gcc=) from:<triple>-gcc<triple>-clangclang --target=<triple>This should be a better UX when cross-compiling with simpler build systems like
dubor even without one at all, offering a out-of-the-box functional setup, on most systems.The advantage to performing this check in the compiler code as opposed to hardcoding a default value in the config file is that:
$CCvalue continues to be respected, which keeps more advanced cross-compiling setups working (for example Meson)A sample implementation of #4978 (comment), I haven't rigorously tested this but I'm interested in comments on the approach/fallback rules.