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

[One .NET] "greenfield" projects opt into trimming #8805

Merged
merged 1 commit into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion external/Java.Interop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
<RootNamespace Condition="'$(name)' != '$(name{-VALUE-FORMS-}safe_namespace)'">AndroidBinding1</RootNamespace>
<Nullable>enable</Nullable>
<ImplicitUsings>enable</ImplicitUsings>
<!--
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>
<!--
NOTE: you can simply add .aar or .jar files in this directory to be included in the project.
To learn more, see: https://learn.microsoft.com/dotnet/maui/migration/android-binding-projects
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@
<ApplicationDisplayVersion>1.0</ApplicationDisplayVersion>
<MSBuildWarningsAsMessages>$(MSBuildWarningsAsMessages);XA4218</MSBuildWarningsAsMessages>
</PropertyGroup>
<!--
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>
<ItemGroup>
<PackageReference Include="Xamarin.AndroidX.Wear" Version="1.2.0.5" />
</ItemGroup>
Expand Down
7 changes: 7 additions & 0 deletions src/Microsoft.Android.Templates/android/AndroidApp1.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,11 @@
<ApplicationVersion>1</ApplicationVersion>
<ApplicationDisplayVersion>1.0</ApplicationDisplayVersion>
</PropertyGroup>
<!--
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>
Comment on lines +13 to +19
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the new project template change for apps.

</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,10 @@
<RootNamespace Condition="'$(name)' != '$(name{-VALUE-FORMS-}safe_namespace)'">AndroidLib1</RootNamespace>
<Nullable>enable</Nullable>
<ImplicitUsings>enable</ImplicitUsings>
<!--
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>
Comment on lines +8 to +12
Copy link
Member Author

Choose a reason for hiding this comment

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

And this would be the project template change for Android class libraries, if we think this is a good idea.

</PropertyGroup>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@
<AndroidLinkMode Condition=" '$(AndroidLinkMode)' == '' and '$(PublishTrimmed)' == 'true' ">SdkOnly</AndroidLinkMode>
<AndroidLinkMode Condition=" '$(AndroidLinkMode)' == '' ">None</AndroidLinkMode>
<!-- For compat with user code not marked trimmable, only trim opt-in by default. -->
<TrimMode Condition=" '$(TrimMode)' == '' and '$(AndroidLinkMode)' == 'Full' ">link</TrimMode>
<TrimMode Condition=" '$(TrimMode)' == '' and '$(AndroidLinkMode)' == 'Full' ">full</TrimMode>
<TrimMode Condition="'$(TrimMode)' == ''">partial</TrimMode>
<SuppressTrimAnalysisWarnings Condition=" '$(SuppressTrimAnalysisWarnings)' == '' and '$(TrimMode)' == 'full' ">false</SuppressTrimAnalysisWarnings>
<SuppressTrimAnalysisWarnings Condition=" '$(SuppressTrimAnalysisWarnings)' == '' ">true</SuppressTrimAnalysisWarnings>
Comment on lines +79 to 80
Copy link
Member Author

Choose a reason for hiding this comment

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

/cc @rolfbjarne this is the only defaults I changed. So if TrimMode=full, customers will see all the trimming warnings.

Existing apps, that are TrimMode=partial. I don't think they should appear when upgrading to .NET 9, as they might have 100s of warnings in NuGet packages?

<!-- Prefer $(RuntimeIdentifiers) plural -->
<RuntimeIdentifiers Condition=" '$(RuntimeIdentifier)' == '' And '$(RuntimeIdentifiers)' == '' ">android-arm;android-arm64;android-x86;android-x64</RuntimeIdentifiers>
Expand Down
31 changes: 27 additions & 4 deletions src/Xamarin.Android.Build.Tasks/Resources/LayoutBinding.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using System;

using System.Diagnostics.CodeAnalysis;
using Android.App;
using Android.Views;

Expand All @@ -9,6 +9,8 @@ namespace Xamarin.Android.Design

abstract class LayoutBinding
{
const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors;

Activity boundActivity;
View boundView;
OnLayoutItemNotFoundHandler onLayoutItemNotFound;
Expand All @@ -25,7 +27,13 @@ protected LayoutBinding (View view, OnLayoutItemNotFoundHandler onLayoutItemNotF
this.onLayoutItemNotFound = onLayoutItemNotFound;
}

protected T FindView <T> (int resourceId, ref T cachedField) where T: View
protected T FindView <
[DynamicallyAccessedMembers (Constructors)]
T
> (
int resourceId,
ref T cachedField)
where T: View
{
if (cachedField != null)
return cachedField;
Expand Down Expand Up @@ -58,7 +66,14 @@ Activity EnsureActivity ()
throw new InvalidOperationException ("Finding fragments is supported only for Activity instances");
}

T __FindFragment<T> (int resourceId, Func<Activity, T> finder, ref T cachedField) where T: Java.Lang.Object
T __FindFragment<
[DynamicallyAccessedMembers (Constructors)]
T
> (
int resourceId,
Func<Activity, T> finder,
ref T cachedField)
where T: Java.Lang.Object
{
if (cachedField != null)
return cachedField;
Expand All @@ -74,7 +89,15 @@ T __FindFragment<T> (int resourceId, Func<Activity, T> finder, ref T cachedField
return ret;
}
#if __ANDROID_11__
protected T FindFragment<T> (int resourceId, global::Android.App.Fragment __ignoreMe, ref T cachedField) where T: global::Android.App.Fragment
protected T FindFragment<
[DynamicallyAccessedMembers (Constructors)]
T
> (
int resourceId,
global::Android.App.Fragment __ignoreMe,
ref T cachedField
)
where T: global::Android.App.Fragment
{
return __FindFragment<T> (resourceId, (activity) => activity.FragmentManager.FindFragmentById<T> (resourceId), ref cachedField);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,10 @@ public void BuildHasNoWarnings (bool isRelease, bool xamarinForms, bool multidex
new XamarinFormsAndroidApplicationProject () :
new XamarinAndroidApplicationProject ();
proj.IsRelease = isRelease;
// Enable full trimming
if (!xamarinForms && isRelease) {
proj.TrimModeRelease = TrimMode.Full;
}
if (multidex) {
proj.SetProperty ("AndroidEnableMultiDex", "True");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,10 @@ public enum AndroidLinkMode
SdkOnly,
Full,
}

public enum TrimMode
{
Partial,
Full,
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;

namespace Xamarin.ProjectTools
{
Expand All @@ -18,6 +18,7 @@ public static class KnownProperties
public const string RuntimeIdentifiers = "RuntimeIdentifiers";
public const string RunAOTCompilation = "RunAOTCompilation";
public const string PublishTrimmed = "PublishTrimmed";
public const string TrimMode = "TrimMode";
public const string SupportedOSPlatformVersion = "SupportedOSPlatformVersion";

public const string Deterministic = "Deterministic";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,11 @@ public AndroidLinkMode AndroidLinkModeRelease {
set { SetProperty (ReleaseProperties, KnownProperties.AndroidLinkMode, value.ToString ()); }
}

public TrimMode TrimModeRelease {
get => Enum.TryParse (GetProperty (ReleaseProperties, KnownProperties.TrimMode), out TrimMode trimMode) ? trimMode : TrimMode.Partial;
set => SetProperty (ReleaseProperties, KnownProperties.TrimMode, value.ToString ().ToLowerInvariant ());
}

public bool EnableMarshalMethods {
get { return string.Equals (GetProperty (KnownProperties.AndroidEnableMarshalMethods), "True", StringComparison.OrdinalIgnoreCase); }
set { SetProperty (KnownProperties.AndroidEnableMarshalMethods, value.ToString ()); }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Reflection;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

using NUnit.Framework;

Expand Down Expand Up @@ -180,6 +181,7 @@ public void VirtualMethodBinding ()
}

[Test]
[RequiresUnreferencedCode ("Tests trimming unsafe features")]
public void JavaAbstractMethodTest ()
{
// Library is referencing APIv1, ICursor is from APIv2
Expand All @@ -198,7 +200,7 @@ public void JavaAbstractMethodTest ()
throw e;
}

var mi = ic.GetType ().GetMethod ("global::Test.Bindings.ICursor.MethodWithCursor", BindingFlags.Instance | BindingFlags.NonPublic);
var mi = typeof (Library.MyClrCursor).GetMethod ("global::Test.Bindings.ICursor.MethodWithCursor", BindingFlags.Instance | BindingFlags.NonPublic);
Assert.IsNotNull (mi, "ICursor.MethodWithCursor not found");
if (mi.GetMethodBody ()?.LocalVariables?.Count is not int x || x == 0)
throw new Exception ("FixAbstractMethodStep broken, MethodWithRT added, while it should not be");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
<OutputPath>..\..\..\bin\Test$(Configuration)</OutputPath>
</PropertyGroup>

<PropertyGroup Condition=" '$(Configuration)' == 'Release' ">
<TrimMode Condition=" '$(TrimMode)' == '' ">full</TrimMode>
</PropertyGroup>

<ItemGroup>
<AndroidJavaSource Include="../Xamarin.Android.McwGen-Tests/java/**/*.java" />
<Compile Remove="TimingTests.cs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using System;

using System.Diagnostics.CodeAnalysis;
using Android.App;
using Android.Content.Res;
using Android.Graphics;
Expand All @@ -25,6 +25,7 @@ public class NinePatchTests
};

[Test, TestCaseSource (nameof (NinePatchDrawables))]
[DynamicDependency (DynamicallyAccessedMemberTypes.All, typeof (NinePatchDrawable))]
Copy link
Member Author

Choose a reason for hiding this comment

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

This test was failing, where the only usage of NinePatchDrawable in C# was:

Assert.IsNotNull (d as NinePatchDrawable);

And so the type NinePatchDrawable existed, but with everything trimmed away.

public void DrawableFromRes_ShouldBeTypeNinePatchDrawable (int resId, string name)
{
var d = Application.Context.Resources.GetDrawable (resId);
Expand All @@ -33,6 +34,7 @@ public void DrawableFromRes_ShouldBeTypeNinePatchDrawable (int resId, string nam
}

[Test, TestCaseSource (nameof (NinePatchDrawables))]
[DynamicDependency (DynamicallyAccessedMemberTypes.All, typeof (NinePatchDrawable))]
public void DrawableFromResStream_ShouldBeTypeNinePatchDrawable (int resId, string name)
{
var value = new Android.Util.TypedValue ();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
using Android.App;
using System.Diagnostics.CodeAnalysis;
using Android.App;
using Android.Content;
using Android.Util;
using Android.Views;
using Android.Widget;
using NUnit.Framework;
using Mono.Android_Test.Library;

namespace Xamarin.Android.RuntimeTests
{
Expand All @@ -12,6 +14,7 @@ public class CustomWidgetTests
{
// https://bugzilla.xamarin.com/show_bug.cgi?id=23880
[Test]
[DynamicDependency (DynamicallyAccessedMemberTypes.All, typeof (CustomTextView))]
Copy link
Member Author

Choose a reason for hiding this comment

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

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

(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

public void UpperCaseCustomWidget_ShouldNotThrowInflateException ()
{
Assert.DoesNotThrow (() => {
Expand All @@ -21,6 +24,7 @@ public void UpperCaseCustomWidget_ShouldNotThrowInflateException ()
}

[Test]
[DynamicDependency (DynamicallyAccessedMemberTypes.All, typeof (CustomTextView))]
public void LowerCaseCustomWidget_ShouldNotThrowInflateException ()
{
Assert.DoesNotThrow (() => {
Expand All @@ -30,6 +34,7 @@ public void LowerCaseCustomWidget_ShouldNotThrowInflateException ()
}

[Test]
[DynamicDependency (DynamicallyAccessedMemberTypes.All, typeof (CustomTextView))]
public void UpperAndLowerCaseCustomWidget_FromLibrary_ShouldNotThrowInflateException ()
{
Assert.DoesNotThrow (() => {
Expand Down
12 changes: 10 additions & 2 deletions tests/Mono.Android-Tests/Java.Lang/ObjectTest.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.IO.Compression;
using System.Linq;
Expand Down Expand Up @@ -47,12 +48,19 @@ public void JavaConvert_FromJavaObject_ShouldNotBreakExistingReferences ()

static Func<IJavaObject, int> GetIJavaObjectToInt32 ()
{
var JavaConvert = typeof (Java.Lang.Object).Assembly.GetType ("Java.Interop.JavaConvert");
[UnconditionalSuppressMessage ("Trimming", "IL2060", Justification = "")]
static MethodInfo MakeGenericMethod (MethodInfo method, Type type) =>
// FIXME: https://github.com/xamarin/xamarin-android/issues/8724
#pragma warning disable IL3050
method.MakeGenericMethod (type);
#pragma warning restore IL3050

var JavaConvert = Type.GetType ("Java.Interop.JavaConvert, Mono.Android");
var FromJavaObject_T = JavaConvert.GetMethods (BindingFlags.Public | BindingFlags.Static)
.First (m => m.Name == "FromJavaObject" && m.IsGenericMethod);
return (Func<IJavaObject, int>) Delegate.CreateDelegate (
typeof(Func<IJavaObject, int>),
FromJavaObject_T.MakeGenericMethod (typeof (int)));
MakeGenericMethod (FromJavaObject_T, typeof (int)));
}

[Test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,13 @@

<PropertyGroup Condition=" '$(Configuration)' == 'Release' ">
<AndroidLinkTool Condition=" '$(AndroidLinkTool)' == '' ">r8</AndroidLinkTool>
<TrimMode Condition=" '$(TrimMode)' == '' ">full</TrimMode>
<!-- Trimmer switches required for tests -->
<JsonSerializerIsReflectionEnabledByDefault>true</JsonSerializerIsReflectionEnabledByDefault>
</PropertyGroup>

<ItemGroup>
<TrimmerRootAssembly Include="Java.Interop-Tests" RootMode="All" />
<_AndroidRemapMembers Include="Remaps.xml" />
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using NUnit.Framework;
using System;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;

namespace SystemTests
Expand Down Expand Up @@ -44,7 +45,11 @@ public void GetData (string name, string expected)

[Test]
[TestCaseSource (nameof (TestPrivateSwitchesSource))]
public void TestPrivateSwitches (string className, string propertyName, object expected)
public void TestPrivateSwitches (
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.All)]
string className,
string propertyName,
object expected)
{
var type = Type.GetType (className, throwOnError: true);
var members = type.GetMember (propertyName, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.ComponentModel;
using System.Diagnostics.CodeAnalysis;
using System.Drawing;

using NUnit.Framework;
Expand All @@ -10,6 +11,7 @@ namespace System.Drawing {
public class TypeConverterTest {

[Test]
[RequiresUnreferencedCode ("Tests trimming unsafe features")]
public void ColorConverter ()
{
var typeConverter = TypeDescriptor.GetConverter (typeof (Color));
Expand All @@ -22,6 +24,7 @@ public void ColorConverter ()
}

[Test]
[RequiresUnreferencedCode ("Tests trimming unsafe features")]
public void RectangleConverter ()
{
var typeConverter = TypeDescriptor.GetConverter (typeof (Rectangle));
Expand All @@ -34,6 +37,7 @@ public void RectangleConverter ()
}

[Test]
[RequiresUnreferencedCode ("Tests trimming unsafe features")]
public void PointConverter ()
{
var typeConverter = TypeDescriptor.GetConverter (typeof (Point));
Expand All @@ -44,6 +48,7 @@ public void PointConverter ()
}

[Test]
[RequiresUnreferencedCode ("Tests trimming unsafe features")]
public void SizeConverter ()
{
var typeConverter = TypeDescriptor.GetConverter (typeof (Size));
Expand All @@ -54,6 +59,7 @@ public void SizeConverter ()
}

[Test]
[RequiresUnreferencedCode ("Tests trimming unsafe features")]
public void SizeFConverter ()
{
var typeConverter = TypeDescriptor.GetConverter (typeof (SizeF));
Expand Down
Loading