Skip to content

Remove ICU from the toolchain #75262

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

Merged
merged 2 commits into from
Jul 18, 2024
Merged

Conversation

jmschonfeld
Copy link
Contributor

Now that Foundation builds against its own swift-foundation-icu repo, we no longer need to build the upstream libicu repo for the toolchain. This removes the libicu build from the toolchain

@jmschonfeld
Copy link
Contributor Author

@swift-ci please smoke test

@jmschonfeld jmschonfeld marked this pull request as ready for review July 16, 2024 15:38
@jmschonfeld jmschonfeld requested a review from shahmishal as a code owner July 16, 2024 15:38
Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a good idea to do a toolchain build on this.

@compnerd
Copy link
Member

@swift-ci please build toolchain

@finagolfin
Copy link
Member

Oh nice, but I see some docs that still refer to this libicu checkout, maybe those should be removed too?

@jmschonfeld
Copy link
Contributor Author

Oh nice, but I see some docs that still refer to this libicu checkout, maybe those should be removed too?

I think the first two files are just quite old and out of date in general - I see lots of details in each file unrelated to ICU that are incorrect or no longer the case at this point. I think that those could definitely use an update, but they should probably be updated wholesale rather than just this piece so I think it might be best to save that for a future PR. The final one seems like it could make sense to remove the part related to ICU (but leave the LFS comment in place in case it applies to other repos). I can update that once the toolchain builds pass so they can finish before adding a commit if we want to remove that comment from the document

@AnthonyLatsis
Copy link
Collaborator

The final one seems like it could make sense to remove the part related to ICU (but leave the LFS comment in place in case it applies to other repos)

That item is ICU-specific, so please remove it entirely. Thanks!

@jmschonfeld
Copy link
Contributor Author

@compnerd it looks like all of the full toolchain builds passed 👍🏻

@jmschonfeld
Copy link
Contributor Author

@swift-ci please smoke test

@jmschonfeld jmschonfeld enabled auto-merge (squash) July 16, 2024 21:14
@compnerd
Copy link
Member

@swift-ci please smoke test Linux platform

1 similar comment
@jmschonfeld
Copy link
Contributor Author

@swift-ci please smoke test Linux platform

@jmschonfeld
Copy link
Contributor Author

@swift-ci please clean smoke test Linux platform

2 similar comments
@jmschonfeld
Copy link
Contributor Author

@swift-ci please clean smoke test Linux platform

@jmschonfeld
Copy link
Contributor Author

@swift-ci please clean smoke test Linux platform

@jmschonfeld jmschonfeld merged commit f3acbb0 into swiftlang:main Jul 18, 2024
3 checks passed
@jmschonfeld jmschonfeld deleted the remove-icu branch July 18, 2024 01:52
MaxDesiatov added a commit that referenced this pull request Jul 18, 2024
susmonteiro pushed a commit that referenced this pull request Jul 19, 2024
* Remove ICU from the toolchain

* Remove icu comment from GettingStarted.md
finagolfin pushed a commit to finagolfin/swift that referenced this pull request Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

utils/update-checkout error from git checkout icu maint/maint-69
6 participants