[cpullvm] Set CC/CXX in the environment for Windows builds#317
[cpullvm] Set CC/CXX in the environment for Windows builds#317jonathonpenix wants to merge 1 commit intoqualcomm:qualcomm-softwarefrom
Conversation
There's a few different aims here: * Setting CMAKE_C_COMPILER, etc. directly can be a bit confusing when looking through our logs. That value is not propagated when we add multilib and the runtimes as external CMake projects so these projects report MSVC as CMAKE_C[XX]_COMPILER. We don't *use* MSVC in these subprojects (we manually pass in/use the just-built LLVM toolchain) so there shouldn't be any functional change here, but it should hopefully avoid confusion if/when people look through logs. So, I think it is best we avoid recommending this if we can. * This also aligns our Windows builds closer to what we recommend for Linux. With this change, I think we can also consolidate our build instructions a bit to make them (hopefully) more clear/concise. Signed-off-by: Jonathon Penix <jpenix@qti.qualcomm.com>
|
Examples from our logs: From one of our nightly runs: From building with this change: Again this is really a cosmetic change, but it makes us a bit more consistent/hopefully preempts some confusion around this. |
|
Successful Windows builds (eld failures unrelated): |
e57c81f to
6830194
Compare
apazos
left a comment
There was a problem hiding this comment.
Is this an issue only on Windows? I think users might be more familiar to setting -DCMAKE_C_COMPILER, -DCMAKE_CXX_COMPILER, so should we keep them and add a note about Windows?
|
It's not specific to Windows, it's just we already use There isn't a particularly good reason why we weren't doing this on Windows before--it's just that when I was rushing through things I wasn't sure if CMake was consistent about this on Windows I'm also not necessarily opposed to going through CMAKE_C_COMPILER (we can always pass this through to subprojects or something), but I think it's worth being consistent between whatever we recommend for Linux/Windows |
There's a few different aims here:
Setting CMAKE_C_COMPILER, etc. directly can be a bit confusing when looking through our logs. That value is not propagated when we add multilib and the runtimes as external CMake projects so these projects report MSVC as CMAKE_C[XX]_COMPILER. We don't use MSVC in these subprojects (we manually pass in/use the just-built LLVM toolchain) so there shouldn't be any functional change here, but it should hopefully avoid confusion if/when people look through logs.
So, I think it is best we avoid recommending this if we can.
This also aligns our Windows builds closer to what we recommend for Linux.
With this change, I think we can also consolidate our build instructions a bit to make them (hopefully) more clear/concise.