Skip to content
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

Use ICU on Windows when available #34645

Merged
merged 21 commits into from
Apr 14, 2020

Conversation

safern
Copy link
Member

@safern safern commented Apr 7, 2020

Contributes towards: #826
Fixes: #4673

Mono is still defaulted to use NLS because we don't produce a shim in the libraries Windows build. Once the shim is linked into the mono runtime I'll update the mono code to use ICU when available. (cc: @marek-safar)

FYI: @GrabYourPitchforks @ericstj

@safern safern added the breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. label Apr 7, 2020
@safern safern added this to the 5.0 milestone Apr 7, 2020
@ghost
Copy link

ghost commented Apr 7, 2020

Tagging @tarekgh, @safern as an area owner

@GrabYourPitchforks
Copy link
Member

FYI I recently changed some of these files in #34616. There will be conflicts until you merge in origin/master.

@mjsabby
Copy link
Contributor

mjsabby commented Apr 10, 2020

Another question. We use the sysglobl APIs to create additional cultures and reparent them to existing ones in most cases. This afaik ultimately are just additions to the registry. Will such a feature will be available to ICU?

@tarekgh
Copy link
Member

tarekgh commented Apr 10, 2020

@mjsabby

Another question. We use the sysglobl APIs to create additional cultures and reparent them to existing ones in most cases. This afaik ultimately are just additions to the registry. Will such a feature will be available to ICU?

Unfortunately, we'll be limited to what the underlying ICU library is providing to us. I checked with the Windows team and learned custom locales are not supported by ICU when running on Windows. It would be nice if you can tell why you are using such custom locales?
If it is really very important to you then you may use the ICU app local feature which will allow you to load the app local version of the ICU and not the system one. And you'll need to build and install your own ICU customized locales data file. Although this can be non-trivial it is not impossible to do.

CC @jefgen @ShawnSteele

@mjsabby
Copy link
Contributor

mjsabby commented Apr 11, 2020

@tarekgh For business and legal reasons we need to define few markets differently than Windows. So we definitely need to use the app local feature.

The question is how much more complicated is it that than the current process which is calling sysglobl APIs.

Is the app-local feature documentation available somewhere?

@tarekgh
Copy link
Member

tarekgh commented Apr 11, 2020

For business and legal reasons we need to define few markets differently than Windows.

Is this mean you need to update some locale data but not adding any more locales?

Is the app-local feature documentation available somewhere?

This is not implemented yet. we are going to implement it next couple of weeks. Basically app local feature is the app will carry its own copy of ICU. Using the tool https://github.com/unicode-org/icu/blob/master/docs/userguide/icu_data/buildtool.md I believe you can build your own customized copy of ICU data and use it. I didn't play with this tool before but I think it will give you full control over the locale data for your app. Also, you'll have control when you can pick a newer version of the ICU at the time you like.
Also, you can just clone the ICU repo and edit the locale data you need and then build it. this will not need to use the tool I think as you'll build a regular way.

@safern safern added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Apr 13, 2020
@safern safern removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Apr 14, 2020
@safern safern merged commit 1db45ed into dotnet:master Apr 14, 2020
@safern safern deleted the GlobalizationIcuManagedImplementation branch April 14, 2020 20:45
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Globalization breaking-change Issue or PR that represents a breaking API or functional change over a prerelease.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

String.StartsWith/EndsWith/IndexOf/LastIndexOf ignoring null characters on Unix
9 participants