build: bootstrap SwiftPM with forked and not host clang

[finagolfin]

This was set in #28710 years ago, but didn't matter until swiftlang/swift-package-manager#5894 a couple months ago, as the --clang-path flag was ignored before that. However, building Swift tools with the host clang can cause problems, as seen on my Android CI since that SPM pull was merged in December, so everything from the Swift stdlib onwards is built with the Swift-forked clang instead, so switch SPM here to use it too.

@MaxDesiatov and @neonichu, please review.

[MaxDesiatov]

@swift-ci test

[MaxDesiatov]

@swift-ci build toolchain

[MaxDesiatov]

This will need additional checks on our side before it's merged. I do think these changes are good in principle, this may in fact unblock a PR or two of mine that were built with the old toolchain on CI and failed because of that. But there some configurations that we have to verify those aren't impacted.

[neonichu]

We should also run the toolchain build with this PR, I believe that still requires opening a PR on the Swift repo and performing a cross-repo test.

[finagolfin]

This is in the Swift repo! 😆

[neonichu]

Ah yes, it is and Max has already done a toolchain build 🤣 Mondays...

[finagolfin]

@MaxDesiatov, still need time to test this out? I will submit for 5.9 next.

[MaxDesiatov]

Yes. This will need more time to be tested with 5.9 too. Although I'm not sure if it would be accepted to a release branch as a risky change.

[finagolfin]

I think it should go into 5.9, as all other Swift toolchain repos after the compiler are built with the Swift-forked clang. This was a regression for SPM from that normal usage, which should be fixed.

[finagolfin]

Ping, ready to go in?

[MaxDesiatov]

I've verified this works on my side when applied to 5.9, but I'm not sure whether it's too late for this to go into that branch. Changes to the build system are considered quite disruptive AFAIU. This feels like safe to merge to main to me. Maybe @etcwilde could also have a look as he's more familiar with how the build system is set up.

[finagolfin]

Ping @etcwilde, Max wanted your input on this.

[etcwilde]

I think this would prevent cross-compiling the toolchain, no? If I've cross-compiled clang to run on another platform, we can't use it to build SwiftPM since it won't run on my builder. All tools going into the newly built toolchain should be built with the same set of tools. If those aren't available, the toolchains are usually staged. See the set of tools built for the stage 1 and stage 2 Fuchsia build (or any of the clang multi-stage builds for that matter): https://github.com/llvm/llvm-project/blob/main/clang/cmake/caches/Fuchsia.cmake, and https://github.com/llvm/llvm-project/blob/main/clang/cmake/caches/Fuchsia-stage2.cmake

(Yes, I recognize that we have the same problem with the Swift compiler we just built too. I didn't say the build makes any sense. :) )

[finagolfin]

I think this would prevent cross-compiling the toolchain, no?

No, it is what makes it possible, as the corelibs and much of the toolchain is cross-compiled with the Swift-forked clang. The C/C++ portions of SwiftPM were also built with the Swift-forked clang, until the recent pull that I linked above inadvertently overrode that.

If I've cross-compiled clang to run on another platform, we can't use it to build SwiftPM since it won't run on my builder.

This pull and the one used by the corelibs that I just linked always use the native Swift-forked clang built for local host, which is built by default, for both native and cross-compilation of the toolchain, so that's not an issue.

All tools going into the newly built toolchain should be built with the same set of tools.

Yep, I agree. Everything from the Swift stdlib onwards is built with the Swift-forked clang, including the corelibs, so SPM should be built with that too. That recent change of SPM being built with the system clang instead is what this pull fixes.

[finagolfin]

In case it's unclear, self.toolchain.cc is the clang installed in the system, which is really only meant to build the Swift compiler itself. I'm replacing that in this pull with self.native_toolchain_path(host_target), which is usually the Swift-forked clang that we just built for the host, which is used to build the stdlib and corelibs.

Historically, this --clang-path flag was ignored, so the SPM build defaulted to using the Swift-forked clang. Since Max wired up --clang-path last December, we're using the system clang to build SPM too, which works on the CI but not in as many scenarios as the Swift-forked clang that was historically used.

[finagolfin]

@etcwilde, just waiting for you to sign off on this, then I'd like to get this into the release branches too. My Android CI has needed this fix since December.

[finagolfin]

@MaxDesiatov, can we go ahead and get this in? Evan is probably too busy to look at this, and I've addressed all concerns.

[MaxDesiatov]

It'd be great if someone with a good knowledge of the build system had a look at this first.

[finagolfin]

Ping @etcwilde, since you just reviewed my other pull, maybe you can approve this now?

[finagolfin]

Ping @neonichu, can we go ahead and get this in? I'd like to submit to the release branches next, get this regression fixed there too.

[neonichu]

I'm afraid I don't know too much about the build-script side of things, so I wouldn't want to make the call on this. So pinging @etcwilde again or maybe @shahmishal wants to take a look.

[finagolfin]

Ping @compnerd, maybe you can chime in here.

[finagolfin]

@edymtt, could you take a look at this small pull, that's been held up for lack of review by someone very familiar with the build system?

[finagolfin]

Ping @edymtt, small pull of a couple lines that needs review.

[finagolfin]

Ping @edymtt, should be an easy sign-off.

[finagolfin]

@al45tair, maybe you could review?

[al45tair]

It looks reasonable to me, but I've spent all of five seconds looking at it; I'll talk to @etcwilde about it just to understand his concerns and get back to you.

[etcwilde]

I'm still concerned. If I'm cross-compiling the toolchain, the clang that I just built won't run on my builder. This happens to work at the moment because the builder and the host we're targeting for the toolchain are the same. I recognize that the rest of the build can't really handle a full Canadian-cross-compile either, but I don't think that means we should continue making that situation worse.

[finagolfin]

I'm still concerned. If I'm cross-compiling the toolchain, the clang that I just built won't run on my builder. This happens to work at the moment because the builder and the host we're targeting for the toolchain are the same.

No, in order to cross-compile the Swift toolchain currently, build-script requires that you must either build the same Swift toolchain source for the native host first or supply a prebuilt native Swift toolchain.

That is why I use native_toolchain_path() instead in this pull, because it invokes either the freshly-built Swift-forked native toolchain or a prebuilt native toolchain, if one is supplied by the user. I moved swiftpm, sourcekit-lsp, and swift-driver to use native_toolchain_path() a couple years ago, #36917, without an issue.

[etcwilde]

Ah, I missed the part where toolchain_path was the builder toolchain and not the toolchain we were building. Too many overloads of the same set of words floating around.

[finagolfin]

@bnbarham, I think one more CI run and we can get this in.

[bnbarham]

@swift-ci please test

[finagolfin]

@al45tair, ready for merge.

[al45tair]

This appears to have broken the --swiftpm option to utils/build-script on macOS. I think we’ll probably have to revert it for now and revisit it after there’s a version that works for that. // @finagolfin

[finagolfin]

@al45tair, I see no reason why this pull would break that, sounds like someone is just using it wrong.