-
Notifications
You must be signed in to change notification settings - Fork 10.5k
utils: build a few extra copies of swift-syntax #75206
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
This is required to build swift-foundation currently due to the use of macros. We currently build the macros for the host rather than the build (though it should arguably be built for multiple separate targets from the host and build as it is meant for the toolchain's hosts).
Exactly. We'll have to build the macros for each of the supported toolchains if we want SDKs to work with separate. I wonder if we can use the static SDK to avoid having 16(?) different Linux macro builds and only have two... But we'll still need them for each arch and each platform. I'm not sure how that's supposed to work if you want the SDK to work on both Windows and macOS. 🤔 |
I'm still rather confused about this. How are the stdlib macros built on Windows? We need just as many swift-syntax there, why is the swift-foundation build any different 🤔? |
We have the same problem there but the bulls system is obscuring it. The thing of that currently the static macros are only built for the single toolchain host, but Foundation is built for each SDK and so exposes the issue. |
Can we split the macros portion of the build and install them into the toolchain instead, similar to the stdlib today IIUC? |
That was the discussion on swiftlang/swift-foundation#714. However, even then we will need this. Consider the Android SDK - we would want the plugins to be hosted on windows x64 and arm64. |
@@ -2372,6 +2395,7 @@ if (-not $SkipBuild) { | |||
# Build platform: SDK, Redist and XCTest | |||
Invoke-BuildStep Build-Runtime Android $Arch | |||
Invoke-BuildStep Build-Dispatch Android $Arch | |||
Invoke-BuildStep Build-Syntax Android $Arch |
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'm a little confused about this part and maybe I'm misreading. Yes, the macros themselves are SDK content, but they still run on the builder with the rest of the toolchain. You shouldn't need to ship a Swift-Syntax to an android-arm*, unless you're planning on running a compiler there? For swift-syntax, if you're pre-building a copy so that you don't need to rebuild it (assuming aligning versions), I would expect that you build it for the machines that toolchain will be used with.
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.
Right, this is currently needed due to the need for swiftlang/swift-foundation#714 to be merged.
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.
Why? The macros in Swift-Foundation are built against what is on the machine doing the build, not the machine it's building for.
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.
Alas, if it were ... that is what that PR sets out to accomplish :)
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.
swift-foundation macros should ideally build for both the machine doing the build and the machine that the toolchain will run on (but not the destination target of each SDK) - will this allow us to accomplish both of those?
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 think that the changes in the referenced PR should allow us to do that. This change will then obviously need to be altered, but, I'm not only fine with that, I'm in favour of that.
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.
It needs to be built "twice", once for the builder machine, and once for the toolchain that they're going into. That part's fine. Twice in quotes because we'll actually need to build them for every toolchain if we want the SDK to work across toolchains (thinking of the static Linux SDK), so maybe 22 times? macos, Ubuntu 20.04, 22.04, 23.10, 24.04, Fedora 39, Debian 12, AmazonLinux2, UBI9, centOS7, and Windows, and each of those for intel and ARM. RIP build times...
That said, I would prefer teaching the swift-foundation build how to not build the macros. We shouldn't need swift-syntax on Android unless someone is actually compiling macros on their phone, but then they can figure out how to get Swift-Syntax on their phone.
Right, this is currently needed due to the need for swiftlang/swift-foundation#714 to be merged.
I maybe misunderstood this comment. I thought you were saying these were both required, not that this PR was working around that issue.
will this allow us to accomplish both of those?
This PR, no. swiftlang/swift-foundation#714, gets us closer to that, yes.
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.
No, this change is working around the fact that I need a build for android currently. IMO, that is not what we want in the long term - we want to build swift-syntax only for the toolchain hosts - Windows x64, ARM64, macOS x64, ARM64, Linux ... I don't know.
That said, I would prefer teaching the swift-foundation build how to not build the macros.
I assume that you mean to not build the macros for the host but rather the build and standalone right? That would allow us to build the macros for the various toolchains that we want to support and not worry about the myriad of hosts that we want to support.
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 assume that you mean to not build the macros for the host but rather the build and standalone right? That would allow us to build the macros for the various toolchains that we want to support and not worry about the myriad of hosts that we want to support.
Yep
@bnbarham, @jmschonfeld ping - this is blocking the android work :( |
I thought the idea was to not take this patch and instead the foundation? |
The foundation changes and build changes for this have been merged, I don't think that this change should be needed any longer. |
This is required to build swift-foundation currently due to the use of macros. We currently build the macros for the host rather than the build (though it should arguably be built for multiple separate targets from the host and build as it is meant for the toolchain's hosts).