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

Problem with Collator.getInstance(), related to ICU4N.resources #69

Closed
michaelhkay opened this issue May 5, 2024 · 6 comments · Fixed by #88
Closed

Problem with Collator.getInstance(), related to ICU4N.resources #69

michaelhkay opened this issue May 5, 2024 · 6 comments · Fixed by #88
Assignees
Labels
is:bug Something isn't working

Comments

@michaelhkay
Copy link

I'm getting an exception with Collator.getInstance(). I'm running in Rider. My project has a nuget dependency on ICU4N 60.1.0-alpha.402. Note the reference to ICU4N.resources version=60.0.0.0.

I thought I previously had this working but I might have been mistaken, because the application has a fallback path that catches the exception.

(Note, it would be great to get an ICU4N version that isn't at alpha status, since this leads to build warnings).

Michael Kay

System.TypeInitializationException: The type initializer for 'ICU4N.Globalization.CultureInfoExtensions' threw an exception.
---> System.TypeInitializationException: The type initializer for 'DotNetLocaleHelper' threw an exception.
---> System.TypeInitializationException: The type initializer for 'ICU4N.Impl.ICUData' threw an exception.
---> System.IO.FileNotFoundException: Could not load file or assembly 'ICU4N.resources, Version=60.0.0.0, Culture=neutral, PublicKeyToken=efb17c8e4f0e291b'. The system cannot find the file specified.

File name: 'ICU4N.resources, Version=60.0.0.0, Culture=neutral, PublicKeyToken=efb17c8e4f0e291b'
at System.Reflection.RuntimeAssembly.InternalLoad(AssemblyName assemblyName, StackCrawlMark& stackMark, AssemblyLoadContext assemblyLoadContext, RuntimeAssembly requestingAssembly, Boolean throwOnFileNotFound)
at System.Reflection.RuntimeAssembly.InternalGetSatelliteAssembly(CultureInfo culture, Version version, Boolean throwOnFileNotFound)
at System.Reflection.RuntimeAssembly.GetSatelliteAssembly(CultureInfo culture, Version version)
at ICU4N.Impl.ICUData..cctor()
--- End of inner exception stack trace ---
at ICU4N.Impl.ICUData.GetLocaleIDFromResourceName(String resourceName)
at ICU4N.Impl.ICUData.GetStream(Assembly loader, String resourceName, Boolean required)
at ICU4N.Impl.ICUBinary.GetData(Assembly assembly, String resourceName, String itemPath, Boolean required)
at ICU4N.Impl.ICUBinary.GetData(Assembly assembly, String resourceName, String itemPath)
at ICU4N.Impl.ICUResourceBundleReader.<>c__DisplayClass35_0.b__0(ReaderCacheKey key)
at ICU4N.Impl.SoftCache2.<>c__DisplayClass1_1.<GetOrCreate>b__1() at System.Lazy1.ViaFactory(LazyThreadSafetyMode mode)
at System.Lazy1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor) at System.Lazy1.CreateValue()
at ICU4N.Impl.SoftCache2.GetOrCreate(TKey key, Func2 valueFactory)
at ICU4N.Impl.ICUResourceBundleReader.GetReader(String baseName, String localeID, Assembly root)
at ICU4N.Impl.ICUResourceBundle.CreateBundle(String baseName, String localeID, Assembly root)
at ICU4N.Impl.ICUResourceBundle.<>c__DisplayClass64_0.b__0(String key)
at ICU4N.Impl.SoftCache2.<>c__DisplayClass1_1.<GetOrCreate>b__1() at System.Lazy1.ViaFactory(LazyThreadSafetyMode mode)
at System.Lazy1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor) at System.Lazy1.CreateValue()
at ICU4N.Impl.SoftCache2.GetOrCreate(TKey key, Func2 valueFactory)
at ICU4N.Impl.ICUResourceBundle.InstantiateBundle(String baseName, String localeID, String defaultID, Assembly root, OpenType openType)
at ICU4N.Impl.ICUResourceBundle.GetBundleInstance(String baseName, String localeID, String defaultID, Assembly root, OpenType openType)
at ICU4N.Impl.ICUResourceBundle.GetBundleInstance(String baseName, String localeID, Assembly root, OpenType openType)
at ICU4N.Impl.ICUResourceBundle.GetBundleInstance(String baseName, String localeID, Assembly root, Boolean disableFallback)
at ICU4N.Util.UResourceBundle.<>c__DisplayClass22_0.b__0(String key)
at System.Collections.Concurrent.ConcurrentDictionary2.GetOrAdd(TKey key, Func2 valueFactory)
at ICU4N.Util.UResourceBundle.GetRootType(String baseName, Assembly root)
at ICU4N.Util.UResourceBundle.InstantiateBundle(String baseName, String localeName, Assembly root, Boolean disableFallback)
at ICU4N.Impl.ICUResourceBundle.CreateUCultureList(String baseName, Assembly root)
at ICU4N.Impl.ICUResourceBundle.AvailEntry.<>c__DisplayClass7_0.b__0()
at System.Threading.LazyInitializer.EnsureInitializedCore[T](T& target, Func1 valueFactory) at ICU4N.Impl.ICUResourceBundle.AvailEntry.GetUCultureList(UCultureTypes types) at ICU4N.Impl.ICUResourceBundle.GetUCultures(String baseName, Assembly assembly, UCultureTypes types) at ICU4N.Impl.ICUResourceBundle.GetUCultures(UCultureTypes types) at ICU4N.Globalization.UCultureInfo.GetCultures(UCultureTypes types) at ICU4N.Globalization.UCultureInfo.DotNetLocaleHelper.LoadNonGregorianDefaultCalendars() at System.Threading.LazyInitializer.EnsureInitializedCore[T](T& target, Boolean& initialized, Object& syncLock, Func1 valueFactory)
at ICU4N.Globalization.UCultureInfo.DotNetLocaleHelper.EnsureInitialized()
at ICU4N.Globalization.UCultureInfo.DotNetLocaleHelper..cctor()
--- End of inner exception stack trace ---
at ICU4N.Globalization.UCultureInfo.DotNetLocaleHelper.EnsureInitialized()
at ICU4N.Globalization.CultureInfoExtensions..cctor()
--- End of inner exception stack trace ---
at ICU4N.Globalization.CultureInfoExtensions.ToUCultureInfo(CultureInfo culture)
at ICU4N.Globalization.UCultureInfo.GetCurrentCulture()
at ICU4N.Globalization.UCultureInfo.get_CurrentCulture()
at ICU4N.Text.Collator.GetInstance()
at Saxon.Eej.expr.sort.UcaCollatorUsingIcu..ctor(String uri) in /Users/mike/GitHub/saxon13/build/cs/ee/Saxon/Eej/expr/sort/UcaCollatorUsingIcu.cs:line 22
The type initializer for 'ICU4N.Globalization.CultureInfoExtensions' threw an exception.
System.TypeInitializationException: The type initializer for 'ICU4N.Globalization.CultureInfoExtensions' threw an exception.
---> System.TypeInitializationException: The type initializer for 'DotNetLocaleHelper' threw an exception.
---> System.TypeInitializationException: The type initializer for 'ICU4N.Impl.ICUData' threw an exception.
---> System.IO.FileNotFoundException: Could not load file or assembly 'ICU4N.resources, Version=60.0.0.0, Culture=neutral, PublicKeyToken=efb17c8e4f0e291b'. The system cannot find the file specified.

@NightOwl888
Copy link
Owner

Yes, you are correct that it should fallback. However, there are 2 exceptions:

  1. If the neutral culture (e.g. en/ICU4N.resources.dll) is not present but a localized version of it (e.g. en-GB/ICU4N.resources.dll) is present, it will fail because most of the data is in the neutral culture assembly. This is a limitation imposed in ICU4J.
  2. If the root assembly ICU4N.resources.dll is missing, ICU4N will fail to execute since this is a required file.

I suspect it is more likely the latter situation that is causing this error. But you will need to check your bin folder to see what is actually on disk.

If you only have a PackageReference to ICU4N and have not customized the build in any way, it should copy the files in appropriately. But a good test to see if the build is working is to close the IDE, delete the /bin and /obj folders, and then build again.


I think we will have to take a step backward in the next alpha release and put all of the resources in a single root ICU4N.resources.dll satellite assembly file, as getting into the weeds of which culture names are appropriate to use turns out to be more nuanced than "gather the list of cultures". Microsoft has added cultures that ICU4C doesn't support out of the box, which makes the decision to follow Microsoft's approach or use the standard ICU resources a tough choice. Not to mention, the appropriate list may vary depending on the .NET target framework. It doesn't seem worth it to delay the next release while arriving at a decision on this, but we definitely need a better approach before then.


I have been putting in several 16-hour days over the past few weeks adding wide support for ReadOnlySpan<char> so we can eliminate the need to generate code with multiple copies of the same business logic in it and maintain APIs that accept char[], StringBuilder, and ICharSequence. This work is complete in J2N and around 2/3 of the way complete in ICU4N. This of course goes a long way toward making the public API more stable.

But ultimately, I think much of the public API surface needs to be made internal (or flagged in a way that the user receives warnings if they use it) so we can do a production release without getting into the weeds of refactoring either because of conventions or performance degradation due to unfortunate API design choices.

@michaelhkay
Copy link
Author

Thanks for the response. I can confirm that there were no ICU4N.resources files in the bin folder. I've deleted the bin and obj folders as suggested, and the immediate effect is an error Microsoft.PackageDependencyResolution.targets(266, 5): [NETSDK1004] Assets file '/Users/mike/RiderProjects/.../obj/project.assets.json' not found. Run a NuGet package restore to generate this file.

I'm attempting to restore the project, with mixed success. I think I'll start rebuilding it from scratch and see what happens. At least I now have some pointers.

@michaelhkay
Copy link
Author

I've now rebuilt everything: did a good clear-out of my .NET and Rider installations, and created a new solution/project.

I'm getting the same failure.

A file named ICU4N.resources.dll is present in all of:

  • bin/Debug/net8.0
  • bin/Debug/net8.0/en
  • bin/Debug/net8.0/en-GB.

I'm not sure what you mean by "If the root assembly ICU4N.resources.dll is missing" - presumably the first of those three. (Which also contains ICU4N.dll and othe dependencies).

@michaelhkay
Copy link
Author

I have now established that the same basic code is working in another project (with an earlier version of Saxon). The most obvious difference between the projects is that the failing one is .net8.0, the working one is .net6.0. But switching the failing project to .net6.0 makes no difference. Any suggestions where I start looking?

@NightOwl888 NightOwl888 added is:bug Something isn't working and removed is:bug Something isn't working labels May 8, 2024
@NightOwl888
Copy link
Owner

I took a look by setting up a new .NET 8 project and noticed some strange behavior. I put 2 projects into a solution, a class library with a package reference on ICU4N and a console app. The resource files get copied into the console app output, but not in the class library.

I then switched the projects to net6.0 and got a similar result. However, I noticed that there is a difference. In the class library that targets net6.0, the <project>.deps.json file doesn't contain any of the resource files, but the file with the same name in the nt8.0 folder lists all of them.

That said, since a class library project is not executable, I don't think this is an issue (at least not in a production scenario). The resources get copied into a console app, so they exist in the place where the code runs. I also added a test project and confirmed that they get copied into that project and the tests run in the IDE.

Then I tried running the test project using dotnet test -c Debug, and reproduced the exception. I tried the test again in Visual Studio 2022 and it errored there, as well. There was a yellow icon next to the ICU4N.Resources package reference in the IDE. So, I then attempted to explicitly add a package reference to ICU4N.Resources from the test project and it now works on both the command line and in the IDE.

So, in summary, I have confirmed this is a bug, but at this point I don't know whether it is something that was introduced with the .NET 8 SDK or it is something that is broken on our end. You should be able to continue working by adding an explicit reference to ICU4N.Resources in your project until a fix is available.

@michaelhkay
Copy link
Author

Thanks, that seems to have solved it for now.

Dynamic loading in .NET is a pretty delicate business. We're pretty well eliminated it in SaxonCS because it was causing us too much support hassle.

