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.

Sign up for GitHub

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

Jump to bottom

Fix known issues with NativeAOT #8724

Open
jonathanpeppers opened this issue Feb 15, 2024 · 2 comments
Open

Fix known issues with NativeAOT #8724

jonathanpeppers opened this issue Feb 15, 2024 · 2 comments
Labels
Area: App+Library Build Issues when building Library projects or Application projects. Area: App Runtime Issues in `libmonodroid.so`. enhancement Proposed change to current functionality.
Milestone
.NET 10

Comments

@jonathanpeppers
Copy link
Member

Android application type

.NET Android (net7.0-android, net8.0-android, etc.)

Affected platform version

.NET 9+

Description

Related: dotnet/java-interop#1192
Related: dotnet/java-interop#1157

When setting $(EnableAotAnalyzer) to true, there are several warnings we will suppress for now. These will likely work under Mono, but will need to be fixed one day in .NET 10 or some future release that supports NativeAOT.

Ignoring AOT warnings like a reasonable place to start, so we can enable the analyzer and not introduce new warnings.

Creating an issue for now, so I can link to this issue from C# code comments.

Steps to Reproduce

Build Mono.Android.csproj with EnableAotAnalyzer=true.

Did you find any workaround?

No response

Relevant log output

No response
@jonathanpeppers jonathanpeppers added Area: App+Library Build Issues when building Library projects or Application projects. Area: App Runtime Issues in `libmonodroid.so`. labels Feb 15, 2024
@jonathanpeppers jonathanpeppers added this to the Under Consideration milestone Feb 15, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issues that need to be assigned. label Feb 15, 2024
@jpobst jpobst removed the needs-triage Issues that need to be assigned. label Feb 15, 2024
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Feb 23, 2024
@jonathanpeppers
[Mono.Android] fix trimming warnings, part 2
2b6a0bf 
This fixes the next set of trimmer warnings found via:

    <IsTrimmable>true</IsTrimmable>
    <EnableAotAnalyzer>true</EnableAotAnalyzer>

~~ JavaObjectExtensions ~~

`JavaCast<T>()` now requires `PublicConstructors` and
`NonPublicConstructors` because of `Activator.CreateInstance<T>()`.
This change bubbles up to various other types that have a
`Find*ById<T>()` method:

* `Activity`
* `Dialog`
* `FragmentManager`
* `View`
* `Window`

`GetInvokerType()` also has suppressions around `Assembly.GetType()`
and `Type.MakeGenericType()`. We track this for the future at:

dotnet#8724

~~ AndroidRuntime ~~

Update `[DynamicallyAccessedMembers]` based on changes to
`RegisterNativeMembers` in:

dotnet/java-interop@b8f6f88

~~ JNINativeWrapper ~~

`$(EnableAotAnalyzer)` found usage of `DynamicMethod`. Suppress for now,
as we track this for the future at:

dotnet#8724

~~ ResourceIdManager ~~

