dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.56k stars 4.54k forks source link

Path for ICU_DAT_FILE_PATH when targeting iOS inside an XCTest bundle #88835

Open lemonmojo opened 12 months ago

lemonmojo commented 12 months ago

Description

As commented here the recently introduced change to relatively locate the icudt.dat file inside an app bundle works fine.

However, when a NativeAOT'd library is included and called from an XCTest bundle, -[NSBundle mainBundle] returns a path inside Xcode instead of the test bundle:

let mainPath = Bundle.main.bundlePath
// /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/Library/Xcode/Agents

let testPath = Bundle(for: SomeTypeInTheTestBundle.self).bundlePath
// ~/Library/Developer/Xcode/DerivedData/TestBundle/Build/Products/Variant-ASan/Debug-iphonesimulator/TestBundle.xctest

Of course this causes the load to fail because the icudt.dat will never be in Xcode's app bundle. If the XCTest project is however set up to copy this file to it's bundle resources folder, it would be there waiting to be consumed by the .NET runtime but currently fails because the logic of locating it just targets the main bundle.

akoeplinger commented 12 months ago

This shouldn't be NativeAOT specific and happen on Mono as well, guess nobody tried that yet :)

ghost commented 12 months ago

Tagging subscribers to 'os-ios': @steveisok, @akoeplinger, @kotlarmilos See info in area-owners.md if you want to be subscribed.

Issue Details
### Description As [commented here](https://github.com/xamarin/xamarin-macios/issues/18471#issuecomment-1634287195) the recently [introduced change](https://github.com/dotnet/runtime/pull/87813) to relatively locate the icudt.dat file inside an app bundle works fine. However, when a NativeAOT'd library is included and called from an XCTest bundle, `-[NSBundle mainBundle]` returns a path inside Xcode instead of the test bundle: ```swift let mainPath = Bundle.main.bundlePath // /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/Library/Xcode/Agents let testPath = Bundle(for: SomeTypeInTheTestBundle.self).bundlePath // ~/Library/Developer/Xcode/DerivedData/TestBundle/Build/Products/Variant-ASan/Debug-iphonesimulator/TestBundle.xctest ``` Of course this causes the load to fail because the icudt.dat will never be in Xcode's app bundle. If the XCTest project is however set up to copy this file to it's bundle resources folder, it would be there waiting to be consumed by the .NET runtime but currently fails because the logic of locating it just targets the main bundle.
Author: lemonmojo
Assignees: -
Labels: `area-CoreLib-mono`, `os-ios`
Milestone: -
lemonmojo commented 12 months ago

@akoeplinger I guess so. Unfortunately I currently have no idea what a proper fix might look like as the location of the file heavily depends on the project/bundle structure.

I mean, even if -[NSBundle mainBundle] would return the XCTest bundle, the icudt.dat file might instead be included in some framework, potentially deeply nested.

Is there some way/hack to set the path at runtime?

akoeplinger commented 12 months ago

hmm not right now, we have the ICU_DAT_FILE_PATH app context key but NativeAOT compiles that into the app "hardcoded" during the build time.

we could probably add an environment variable override.

another idea might be calling AppContext.SetData("ICU_DAT_FILE_PATH", "<path>") early on in startup so that it overrides the value that NativeAOT compiled into the app

lemonmojo commented 11 months ago

@akoeplinger I can confirm that using AppContext to set the file path at runtime does indeed work.

I think it's still a good idea to change the loading code in the runtime to use -[NSBundle bundleForClass:] and provide a way to overwrite the path using an env var in case the bundle is structured in another way.

lemonmojo commented 11 months ago

@akoeplinger Any chance that the switch to -[NSBundle bundleForClass:] will make it into .NET 8?

akoeplinger commented 11 months ago

@lemonmojo I was planning to but we recently hit an issue in our so called "library mode" where linking the runtime into two libraries causes an issue because of duplicate Objective-C classes and we got rid of the classes: https://github.com/dotnet/runtime/pull/89956

So the bundleForClass won't work since we don't have a class to point it to.

I think the env var is the only good option we have left, do you think that'd still help or is the AppContext solution good enough?

akoeplinger commented 11 months ago

Actually, can you try setting ICU_DAT_FILE_PATH as an environment variable? it looks like AppContext will read from the environment if it's not set already.

akoeplinger commented 10 months ago

For XCTest we could probably look at XCTestBundlePath to find the bundle path but we'd hit the same issue when we're embedded inside a .framework.

Alternatively we could make the build pass in the bundle identifier via AppContext and then use +[NSBundle bundleWithIdentifier] (see the perf optimizations Flutter had to do with that approach: https://github.com/flutter/engine/pull/39975)

I wonder if the easiest solution is embedding the icudt.dat file itself somehow.

lemonmojo commented 10 months ago

@akoeplinger Sorry for the delay. I just tried setting the ICU_DAT_FILE_PATH as an environment variable instead of writing it to AppContext but that didn't work unfortunately.

At least for my case, just looking at XCTestBundlePath won't do the trick as I use nested frameworks and the ICU file could be deep inside the framework tree.

I'm not sure I understand your second suggestion regarding passing in the bundle identifier. Could you please elaborate?

Is embedding the icudt.dat file a viable option at that level? If so, that would absolutely be the easiest approach from a .NET user's perspective. To be honest, I didn't even know what that file is and why it's required until I hit some issues because I wasn't embedding it. So yeah, if you can make that work I'd be awesome.

ghost commented 5 months ago

Tagging subscribers to this area: @dotnet/area-system-globalization See info in area-owners.md if you want to be subscribed.

Issue Details
### Description As [commented here](https://github.com/xamarin/xamarin-macios/issues/18471#issuecomment-1634287195) the recently [introduced change](https://github.com/dotnet/runtime/pull/87813) to relatively locate the icudt.dat file inside an app bundle works fine. However, when a NativeAOT'd library is included and called from an XCTest bundle, `-[NSBundle mainBundle]` returns a path inside Xcode instead of the test bundle: ```swift let mainPath = Bundle.main.bundlePath // /Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/Library/Xcode/Agents let testPath = Bundle(for: SomeTypeInTheTestBundle.self).bundlePath // ~/Library/Developer/Xcode/DerivedData/TestBundle/Build/Products/Variant-ASan/Debug-iphonesimulator/TestBundle.xctest ``` Of course this causes the load to fail because the icudt.dat will never be in Xcode's app bundle. If the XCTest project is however set up to copy this file to it's bundle resources folder, it would be there waiting to be consumed by the .NET runtime but currently fails because the logic of locating it just targets the main bundle.
Author: lemonmojo
Assignees: akoeplinger
Labels: `area-System.Globalization`, `os-ios`
Milestone: 9.0.0