Skip to content

Allow the icu native code to be linked out when invariant globalization is enabled. #48120

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Feb 16, 2021
Merged

Allow the icu native code to be linked out when invariant globalization is enabled. #48120

merged 10 commits into from
Feb 16, 2021

Conversation

vargaz
Copy link
Contributor

@vargaz vargaz commented Feb 10, 2021 •
edited by marek-safar
Loading

Fixes #43689

@vargaz vargaz requested a review from marek-safar as a code owner February 10, 2021 19:23
@ghost
Copy link

ghost commented Feb 10, 2021

Tagging subscribers to this area: @tarekgh, @safern, @krwq
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: vargaz
Assignees: -
Labels:

area-System.Globalization
Milestone: -

@vargaz vargaz marked this pull request as draft February 10, 2021 19:24
@tarekgh tarekgh added the linkable-framework Issues associated with delivering a linker friendly framework label Feb 10, 2021
@ghost
Copy link

ghost commented Feb 10, 2021

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @tannergooding, @sbomer
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: vargaz
Assignees: -
Labels:

area-System.Globalization, linkable-framework
Milestone: -

@vargaz vargaz marked this pull request as ready for review February 11, 2021 03:17
@vargaz vargaz marked this pull request as draft February 11, 2021 06:20
@lewing lewing added the arch-wasm WebAssembly architecture label Feb 12, 2021
@ghost
Copy link

ghost commented Feb 12, 2021

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: vargaz
Assignees: -
Labels:

arch-wasm, area-System.Globalization, linkable-framework
Milestone: -

lewing
lewing reviewed Feb 12, 2021
View reviewed changes
src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Is there some reason we aren't changing the eng/illink tasks to use ILLink/ILLink.Descriptors.LibraryBuild.xml for this sort of thing by default? Does it make sense to generalize the ILLink patterns for runtime?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's already generalized but SPC is special because it has shared part

@vargaz vargaz marked this pull request as ready for review February 12, 2021 20:48
@runfoapp runfoapp bot mentioned this pull request Feb 12, 2021
Closed
marek-safar
marek-safar reviewed Feb 15, 2021
View reviewed changes
@marek-safar
Copy link
Contributor

@steveisok could you ensure this scenario is covered in functional tests

@vargaz vargaz merged commit 2e36f1b into dotnet:master Feb 16, 2021
@vargaz vargaz deleted the wasm-invariant branch February 16, 2021 00:25
@eerhardt eerhardt mentioned this pull request Feb 16, 2021
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Mar 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lewing lewing lewing left review comments

@eerhardt eerhardt eerhardt left review comments

@marek-safar marek-safar marek-safar approved these changes

Assignees
No one assigned
Labels
arch-wasm WebAssembly architecture area-System.Globalization linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ensure ICU is fully linked out in invariant mode
5 participants
@vargaz @marek-safar @lewing @eerhardt @tarekgh