Usage of `Type.GetMethod ("UpdateIdValues")` leads to decoration of
`[ResourceDesignerAttribute]` with:

    [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
    public string FullName { get; set; }

I also had to suppress warnings around `Assembly.GetType()`. This
*should* be OK, as `Resource.designer.cs` is always in the "root
assembly" of Android application projects.

~~ JavaProxyThrowable ~~

Suppress warning around `StackFrame.GetMethod()`, we already handle
null return values and exceptions. The existing code appears to be
"best effort" to provide additional stack trace information.

~~ TypeManager ~~

Suppress warning around a call to `Type.GetType()` with a string
passed in from Java. Not really much we can do yet, except rely on
`MarkJavaObjects` trimmer step.

Likely also a problem for the future:

dotnet#8724
@jonathanpeppers jonathanpeppers mentioned this issue Feb 23, 2024
Merged
@jonathanpeppers
Copy link
Member Author

Update

We'll probably actually suppress the warnings like:

// FIXME: https://github.com/xamarin/xamarin-android/issues/8724
// IL3050 disabled in source: if someone uses NativeAOT, they will get the warning.
#pragma warning disable IL3050
var dynamic = new DynamicMethod (DynamicMethodNameCounter.GetUniqueName (), ret_type, param_types, typeof (DynamicMethodNameCounter), true);
#pragma warning restore IL3050

@jonathanpeppers jonathanpeppers changed the title [NativeAOT] fix places we use [UnconditionalSuppressMessage ("AOT")] Fix known issues with NativeAOT Feb 27, 2024
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Feb 29, 2024
@jonathanpeppers
[Mono.Android] fix trimming warnings, part 2
6a8022a 
This fixes the next set of trimmer warnings found via:

    <IsTrimmable>true</IsTrimmable>
    <EnableAotAnalyzer>true</EnableAotAnalyzer>

~~ JavaObjectExtensions ~~

`JavaCast<T>()` now requires `PublicConstructors` and
`NonPublicConstructors` because of `Activator.CreateInstance<T>()`.
This change bubbles up to various other types that have a
`Find*ById<T>()` method:

* `Activity`
* `Dialog`
* `FragmentManager`
* `View`
* `Window`

`GetInvokerType()` also has suppressions around `Assembly.GetType()`
and `Type.MakeGenericType()`. We track this for the future at:

dotnet#8724

~~ AndroidRuntime ~~

Update `[DynamicallyAccessedMembers]` based on changes to
`RegisterNativeMembers` in:

dotnet/java-interop@b8f6f88

~~ JNINativeWrapper ~~

`$(EnableAotAnalyzer)` found usage of `DynamicMethod`. Suppress for now,
as we track this for the future at:

dotnet#8724

~~ ResourceIdManager ~~

Usage of `Type.GetMethod ("UpdateIdValues")` leads to decoration of
`[ResourceDesignerAttribute]` with:

    [DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
    public string FullName { get; set; }

I also had to suppress warnings around `Assembly.GetType()`. This
*should* be OK, as `Resource.designer.cs` is always in the "root
assembly" of Android application projects.

~~ JavaProxyThrowable ~~

Suppress warning around `StackFrame.GetMethod()`, we already handle
null return values and exceptions. The existing code appears to be
"best effort" to provide additional stack trace information.

~~ TypeManager ~~

Suppress warning around a call to `Type.GetType()` with a string
passed in from Java. Not really much we can do yet, except rely on
`MarkJavaObjects` trimmer step.

Likely also a problem for the future:

dotnet#8724
jonpryor pushed a commit that referenced this issue Mar 1, 2024
@jonathanpeppers
[Mono.Android] fix trimming warnings, part 2 (#8758)
5205a5f 
Context: #5652
Context: #8724
Context: dotnet/java-interop#1165
Context: dotnet/java-interop@b8f6f88
Context: dc3dc3ccf28cdbe9f8c0a705400b83c11a85c81a980ccf2

Fix another set of trimmer warnings found via:

	<IsTrimmable>true</IsTrimmable>
	<EnableAotAnalyzer>true</EnableAotAnalyzer>

~~ JavaObjectExtensions ~~

`Extensions.JavaCast<T>()` now requires `PublicConstructors` and
`NonPublicConstructors` because `TypeManager.CreateProxy()` uses
`ConstructorInfo.Invoke()`.  This change bubbles up to various other
types that have a `Find*ById<T>()` method:

  * `Activity`
  * `Dialog`
  * `FragmentManager`
  * `View`
  * `Window`

`JavaObjectExtensions.GetInvokerType()` also has suppressions around
`Assembly.GetType()` and `Type.MakeGenericType()`.  We track this for
the future at #8724.


~~ AndroidRuntime ~~

Update `[DynamicallyAccessedMembers]` based on changes to
`RegisterNativeMembers` in dotnet/java-interop@b8f6f888.


~~ JNINativeWrapper ~~

`$(EnableAotAnalyzer)` found usage of `DynamicMethod`.  Suppress for
now, as we track this for the future at #8724.


~~ ResourceIdManager ~~

Usage of `Type.GetMethod ("UpdateIdValues")` leads to decoration of
`[ResourceDesignerAttribute]` with:

	[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
	public string FullName { get; set; }

I also had to suppress warnings around `Assembly.GetType()`.
This *should* be OK, as `Resource.designer.cs` is always in the
"root assembly" of Android application projects.

Additionally, this code should no longer be used in .NET 8+ apps;
see dc3ccf2.


~~ JavaProxyThrowable ~~

Suppress warning around `StackFrame.GetMethod()`; we already handle
`null` return values and exceptions.  The existing code appears to be
"best effort" to provide additional stack trace information.


~~ TypeManager ~~

Suppress warning around a call to `Type.GetType()` with a string
passed in from Java.  There is not much we can really do yet, except
rely on the `MarkJavaObjects` trimmer step.

Likely also a problem for the future:

  * dotnet/java-interop#1165
  * #8724


~~ Impact on `.apk` size ~~

`BuildReleaseArm64XFormsDotNet.apkdesc` shows a ~33KB size increase
in the `.apk`.  Much of this is attributable to changes from
dotnet/runtime (`System.Private.CoreLib.dll` is ~20KB larger).

Some of this is due to increases in the size of `classes*.dex`.
These changes are because more managed constructors are now preserved
by the trimmer, which causes more constructors to be emitted into the
Java Callable Wrappers.
jonpryor pushed a commit that referenced this issue Mar 13, 2024
@jonathanpeppers
[Mono.Android] is now "trimming safe" (#8778)
8a5b2a0 
Fixes: #5652

Add `build-tools/trim-analyzers/trim-analyzers.props`, which when
`<Import/>`ed into a project will enable trimmer analyzers and enable
`$(WarningsAsErrors)` for the warnings listed below so that that these
trimmer warnings need to be fixed as part of the build:

  * IL2000 through IL2129
  * IL3050 through IL3056

Update `Mono.Android.csproj` to import `trim-analyzers.props`.

Address the trimmer warnings that are now errors


~~ `$(AndroidHttpClientHandlerType)` and AndroidEnvironment ~~

The default `HttpClientHandler` type used with `new HttpClient()` can
be controlled via the [`$(AndroidHttpClientHandlerType)`][0] MSBuild
property, which sets the `$XA_HTTP_CLIENT_HANDLER_TYPE` environment
variable, which is used with `Type.GetType()`.  (Setting
`$(AndroidHttpClientHandlerType)` [is rare][1], has not been
deprecated, and thus is supported.)

The trimmer only supports `Type.GetType()` with string constants, not
with runtime values such as environment variables.

This results in numerous trimmer warnings:

	src\Mono.Android\Android.Runtime\AndroidEnvironment.cs(373,19): error IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
	src\Mono.Android\Android.Runtime\AndroidEnvironment.cs(379,22): error IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
	src\Mono.Android\Android.Runtime\AndroidEnvironment.cs(342,20): error IL2057: Unrecognized value passed to the parameter 'typeName' of method 'System.Type.GetType(String, Boolean)'. It's not possible to guarantee the availability of the target type.
	src\Mono.Android\Android.Runtime\AndroidEnvironment.cs(352,11): error IL2077: 'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Activator.CreateInstance(Type)'.
	  The field 'Android.Runtime.AndroidEnvironment.httpMessageHandlerType' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

Partially address these issues by:

  * Using string constants instead of default parameter values.

        // This works
        static Type GetFallbackHttpMessageHandlerType() {
            const string typeName = "Xamarin.Android.Net.AndroidMessageHandler, Mono.Android";
            Type.GetType(typeName, …);
        }

        // This does not, as the *caller* gets the value
        static Type GetFallbackHttpMessageHandlerType(string typeName = "Xamarin.Android.Net.AndroidMessageHandler, Mono.Android") {
            Type.GetType(typeName, …);
        }

  * Spread `[DynamicallyAccessedMembers]` around to fix IL2077.

  * Use `[UnconditionalSuppressMessage]` to explicitly suppress IL2057

TODO: we need to *actually preserve* the type referenced by
`$(AndroidHttpClientHandlerType)`.

  * #8797


~~ JavaConvert ~~

`JavaConvert` is used internally to marshal between Java and managed
types, and elicits numerous trimmer warnings:

	src\Mono.Android\Java.Interop\JavaConvert.cs(223,12): error IL2091: 'TResult' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors',
	  'DynamicallyAccessedMemberTypes.NonPublicConstructors' in 'Java.Interop.JavaObjectExtensions._JavaCast<TResult>(IJavaObject)'.
	  The generic parameter 'T' of 'Java.Interop.JavaConvert.FromJavaObject<T>(IJavaObject, out Boolean)' does not have matching annotations.
	  The source value must declare at least the same requirements as those declared on the target location it is assigned to.
	src\Mono.Android\Java.Interop\JavaConvert.cs(254,12): error IL2067: 'resultType' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors',
	  'DynamicallyAccessedMemberTypes.NonPublicConstructors' in call to 'Java.Interop.JavaObjectExtensions.JavaCast(IJavaObject, Type)'.
	  The parameter 'targetType' of method 'Java.Interop.JavaConvert.FromJavaObject(IJavaObject, Type)' does not have matching annotations.
	  The source value must declare at least the same requirements as those declared on the target location it is assigned to.
	src\Mono.Android\Java.Interop\JavaConvert.cs(67,14): 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.
	src\Mono.Android\Java.Interop\JavaConvert.cs(73,14): 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.
	src\Mono.Android\Java.Interop\JavaConvert.cs(79,14): 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.

Address these by adding `[DynamicallyAccessedMembers]` to assist the
trimmer, `#pragma warning disable` to suppress the IL3050 warning,
and `[UnconditionalSuppressMessage]` to ignore the IL2055 warnings.

This trickled over to require more attributes for:

  * `AdapterView`
  * `ArrayAdapter`
  * `AsyncTask`
  * `JavaCollection`
  * `JavaDictionary`
  * `JavaList`
  * `JavaList`
  * `JavaObjectExtensions`
  * `JavaSet`
  * `SparseArray`
  * `System.Linq\Extensions`


~~ JNIEnv ~~

`JNIEnv` is also used internally to marshal between Java and managed
types, and elicits numerous trimmer warnings:

	src\Mono.Android\Android.Runtime\JNIEnv.cs(810,38): 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.
	src\Mono.Android\Android.Runtime\JNIEnv.cs(953,33): 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.
	src\Mono.Android\Android.Runtime\JNIEnv.cs(1078,44): 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.
	src\Mono.Android\Android.Runtime\JNIEnv.cs(1139,15): error IL2091: 'T' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors',
	  'DynamicallyAccessedMemberTypes.NonPublicConstructors' in 'Java.Interop.JavaConvert.FromJavaObject<T>(IJavaObject)'.
	  The generic parameter 'T' of 'Android.Runtime.JNIEnv.GetArray<T>(Object[])' does not have matching annotations.
	  The source value must declare at least the same requirements as those declared on the target location it is assigned to.
	src\Mono.Android\Android.Runtime\JNIEnv.cs(1060,14): error IL3050: Using member 'System.Array.CreateInstance(Type, Int32)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
	src\Mono.Android\Android.Runtime\JNIEnv.cs(1065,14): error IL3050: Using member 'System.Array.CreateInstance(Type, Int32)' which has 'RequiresDynamicCodeAttribute' can break functionality when AOT compiling. The code for an array of the specified type might not be available.
	src\Mono.Android\Android.Runtime\JNIEnv.cs(1257,23): error IL2091: 'T' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors',
	  'DynamicallyAccessedMemberTypes.NonPublicConstructors' in 'Java.Interop.JavaConvert.FromJniHandle<T>(nint, JniHandleOwnership)'.
	  The generic parameter 'T' of 'Android.Runtime.JNIEnv.CopyObjectArray<T>(nint, T[])' does not have matching annotations.
	  The source value must declare at least the same requirements as those declared on the target location it is assigned to.

Adding `[DynamicallyAccessedMembers]` fixes the IL2091 warnings.

Use `#pragma warning disable` to ignore the IL3050 warnings around
`Array.CreateInstance()` and `Type.MakeArrayType()`.

TODO: try to fix these issues instead of suppressing them:

  * #8724

[0]: https://github.com/xamarin/xamarin-android/blob/c20d51fcf8e910b8fb46c5351c26e55ed1fab90c/Documentation/guides/building-apps/build-properties.md#androidhttpclienthandlertype
[1]: https://github.com/search?q=%3CAndroidHttpClientHandlerType%3E+NOT+Xamarin.Android.Net+NOT+System.Net.Http+NOT+Default&type=code
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Mar 14, 2024
@jonathanpeppers
[One .NET] "greenfield" projects opt into trimming
b521911 
As we have solved all trimming warnings in the Android workload, we
can now go "all in" on trimming.

Early in .NET 6 (maybe even 5?) we "hid" many trimming warnings as we
did not yet plan to solve them:

    <SuppressTrimAnalysisWarnings Condition=" '$(SuppressTrimAnalysisWarnings)' == '' ">true</SuppressTrimAnalysisWarnings>

These warnings were not *actionable* at the time for customers, as
many warnings were in `Mono.Android.dll`, `Java.Interop.dll`, etc.

Going forward, let's stop suppressing these warnings if `TrimMode=full`.

We can also enable trimming for new projects:

* `dotnet new android`
* `dotnet new android-wear`

These will have the `TrimMode` property set to `Full` by default:

    <!--
      Enable full trimming in Release mode.
      To learn more, see: https://learn.microsoft.com/dotnet/core/deploying/trimming/trimming-options#trimming-granularity
    -->
    <PropertyGroup Condition="'$(Configuration)' == 'Release'">
      <TrimMode>full</TrimMode>
    </PropertyGroup>

We wouldn't want to do this for existing projects *yet*, as they might
have existing code, NuGet packages, etc. where trimming warnings might
be present.

We can also improve the templates for Android class libraries:

* `dotnet new androidlib`
* `dotnet new android-bindinglib`

    <!--
      Enable trim analyzers for Android class libraries.
      To learn more, see: https://learn.microsoft.com/dotnet/core/deploying/trimming/prepare-libraries-for-trimming
    -->
    <IsTrimmable>true</IsTrimmable>

This way, new class libraries will be "trimmable" by default and be
able to react to trimming warnings.

We can also use `TrimMode=full` in many of our existing tests:

* MSBuild tests that assert 0 warnings can use `TrimMode=full`.

* On-device tests can use `TrimMode=full`.

~~ General trimming warnings ~~

This was discovered through `Mono.Android-NET-Tests.csproj`, but there
were a few trimmer warnings in the "layout bindings" feature:

    dotnet\packs\Microsoft.Android.Sdk.Windows\...\tools\LayoutBinding.cs(79,56): warning IL2091: Xamarin.Android.Design.LayoutBinding.<>c__DisplayClass8_0<T>.<FindFragment>b__0(Activity): 'T' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.NonPublicConstructors' in 'Android.App.FragmentManager.FindFragmentById<T>(Int32)'. The generic parameter 'T' of 'Xamarin.Android.Design.LayoutBinding.<>c__DisplayClass8_0<T>' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
    dotnet\packs\Microsoft.Android.Sdk.Windows\...\tools\LayoutBinding.cs(35,5): warning IL2091: Xamarin.Android.Design.LayoutBinding.FindView<T>(Int32, T&): 'T' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.NonPublicConstructors' in 'Android.App.Activity.FindViewById<T>(Int32)'. The generic parameter 'T' of 'Xamarin.Android.Design.LayoutBinding.FindView<T>(Int32, T&)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
    dotnet\packs\Microsoft.Android.Sdk.Windows\...\tools\LayoutBinding.cs(37,5): warning IL2091: Xamarin.Android.Design.LayoutBinding.FindView<T>(Int32, T&): 'T' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.NonPublicConstructors' in 'Android.Views.View.FindViewById<T>(Int32)'. The generic parameter 'T' of 'Xamarin.Android.Design.LayoutBinding.FindView<T>(Int32, T&)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

We can `[DynamicallyAccessedMembers (Constructors)]` to fix these.

~~ Trimming warnings in tests ~~

Several tests that verify "trimming unsafe features" just specify:

    [RequiresUnreferencedCode ("Tests trimming unsafe features")]

If the test might have an issue under NativeAOT, I used:

    // FIXME: dotnet#8724
    #pragma warning disable IL3050

Places that use `Assembly.GetType()` can use `Type.GetType()` instead:

    --var JavaProxyThrowable_type = typeof (Java.Lang.Object)
    --    .Assembly
    --    .GetType ("Android.Runtime.JavaProxyThrowable");
    ++var JavaProxyThrowable_type = Type.GetType ("Android.Runtime.JavaProxyThrowable, Mono.Android");

`AppDomainTest` was just ignored (and had warnings). Updated to just
verify `PlatformNotSupportedException` is thrown.

~~ Test failures ~~

`JsonSerializerTest` requires a feature flag:

    <JsonSerializerIsReflectionEnabledByDefault>true</JsonSerializerIsReflectionEnabledByDefault>

Otherwise, an exception is thrown:

    System.InvalidOperationException : JsonSerializerIsReflectionDisabled

`Java.Interop-Tests` were initially not loaded at all. With the log
message:

    03-13 16:13:59.940  5262  5344 W NUnit   : Failed to load tests from assembly 'Java.Interop-Tests, Version=1.0.0.0, Culture=neutral, PublicKeyToken=84e04ff9cfb79065

If we make `Java.Interop-Tests.dll` a `@(TrimmerRootAssembly)`:

    <TrimmerRootAssembly Include="Java.Interop-Tests" RootMode="All" />

Then all the tests cases are preserved and can be run, in the same way
the "main app assembly" is preserved.

`NinePatchTests` failed with:

    The drawable created from resource tile should be a NinePatchDrawable.
    Expected: not null
    But was:  null

The only usage of `NinePatchDrawable` was:

    Assert.IsNotNull (d as NinePatchDrawable);

`NinePatchDrawable` likely needs its interfaces and constructors
preserved for this test to pass. I added an attribute for just `All`
for the test to pass:

    [DynamicDependency (DynamicallyAccessedMemberTypes.All, typeof (NinePatchDrawable))]

`CustomWidgetTests` failed with:

    (Binary XML file line #1 in Mono.Android.NET_Tests:layout/uppercase_custom: Binary XML file line #1 in Mono.Android.NET_Tests:layout/uppercase_custom: Error inflating class Mono.Android_Test.Library.CustomTextView)
    at Java.Interop.JniEnvironment.InstanceMethods.CallObjectMethod(JniObjectReference , JniMethodInfo , JniArgumentValue* )
    at Java.Interop.JniPeerMembers.JniInstanceMethods.InvokeVirtualObjectMethod(String , IJavaPeerable , JniArgumentValue* )
    at Android.Views.LayoutInflater.Inflate(Int32 , ViewGroup )
    at Xamarin.Android.RuntimeTests.CustomWidgetTests.<>c.<UpperCaseCustomWidget_ShouldNotThrowInflateException>b__0_0()
    at NUnit.Framework.Constraints.VoidInvocationDescriptor.Invoke()
    at NUnit.Framework.Constraints.ExceptionInterceptor.Intercept(Object )
    --- End of managed Java.Lang.RuntimeException stack trace ---
    android.view.InflateException: Binary XML file line #1 in Mono.Android.NET_Tests:layout/uppercase_custom: Binary XML file line #1 in Mono.Android.NET_Tests:layout/uppercase_custom: Error inflating class Mono.Android_Test.Library.CustomTextView
    Caused by: android.view.InflateException: Binary XML file line #1 in Mono.Android.NET_Tests:layout/uppercase_custom: Error inflating class Mono.Android_Test.Library.CustomTextView
    Caused by: java.lang.ClassNotFoundException: Mono.Android_Test.Library.CustomTextView
    at java.lang.Class.classForName(Native Method)
    at java.lang.Class.forName(Class.java:454)
    at android.view.LayoutInflater.createView(LayoutInflater.java:815)
    at android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:1006)
    at android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:961)
    at android.view.LayoutInflater.rInflate(LayoutInflater.java:1123)
    at android.view.LayoutInflater.rInflateChildren(LayoutInflater.java:1084)
    at android.view.LayoutInflater.inflate(LayoutInflater.java:682)
    at android.view.LayoutInflater.inflate(LayoutInflater.java:534)
    at android.view.LayoutInflater.inflate(LayoutInflater.java:481)
    at crc643df67da7b13bb6b1.TestInstrumentation_1.n_onStart(Native Method)
    at crc643df67da7b13bb6b1.TestInstrumentation_1.onStart(TestInstrumentation_1.java:32)
    at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:2189)
    Caused by: java.lang.ClassNotFoundException: Didn't find class "Mono.Android_Test.Library.CustomTextView" on path

In this case, `Mono.Android_Test.Library.CustomTextView` was used from
an Android layout, but not used anywhere in managed code.

To fix, I added:

    [DynamicDependency (DynamicallyAccessedMemberTypes.All, typeof (Mono.Android_Test.Library.CustomTextView))]

I could have also made `Mono.Android_Test.Library` a `@(TrimmerRootAssembly)`.
jonpryor pushed a commit that referenced this issue Mar 20, 2024
@jonathanpeppers
[One .NET] new "greenfield" projects are trimmed by default (#8805)
9a782d7 
Context: 5205a5f
Context: 8a5b2a0
Context: #8724
Context: #8797

As we have solved all trimming warnings (5205a5f. 8a5b2a0) in the
Android workload, we can now go "all in" on trimming.

Early in .NET 6 (maybe even 5?) we "hid" many trimming warnings as we
did not yet plan to solve them:

	<SuppressTrimAnalysisWarnings Condition=" '$(SuppressTrimAnalysisWarnings)' == '' ">true</SuppressTrimAnalysisWarnings>

These warnings were not *actionable* at the time for customers, as
many warnings were in `Mono.Android.dll`, `Java.Interop.dll`, etc.

Going forward, let's stop suppressing these warnings for
`$(TrimMode)`=full.

We can also enable trimming for new projects:

  * `dotnet new android`
  * `dotnet new android-wear`

New projects will have the [`$(TrimMode)`][0] property set to `Full`
by default:

	<!--
	  Enable full trimming in Release mode.
	  To learn more, see: https://learn.microsoft.com/dotnet/core/deploying/trimming/trimming-options#trimming-granularity
	-->
	<PropertyGroup Condition="'$(Configuration)' == 'Release'">
	  <TrimMode>full</TrimMode>
	</PropertyGroup>

We wouldn't want to do this for existing projects *yet*, as they
might have existing code, NuGet packages, etc. where trimming
warnings might be present.

We can also improve the templates for Android class libraries:

  * `dotnet new androidlib`
  * `dotnet new android-bindinglib`

New class library projects will have the [`$(IsTrimmable)`][1]
property set to `true` by default:

	<!--
	  Enable trim analyzers for Android class libraries.
	  To learn more, see: https://learn.microsoft.com/dotnet/core/deploying/trimming/prepare-libraries-for-trimming
	-->
	<IsTrimmable>true</IsTrimmable>

This way, new class libraries will be "trimmable" by default and be
able to react to trimming warnings.

We can also use `$(TrimMode)=full` in many of our existing tests:

  * MSBuild tests that assert 0 warnings can use `$(TrimMode)=full`.

  * On-device tests can use `$(TrimMode)=full`.


~~ General trimming warnings ~~

This was discovered through `Mono.Android-NET-Tests.csproj`, but
there were a few trimmer warnings in the "layout bindings" feature:

	…\dotnet\packs\Microsoft.Android.Sdk.Windows\…\tools\LayoutBinding.cs(79,56):
	  warning IL2091: Xamarin.Android.Design.LayoutBinding.<>c__DisplayClass8_0<T>.<FindFragment>b__0(Activity):
	  'T' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors',
	    'DynamicallyAccessedMemberTypes.NonPublicConstructors' in 'Android.App.FragmentManager.FindFragmentById<T>(Int32)'.
	  The generic parameter 'T' of 'Xamarin.Android.Design.LayoutBinding.<>c__DisplayClass8_0<T>' does not have matching annotations.
	  The source value must declare at least the same requirements as those declared on the target location it is assigned to.
	…\dotnet\packs\Microsoft.Android.Sdk.Windows\…\tools\LayoutBinding.cs(35,5):
	  warning IL2091: Xamarin.Android.Design.LayoutBinding.FindView<T>(Int32, T&):
	  'T' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors',
	    'DynamicallyAccessedMemberTypes.NonPublicConstructors' in 'Android.App.Activity.FindViewById<T>(Int32)'.
	  The generic parameter 'T' of 'Xamarin.Android.Design.LayoutBinding.FindView<T>(Int32, T&)' does not have matching annotations.
	  The source value must declare at least the same requirements as those declared on the target location it is assigned to.
	…\dotnet\packs\Microsoft.Android.Sdk.Windows\…\tools\LayoutBinding.cs(37,5):
	  warning IL2091: Xamarin.Android.Design.LayoutBinding.FindView<T>(Int32, T&):
	  'T' generic argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors',
	    'DynamicallyAccessedMemberTypes.NonPublicConstructors' in 'Android.Views.View.FindViewById<T>(Int32)'.
	  The generic parameter 'T' of 'Xamarin.Android.Design.LayoutBinding.FindView<T>(Int32, T&)' does not have matching annotations.
	  The source value must declare at least the same requirements as those declared on the target location it is assigned to.

We can `[DynamicallyAccessedMembers(Constructors)]` to fix these.


~~ Trimming warnings in tests ~~

Several tests that verify "trimming unsafe features" just specify:

	[RequiresUnreferencedCode ("Tests trimming unsafe features")]

If the test might have an issue under NativeAOT, I used:

	// FIXME: #8724
	#pragma warning disable IL3050

Places that use `Assembly.GetType()` can use `Type.GetType()` instead:

	-var JavaProxyThrowable_type = typeof (Java.Lang.Object)
	-    .Assembly
	-    .GetType ("Android.Runtime.JavaProxyThrowable");
	+var JavaProxyThrowable_type = Type.GetType ("Android.Runtime.JavaProxyThrowable, Mono.Android");

`SystemTests.AppDomainTest` was just ignored (and had warnings).
Update to just verify `PlatformNotSupportedException` is thrown.


~~ Test failures ~~

`JsonSerializerTest` requires setting
[`$(JsonSerializerIsReflectionEnabledByDefault)`][2]=true:

	<JsonSerializerIsReflectionEnabledByDefault>true</JsonSerializerIsReflectionEnabledByDefault>

Otherwise, an exception is thrown:

	System.InvalidOperationException : JsonSerializerIsReflectionDisabled

`Java.Interop-Tests` were initially not loaded at all, with the log
message:

	W NUnit   : Failed to load tests from assembly 'Java.Interop-Tests, Version=1.0.0.0, Culture=neutral, PublicKeyToken=84e04ff9cfb79065

If we make `Java.Interop-Tests.dll` a `@(TrimmerRootAssembly)`:

	<TrimmerRootAssembly Include="Java.Interop-Tests" RootMode="All" />

Then all the tests cases are preserved and can be run, in the same
way the "main app assembly" is preserved.

`Android.GraphicsTests.NinePatchTests` failed with:

	The drawable created from resource tile should be a NinePatchDrawable.
	Expected: not null
	But was:  null

The only usage of `NinePatchDrawable` was:

	Assert.IsNotNull (d as NinePatchDrawable);

`NinePatchDrawable` likely needs its interfaces and constructors
preserved for this test to pass.  I added an attribute for just `All`
members for the test to pass:

	[DynamicDependency (DynamicallyAccessedMemberTypes.All, typeof (NinePatchDrawable))]

`Xamarin.Android.RuntimeTests.CustomWidgetTests` failed with:

	(Binary XML file line #1 in Mono.Android.NET_Tests:layout/uppercase_custom: Binary XML file line #1 in Mono.Android.NET_Tests:layout/uppercase_custom: Error inflating class Mono.Android_Test.Library.CustomTextView)
	   at Java.Interop.JniEnvironment.InstanceMethods.CallObjectMethod(JniObjectReference , JniMethodInfo , JniArgumentValue* )
	   at Java.Interop.JniPeerMembers.JniInstanceMethods.InvokeVirtualObjectMethod(String , IJavaPeerable , JniArgumentValue* )
	   at Android.Views.LayoutInflater.Inflate(Int32 , ViewGroup )
	   at Xamarin.Android.RuntimeTests.CustomWidgetTests.<>c.<UpperCaseCustomWidget_ShouldNotThrowInflateException>b__0_0()
	   at NUnit.Framework.Constraints.VoidInvocationDescriptor.Invoke()
	   at NUnit.Framework.Constraints.ExceptionInterceptor.Intercept(Object )
	--- End of managed Java.Lang.RuntimeException stack trace ---
	android.view.InflateException: Binary XML file line #1 in Mono.Android.NET_Tests:layout/uppercase_custom: Binary XML file line #1 in Mono.Android.NET_Tests:layout/uppercase_custom: Error inflating class Mono.Android_Test.Library.CustomTextView
	Caused by: android.view.InflateException: Binary XML file line #1 in Mono.Android.NET_Tests:layout/uppercase_custom: Error inflating class Mono.Android_Test.Library.CustomTextView
	Caused by: java.lang.ClassNotFoundException: Mono.Android_Test.Library.CustomTextView
	   at java.lang.Class.classForName(Native Method)
	   at java.lang.Class.forName(Class.java:454)
	   at android.view.LayoutInflater.createView(LayoutInflater.java:815)
	   at android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:1006)
	   at android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:961)
	   at android.view.LayoutInflater.rInflate(LayoutInflater.java:1123)
	   at android.view.LayoutInflater.rInflateChildren(LayoutInflater.java:1084)
	   at android.view.LayoutInflater.inflate(LayoutInflater.java:682)
	   at android.view.LayoutInflater.inflate(LayoutInflater.java:534)
	   at android.view.LayoutInflater.inflate(LayoutInflater.java:481)
	   at crc643df67da7b13bb6b1.TestInstrumentation_1.n_onStart(Native Method)
	   at crc643df67da7b13bb6b1.TestInstrumentation_1.onStart(TestInstrumentation_1.java:32)
	   at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:2189)
	Caused by: java.lang.ClassNotFoundException: Didn't find class "Mono.Android_Test.Library.CustomTextView" on path

In this case, `Mono.Android_Test.Library.CustomTextView` was used
from an Android layout, but not used anywhere in managed code.

To fix, I added:

	[DynamicDependency (DynamicallyAccessedMemberTypes.All, typeof (Mono.Android_Test.Library.CustomTextView))]

I could have also made `Mono.Android_Test.Library` a `@(TrimmerRootAssembly)`.

TODO: `View` subclasses used within Android Layout `.axml` files
should be automatically preserved; see #8797.

[0]: https://learn.microsoft.com/dotnet/core/deploying/trimming/trimming-options?pivots=dotnet-8-0#trimming-granularity
[1]: https://learn.microsoft.com/dotnet/core/deploying/trimming/prepare-libraries-for-trimming?pivots=dotnet-8-0#enable-project-specific-trimming
[2]: https://learn.microsoft.com/dotnet/core/compatibility/serialization/8.0/publishtrimmed
@jonathanpeppers
Copy link
Member Author

We will need to suppress warnings from the trimmer, but rely on warnings coming from the AOT compiler (ILC):

@jonathanpeppers jonathanpeppers mentioned this issue Jul 2, 2024
Merged
@vyacheslav-volkov vyacheslav-volkov mentioned this issue Aug 14, 2024
Open
@vyacheslav-volkov vyacheslav-volkov mentioned this issue Aug 21, 2024
Open
@jpobst jpobst modified the milestones: Under Consideration, .NET 10 Oct 1, 2024
@jonathanpeppers jonathanpeppers mentioned this issue Oct 28, 2024
Closed
@jpobst jpobst added the enhancement Proposed change to current functionality. label Nov 1, 2024
@jonathanpeppers jonathanpeppers mentioned this issue Dec 18, 2024
Closed
@jamescrosswell jamescrosswell mentioned this issue Dec 19, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area: App+Library Build Issues when building Library projects or Application projects. Area: App Runtime Issues in `libmonodroid.so`. enhancement Proposed change to current functionality.
Projects
None yet
Milestone
.NET 10
Development

No branches or pull requests
2 participants
@jpobst @jonathanpeppers