When building standalone stdlib, use Clang resource dir from the toolchain, not the system default

[kubamracek]

This is to resolve this failure: https://ci.swift.org/job/swift-PR-stdlib-with-toolchain-osx/6/consoleFull

[kubamracek]

@swift-ci test stdlib with toolchain

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - e9501b269357885a2243404478180c7fa254dec3

[kubamracek]

@swift-ci please test

[compnerd]

This seems wrong, this is under the use the host compiler path, and the resource dir cannot be mixed and matched as the headers may not be parseable.

[kubamracek]

The alternative is to change build-script-impl to explicitly set CMAKE_C_COMPILER to clang from native_clang_tools_path. Would that work?

[kubamracek]

@swift-ci please test

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - e9501b269357885a2243404478180c7fa254dec3

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - e9501b269357885a2243404478180c7fa254dec3

[kubamracek]

@swift-ci test stdlib with toolchain

[shahmishal]

@swift-ci test stdlib with toolchain

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - edd3f38

[kubamracek]

@swift-ci please test Linux platform

[finagolfin]

@kubamracek, this is wrong and will need to be reverted. The reason the standalone stdlib preset uses the host clang is because the build-runtime-with-host-compiler=1 flag is explicitly set by that preset, which you're now incoherently overriding. Not only that but this solution now applies the native-clang-tools-path to all build-script-impl products, not just the stdlib, which could cause problems.

@gottesmm, you first added this flag in #25108 last year, perhaps you can chime in on whether it's really necessary and how best to fix the issue he was having.

[kubamracek]

Can you describe what actual problem are you seeing that's caused by this change? It's been merged in for 2 months, so I'm not sure reverting is a good option. But it depends on what kind of regression are you seeing.

I guess the important point is "What exactly is native-clang-tools-path supposed to mean?" I would argue that it's technically supposed to mean something like "When set, use this path for clang/clang++ for all C/C++ compilations, avoid using any other compilers". Given that, this PR is actually fixing an important bug, which has nothing to do with the other flags or with the preset.

Let's also get @compnerd's take on this.

[finagolfin]

Can you describe what actual problem are you seeing that's caused by this change?

It has not caused a problem for me yet, but it's the wrong way to fix your problem.

I guess the important point is "What exactly is native-clang-tools-path supposed to mean?" I would argue that it's technically supposed to mean something like "When set, use this path for clang/clang++ for all C/C++ compilations, avoid using any other compilers".

Well, that's not what it does even now, let me explain as I have been looking into this lately. The build-script requires a host_cc to be set, which it looks up in the path or can be passed in with --host-cc and then it sets CMAKE_{C,CXX}_COMPILER for all CMake invocations with that host clang/clang++. Then, several build products override that with another CMAKE_{C,CXX}_COMPILER invocation, either set to the freshly built Swift clang or native_clang_tools_path, #32922.

On top of this, you've now added a third CMAKE_{C,CXX}_COMPILER invocation for all build-script-impl CMake invocations, including when compiling CMark, LLVM, and Swift. These multiple redefinitions of the C/C++ compiler used can cause problems, as I mentioned in #33724.

this PR is actually fixing an important bug, which has nothing to do with the other flags or with the preset.

It matters because the reason given above is to fix that preset, which explicitly has a flag set to only use the host_cc and so you're subverting the preset by changing this. Maybe that preset doesn't really need that build-runtime-with-host-compiler=1 flag though, in which case, the correct fix for your problem is removing that flag.

If you change the reason for this pull to other products, I noted above that several were already using native_clang_tools_path and you've simply added another possible breakage for products that for whatever reason don't override the host_cc till this pull.