dotnet / runtime

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

iOS Release can't build with CosmosDB #101967

Open mdbill opened 4 months ago

mdbill commented 4 months ago

Description

Attempt to build an ipa file (Release) fails.

error : ILStrip failed for C:/Users/xxx/.nuget/packages/microsoft.azure. cosmos/3.38.1/runtimes/win-x64/native/Microsoft.Azure.Cosmos.ServiceInterop.dll: The image is not a managed assembly

Oddly, it's a windows-only DLL. Worked in Xamarin.Forms. Works on Android. Works on iOS Debug/Simulator

See: https://github.com/Azure/azure-cosmos-dotnet-v3/issues/3171

Steps to Reproduce

build an ipa file (Release) of the MAUI template with a reference to the Cosmos nuget. I used 3.38.1.

Link to public reproduction project repository

No response

Version with bug

8.0.10 SR3

Is this a regression from previous behavior?

Yes, this used to work in Xamarin.Forms

Last version that worked well

Unknown/Other

Affected platforms

iOS

Affected platform versions

No response

Did you find any workaround?

The Cosmos group said this DLL is not needed and will work fine if missing, so I just need to get past the build.

Relevant log output

No response

drasticactions commented 4 months ago

Your issue is unrelated to the MAUI UI project and this repo, it's a runtime issue. I'm not unsure if it's an issue with ILStrip trying to strip an assembly it should not be or with the Cosmos library including that assembly if it shouldn't be, but I believe this is something that would need to go to runtime or to Cosmos. It must be addressed elsewhere.

If you want to work around it, you can enable the Mono Interpreter with <UseInterpreter> in your csproj, and that should disable the stripping.

スクリーンショット 2024-04-06 23 00 21

To show my point, I wrote an example app in .NET iOS app with no MAUI tooling. Enabling the interpreter in Release mode and publishing it gets me the IPA, disabling it throws the same exception you're getting in your MAUI app. Remember that a MAUI app is really a .NET platform app with an abstracted UI layer library on top and some extra tooling.

If you see library errors like this, try deploying it with a straight .NET app first without the MAUI UI layer. If it works there and doesn't work with the MAUI Template, there could be an issue in this repo. But if it doesn't work with a .NET app at all, it's probably something more fundamental.

@dalexsoto I'm not sure where this issue should go, would you know?

e. Also to note, it "worked" in Xamarin.Forms because it's based on older Mono tooling. The underlying .NET runtime used by MAUI is similar, but not the same. The UI layer has nothing to do with your issue.

mdbill commented 4 months ago

Thanks for the help.
How would: <EnableAssemblyILStripping>false</EnableAssemblyILStripping> compare to <UseInterpreter>? Is one less of an effect than the other? Is there a way to focus the effect only on the Cosmos dll? Is it really OK to do either of these for production?

mdbill commented 4 months ago

I realized my tags weren't wrapped as code and parts of my previous post weren't visible and made no sense. Can anyone comment now?

dotnet-policy-service[bot] commented 3 months ago

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

dotnet-policy-service[bot] commented 3 months ago

Tagging subscribers to this area: @directhex See info in area-owners.md if you want to be subscribed.

ViktorHofer commented 3 months ago

@akoeplinger do you have an area label for ILStrip issues?

akoeplinger commented 3 months ago

hmm we don't have one for ILStrip. @vitek-karas should we add one or use area-Tools-ILLink?

To the issue: we need to add a check and skip non-managed DLLs in ILStrip (e.g. by catching ImageFormatException). That said, I'm not sure if that'll be enough since looking at the xamarin-macios task I think it relies on ILStrip writing the stripped file to the output path and just skipping would "remove" the assembly. Maybe we can avoid passing in non-managed assemblies to the task instead. /cc @rolfbjarne

@mdbill I'd recommend EnableAssemblyILStripping=false as a workaround since that'll still AOT compile your app, it just won't remove the (now unused) IL from the .dll's.

vitek-karas commented 3 months ago

We can probably leave it in a mono area either as infra or as AOT code gen related.

Where is ILStrip used - is it only from mono AOT compiler on both Android/iOS?

@jkurdek could you please take a quick look?

akoeplinger commented 3 months ago

Where is ILStrip used - is it only from mono AOT compiler on both Android/iOS?

There is a bit of a complex history here :)

Originally ILStrip was only used by xamarin-macios to remove IL since they do "full" AOT and don't need the IL anymore. It trims method bodies from all methods in an assembly unconditionally. It uses a very old version of Cecil that keeps the metadata tokens the same when removing method bodies which is required so the mono runtime can resolve methods in the AOT image from a stripped .dll but newer Cecil versions automatically regenerate the metadata token. We imported that copy of Cecil into runtime-assets and compile it into the task directly (which is kinda bad: https://github.com/dotnet/runtime/issues/60273).

Some time ago Fan added a way to trim only specific methods (TrimIndividualMethods flag in the task), this was so that xamarin-android can use ILStrip as well since FullAOT/stripping everything isn't realistic there. The way it works is that the mono aot compiler writes a list of methods it AOTd into a file and that file is passed to ILStrip so it only trims those methods. This part of ILStrip uses System.Reflection.Metadata instead of Cecil and "solves" the metadata token issue by just zero-ing the method body bytes.

I assume only the first one used by xamarin-macios is affected.

rolfbjarne commented 3 months ago

@mdbill I'd recommend EnableAssemblyILStripping=false as a workaround since that'll still AOT compile your app, it just won't remove the (now unused) IL from the .dll's.

Agreed, this is the best option (pretty much the only downside is a bigger app).

To the issue: we need to add a check and skip non-managed DLLs in ILStrip (e.g. by catching ImageFormatException). That said, I'm not sure if that'll be enough since looking at the xamarin-macios task I think it relies on ILStrip writing the stripped file to the output path and just skipping would "remove" the assembly. Maybe we can avoid passing in non-managed assemblies to the task instead. /cc @rolfbjarne

We could adjust our ILStrip task subclass to filter out non-managed assemblies:

https://github.com/xamarin/xamarin-macios/blob/6d137cbeb228818ff1f3d578114e4027402e347d/msbuild/Xamarin.MacDev.Tasks/Tasks/ILStrip.cs#L26-L27

That said, to me it feels like the best option would be to add a property to the ILTask that allows the user to decide what to do with non-managed assemblies (throw/warn/ignore) - just from a simple performance perspective it's best to open and read the file(s) in question only once, and ILTask already opens and reads these files.

akoeplinger commented 3 months ago

We could also populate the UpdatedAssemblies output which is currently only updated by the TrimIndividualMethods codepath:

https://github.com/dotnet/runtime/blob/9926d609dab5929941b7924a04521ea2c2df27f3/src/tasks/MonoTargetsTasks/ILStrip/ILStrip.cs#L45-L54

Only downside is that this existed in a different form in 8.0 so backporting wouldn't be straighforward.

Sidenote: it is kinda weird that a file from runtimes/win-x64 gets included in the build, we should investigate that separately.

jkurdek commented 3 months ago

That said, I'm not sure if that'll be enough since looking at the [xamarin-macios task](https://github.com/xamarin/xamarin- macios/blob/6d137cbeb228818ff1f3d578114e4027402e347d/dotnet/targets/Xamarin.Shared.Sdk.targets#L894) I think it relies on ILStrip writing the stripped file to the output path and just skipping would "remove" the assembly.

Yeah, just handling the error in ILStrip.cs/StripAssembly doesn't look enough. The xamarin-macios task would still write it information about the assembly to the output, yet the assembly would not be written if skipped.

We could also populate the UpdatedAssemblies output which is currently only updated by the TrimIndividualMethods codepath

This looks hopeful. You mean to create StrippedAssemblies in xamarin-macios using UpdatedAssemblies right?

akoeplinger commented 3 months ago

This looks hopeful. You mean to create StrippedAssemblies in xamarin-macios using UpdatedAssemblies right?

Yep.