NightOwl888 / ICU4N

International Components for Unicode for .NET
Apache License 2.0
27 stars 7 forks source link

Fix for many issues with copying, naming, and loading satellite assemblies #88

Closed NightOwl888 closed 1 month ago

NightOwl888 commented 1 month ago

Fixes #69, Fixes #65, Fixes #79

This addresses the following issues:

  1. MSBuild produced several NETSDK1187 and NETSDK1188 warning messages when referencing ICU4N.Resources. Addressed by repackaging the satellite assemblies into neutral culture names and providing mappings for the names that didn't match up between ICU and .NET.
  2. MSBuild failed to copy the root ICU4N.resources.dll file to the build output (See: #69 and https://github.com/dotnet/sdk/issues/40834). This was addressed by adding a direct reference to ICU4N.Resources rather than one that was generated transitively. MSBuild doesn't take transitive conditions into account in the NuGet restore operation.
  3. MSBuild fails to copy satellite assemblies with 3-letter language codes to the build output. This is a bug with MSBuild that hasn't been reported yet. But since there still are and will be broken SDKs in the wild, this was addressed by adding the ICU4N.Resources.CopyPatch.targets file to buildTransitive of both ICU4N.Resources and ICU4N.Resources.NETFramework4.0.
  4. Using only .NET supported names should also fix #65, but that has yet to be confirmed.
  5. ICU4N loaded all assemblies when listing cultures using the GetUCultures() method of several classes, such as UCultureInfo (See: #79). This has been addressed by reading the embedded file names from the metadata using System.Reflection.Metadata (except on net40, we still load the assemblies to get this list).
  6. Adds support for the MSBuild SatelliteResourceLanguages property, which allows making the distribution size smaller without having to re-package satellite assemblies.
  7. Fixes an issue with the Visual Studio 2022 debugger due to the fact it would stop functioning when a satellite assembly was loaded using Assembly.GetSatelliteAssembly(). This was also addressed by the fact we are sticking with .NET culture names instead of supplying ICU locale names to Assembly.GetSatelliteAssembly().
  8. Set up a NuGet version constraint to ensure the version of ICU4N.Resources or ICU4N.Resources.NETFramwork4.0 are the same version as ICU4N. Upgrading resource files to a higher version without also upgrading ICU4N is not supported.

We may still revisit resource packaging going forward, but this addresses most of the known usability problems with our previous attempt.

NOTE: We still have some issues with the MSBuild patch to copy resources. It is now copying the files in more cases than it should. For example, it will copy them to the output and update the .deps.json file for .NET Core and .NET Standard class libraries, even though class libraries don't require them during build. However, this is an improvement over not copying resource files to the build output when they should be, which is how MSBuild currently behaves with satellite assemblies that have 3-letter language codes.