@NightOwl888 NightOwl888 self-assigned this Oct 7, 2024
NightOwl888 added a commit that referenced this issue Oct 12, 2024
…s when building so we have a PackageReference when packing ICU4N.csproj. After the build, we delete the resources from the .nuget cache and pack real resource packages. (fixes #69)
NightOwl888 added a commit that referenced this issue Oct 12, 2024
…blies (#88)

* ICU4N.csproj: Added direct reference on ICU4N.Resources or ICU4N.Resources.NETFramework4.0 depending on target framework. Commented out transitive reference.

* ICU4N.csproj: Changed from ProjectReference to PackageReference to avoid interfering with debugging and made the PackageReference conditional so it can be included only when packing.

* azure-pipelines.yml: Commented check for SDKs

* .build/runbuild.ps1: Fixed build so it creates stub resources packages when building so we have a PackageReference when packing ICU4N.csproj. After the build, we delete the resources from the .nuget cache and pack real resource packages. (fixes #69)

* ICU4N.csproj: Added version constraint to lock specific version of resources to the binaries (upgrades not allowed).

* ICU4N.targets: Set SatelliteResourceLanguages manually to try to override MSBuild allowed values

* Revert "ICU4N.csproj: Added version constraint to lock specific version of resources to the binaries (upgrades not allowed)."

This reverts commit 093ed68.

* ICU4N.Globalization.CultureInfoUtil: Added TODO about reviewing this, since it doesn't seem like it should be public (and may not be required at all)

* ICU4N.targets: Removed SatelliteResourceLanguages property - we don't want to filter these cultures here

* ICU4N.Support.Globalization.ResourceUtil: Moved to ICU4N.Globalization namespace

* ICU4N.Resources: Added buildTransitive .targets file to list the cultures in the package

* ICU4N.csproj + ICU4N.Resources.csproj: Patching MSBuild is far easier if we don't have dependencies on netstandard1.0, so bumping ICU4N.Resources to netstandard2.0

* ICU4N.Resources + ICU4N.Resources.NETFramework4.0: Added buildTransitive .targets files to patch copying any missing resource files that MSBuild failed to copy to the build or publish output on the user's build machine.

* ICU4N.Resources.CopyPatch.targets: Fixed patch so it will be tolerant of the fact that not all project types have a .deps.json file. This file only applies to .NET Core projects. To be safe, if there is no .deps.json, copy the resource files regardless of other factors, such as whether it is a class library.

* ICU4N.Resources.CopyPatch.targets: Skip the patch during build on .NET Core and .NET Standard when building a class library.

* ICU4N.Resources.CopyPatch.targets: Removed check for .NET Core and OutputType == Library, because it doesn't function. Need to revisit.

* ICU4N.Impl.LocaleIDParser: Added methods to get the neutral culture from a locale for use in looking up resources in satellite assemblies.

* ICU4N.Impl.ICUResourceBundle::AddLocaleIDsFromSatelliteAndGACFolderNames(): Use metadata for checking the filenames of the embedded resources of satellite assemblies rather than loading the assembly into the AppDomain. Fixes #79.

* ICU4N.Globalization.ResourceUtil: Added GetDotNetNeutralCultureName() method that is used to map ICU locale names to neutral .NET culture names. This allows us to avoid issues with culture names that .NET doesn't consider valid by re-mapping them to valid .NET cultures. Fixed satellite assembly generation so it won't fail if the command length goes over 32768 chars by using a .rsp file that is generated on the fly.

* WIP: Unit tests for neutral cultures

* Revert "WIP: Unit tests for neutral cultures"

This reverts commit 043c911.

* ICU4JResourceConverter.ResourceUtil: Removed some commented code

* ICU4N.Globalization.ResourceUtil: Remapped "qu" to "quz" to conform with .NET's satellite assembly naming convention

* Removed ICU4N.targets (which was buildTransitive in ICU4N.nupkg), since the issues with filtering satellite assembly cultures has been addressed with the MSBuild SatelliteResourcesLanguage property and excluding the package can be done with supported mechanisms (specifically adding a PackageReference for ICU4N.Resources and using ExcludeAssets to remove the resource files from the build and publish output).

* README.md: Updated information about managing resources.

* ICU4N.csproj: Added version constraints for ICU4N.Resources and ICU4N.Resources.NETFramework4.0 (exact version match only).

* ICU4N.Resources.CopyPatch.targets: Updated documentation header to fix formatting and make it more readable

* ICU4N.Resources.CopyPatch.targets: Lowered message importance for logs so we don't output anything on minimal verbosity.

* ICU4N.Resources.CopyPatch.targets: Made logging messages a bit more clear.

* README.md: Made some minor corrections to the Managing Resources section for clarity.

* .build/runbuild.ps1: Updated test condition and path for .nuget cache so we use the default location if the NUGET_PACKAGES env variable is not defined.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is:bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants