dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.21k stars 1.35k forks source link

Satellite Assemblies not generated for some cultures (zh-CN) in .NET 6 #7331

Open nick-beer opened 2 years ago

nick-beer commented 2 years ago

Issue Description

This appears to be a regression on Windows. Starting in .NET 6, a satellite assemblies are not generated for some cultures. For instance, a project with an embedded resource resources.zh-CN.resx will not generate a satellite assembly when using the .NET 6 SDK, but will when using the .NET 5 SDK. I'm not sure if this is limited to zh-CN or not - though I suspect it's related to #3897.

If I build from Visual Studio (2022 17.1.0 Preview 2), the satellite assembly will be generated. Building with the dotnet cli (SDK 6.0.101) will not generate the satellite assembly.

Steps to Reproduce

Build the attached project: dotnet build LocalizationTest.csproj

LocalizationTest.zip

Expected Behavior

A satellite assembly will be generated into a zh-CN subfolder in the output folder.

Actual Behavior

No satellite assembly is generated.

Versions & Configurations

Windows 11 (21H2 22000.132) dotnet SDK 5.0.400 correctly generates the satellite assembly dotnet SDK 6.0.101 does not Visual Studio 2022 (17.1.0 Preview 2) does generate the satellite assembly

benvillalobos commented 2 years ago

This does look directly related to https://github.com/dotnet/msbuild/issues/3897, which will get more dev time once https://github.com/dotnet/msbuild/pull/6148 gets merged in.

Thank you for the repro!

nick-beer commented 2 years ago

In the meantime, is there any advice on what my approach should be using the current runtime/SDK? This is blocking my update of a very large repo to .NET 6 - lots of failing tests and localization is broken.

Specifically, I'm looking for guidance whether using zh-CN is ok/good long term, or if that's something we should ultimately be moving away from?

benvillalobos commented 2 years ago

@nick-beer Using zh-CN should be fine in the long run (cc @tarekgh). Once we're on net6.0 we should be able to use the CultureInfo api's to detect cultures like zh-CN properly.

nick-beer commented 2 years ago

I know that this is related but I'm also seeing now that zh-CN isn't returned from CultureInfo.GetCultures(CultureTypes.AllCultures). That doesn't seem like something that will be addressed by the issues you mentioned above - is that an intentional change in behavior since .NET 5? I wasn't able to find it listed in the breaking changes. Is that something I should open an issue for on the runtime repo?

benvillalobos commented 2 years ago

@nick-beer I'm not too sure about the specifics here, I'd file an issue in the runtime repo.

tarekgh commented 2 years ago

I know that this is related but I'm also seeing now that zh-CN isn't returned from CultureInfo.GetCultures(CultureTypes.AllCultures)

This is intentional. the correct name of this culture is zh-Hans-CN. zh-CN is just an alias for that. This is the CLDR Unicode standard. @BenVillalobos I don't think we need to open issue for that in runtime repo as we need to follow whatever Unicode/ICU give us. We should stick with the following names in the culture enumeration:

zh                  Chinese
zh-Hans             Chinese (Simplified)
zh-Hans-CN          Chinese (Simplified, China)
zh-Hans-HK          Chinese (Simplified, Hong Kong SAR)
zh-Hans-MO          Chinese (Simplified, Macao SAR)
zh-Hans-SG          Chinese (Simplified, Singapore)
zh-Hant             Chinese (Traditional)
zh-Hant-HK          Chinese (Traditional, Hong Kong SAR)
zh-Hant-MO          Chinese (Traditional, Macao SAR)
zh-Hant-TW          Chinese (Traditional, Taiwan)
nick-beer commented 2 years ago

Thanks for the information. As a summary:

These are both significant breakages for our code base - we use zh-CN for most of our localized Chinese assets, and have a non-trivial amount of localization code that (in a way) depends on zh-CN being returned from GetCultures (because we're looking for resources that exist in a zh-CN subfolder).

So, it sounds like I will need to update all of our localized Chinese assets to use zh-Hans-CN rather than zh-CN if we want to update to .NET 6. Ouch. I'm not missing anything, right?

tarekgh commented 2 years ago

@BenVillalobos is it possible we consider adding a small hack in msbuild for .NET 6.0 to manually add zh cultures to the list it enumerates? like zh-CN.

@nick-beer do you localize to other simplified Chinese languages? if not, I would recommend you to use zh-Hans instead which make the localization work for any simplified Chinese culture (like zh-Hans-HK in addition to zh-CN).

rainersigwald commented 2 years ago

I wouldn't be totally opposed to the hack, but I expect that we can get the aliasing work in MSBuild in for SDK 6.0.300, corresponding with VS 17.3.

nick-beer commented 2 years ago

@tarekgh - thanks for the information. zh-Hans is right for us.

I'll keep my eye out for the aliasing changes in 6.0.300 (or later?), though I suspect we'll need to perform similar work in our runtime code that depends on GetCultures (or drop zh-CN altogether...).

amandal1810 commented 2 years ago

Is there some known hack that could give me both the list of aliased cultures and non-aliased cultures? Using a union of these two lists might work to avoid this breaking change.

tarekgh commented 2 years ago

Is there some known hack that could give me both the list of aliased cultures and non-aliased cultures? Using a union of these two lists might work to avoid this breaking change.

I don't think there is a hack for that. I am wondering why you don't localize with language tags like zh-Hans and zh-Hant which should cover all Chinese cultures Simplified and Traditional cases? You don't need any hacks I guess at that time.

nick-beer commented 1 year ago

@tarekgh - I just noticed CultureInfo.GetCultureInfo("zh-CN", predefinedOnly: true) returns a valid culture (effectively zh-Hans-CN?). I found it unexpected that zh-CN is considered "predefined" but is not returned from GetCultures. Is that basically because zh-CN is a known, "predefined" alias, but not an actual culture name as defined by ICU? Is this a bug in GetCultureInfo?

tarekgh commented 1 year ago

zh-CN is not a culture by itself, it is just an alias to the other culture zh-Hans-CN. Returning both cultures from GetCultures is wrong because it can cause other problems. Like users in the UI list the culture display names and they will get a duplicates entry at that time.

nick-beer commented 1 year ago

The behavior of GetCultures makes sense to me. I was surprised that zh-CN is accepted as "predefined" via GetCultureInfo. Because it's not returned from GetCultures, I would not have expected it to be "predefined" as determined by GetCultureInfo. Mostly, just curious if that's expected behavior from GetCultureInfo.

tarekgh commented 1 year ago

predefined means there is a real culture data backing the specific culture name. This is the case for zh-CN which happens to have real data to support this culture name. The behavior you are seeing in GetCultures and GetCultureInfo is expected and correct. Let me know if you have any more questions I can help with. Thanks!

maltalex commented 1 year ago

For everyone's convenience I'd like to spell out the issue and its fix since it took us a considerable amount of head scratching and reading to figure this one out.

  1. The issue stems from a switch from NLS to ICU. There are differences in locales between these two standards, including zh-Hans/zh-CN, zh-Hant/zh-TW and others.

  2. The issue used to exist in only Unix-based environments (See #3897) since only they were using ICU before .NET 5:

Before .NET 5, the .NET globalization APIs used different underlying libraries on different platforms. On Unix, the APIs used International Components for Unicode (ICU), and on Windows, they used National Language Support (NLS). This resulted in some behavioral differences in a handful of globalization APIs when running applications on different platforms.

  1. At some point Windows started incorporating ICU and it became the default for .NET 5 (and above) for these versions:

image

This point especially created a lot of head scratching for us since we have CI pipelines on different versions of Windows, some of which produced valid Satellite Assemblies while others didn't.

  1. This was documented in the .NET 5's list of breaking changes here under the title "Use ICU libraries on Windows". Honestly, for a change this big I would have expected very clear warnings/errors from the tooling. Silently ignoring locales which were perfectly valid a minute ago is completely unreasonable. But unfortunately, this is quite typical of dotnet tooling in my experience. The official IDE does one thing, msbuild.exe another thing, and dotnet build a third thing and it all depends on the exact version of both Windows and the SDK. Moreover, the fix (#7853) went in only into .NET 7 despite .NET 6 being an LTS release. So, a workaround is needed "only" for the LTS release and only for recent versions of Windows.

  2. To fix the problem short-term you can switch back to NLS using one of the methods described here:

Solution 1 (in the project file):

<ItemGroup>
  <RuntimeHostConfigurationOption Include="System.Globalization.UseNls" Value="true" />
</ItemGroup>

Solution 2 (runtimeconfig.json):

{
  "runtimeOptions": {
     "configProperties": {
       "System.Globalization.UseNls": true
      }
  }
}

Solution 3(env variable, which I think is the only way to fix the issue at build time):

DOTNET_SYSTEM_GLOBALIZATION_USENLS=1 or DOTNET_SYSTEM_GLOBALIZATION_USENLS=true

codelovercc commented 1 year ago

Hi, zh-CN is an alias of zh-hans-CN , and there is more alias. So I created a map configuration that transform zh-CN to zh-hans-CN and so on. This would solve this problem.