From 2163693ef1d57fe55317b80aa86603235bec6e8f Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 13 May 2024 14:59:22 -0500 Subject: [PATCH 1/5] [trimming] preserve custom views and `$(AndroidHttpClientHandlerType)` Fixes: https://github.com/xamarin/xamarin-android/issues/8797 Here are two cases `TrimMode=full` can break applications: * `$(AndroidHttpClientHandlerType)` set to a custom type * Custom views (Android `.xml`) that are not referenced in C# code In the `MarkJavaObjects` trimmer step we can preserve both of these cases by: * Passing in `$(AndroidHttpClientHandlerType)`, preserve the public, parameterless constructor of the type * Pass in `$(_CustomViewMapFile)`, preserve `IJavaObject` types if they are found in the map file --- .../MarkJavaObjects.cs | 51 ++++++++++++++++++ .../Android.Runtime/AndroidEnvironment.cs | 4 +- ...oft.Android.Sdk.AssemblyResolution.targets | 1 + .../Microsoft.Android.Sdk.ILLink.targets | 2 + .../Tasks/LinkerTests.cs | 53 ++++++++++++++++--- .../Utilities/MonoAndroidHelper.Linker.cs | 19 +++++++ .../Utilities/MonoAndroidHelper.cs | 14 +---- .../Android.Widget/CustomWidgetTests.cs | 7 +-- 8 files changed, 123 insertions(+), 28 deletions(-) diff --git a/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs b/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs index feb182dfad6..2cfbc2ad0e0 100644 --- a/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs +++ b/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs @@ -5,6 +5,7 @@ using Mono.Linker; using Mono.Linker.Steps; using Mono.Tuner; +using Java.Interop.Tools.Cecil; using Xamarin.Android.Tasks; namespace MonoDroid.Tuner { @@ -16,9 +17,44 @@ public class MarkJavaObjects : BaseMarkHandler public override void Initialize (LinkContext context, MarkContext markContext) { base.Initialize (context, markContext); + context.TryGetCustomData ("AndroidHttpClientHandlerType", out string androidHttpClientHandlerType); + context.TryGetCustomData ("AndroidCustomViewMapFile", out string androidCustomViewMapFile); + var customViewMap = MonoAndroidHelper.LoadCustomViewMapFile (androidCustomViewMapFile); + + markContext.RegisterMarkAssemblyAction (assembly => ProcessAssembly (assembly, androidHttpClientHandlerType, customViewMap)); markContext.RegisterMarkTypeAction (type => ProcessType (type)); } + bool IsActiveFor (AssemblyDefinition assembly) + { + return assembly.MainModule.HasTypeReference ("System.Net.Http.HttpMessageHandler") || assembly.MainModule.HasTypeReference ("Android.Runtime.IJavaObject"); + } + + public void ProcessAssembly (AssemblyDefinition assembly, string androidHttpClientHandlerType, Dictionary> customViewMap) + { + if (!IsActiveFor (assembly)) + return; + + foreach (var type in assembly.MainModule.Types) { + // Custom HttpMessageHandler + if (!string.IsNullOrEmpty (androidHttpClientHandlerType) && + androidHttpClientHandlerType.StartsWith (type.Name, StringComparison.Ordinal)) { + var assemblyQualifiedName = type.GetPartialAssemblyQualifiedName (Context); + if (assemblyQualifiedName == androidHttpClientHandlerType) { + Annotations.Mark (type); + PreservePublicParameterlessConstructors (type); + continue; + } + } + + // Custom views in Android .xml files + if (customViewMap.ContainsKey (type.FullName)) { + Annotations.Mark (type); + PreserveJavaObjectImplementation (type); + } + } + } + public void ProcessType (TypeDefinition type) { // If this isn't a JLO or IJavaObject implementer, @@ -48,6 +84,21 @@ void PreserveJavaObjectImplementation (TypeDefinition type) PreserveInterfaces (type); } + void PreservePublicParameterlessConstructors (TypeDefinition type) + { + if (!type.HasMethods) + return; + + foreach (var constructor in type.Methods) + { + if (!constructor.IsConstructor || constructor.IsStatic || !constructor.IsPublic || constructor.HasParameters) + continue; + + PreserveMethod (type, constructor); + break; // We can stop when found + } + } + void PreserveAttributeSetConstructor (TypeDefinition type) { if (!type.HasMethods) diff --git a/src/Mono.Android/Android.Runtime/AndroidEnvironment.cs b/src/Mono.Android/Android.Runtime/AndroidEnvironment.cs index 329ee6aa67a..902d7c53eb6 100644 --- a/src/Mono.Android/Android.Runtime/AndroidEnvironment.cs +++ b/src/Mono.Android/Android.Runtime/AndroidEnvironment.cs @@ -336,9 +336,7 @@ static IWebProxy GetDefaultProxy () [DynamicDependency (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor, typeof (Xamarin.Android.Net.AndroidMessageHandler))] static object GetHttpMessageHandler () { - // FIXME: https://github.com/xamarin/xamarin-android/issues/8797 - // Note that this is a problem for custom $(AndroidHttpClientHandlerType) or $XA_HTTP_CLIENT_HANDLER_TYPE - [UnconditionalSuppressMessage ("Trimming", "IL2057", Justification = "DynamicDependency should preserve AndroidMessageHandler.")] + [UnconditionalSuppressMessage ("Trimming", "IL2057", Justification = "Preserved by the MarkJavaObjects trimmer step.")] [return: DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] static Type TypeGetType (string typeName) => Type.GetType (typeName, throwOnError: false); diff --git a/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.AssemblyResolution.targets b/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.AssemblyResolution.targets index 05c6c56639f..0a0cd54970d 100644 --- a/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.AssemblyResolution.targets +++ b/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.AssemblyResolution.targets @@ -96,6 +96,7 @@ _ResolveAssemblies MSBuild target. ;_OuterIntermediateAssembly=@(IntermediateAssembly) ;_OuterOutputPath=$(OutputPath) ;_OuterIntermediateOutputPath=$(IntermediateOutputPath) + ;_OuterCustomViewMapFile=$(_CustomViewMapFile) <_AndroidBuildRuntimeIdentifiersInParallel Condition=" '$(_AndroidBuildRuntimeIdentifiersInParallel)' == '' ">true diff --git a/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.ILLink.targets b/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.ILLink.targets index d9478aed722..e99bfb0e722 100644 --- a/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.ILLink.targets +++ b/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.ILLink.targets @@ -34,6 +34,8 @@ This file contains the .NET 5-specific targets to customize ILLink Used for the value: https://github.com/dotnet/sdk/blob/a5393731b5b7b225692fff121f747fbbc9e8b140/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.ILLink.targets#L147 --> + <_TrimmerCustomData Include="AndroidHttpClientHandlerType" Value="$(AndroidHttpClientHandlerType)" /> + <_TrimmerCustomData Include="AndroidCustomViewMapFile" Value="$(_OuterCustomViewMapFile)" /> <_TrimmerCustomData Include="XATargetFrameworkDirectories" Value="$(_XATargetFrameworkDirectories)" /> <_TrimmerCustomData Condition=" '$(_ProguardProjectConfiguration)' != '' " diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs index 86f2a319cc0..1248eb531c4 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs @@ -156,14 +156,53 @@ static AssemblyDefinition CreateFauxMonoAndroidAssembly () return assm; } - private void PreserveCustomHttpClientHandler (string handlerType, string handlerAssembly, string testProjectName, string assemblyPath) + private void PreserveCustomHttpClientHandler ( + string handlerType, + string handlerAssembly, + string testProjectName, + string assemblyPath, + TrimMode trimMode) { - var proj = new XamarinAndroidApplicationProject () { IsRelease = true }; + testProjectName += trimMode.ToString (); + + var class_library = new XamarinAndroidLibraryProject { + IsRelease = true, + ProjectName = "MyClassLibrary", + Sources = { + new BuildItem.Source ("MyCustomHandler.cs") { + TextContent = () => """ + class MyCustomHandler : System.Net.Http.HttpMessageHandler + { + protected override Task SendAsync (HttpRequestMessage request, CancellationToken cancellationToken) => + throw new NotImplementedException (); + } + """ + }, + new BuildItem.Source ("Bar.cs") { + TextContent = () => "public class Bar { }", + } + } + }; + using (var libBuilder = CreateDllBuilder ($"{testProjectName}/{class_library.ProjectName}")) { + Assert.IsTrue (libBuilder.Build (class_library), $"Build for {class_library.ProjectName} should have succeeded."); + } + + var proj = new XamarinAndroidApplicationProject { + ProjectName = "MyApp", + IsRelease = true, + TrimModeRelease = trimMode, + Sources = { + new BuildItem.Source ("Foo.cs") { + TextContent = () => "public class Foo : Bar { }", + } + } + }; + proj.AddReference (class_library); proj.AddReferences ("System.Net.Http"); string handlerTypeFullName = string.IsNullOrEmpty(handlerAssembly) ? handlerType : handlerType + ", " + handlerAssembly; proj.SetProperty (proj.ActiveConfigurationProperties, "AndroidHttpClientHandlerType", handlerTypeFullName); proj.MainActivity = proj.DefaultMainActivity.Replace ("base.OnCreate (bundle);", "base.OnCreate (bundle);\nvar client = new System.Net.Http.HttpClient ();"); - using (var b = CreateApkBuilder (testProjectName)) { + using (var b = CreateApkBuilder ($"{testProjectName}/{proj.ProjectName}")) { Assert.IsTrue (b.Build (proj), "Build should have succeeded."); using (var assembly = AssemblyDefinition.ReadAssembly (Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath, assemblyPath))) { @@ -173,12 +212,14 @@ private void PreserveCustomHttpClientHandler (string handlerType, string handler } [Test] - public void PreserveCustomHttpClientHandlers () + public void PreserveCustomHttpClientHandlers ([Values (TrimMode.Partial, TrimMode.Full)] TrimMode trimMode) { PreserveCustomHttpClientHandler ("Xamarin.Android.Net.AndroidMessageHandler", "", - "temp/PreserveAndroidMessageHandler", "android-arm64/linked/Mono.Android.dll"); + "temp/PreserveAndroidMessageHandler", "android-arm64/linked/Mono.Android.dll", trimMode); PreserveCustomHttpClientHandler ("System.Net.Http.SocketsHttpHandler", "System.Net.Http", - "temp/PreserveSocketsHttpHandler", "android-arm64/linked/System.Net.Http.dll"); + "temp/PreserveSocketsHttpHandler", "android-arm64/linked/System.Net.Http.dll", trimMode); + PreserveCustomHttpClientHandler ("MyCustomHandler", "MyClassLibrary", + "temp/MyCustomHandler", "android-arm64/linked/MyClassLibrary.dll", trimMode); } [Test] diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.Linker.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.Linker.cs index 480f0d1c09b..3a2ba68f765 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.Linker.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.Linker.cs @@ -60,5 +60,24 @@ public static bool ExistsInFrameworkPath (string assembly) .Select (p => Path.GetDirectoryName (p.TrimEnd (Path.DirectorySeparatorChar))) .Any (p => assembly.StartsWith (p, StringComparison.OrdinalIgnoreCase)); } + + static readonly char [] CustomViewMapSeparator = [';']; + + public static Dictionary> LoadCustomViewMapFile (string mapFile) + { + var map = new Dictionary> (); + if (!File.Exists (mapFile)) + return map; + foreach (var s in File.ReadLines (mapFile)) { + var items = s.Split (CustomViewMapSeparator, count: 2); + var key = items [0]; + var value = items [1]; + HashSet set; + if (!map.TryGetValue (key, out set)) + map.Add (key, set = new HashSet ()); + set.Add (value); + } + return map; + } } } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs index dc768408fe3..bfd3c67eb48 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs @@ -408,19 +408,7 @@ public static Dictionary> LoadCustomViewMapFile (IBuildE var cachedMap = engine?.GetRegisteredTaskObjectAssemblyLocal>> (mapFile, RegisteredTaskObjectLifetime.Build); if (cachedMap != null) return cachedMap; - var map = new Dictionary> (); - if (!File.Exists (mapFile)) - return map; - foreach (var s in File.ReadLines (mapFile)) { - var items = s.Split (new char [] { ';' }, count: 2); - var key = items [0]; - var value = items [1]; - HashSet set; - if (!map.TryGetValue (key, out set)) - map.Add (key, set = new HashSet ()); - set.Add (value); - } - return map; + return LoadCustomViewMapFile (mapFile); } public static bool SaveCustomViewMapFile (IBuildEngine4 engine, string mapFile, Dictionary> map) diff --git a/tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs b/tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs index ec2ab5bdc1d..9ffd36a7328 100644 --- a/tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs +++ b/tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs @@ -1,11 +1,9 @@ -using System.Diagnostics.CodeAnalysis; -using Android.App; +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 { @@ -14,7 +12,6 @@ public class CustomWidgetTests { // https://bugzilla.xamarin.com/show_bug.cgi?id=23880 [Test] - [DynamicDependency (DynamicallyAccessedMemberTypes.All, typeof (CustomTextView))] public void UpperCaseCustomWidget_ShouldNotThrowInflateException () { Assert.DoesNotThrow (() => { @@ -24,7 +21,6 @@ public void UpperCaseCustomWidget_ShouldNotThrowInflateException () } [Test] - [DynamicDependency (DynamicallyAccessedMemberTypes.All, typeof (CustomTextView))] public void LowerCaseCustomWidget_ShouldNotThrowInflateException () { Assert.DoesNotThrow (() => { @@ -34,7 +30,6 @@ public void LowerCaseCustomWidget_ShouldNotThrowInflateException () } [Test] - [DynamicDependency (DynamicallyAccessedMemberTypes.All, typeof (CustomTextView))] public void UpperAndLowerCaseCustomWidget_FromLibrary_ShouldNotThrowInflateException () { Assert.DoesNotThrow (() => { From cebeb8ac751cffa6ee9ccb528fe104be59caa9b8 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 16 May 2024 16:00:59 -0500 Subject: [PATCH 2/5] Fix apk tests --- src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs | 2 +- .../Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs | 6 ++++++ tests/Mono.Android-Tests/Mono.Android-Test.Library/Foo.cs | 4 ++++ 3 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 tests/Mono.Android-Tests/Mono.Android-Test.Library/Foo.cs diff --git a/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs b/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs index 2cfbc2ad0e0..371a6d6f5cb 100644 --- a/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs +++ b/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs @@ -27,7 +27,7 @@ public override void Initialize (LinkContext context, MarkContext markContext) bool IsActiveFor (AssemblyDefinition assembly) { - return assembly.MainModule.HasTypeReference ("System.Net.Http.HttpMessageHandler") || assembly.MainModule.HasTypeReference ("Android.Runtime.IJavaObject"); + return assembly.MainModule.HasTypeReference ("System.Net.Http.HttpMessageHandler") || assembly.MainModule.HasTypeReference ("Android.Util.IAttributeSet"); } public void ProcessAssembly (AssemblyDefinition assembly, string androidHttpClientHandlerType, Dictionary> customViewMap) diff --git a/tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs b/tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs index 9ffd36a7328..f3f1d697103 100644 --- a/tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs +++ b/tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs @@ -10,6 +10,12 @@ namespace Xamarin.Android.RuntimeTests [TestFixture] public class CustomWidgetTests { + public CustomWidgetTests() + { + // NOTE: ensure the C# compiler has a reference to the library + new Mono.Android_Test.Library.Foo (); + } + // https://bugzilla.xamarin.com/show_bug.cgi?id=23880 [Test] public void UpperCaseCustomWidget_ShouldNotThrowInflateException () diff --git a/tests/Mono.Android-Tests/Mono.Android-Test.Library/Foo.cs b/tests/Mono.Android-Tests/Mono.Android-Test.Library/Foo.cs new file mode 100644 index 00000000000..9fbf3891225 --- /dev/null +++ b/tests/Mono.Android-Tests/Mono.Android-Test.Library/Foo.cs @@ -0,0 +1,4 @@ +namespace Mono.Android_Test.Library; + +// NOTE: Type to be used, so there is a reference to this library +public class Foo { } From ef77b4e5915c4026bdf88daf8ae976c0a3fbcfdf Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Fri, 17 May 2024 09:44:59 -0500 Subject: [PATCH 3/5] Skip custom views if not IJavaObject --- src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs b/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs index 371a6d6f5cb..290f45a70e6 100644 --- a/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs +++ b/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs @@ -48,6 +48,8 @@ public void ProcessAssembly (AssemblyDefinition assembly, string androidHttpClie } // Custom views in Android .xml files + if (!type.ImplementsIJavaObject (cache)) + continue; if (customViewMap.ContainsKey (type.FullName)) { Annotations.Mark (type); PreserveJavaObjectImplementation (type); From 7d84df724fda29680fd8ddc23c6e55ea510cee6e Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 6 Jun 2024 14:43:42 -0500 Subject: [PATCH 4/5] https://github.com/xamarin/xamarin-android/issues/9008 --- tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs | 2 +- tests/Mono.Android-Tests/Mono.Android-Test.Library/Foo.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs b/tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs index f3f1d697103..f26fa123671 100644 --- a/tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs +++ b/tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs @@ -12,7 +12,7 @@ public class CustomWidgetTests { public CustomWidgetTests() { - // NOTE: ensure the C# compiler has a reference to the library + // FIXME: https://github.com/xamarin/xamarin-android/issues/9008 new Mono.Android_Test.Library.Foo (); } diff --git a/tests/Mono.Android-Tests/Mono.Android-Test.Library/Foo.cs b/tests/Mono.Android-Tests/Mono.Android-Test.Library/Foo.cs index 9fbf3891225..36d1d0d9374 100644 --- a/tests/Mono.Android-Tests/Mono.Android-Test.Library/Foo.cs +++ b/tests/Mono.Android-Tests/Mono.Android-Test.Library/Foo.cs @@ -1,4 +1,4 @@ namespace Mono.Android_Test.Library; -// NOTE: Type to be used, so there is a reference to this library +// FIXME: https://github.com/xamarin/xamarin-android/issues/9008 public class Foo { } From 02e0514e418f367a29169612ca021618eefd0b4f Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 6 Jun 2024 14:44:30 -0500 Subject: [PATCH 5/5] Put namespace back --- tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs b/tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs index f26fa123671..78d62576a0e 100644 --- a/tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs +++ b/tests/Mono.Android-Tests/Android.Widget/CustomWidgetTests.cs @@ -4,6 +4,7 @@ using Android.Views; using Android.Widget; using NUnit.Framework; +using Mono.Android_Test.Library; namespace Xamarin.Android.RuntimeTests { @@ -13,7 +14,7 @@ public class CustomWidgetTests public CustomWidgetTests() { // FIXME: https://github.com/xamarin/xamarin-android/issues/9008 - new Mono.Android_Test.Library.Foo (); + new Foo (); } // https://bugzilla.xamarin.com/show_bug.cgi?id=23880