From 4352b86aa4117a8fab3ee71e14fde3b8542cf0f0 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 22 Jun 2020 12:08:15 -0500 Subject: [PATCH] [Xamarin.Android.Build.Tasks] fix for Hybrid AOT Fixes: https://github.com/xamarin/xamarin-android/issues/4818 Use of Hybrid AOT currently results in assemblies that have not been CIL-stripped. In 77ab2404, a change was made to fix how `Mono.Android.dll` was treated during AOT. `Mono.Android.dll` used to be the only assembly copied to the `shrunk` directory, and we had two copies of the this assembly passed to the AOT compiler. This change unintentionally made `` strip the wrong set of assemblies... The initial fix would be: + ResolvedAssemblies="@(_ShrunkAssemblies)"> Unfortunately, this causes a crash: 06-11 16:19:08.557 E/AndroidRuntime(18806): java.lang.UnsatisfiedLinkError: No implementation found for void mono.android.TypeManager.n_activate(java.lang.String, java.lang.String, java.lang.Object, java.lang.Object[]) (tried Java_mono_android_TypeManager_n_1activate and Java_mono_android_TypeManager_n_1activate__Ljava_lang_String_2Ljava_lang_String_2Ljava_lang_Object_2_3Ljava_lang_Object_2) 06-11 16:19:08.557 E/AndroidRuntime(18806): at mono.android.TypeManager.n_activate(Native Method) 06-11 16:19:08.557 E/AndroidRuntime(18806): at mono.android.TypeManager.Activate(:7) 06-11 16:19:08.557 E/AndroidRuntime(18806): at crc64446c24ccf511bf5f.SplashScreenActivity.(:25) 06-11 16:19:08.557 E/AndroidRuntime(18806): at java.lang.Class.newInstance(Native Method) 06-11 16:19:08.557 E/AndroidRuntime(18806): at android.app.Instrumentation.newActivity(Instrumentation.java:1086) 06-11 16:19:08.557 E/AndroidRuntime(18806): at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2809) 06-11 16:19:08.557 E/AndroidRuntime(18806): at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2988) 06-11 16:19:08.557 E/AndroidRuntime(18806): at android.app.ActivityThread.-wrap14(ActivityThread.java) 06-11 16:19:08.557 E/AndroidRuntime(18806): at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1631) 06-11 16:19:08.557 E/AndroidRuntime(18806): at android.os.Handler.dispatchMessage(Handler.java:102) 06-11 16:19:08.557 E/AndroidRuntime(18806): at android.os.Looper.loop(Looper.java:154) 06-11 16:19:08.557 E/AndroidRuntime(18806): at android.app.ActivityThread.main(ActivityThread.java:6682) 06-11 16:19:08.557 E/AndroidRuntime(18806): at java.lang.reflect.Method.invoke(Native Method) 06-11 16:19:08.557 E/AndroidRuntime(18806): at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1520) 06-11 16:19:08.557 E/AndroidRuntime(18806): at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1410) Thinking about how things "used to work", we weren't stripping the *correct* `Mono.Android.dll`. So we could run `` on every assembly *besides* `Mono.Android.dll`? I think this could be improved further if we could strip `Mono.Android.dll`, but this at least gets things back to working the way they used to. I updated the `BuildIncrementalAot` test that had a `//TODO` comment, which works properly now. I also added a new test to check that method bodies are stripped. --- Documentation/release-notes/4818.md | 5 +++ .../Xamarin.Android.Build.Tests/BuildTest.cs | 32 +++++++++++++++++++ .../IncrementalBuildTest.cs | 5 --- .../Xamarin.Android.Common.targets | 6 +++- 4 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 Documentation/release-notes/4818.md diff --git a/Documentation/release-notes/4818.md b/Documentation/release-notes/4818.md new file mode 100644 index 00000000000..4d3e27ab651 --- /dev/null +++ b/Documentation/release-notes/4818.md @@ -0,0 +1,5 @@ +#### Application and library build and deployment + + * [GitHub 4818](https://github.com/xamarin/xamarin-android/issues/4818): + Applications using `AndroidAotMode=Hybrid` did not properly strip + away the IL from the resulting .NET assemblies. diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs index 9f1a02967be..fc8ec6b8944 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs @@ -4331,5 +4331,37 @@ public void XA4310 ([Values ("apk", "aab")] string packageFormat) } } + + [Test] + public void HybridAOT () + { + var proj = new XamarinAndroidApplicationProject () { + IsRelease = true, + AotAssemblies = true, + }; + proj.SetProperty ("AndroidAotMode", "Hybrid"); + // So we can use Mono.Cecil to open assemblies directly + proj.SetProperty ("AndroidEnableAssemblyCompression", "False"); + + using (var b = CreateApkBuilder ()) { + b.Build (proj); + + var apk = Path.Combine (Root, b.ProjectDirectory, proj.OutputPath, $"{proj.PackageName}.apk"); + FileAssert.Exists (apk); + using (var zip = ZipHelper.OpenZip (apk)) { + var entry = zip.ReadEntry ($"assemblies/{proj.ProjectName}.dll"); + Assert.IsNotNull (entry, $"{proj.ProjectName}.dll should exist in apk!"); + using (var stream = new MemoryStream ()) { + entry.Extract (stream); + stream.Position = 0; + using (var assembly = AssemblyDefinition.ReadAssembly (stream)) { + var type = assembly.MainModule.GetType ($"{proj.ProjectName}.MainActivity"); + var method = type.Methods.First (m => m.Name == "OnCreate"); + Assert.LessOrEqual (method.Body.Instructions.Count, 1, "OnCreate should have stripped method bodies!"); + } + } + } + } + } } } diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs index 8acaa5bf160..0dcb3ee0731 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs @@ -929,11 +929,6 @@ public void BuildIncrementalAot (string supportedAbis, string androidAotMode, bo Assert.IsFalse (b.Output.IsTargetSkipped (target), $"`{target}` should *not* be skipped on first build!"); } - if (androidAotMode == "Hybrid") { - // FIXME: with Hybrid AOT, modifies assemblies in-place - Assert.Ignore ("Ignoring, Hybrid AOT triggers _BuildApkEmbed."); - } - b.BuildLogFile = "second.log"; b.CleanupAfterSuccessfulBuild = false; b.CleanupOnDispose = false; diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index b074d06f4d4..206d5937f30 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -2151,13 +2151,17 @@ because xbuild doesn't support framework reference assemblies. + + <_CilStripAssemblies Include="@(_ShrunkAssemblies)" Condition=" '%(FileName)' != 'Mono.Android' " /> + + + ResolvedAssemblies="@(_CilStripAssemblies)">