swift icon indicating copy to clipboard operation
swift copied to clipboard

[6.0.x] Remove libicu build and update list of Swift runtime libraries to be de-duplicated

Open finagolfin opened this issue 1 year ago • 3 comments

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.

finagolfin avatar Sep 20 '24 18:09 finagolfin

@jmschonfeld, need a CI run here.

finagolfin avatar Sep 20 '24 18:09 finagolfin

@swift-ci please test

jmschonfeld avatar Sep 20 '24 18:09 jmschonfeld

Passed CI, just needs approval from @compnerd and other owners.

finagolfin avatar Sep 21 '24 04:09 finagolfin

@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.

finagolfin avatar Sep 24 '24 05:09 finagolfin

I would like to get a review from @parkera and @airspeedswift.

shahmishal avatar Sep 24 '24 05:09 shahmishal

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.

parkera avatar Sep 24 '24 05:09 parkera

@parkera, this has nothing to do with any bug, I think you might have got this mixed up with #76606. This is just about a simple cleanup on the toolchain for libicu libraries that are now unused on linux and Windows after the Foundation re-core, which uses _FoundationICU instead.

finagolfin avatar Sep 24 '24 05:09 finagolfin

Ok, thanks

parkera avatar Sep 24 '24 05:09 parkera

Ping @shahmishal, do we need anyone else to sign off? I think you and I pinged all relevant reviewers weeks ago.

finagolfin avatar Oct 08 '24 09:10 finagolfin

Ping @airspeedswift, I think Mishal just wanted you to sign off.

finagolfin avatar Oct 28 '24 12:10 finagolfin

Ping @shahmishal, would be good to get this in before the next patch release.

finagolfin avatar Nov 11 '24 13:11 finagolfin

cc: @airspeedswift

shahmishal avatar Nov 11 '24 15:11 shahmishal

It's been a while, so re-running CI before merging

DougGregor avatar Dec 02 '24 20:12 DougGregor

@swift-ci please test

DougGregor avatar Dec 02 '24 20:12 DougGregor

Passed CI, ready for merge.

finagolfin avatar Dec 03 '24 17:12 finagolfin