Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Mono.Android.Export] decorate types to improve trimming warnings #9300

Merged

Conversation

jonathanpeppers
Copy link
Member

The entire Mono.Android.Export.dll assembly is not trimming safe, and never will be: it relies on many dynamic features. But it is possible to get the warning:

warning IL2104: Assembly 'Mono.Android.Export' produced trim warnings. For more information see https://aka.ms/dotnet-illink/libraries

First, import trim-analyzers.props so we have the right analyzers set for Mono.Android.Export.csproj. We can also remove $(EnableSingleFileAnalyzer) as it is in trim-analyzers.props.

This results in ~41 errors, which are mostly resolved by decorating every type with:

[RequiresUnreferencedCode (MonoAndroidExport.DynamicFeatures)]

This seems simpler than decorating methods, as there are quite a few more methods involved than classes.

After this change, we are down to a handful of warnings:

Mono.Android.Export\CallbackCode.cs(526,19): error IL3050: Using member 'System.Reflection.Emit.AssemblyBuilder.DefineDynamicAssembly(AssemblyName, AssemblyBuilderAccess)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. Defining a dynamic assembly requires dynamic code.
Mono.Android.Export\CallbackCode.cs(197,32): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
Mono.Android.Export\Mono.CodeGeneration\CodeCustomAttribute.cs(82,23): error IL3050: Using member 'System.Collections.ArrayList.ToArray(Type)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
Mono.Android.Export\Mono.CodeGeneration\CodeCustomAttribute.cs(83,20): error IL3050: Using member 'System.Collections.ArrayList.ToArray(Type)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
Mono.Android.Export\CallbackCode.cs(609,59): error IL3050: Using member 'System.Type.MakeGenericType(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
Mono.Android.Export\CallbackCode.cs(612,22): error IL3050: Using member 'System.Type.MakeGenericType(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
Mono.Android.Export\CallbackCode.cs(270,32): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
Mono.Android.Export\Mono.CodeGeneration\CodeModule.cs(48,35): error IL3050: Using member 'System.Reflection.Emit.AssemblyBuilder.DefineDynamicAssembly(AssemblyName, AssemblyBuilderAccess)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. Defining a dynamic assembly requires dynamic code.
Mono.Android.Export\CallbackCode.cs(426,32): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
Mono.Android.Export\CallbackCode.cs(643,13): error IL3050: Using member 'System.Reflection.Emit.DynamicMethod.DynamicMethod(String, MethodAttributes, CallingConventions, Type, Type[], Module, Boolean)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. Creating a DynamicMethod requires dynamic code.
Mono.Android.Export\CallbackCode.cs(336,32): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
Mono.Android.Export\CallbackCode.cs(336,154): error IL3050: Using member 'System.Type.MakeArrayType()' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
Mono.Android.Export\CallbackCode.cs(341,32): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
Mono.Android.Export\CallbackCode.cs(346,32): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
Mono.Android.Export\CallbackCode.cs(368,32): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
Mono.Android.Export\CallbackCode.cs(373,32): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
Mono.Android.Export\CallbackCode.cs(671,6): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.

This is a different analyzer for NativeAOT, which is IL3050.

The types involved are a smaller list, I decorated each with:

[RequiresDynamicCode (MonoAndroidExport.DynamicFeatures)]

With these changes, there are no no warnings present any longer.

Other changes:

  • Delete CodeAdd.cs, it only contains a comment

jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Sep 12, 2024
Context: dotnet#23769
Context: dotnet/android#9300

026e046 introduced `HybridWebView`, which unfortunately introduces
trimmer warnings in the `dotnet new maui` project template:

    > dotnet new maui
    > dotnet build -f net9.0-android -c Release -p:TrimMode=Full
    ...
    hellomaui succeeded with 1 warning(s) (7.9s)
    /_/src/Core/src/Handlers/HybridWebView/HybridWebViewHandler.Android.cs(53,5):
    Trim analysis warning IL2026: Microsoft.Maui.Handlers.HybridWebViewHandler.HybridWebViewJavaScriptInterface.SendMessage(String):
    Using member 'Java.Interop.ExportAttribute.ExportAttribute(String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code.
    [ExportAttribute] uses dynamic features.

This is due to usage of `Java.Interop.ExportAttribute`:

    private sealed class HybridWebViewJavaScriptInterface : Java.Lang.Object
    {
        //...
        [JavascriptInterface]
        [Export("sendMessage")]
        public void SendMessage(string message)

`Java.Interop.ExportAttribute` makes heavy usage of unbounded
System.Reflection, System.Reflection.Emit, etc. for it to be able to
work. It brings in an additional assembly `Mono.Android.Export.dll` as
well.

It is inherently trimming unsafe, but how did it get through these
tests?

https://github.com/dotnet/maui/blob/08ff1246383ed4fdaef84a40d5b2ae8e6096bb56/src/TestUtils/src/Microsoft.Maui.IntegrationTests/AndroidTemplateTests.cs#L50-L61

This slipped through, unfortunately, as we had missed solving all the
trimmer warnings in `Mono.Android.Export.dll`! dotnet/android#9300
aims to fix that.

After dotnet/android#9300, new trimming warnings specific to .NET MAUI
will surface such as the one above.

But we can easily replace `[Export]` by:

* Define a Java interface & create a binding for it in C#, we already
  have `maui.aar` setup for this.

* We can simply implement the interface in C# and remove `[Export]`.

Lastly, I fixed some of the defaults in `Metadata.xml`, it didn't look
like we were automatically making Java interfaces `internal`. It looks
like we probably made `ImageLoaderCallback` public by mistake.
@jonathanpeppers
Copy link
Member Author

Ok wow MAUI Integration test lane will fail with:

D:\a\_work\1\s\maui\src\Core\src\Handlers\HybridWebView\HybridWebViewHandler.Android.cs(50,5): error IL2026: Using member 'Java.Interop.ExportAttribute.ExportAttribute(String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. [ExportAttribute] uses dynamic features. [D:\a\_work\1\s\maui\src\Core\src\Core.csproj::TargetFramework=net9.0-android35.0]
    68 Warning(s)
    1 Error(s)

Until we get this one:

@jonathanpeppers jonathanpeppers marked this pull request as ready for review September 12, 2024 19:24
rmarinho pushed a commit to dotnet/maui that referenced this pull request Sep 20, 2024
Context: #23769
Context: dotnet/android#9300

026e046 introduced `HybridWebView`, which unfortunately introduces
trimmer warnings in the `dotnet new maui` project template:

    > dotnet new maui
    > dotnet build -f net9.0-android -c Release -p:TrimMode=Full
    ...
    hellomaui succeeded with 1 warning(s) (7.9s)
    /_/src/Core/src/Handlers/HybridWebView/HybridWebViewHandler.Android.cs(53,5):
    Trim analysis warning IL2026: Microsoft.Maui.Handlers.HybridWebViewHandler.HybridWebViewJavaScriptInterface.SendMessage(String):
    Using member 'Java.Interop.ExportAttribute.ExportAttribute(String)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code.
    [ExportAttribute] uses dynamic features.

This is due to usage of `Java.Interop.ExportAttribute`:

    private sealed class HybridWebViewJavaScriptInterface : Java.Lang.Object
    {
        //...
        [JavascriptInterface]
        [Export("sendMessage")]
        public void SendMessage(string message)

`Java.Interop.ExportAttribute` makes heavy usage of unbounded
System.Reflection, System.Reflection.Emit, etc. for it to be able to
work. It brings in an additional assembly `Mono.Android.Export.dll` as
well.

It is inherently trimming unsafe, but how did it get through these
tests?

https://github.com/dotnet/maui/blob/08ff1246383ed4fdaef84a40d5b2ae8e6096bb56/src/TestUtils/src/Microsoft.Maui.IntegrationTests/AndroidTemplateTests.cs#L50-L61

This slipped through, unfortunately, as we had missed solving all the
trimmer warnings in `Mono.Android.Export.dll`! dotnet/android#9300
aims to fix that.

After dotnet/android#9300, new trimming warnings specific to .NET MAUI
will surface such as the one above.

But we can easily replace `[Export]` by:

* Define a Java interface & create a binding for it in C#, we already
  have `maui.aar` setup for this.

* We can simply implement the interface in C# and remove `[Export]`.

Lastly, I fixed some of the defaults in `Metadata.xml`, it didn't look
like we were automatically making Java interfaces `internal`. It looks
like we probably made `ImageLoaderCallback` public by mistake.
The entire `Mono.Android.Export.dll` assembly is not trimming safe,
and never will be: it relies on many dynamic features. But it is
possible to get the warning:

    warning IL2104: Assembly 'Mono.Android.Export' produced trim warnings. For more information see https://aka.ms/dotnet-illink/libraries

First, import `trim-analyzers.props` so we have the right analyzers
set for `Mono.Android.Export.csproj`. We can also remove
`$(EnableSingleFileAnalyzer)` as it is in `trim-analyzers.props`.

This results in ~41 errors, which are mostly resolved by decorating
every type with:

    [RequiresUnreferencedCode (MonoAndroidExport.DynamicFeatures)]

This seems simpler than decorating methods, as there are quite a few
more methods involved than classes.

After this change, we are down to a handful of warnings:

    Mono.Android.Export\CallbackCode.cs(526,19): error IL3050: Using member 'System.Reflection.Emit.AssemblyBuilder.DefineDynamicAssembly(AssemblyName, AssemblyBuilderAccess)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. Defining a dynamic assembly requires dynamic code.
    Mono.Android.Export\CallbackCode.cs(197,32): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
    Mono.Android.Export\Mono.CodeGeneration\CodeCustomAttribute.cs(82,23): error IL3050: Using member 'System.Collections.ArrayList.ToArray(Type)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
    Mono.Android.Export\Mono.CodeGeneration\CodeCustomAttribute.cs(83,20): error IL3050: Using member 'System.Collections.ArrayList.ToArray(Type)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
    Mono.Android.Export\CallbackCode.cs(609,59): error IL3050: Using member 'System.Type.MakeGenericType(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
    Mono.Android.Export\CallbackCode.cs(612,22): error IL3050: Using member 'System.Type.MakeGenericType(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
    Mono.Android.Export\CallbackCode.cs(270,32): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
    Mono.Android.Export\Mono.CodeGeneration\CodeModule.cs(48,35): error IL3050: Using member 'System.Reflection.Emit.AssemblyBuilder.DefineDynamicAssembly(AssemblyName, AssemblyBuilderAccess)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. Defining a dynamic assembly requires dynamic code.
    Mono.Android.Export\CallbackCode.cs(426,32): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
    Mono.Android.Export\CallbackCode.cs(643,13): error IL3050: Using member 'System.Reflection.Emit.DynamicMethod.DynamicMethod(String, MethodAttributes, CallingConventions, Type, Type[], Module, Boolean)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. Creating a DynamicMethod requires dynamic code.
    Mono.Android.Export\CallbackCode.cs(336,32): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
    Mono.Android.Export\CallbackCode.cs(336,154): error IL3050: Using member 'System.Type.MakeArrayType()' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
    Mono.Android.Export\CallbackCode.cs(341,32): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
    Mono.Android.Export\CallbackCode.cs(346,32): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
    Mono.Android.Export\CallbackCode.cs(368,32): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
    Mono.Android.Export\CallbackCode.cs(373,32): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.
    Mono.Android.Export\CallbackCode.cs(671,6): error IL3050: Using member 'System.Reflection.MethodInfo.MakeGenericMethod(params Type[])' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The native code for this instantiation might not be available at runtime.

This is a *different* analyzer for NativeAOT, which is IL3050.

The types involved are a smaller list, I decorated each with:

    [RequiresDynamicCode (MonoAndroidExport.DynamicFeatures)]

With these changes, there are no no warnings present any longer.

Other changes:

* Delete `CodeAdd.cs`, it only contains a comment
    src/Xamarin.Android.NamingCustomAttributes/Java.Interop/ExportAttribute.cs(21,3):
    warning IL2026: Java.Interop.ExportAttribute.ExportAttribute(String): Using member 'Java.Interop.DynamicCallbackCodeGenerator..cctor()' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Mono.Android.Export uses dynamic features.
@jonathanpeppers jonathanpeppers force-pushed the Mono.Android.Export-TrimmerWarnings branch from 645fc66 to 3c6ecd1 Compare September 24, 2024 00:21

static class MonoAndroidExport
{
public const string DynamicFeatures = "Mono.Android.Export uses dynamic features.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I would prefer:

[Java.Interop.ExportAttribute] requires dynamic features.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed in meeting:

  • We realized customers would only get a warning on [ExportAttribute] which says: [RequiresUnreferencedCode ("[ExportAttribute] uses dynamic features.")]
  • The warning here should only be possibly seen by us, so it's probably fine as-is.

@jonpryor jonpryor merged commit ce0913e into dotnet:main Sep 25, 2024
55 of 57 checks passed
@jonathanpeppers jonathanpeppers deleted the Mono.Android.Export-TrimmerWarnings branch September 25, 2024 19:52
@github-actions github-actions bot locked and limited conversation to collaborators Oct 26, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants