-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Cultures aliased by ICU cannot be used for resource localization on non-Windows environments #3897
Comments
Thanks @CodingDinosaur for reporting the issue and listing the details. this is very helpful. we'll take a look. |
@cdmihai it is wrong to have msbuild depends on only the list returned from CultureInfo.GetCultures. this was ok before Windows 10 and Linux support but now it is not valid approach. for example, in Windows 10, if you try to create any culture which Windows doesn't have data for, the operation still succeed and the culture can be created as long as the culture name conform to BCP-47 spec. We'll try to look how we can enhance the support for aliased culture as this issue suggested but whatever we do here, msbuild will need to do something more. do you want me open a new issue in msbuild to track that? |
From @cdmihai on October 15, 2018 18:23 @tarekgh would this be a suitable issue? #1454 Regarding removing valid locale checks, the biggest issue with this is that it would be a breaking change for MSBuild. The msbuild repo build logic itself has the assumption that non-existing locales are rejected, so a lot of strings are put in Microsoft.shared.resx. If we remove the locale check in the FYI @rainersigwald |
I am not sure I understand the breaking scenario here. The scenario you are describing can occur today. for example build some project with some culture introduced in Windows 10 and then run on down-level platform don't have this culture. |
From @cdmihai on October 15, 2018 20:40 True, but that's the class of valid locales introduced in different windows versions. The new breaking scenario is for the class of always invalid locales, that were never meant to be locales, and users expect msbuild to not treat them as locales. |
I am not sure I agree with that. If the OS/.Net can create a culture for these, then those should be a valid locales to use. Why you think msbuild should reject such cultures? |
From @cdmihai on October 15, 2018 23:16 Personally I agree that msbuild should not care and just do what the OS does, but I fear there are actual customers who depend on this behaviour, and changing this might break them. But this is just gut feeling based on the fact that the msbuild repo itself is doing it, and I don't have actual data on it. Alternatively we can only enable it in .net core msbuild, and then customers will have to opt-in to the break by transitioning to .net core. But it's not nice to diverge behaviour. |
The only breaking scenario I can think of is when we allow creating resources with a culture not returned by CultureInfo.GetCultures and then move this resources to other machine which cannot understand the used culture. This scenario can happen today anyway. do you have any breaking scenario you can think of? #1454 is specific to custom cultures. I would suggest updating it to include the other supported system cultures which is not enumerated by CultureInfo.GetCultures |
After looking at this issue, it looks ICU not enumerating the aliased cultures for good reasons. I believe the framework should follow that too and not enumerate such aliased cultures. The framework still can create such aliased culture if anyone want to use them. e.g.: new CultureInfo("zh-TW"); will work fine. Considering that, I believe the resource issue should be fixed from msbuild side. I am going to move this issue to msbuild repo. @CodingDinosaur thanks again for reporting this issue. |
Attempt to solve dotnet/msbuild#3897
Is there any update to this issue, or at least a workaround? My understanding is, that it's currently not possible to have an ASP.NET Core application localized with |
Any update? |
Team Triage: The design of resource localization is that we infer based on names whether the resource is culture specific. That means we need to be able to take a string and ask if it's a culture. Before the Windows 10 changes to how cultures worked, we just used @tarekgh is there a multi-platform check for cultures that doesn't accept dramatically more strings as cultures on windows? Based on @Forgind 's testing it looks like if we just call /cc: @wli3 for loc |
@benvillalobos the PR #6003 had the detailed discussion. Please let me know if there is any question not answered there, I can help answering here. My comment #6003 (comment) is answering your posted question, I guess. |
Any updates? So I can't build my asp.net core project to Linux image, this is unacceptable. |
@benvillalobos, you've been looking at this, right? |
That is right. Do you want to build the resources on Linux? You can just create them on Windows and use it with Linux runs as needed. I am trying to say, make a job in your pipeline to build these resources and on Linux you don't need to build the resources and instead copy the resources to output folder after finish building. I am just trying to provide some workaround to unblock you. I am not saying this is a solution as we still need to fix msbuild. |
Makes sense. I'll try that as I wait for the response to other questions in this thread. |
Q&A time 🤔
Definitely during build time. Can you describe your runtime scenario? Does this just mean "calling our API for valid cultures?" If so, it applies to both.
If we add this workaround as "if it's not seen in the culture API, use our hardcoded list as a backup," I expect windows/non-windows to behave the same. The only situation to be worried about would be some culture alias not existing in the hardcoded list, so it would still fail on unix. We'd need to handle those on a case by case basis.
I believe it'll work as long as the env var is set by the time our API gets called. Though consider that
cc @marcpopMSFT . The situation that concerns me is getting a flood of "we need this alias to be supported" in the long term, which isn't very maintainable.
Starting up a PR for it as soon as this comment gets posted.
It's a new codepath within our binaries, so It'd need an upgrade to net7 unless we backport. |
Thanks for the detail @benvillalobos !
Mostly our runtime scenario is just setting However, we do also have some code that tries to iterate through all cultures for which we have translations at runtime using
It would be great to know whether this is possible/likely. If this doesn't happen, is there any way to work around this at build time (aside from renaming all the resource files of course)? I suppose we could have a build task which just copies and renames all the files. |
Note: I have updated some of these after @tarekgh let me know (and I confirmed) that the NLS mode is not enabled. @tarekgh I've tried setting Command Ran on Git Bash - Linux Container version
Command Ran on Git Bash - Windows version
Specs for the Windows Machine Used
SetupI tried 4 different cases. In each case, I:
Results
Miscellaneous Info
Questions:
Thank you very much in advance, and I look forward to hearing back from you. |
Reading these two comments suggests the NLS mode is not enabled correctly. Could you add the following line in your code and send the output? Console.WriteLine($".... UseNls: {typeof(object).Assembly.GetType("System.Globalization.GlobalizationMode")!.GetProperty("UseNls", BindingFlags.Static | BindingFlags.NonPublic)!.GetValue(null)} ...."); |
@tarekgh Thanks for pointing that out! I have updated my comment above accordingly. It appears that the changes I made to environment variable were not applied for some reason. Properly enable the NLS mode answered two of my questions, but I'm still curious about the other two. I'm especially curious about how long .NET will support NLS mode through runtime configuration. Will it survive in .NET 7? |
Yes, it will still be supported in .NET 7.0. We don't have any plan to abandon NLS support in the near future. @Bartleby2718 if there any questions now answered yet, please point at it and I'll be happy to answer it. |
One last recommendation to @Bartleby2718 is try not to set |
@tarekgh That makes sense. I think that answers all my questions. Thanks for the prompt responses! |
@tarekgh Actually I do have one more question. I believe there was one more option besides |
In your case, it is not recommended. This one will have the same effect as if you set the property in the csproj. |
Just another point of context on why supporting |
@tarekgh I see that a PR has been merged to fix this issue.
|
@benvillalobos could you please help answering @Bartleby2718 questions? |
@Bartleby2718 Testing should involve running builds that previously didn't output resources for aliases like zh-TW and ensuring you see the expected output. Based on how the SDK flows, it should be available whenever the next release is available. In a few days I believe |
There should be a new daily build available in a few days but the next official release (RC1) won't be available until September. |
@tarekgh I used DOTNET_SYSTEM_GLOBALIZATION_USENLS=1 to sort out our Windows build servers, but also build on Linux which is failing, is there a work around for that? |
@Jetski5822 #3897 (comment). In short, there is no workaround for that except if you build the resources on Windows and then copy it to Linux build. Also, the issue should be fixed in #7853. You may consider using the new SDK? |
@tarekgh Okay cool - so I flipped zh-CN to zh-Hans-CN; but to get them to compile, theres a difference between VS (im using latest stable) and Command line. VS = the resource set wont get built, and when debugging the test, it also wont load. Any ideas why this would be happening?, also - the latest SDK, is that NET7? |
@Jetski5822 I guess, VS is using msbuild based on .NET Framework. @benvillalobos will know for sure. My question now is, is it possible you can just generate the resources with the name |
@Jetski5822 yeah this behavior difference is unfortunately expected. The fix only works for net core scenarios, since that's where we get our fancy new API from. |
Problem: Refer to dotnet/msbuild#3897 When building with dotnet cli, certain cultures cannot be utilized for resource localization, such as getting localized strings. This impacts both the build process (e.g., identifying and processing resource files) and the lookup of resources at runtime. For example: MyStrings.zh-TW.resx will not have a resource assembly created. Solution: This change renames all *.zh-CN.resx to *.zh-Hans.resx and *.zh-TW.resx to *.zh-Hant.resx. Test Done: Local build with dotnet cli and verified chinese assemblies are created.
How to use this code in project? We have lot of services use zh-CN/zh-TW, I don't want to rename all one by one, is there any better way to fix? Any insight is helpful. |
From @CodingDinosaur on October 12, 2018 3:30
When building or running under .NET Core on a Unix-based environment, certain cultures cannot be utilized for resource localization, such as getting localized strings. This impacts both the build process (e.g., identifying and processing resource files) and the lookup of resources at runtime. These cultures can be used as expected when building or running under Windows.
The affected cultures are those which are aliased by ICU -- that is, to save on DB space for certain cases, ICU defines some locales as an "alias" of another. There are 42 locale aliases in ICU 57, of those two of the most common are zh-TW and zh-CN. For a full list of affected locales, see: ICU Aliased Locale List - CultureIssueDemonstration Readme.
This platform-inconsistent behavior when trying to localize certain resources, necessitates using special code and workarounds both at build and deploy time when developing cross-platform applications.
See a demo of this issue in CodingDinosaur/CultureIssueDemonstration
Symptoms
Expected behavior
Brief Analysis
Most of the above symptoms boil down to uloc_getAvailable in ICU's C API.
For example, the zh-TW resource files do not get copied during build, because during the task SplitResourcesByCulture, the culture is validated against a cache based ultimately on CultureInfo.GetCultures, which in turn, on Unix, ultimately relies on ICU. A diagnostic MSBuild log shows why the file is missing:
From which we can follow back to the offending path:
Microsoft/msbuild/src/Tasks/Microsoft.Common.CurrentVersion.targets - SplitResourceByCulture ->
Microsoft.Build.Tasks.AssignCulture.Execute ->
Microsoft.Build.Tasks.Culture.GetItemCultureInfo ->
Microsoft.Build.Tasks.CultureInfoCache.IsValidCultureString ->
Microsoft.Build.Shared.AssemblyUtilities.GetAllCultures ->
CultureInfo.GetCultures ->
CultureData.GetCultures ->
CultureData(Unix).EnumCultures ->
System.Globalization.Native/locale.cpp:GlobalizationNative_GetLocales
https://github.com/dotnet/coreclr/blob/8ba838fb54d6c07271d026b2d77bedcb9e2a786a/src/corefx/System.Globalization.Native/locale.cpp#L162-L171
ICU does not return aliases when getting a list of locales -- whether with uloc_getAvailable or Locale::getAvailableLocales (and uloc_countAvailable does not include them in its count).
That ICU does not return the aliases in this manner appears to be intentional, both based on the numerous references to a lack of alias mapping in the uloc documentation, and the following bug:
https://unicode-org.atlassian.net/browse/ICU-4309
ICU-4309 was fixed via: unicode-org/icu@ab68bb3
Which seems to further indicate that ICU not returning aliases when calling uloc_getAvailable is intentional.
In-Depth Analysis
A full analysis can be seen in the test repo README: CodingDinosaur/CultureIssueDemonstration
Test Repos
I have two test repos that help demonstrate this issue:
CultureIssueDemonstration
CultureIcuTest
Copied from original issue: dotnet/coreclr#20388
The text was updated successfully, but these errors were encountered: