-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[6.0.x] Remove libicu build and update list of Swift runtime libraries to be de-duplicated #76613
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
* Remove ICU from the toolchain
…tolink-extract tool doesn't repeat them over and over again (swiftlang#76224)
@jmschonfeld, need a CI run here. |
@swift-ci please test |
Passed CI, just needs approval from @compnerd and other owners. |
@shahmishal, let me know what you think as the linux platform manager about shaving these unused libraries off the 6.0 toolchain in a patch release. |
I would like to get a review from @parkera and @airspeedswift. |
I don't want to aggressively cherry pick this back without any evidence that it resolves the named bug. We're already doing it on main, though. |
Ok, thanks |
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 this is fine since we've already landed similar changes on main. I see it as an opportunity fix to along with other changes already staged for a 6.0.x but not a driver for the release itself.
Ping @shahmishal, do we need anyone else to sign off? I think you and I pinged all relevant reviewers weeks ago. |
Ping @airspeedswift, I think Mishal just wanted you to sign off. |
Ping @shahmishal, would be good to get this in before the next patch release. |
cc: @airspeedswift |
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.
Yeah, we should take this
It's been a while, so re-running CI before merging |
@swift-ci please test |
Passed CI, ready for merge. |
Explanation: Libicu is entirely unused on non-Darwin platforms too since the Foundation re-core to use
_FoundationICU
, so shave 35 MB off the 6.0 non-Darwin toolchains by removing it. Also, update the list of libraries that are deduplicated by the autolink-extract tool.Scope: Only affects code for non-Darwin platforms, that is now entirely unused.
Issue: None
Original PRs: #75262, #75342, and #76224
Risk: Very low
Testing: Passed all CI on trunk
Reviewer: @jmschonfeld @compnerd @parkera @etcwilde
We are unnecessarily still shipping libicu in the 6.0 toolchains despite having another copy in
lib_FoundationICU
. This was removed in 6.1 months ago without a problem.