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

Alpine 3.16 icu-libs now contains only en. #3844

Closed
Belorus opened this issue Jun 15, 2022 · 16 comments · Fixed by #3847
Closed

Alpine 3.16 icu-libs now contains only en. #3844

Belorus opened this issue Jun 15, 2022 · 16 comments · Fixed by #3847

Comments

@Belorus
Copy link

Belorus commented Jun 15, 2022

Unit tests that run under SDK image (in our case) started to fail on culture-specific things.

In order to keep full compatibility with previous behavior (all locales) one should install also icu-data-full package.

https://wiki.alpinelinux.org/wiki/Release_Notes_for_Alpine_3.16.0#ICU_data_split

On the other side - alpine based runtime images don't contain icu and people always add this line manually, from that perspective it is a good idea to keep SDK with limited ICU to make sure people are aware of the ICU question.

This is probably more of a question. What is the right behavior for SDK in such situation?

@richlander
Copy link
Member

I was reading the release notes.

nodejs is now compiled with system ICU.

Is that "system ICU" the same as icu-data-en or that's a third variant? It doesn't address your question, but I'm curious. It also defines (I assume) what node developers get? Not that we need to make the same choice.

@richlander
Copy link
Member

richlander commented Jun 15, 2022

It seems that there are two questions to answer:

  1. Should we update the SDK image to maintain existing behavior? Seems like the answer should be "yes".
  2. Should we update runtime-deps to include icu-data-en and then stop enabling globalization invariant mode. This one is less convincing but worth discussion.

@Belorus
Copy link
Author

Belorus commented Jun 15, 2022

My 2 cents:

1 - Agree with you
2 - I believe current strategy (i'm none of a MSFT employee, so i can be completely wrong here) is to have slim docker runtime image without ICU and with invariant mode enabled. And therefore runtime image doesn't contain any ICU stuff. Those who add ICU in their Dockerfiles manually should now add icu-data-full in addition to currently recommeded of icu-libs. This probably implies update of documentation, github answers and all the places where dotnet + icu topic was raised

UPD:
I have a feeling that without icu-libs .net6 app doesn't even start. So for dotnet safety check we should add icu-libs, but for real data - icu-data-full

@spenceclark
Copy link

For me swapping icu-libs for icu-data-full didn't work as .net complained it couldn't find icu at all.
So I added both and now I get the full set of locales again:

RUN apk add --no-cache icu-libs
RUN apk add --no-cache icu-data-full

@mthalman
Copy link
Member

I was reading the release notes.

nodejs is now compiled with system ICU.

Is that "system ICU" the same as icu-data-en or that's a third variant? It doesn't address your question, but I'm curious. It also defines (I assume) what node developers get? Not that we need to make the same choice.

"System" in this instance seems to be icu-libs. Here's the output of installing nodejs:

/ # apk add nodejs
(1/9) Installing ca-certificates (20211220-r0)
(2/9) Installing nghttp2-libs (1.47.0-r0)
(3/9) Installing brotli-libs (1.0.9-r6)
(4/9) Installing c-ares (1.18.1-r0)
(5/9) Installing libgcc (11.2.1_git20220219-r2)
(6/9) Installing icu-data-en (71.1-r2)
Executing icu-data-en-71.1-r2.post-install
*
* If you need ICU with non-English locales and legacy charset support, install
* package icu-data-full.
*
(7/9) Installing libstdc++ (11.2.1_git20220219-r2)
(8/9) Installing icu-libs (71.1-r2)
(9/9) Installing nodejs (16.15.0-r1)
Executing busybox-1.35.0-r13.trigger
Executing ca-certificates-20211220-r0.trigger
OK: 48 MiB in 23 packages

@richlander
Copy link
Member

It's talks about "compiled". Are there different header files for the -en and -full versions?

@kaniini
Copy link
Member

kaniini commented Jun 15, 2022

It is just the data files, installing icu-data-full along with icu-libs will work.

@mthalman
Copy link
Member

[Triage] - We'll work on getting the icu-data-full package added into the sdk images for Alpine 3.16. Apologies for the break that occurred here.

@Belorus
Copy link
Author

Belorus commented Jun 15, 2022

[Triage] - We'll work on getting the icu-data-full package added into the sdk images for Alpine 3.16. Apologies for the break that occurred here.

Thanks, but seems the problem is a bit wider than SDK.

It probably makes sense to teach runtime to accept icu-data-full (not only "deprecated" icu-libs) to avoid situation when we need to install 2 packages where 2nd supercedes first one.

WDYT?

This is what i see if i only install new icu-data-full (which contains all the locales, but is unknown to runtime safety checker)
Process terminated. Couldn't find a valid ICU package installed on the system. Please install libicu using your package manager and try again. Alternatively you can set the configuration flag System.Globalization.Invariant to true if you want to run with no globalization support. Please see https://aka.ms/dotnet-missing-libicu for more information. at System.Environment.FailFast(System.String) at System.Globalization.GlobalizationMode+Settings..cctor() at System.Globalization.CultureData.CreateCultureWithInvariantData() at System.Globalization.CultureData.get_Invariant() at System.Globalization.CultureInfo..cctor() at System.Globalization.CultureInfo.get_CachedCulturesByName() at System.Globalization.CultureInfo.GetCultureInfo(System.String) at System.Reflection.RuntimeAssembly.GetLocale() at System.Reflection.RuntimeAssembly.GetName(Boolean) at System.Reflection.Assembly.GetName() at Microsoft.AspNetCore.Hosting.GenericWebHostBuilder.UseStartup(System.Type)

@mthalman
Copy link
Member

This is what i see if i only install new icu-data-full

Can you explain why your expectation is that it should work with just the icu-data-full package installed? It sounds like that's just the data package and icu-libs is still required.

@Belorus
Copy link
Author

Belorus commented Jun 15, 2022

This is what i see if i only install new icu-data-full

Can you explain why your expectation is that it should work with just the icu-data-full package installed? It sounds like that's just the data package and icu-libs is still required.

Ah, you're right. I've compared contents of the packages, and got that data don't duplicate libs one.

I got a bit confused when noticed that "purged" log
image

Sorry for misleading comment. Seems with your fix we're good :)

@mthalman
Copy link
Member

Updated sdk images have been published that now include icu-data-full.

@andtii
Copy link

andtii commented Oct 5, 2022

@mthalman @richlander The examples how to enable globalization on alpine is wrong since its missing icu-data-full
https://github.com/dotnet/dotnet-docker/blob/main/samples/dotnetapp/Dockerfile.alpine-x64#L19

@dionisvl
Copy link

dionisvl commented Dec 9, 2022

works for me:
RUN apk add icu-dev icu-libs icu-data-full

@shyameasternent
Copy link

works for me: RUN apk add icu-dev icu-libs icu-data-full

Thanks @dionisvl this helped me to add for docker instance.

@yisibl
Copy link

yisibl commented Feb 2, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

9 participants