From 9798fb889ee251b65264b0b4637454a3c68ebe91 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Fri, 26 Jan 2024 14:41:26 -0500 Subject: [PATCH 01/14] Try xamarin/java.interop#1181 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://github.com/xamarin/java.interop/pull/1181 Context: https://github.com/xamarin/xamarin-android/pull/8625 Context: https://github.com/xamarin/java.interop/commit/005c914170a0af9069ff18fd4dd9d45463dd5dc6 Does It Build™? PR #8625 has turned into a cluster, *probably* because of xamarin/java.interop@005c9141, which implicitly requires typemap support for *non*-`Java.Lang.Object` subclasses such as the `CallVirtualFromConstructorDerived` test type. Java.Interop#1181 updates typemap and JCW generation to support `Java.Interop.JavaObject` and `Java.Interop.JavaException` subclasses, which will *hopefully* allow the `CallVirtualFromConstructorDerived`-using tests to work. --- .gitmodules | 2 +- external/Java.Interop | 2 +- src/Xamarin.Android.Build.Tasks/Utilities/XAJavaTypeScanner.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.gitmodules b/.gitmodules index c9883e88b80..7900eb9c650 100644 --- a/.gitmodules +++ b/.gitmodules @@ -9,7 +9,7 @@ [submodule "external/Java.Interop"] path = external/Java.Interop url = https://github.com/xamarin/java.interop.git - branch = main + branch = dev/jonp/ji-typemap-support [submodule "external/lz4"] path = external/lz4 url = https://github.com/xamarin/lz4 diff --git a/external/Java.Interop b/external/Java.Interop index 8b85462e5f3..3e583fea597 160000 --- a/external/Java.Interop +++ b/external/Java.Interop @@ -1 +1 @@ -Subproject commit 8b85462e5f304d9aa2d91c02966d7dc9751516c7 +Subproject commit 3e583fea597ffb169a4df1313edc2259a00ceb22 diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/XAJavaTypeScanner.cs b/src/Xamarin.Android.Build.Tasks/Utilities/XAJavaTypeScanner.cs index 5f965ab46ff..4b82016eaa9 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/XAJavaTypeScanner.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/XAJavaTypeScanner.cs @@ -77,7 +77,7 @@ public List GetJavaTypes (ICollection inputAssemblies, XAAs void AddJavaType (TypeDefinition type, Dictionary types, AndroidTargetArch arch) { - if (type.IsSubclassOf ("Java.Lang.Object", cache) || type.IsSubclassOf ("Java.Lang.Throwable", cache) || (type.IsInterface && type.ImplementsInterface ("Java.Interop.IJavaPeerable", cache))) { + if (type.HasJavaPeer (cache)) { // For subclasses of e.g. Android.App.Activity. string typeName = type.GetPartialAssemblyQualifiedName (cache); if (!types.TryGetValue (typeName, out TypeData typeData)) { From f0305ba69c4a87f65c6f3d98cb05a4fd7fad3ab1 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Fri, 26 Jan 2024 21:35:13 -0500 Subject: [PATCH 02/14] Fix "JNI DETECTED ERROR IN APPLICATION: use of deleted local reference" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://github.com/xamarin/monodroid/commit/e3e4f123d8 Context: https://github.com/xamarin/java.interop/commit/005c9141 Context: https://github.com/xamarin/java.interop/pull/1181 We've been trying to track down a JNI error which occurs when trying to use xamarin/java.interop@005c9141, resembling: I monodroid-lref: +l+ lrefc 1 handle 0x71/L from thread '(null)'(1) D monodroid-gref: at Android.Runtime.AndroidObjectReferenceManager.CreatedLocalReference(JniObjectReference , Int32& ) D monodroid-gref: at Java.Interop.JniRuntime.JniObjectReferenceManager.CreatedLocalReference(JniEnvironmentInfo , JniObjectReference ) D monodroid-gref: at Java.Interop.JniEnvironment.LogCreateLocalRef(JniObjectReference ) D monodroid-gref: at Java.Interop.JniEnvironment.LogCreateLocalRef(IntPtr ) D monodroid-gref: at Java.Interop.JniEnvironment.InstanceMethods.CallObjectMethod(JniObjectReference , JniMethodInfo ) D monodroid-gref: at Android.Runtime.JNIEnv.CallObjectMethod(IntPtr , IntPtr ) D monodroid-gref: at Android.Runtime.JavaSet.Iterator() D monodroid-gref: at Android.Runtime.JavaSet`1[[System.String, System.Private.CoreLib, Version=9.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].GetEnumerator() D monodroid-gref: at Xamarin.Android.UnitTests.TestInstrumentation`1[[Xamarin.Android.UnitTests.NUnit.NUnitTestRunner, TestRunner.NUnit.NET, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].ProcessArguments() D monodroid-gref: at Xamarin.Android.UnitTests.TestInstrumentation`1[[Xamarin.Android.UnitTests.NUnit.NUnitTestRunner, TestRunner.NUnit.NET, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].OnCreate(Bundle arguments) D monodroid-gref: at Android.App.Instrumentation.n_OnCreate_Landroid_os_Bundle_(IntPtr jnienv, IntPtr native__this, IntPtr native_arguments) D monodroid-gref: at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PPL_V(_JniMarshal_PPL_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0) … I monodroid-lref: -l- lrefc 0 handle 0x71/L from thread '(null)'(1) D monodroid-gref: at Android.Runtime.AndroidObjectReferenceManager.DeleteLocalReference(JniObjectReference& , Int32& ) D monodroid-gref: at Java.Interop.JniRuntime.JniObjectReferenceManager.DeleteLocalReference(JniEnvironmentInfo , JniObjectReference& ) D monodroid-gref: at Java.Interop.JniObjectReference.Dispose(JniObjectReference& reference) D monodroid-gref: at Android.Runtime.JNIEnv.DeleteLocalRef(IntPtr ) D monodroid-gref: at Android.Runtime.JNIEnv.DeleteRef(IntPtr , JniHandleOwnership ) D monodroid-gref: at Java.Interop.TypeManager.CreateInstance(IntPtr , JniHandleOwnership , Type ) D monodroid-gref: at Java.Lang.Object.GetObject(IntPtr , JniHandleOwnership , Type ) D monodroid-gref: at Java.Lang.Object._GetObject[IIterator](IntPtr , JniHandleOwnership ) D monodroid-gref: at Java.Lang.Object.GetObject[IIterator](IntPtr handle, JniHandleOwnership transfer) D monodroid-gref: at Android.Runtime.JavaSet.Iterator() D monodroid-gref: at Android.Runtime.JavaSet`1[[System.String, System.Private.CoreLib, Version=9.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].GetEnumerator() D monodroid-gref: at Xamarin.Android.UnitTests.TestInstrumentation`1[[Xamarin.Android.UnitTests.NUnit.NUnitTestRunner, TestRunner.NUnit.NET, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].ProcessArguments() D monodroid-gref: at Xamarin.Android.UnitTests.TestInstrumentation`1[[Xamarin.Android.UnitTests.NUnit.NUnitTestRunner, TestRunner.NUnit.NET, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].OnCreate(Bundle arguments) D monodroid-gref: at Android.App.Instrumentation.n_OnCreate_Landroid_os_Bundle_(IntPtr jnienv, IntPtr native__this, IntPtr native_arguments) D monodroid-gref: at Android.Runtime.JNINativeWrapper.Wrap_JniMarshal_PPL_V(_JniMarshal_PPL_V callback, IntPtr jnienv, IntPtr klazz, IntPtr p0) D monodroid-gref: E droid.NET_Test: JNI ERROR (app bug): accessed stale Local 0x71 (index 7 in a table of size 7) F droid.NET_Test: java_vm_ext.cc:570] JNI DETECTED ERROR IN APPLICATION: use of deleted local reference 0x71 … F droid.NET_Test: runtime.cc:630] native: #13 pc 00000000003ce865 /apex/com.android.runtime/lib64/libart.so (art::(anonymous namespace)::CheckJNI::GetObjectClass(_JNIEnv*, _jobject*)+837) This has been "fun". The problem: 1. xamarin/java.interop@005c9141 relies on/requires additional typemaps in order to "fix" some linker warnings. This felt "fine" at the time. 2. However, the Java.Interop *unit tests* which test (1) involve "hand-written" typemap entries to allow things to work. 3. In .NET Android, those hand-written typemap entries aren't used; instead, the normal .NET Android typemaps are used. 4. .NET Android typemaps did not contain entries for the types introduced in (2), so various tests started failing. 5. xamarin/java.interop#1181 attempts to fix this by extending Java Callable Wrappers and associated typemaps to support `Java.Interop.JavaObject` subclasses, which brings the new types in (2) into the "normal" typemap fold. 6. However, some of those types "alias" `java.lang.Object`, and -- for some "bizarre" random ordering reason -- a type within `Java.Interop-Tests.dll` becomes the preferred `System.Type` to return when looking up `java/lang/Object`. 7. Which would *probably* be okay (if *really* weird), except that `GetJavaToManagedType()` returns null when the binding is within an assembly that hasn't been loaded yet. As this codepath is getting hit during app startup, `Java.Interop-Tests` hasn't been loaded, so `GetJavaToManagedType()` returns null. 8. Which means we're now in the scenario of being unable to find a binding/"wrapper class" for `java/lang/Object`, which we consider to be an error state. Because it's an error state, we dutifully throw. …except we've never actually hit this error state before -- HOW COULD WE?! -- which means we've found a bug in our error handling. Quick, find the problem! JNIEnv.DeleteRef (handle, transfer); throw new NotSupportedException ( FormattableString.Invariant ($"Internal error finding wrapper class for '{JNIEnv.GetClassNameFromInstance (handle)}'. (Where is the Java.Lang.Object wrapper?!)"), CreateJavaLocationException ()); The problem is a "use after free" bug: `JNIEnv.DeleteRef(handle, transfer)` *invalidates `handle`*, and then *immediately* afterward we call `JNIEnv.GetClassNameFromInstance(handle)`, on the now invalid value. BOOM goes the Android runtime. (The `DeleteRef()` call was introduced in xamarin/monodroid@e3e4f123d8, on 2011-Oct-19. Over 12 years to encounter this scenario!) Unfortunately, *just* fixing the "use-after-free" bug is insufficient; if we throw that `NotSupportedException`, things *will* break elsewhere. We'll just have an "elegant unhandled exception" app crash instead of a "THE WORLD IS ENDING" failed assertion crash. We could go with the simple fix for the crash, but this means that in order to integrate xamarin/java.interop@005c9141 & xamarin/java.interop#1181 we'd have to figure out how to *ensure* that `java/lang/Object` is bound as `Java.Lang.Object, Mono.Android`, not `Java.InteropTests.JavaLangRemappingTestObject, Java.Interop-Tests`. There may be a *slightly* more complicated fix which fixes both issues: consider the `-l-` callstack: at Android.Runtime.JNIEnv.DeleteRef(IntPtr , JniHandleOwnership ) at Java.Interop.TypeManager.CreateInstance(IntPtr , JniHandleOwnership , Type ) at Java.Lang.Object.GetObject(IntPtr , JniHandleOwnership , Type ) at Java.Lang.Object._GetObject[IIterator](IntPtr , JniHandleOwnership ) at Java.Lang.Object.GetObject[IIterator](IntPtr handle, JniHandleOwnership transfer) at Android.Runtime.JavaSet.Iterator() This is part of a generic `Object.GetObject()` invocation! Additionally, because `IIterator` is an interface, in *normal* use the `type` variable within `TypeManager.CreateInstance()` would be `Java.Lang.Object, Mono.Android` and then ~immediately "discarded" because `Java.Lang.Object` cannot be assigned to `IIterator`. If we move the type compatibility check to *before* the `type == null` check, we *may* also fix the "`java/lang/Object` is bound as some unloadable type" issue. Let's try that! --- src/Mono.Android/Java.Interop/TypeManager.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Mono.Android/Java.Interop/TypeManager.cs b/src/Mono.Android/Java.Interop/TypeManager.cs index 3b900017db2..9d71981804f 100644 --- a/src/Mono.Android/Java.Interop/TypeManager.cs +++ b/src/Mono.Android/Java.Interop/TypeManager.cs @@ -286,16 +286,20 @@ internal static IJavaPeerable CreateInstance (IntPtr handle, JniHandleOwnership JNIEnv.DeleteLocalRef (class_ptr); + if (targetType != null && + (type == null || + !targetType.IsAssignableFrom (type))) { + type = targetType; + } + if (type == null) { + class_name = JNIEnv.GetClassNameFromInstance (handle); JNIEnv.DeleteRef (handle, transfer); throw new NotSupportedException ( - FormattableString.Invariant ($"Internal error finding wrapper class for '{JNIEnv.GetClassNameFromInstance (handle)}'. (Where is the Java.Lang.Object wrapper?!)"), + FormattableString.Invariant ($"Internal error finding wrapper class for '{class_name}'. (Where is the Java.Lang.Object wrapper?!)"), CreateJavaLocationException ()); } - if (targetType != null && !targetType.IsAssignableFrom (type)) - type = targetType; - if (type.IsInterface || type.IsAbstract) { var invokerType = JavaObjectExtensions.GetInvokerType (type); if (invokerType == null) From 9bd77a5ed2216b50acf3490f3bd915d1d50cf029 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Mon, 29 Jan 2024 15:42:24 -0500 Subject: [PATCH 03/14] Fix "missing" `net/dot/jni/test/tests/GenericHolder`. --- external/Java.Interop | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/Java.Interop b/external/Java.Interop index 3e583fea597..18a7e028ae5 160000 --- a/external/Java.Interop +++ b/external/Java.Interop @@ -1 +1 @@ -Subproject commit 3e583fea597ffb169a4df1313edc2259a00ceb22 +Subproject commit 18a7e028ae508fabff6fbc586dca3efbbabad3f9 From 8dc95b5ce7be9a77b93f174c5b9b77f3a3de98f2 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Mon, 29 Jan 2024 16:13:36 -0500 Subject: [PATCH 04/14] TEMPORARY: disable retries on task failures. We need to get the binlog for the *first* build of `tests/Mono.Android-Tests/Runtime-Microsoft.Android.Sdk/*.csproj`. Right now, it's auto-retrying, so we only capture the binlog for the *second* build, and I need the first binlog to figure out what's going wrong. Temporarily disable retries. --- build-tools/automation/yaml-templates/apk-instrumentation.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-tools/automation/yaml-templates/apk-instrumentation.yaml b/build-tools/automation/yaml-templates/apk-instrumentation.yaml index 7f8b03133bf..a60c98af275 100644 --- a/build-tools/automation/yaml-templates/apk-instrumentation.yaml +++ b/build-tools/automation/yaml-templates/apk-instrumentation.yaml @@ -11,7 +11,7 @@ parameters: artifactFolder: "" useDotNet: true condition: succeeded() - retryCountOnTaskFailure: 1 + retryCountOnTaskFailure: 0 steps: - ${{ if eq(parameters.useDotNet, false) }}: From eacd356cfed39fc8ba227376c86578af5c04fa11 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Mon, 29 Jan 2024 22:00:24 -0500 Subject: [PATCH 05/14] MOAR LOGGING Context: https://github.com/xamarin/xamarin-android/pull/8681#issuecomment-1915504146 For some reason we see `crc641855b07eca6dcc03/GenericHolder_1.java` in the log output, part of `@(_JavaStubFiles)`, but as far as we can tell it *isn't* being compiled. Which makes no sense. My best conjecture is that `@(_JavaBindingSource)` doesn't include everything within `@(_JavaStubFiles)`. Update the `` task to print out the paths of all files that at added to the response file, so that we can see what `javac` is *actually* compiling. --- .../Tasks/JavaCompileToolTask.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/JavaCompileToolTask.cs b/src/Xamarin.Android.Build.Tasks/Tasks/JavaCompileToolTask.cs index a3c1a73f6c3..781fe96d73f 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/JavaCompileToolTask.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/JavaCompileToolTask.cs @@ -63,8 +63,11 @@ private void GenerateResponseFile () WriteOptionsToResponseFile (sw); // Include any user .java files if (JavaSourceFiles != null) - foreach (var file in JavaSourceFiles.Where (p => Path.GetExtension (p.ItemSpec) == ".java")) - sw.WriteLine (string.Format ("\"{0}\"", file.ItemSpec.Replace (@"\", @"\\"))); + foreach (var file in JavaSourceFiles.Where (p => Path.GetExtension (p.ItemSpec) == ".java")) { + var path = file.ItemSpec.Replace (@"\", @"\\"); + sw.WriteLine (string.Format ("\"{0}\"", path)); + Log.LogDebugMessage ($"javac source: {path}"); + } if (string.IsNullOrEmpty (StubSourceDirectory)) return; @@ -86,8 +89,10 @@ private void GenerateResponseFile () // Solution: // Since '\' is an escape character, we need to escape it. // [0] http://download.oracle.com/javase/1.4.2/docs/api/java/io/StreamTokenizer.html#quoteChar(int) + var path = file.Replace (@"\", @"\\").Normalize (NormalizationForm.FormC); sw.WriteLine (string.Format ("\"{0}\"", - file.Replace (@"\", @"\\").Normalize (NormalizationForm.FormC))); + path)); + Log.LogDebugMessage ($"javac source: {path}"); } } } From 94abc6393a141db76a9b00c1da6ced516faae280 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Tue, 30 Jan 2024 09:05:07 -0500 Subject: [PATCH 06/14] Properly parse acw-map.txt `acw-map.txt` contains mappings from .NET types to Java types, and implicitly vice-versa; see (TODO commit). *Normally* it contains three entries: 1. The fully-qualified .NET type name 2. The .NET type name, no assembly 3. (2) with a lowercased namespace name. For example: Mono.Android_Test.Library.CustomTextView, Mono.Android-Test.Library.NET;crc6456ab8145c81c4100.CustomTextView Mono.Android_Test.Library.CustomTextView;crc6456ab8145c81c4100.CustomTextView mono.android_test.library.CustomTextView;crc6456ab8145c81c4100.CustomTextView However, when XA4214 is emitted, there is a "collision" on the .NET side (but *not* the Java side); (2) and (3) are *ambiguous*, so one .NET type is arbitrarily chosen. The first line is still possible, because of assembly qualification. Enter ``Java.InteropTests.GenericHolder`1``: this type is present in *both* `Java.Interop-Tests.dll` *and* `Mono.Android-Tests.dll`. Before xamarin/java.interop#1181, this was "fine" because the `GenericHolder` within `Java.Interop-Tests.dll` did not participate in typemap generation. Now it does, resulting in the XA4214. XA4214 *also* means that instead of three lines, it's *one* line: Java.InteropTests.GenericHolder`1, Mono.Android.NET-Tests;crc641855b07eca6dcc03.GenericHolder_1 Enter ``, which parses `acw-map.txt` to create a `proguard_project_primary.cfg` file. `` did it's *own* parsing of `acw-map.txt`, parsing only *one of every three lines*, on the assumption that *all* entries took three lines. This breaks in the presence of XA4214, because some entries only take one line, not three lines. Update `` to instead use `MonoAndroidHelper.LoadMapFile()`, which reads all lines within `acw-map.txt`. This should result in a `proguard_project_primary.cfg` file which properly contains a `-keep` entry for `crc641855b07eca6dcc03.GenericHolder_1`. --- src/Xamarin.Android.Build.Tasks/Tasks/R8.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/R8.cs b/src/Xamarin.Android.Build.Tasks/Tasks/R8.cs index 81d65a2c218..d9cafeb4e3d 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/R8.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/R8.cs @@ -1,3 +1,4 @@ +using System; using Microsoft.Build.Framework; using Microsoft.Build.Utilities; using System.Collections.Generic; @@ -82,16 +83,15 @@ protected override CommandLineBuilder GetCommandLineBuilder () if (EnableShrinking) { if (!string.IsNullOrEmpty (AcwMapFile)) { - var acwLines = File.ReadAllLines (AcwMapFile); + var acwMap = MonoAndroidHelper.LoadMapFile (BuildEngine4, Path.GetFullPath (AcwMapFile), StringComparer.OrdinalIgnoreCase); + var javaTypes = new List (acwMap.Values.Count); + foreach (var v in acwMap.Values) { + javaTypes.Add (v); + } + javaTypes.Sort (StringComparer.Ordinal); using (var appcfg = File.CreateText (ProguardGeneratedApplicationConfiguration)) { - for (int i = 0; i + 2 < acwLines.Length; i += 3) { - try { - var line = acwLines [i + 2]; - var java = line.Substring (line.IndexOf (';') + 1); - appcfg.WriteLine ("-keep class " + java + " { *; }"); - } catch { - // skip invalid lines - } + foreach (var java in javaTypes) { + appcfg.WriteLine ($"-keep class {java} {{ *; }}"); } } } From 1d80b8c346e71af7e56089bfd8c04c85673f9583 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Tue, 30 Jan 2024 21:03:01 -0500 Subject: [PATCH 07/14] Use latest xamarin/java.interop#1181 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shouldn't change anything on this side… --- external/Java.Interop | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/Java.Interop b/external/Java.Interop index 18a7e028ae5..1af1d9ab3d0 160000 --- a/external/Java.Interop +++ b/external/Java.Interop @@ -1 +1 @@ -Subproject commit 18a7e028ae508fabff6fbc586dca3efbbabad3f9 +Subproject commit 1af1d9ab3d0b3dee1eef5632ad7151aa3d513f3b From e38e3b74815548a2437019459cd7a16ef4baad73 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Tue, 30 Jan 2024 21:03:37 -0500 Subject: [PATCH 08/14] Parse Mono.Android.dll FIRST for typemaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The remaining unit test failures are because `java/lang/Object` is bound as `Java.InteropTests.JavaLangRemappingTestObject, Java.Interop-Tests`, NOT `Java.Lang.Object, Mono.Android`. `Android.RuntimeTests.XmlReaderPullParserTest.ToLocalJniHandle()`: System.NotSupportedException : Unable to activate instance of type Java.InteropTests.JavaLangRemappingTestObject from native handle 0x19 (key_handle 0x2408476). ----> System.MissingMethodException : No constructor found for Java.InteropTests.JavaLangRemappingTestObject::.ctor(System.IntPtr, Android.Runtime.JniHandleOwnership) ----> Java.Interop.JavaLocationException : Exception_WasThrown, Java.Interop.JavaLocationException at Java.Interop.TypeManager.CreateInstance(IntPtr , JniHandleOwnership , Type ) at Java.Interop.TypeManager.CreateInstance(IntPtr , JniHandleOwnership ) at Android.Runtime.XmlResourceParserReader.FromNative(IntPtr , JniHandleOwnership ) at Android.Runtime.XmlResourceParserReader.FromJniHandle(IntPtr handle, JniHandleOwnership transfer) at Android.Content.Res.Resources.GetXml(Int32 ) at Android.RuntimeTests.XmlReaderPullParserTest.ToLocalJniHandle() at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args) at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags ) --MissingMethodException at Java.Interop.TypeManager.CreateProxy(Type , IntPtr , JniHandleOwnership ) at Java.Interop.TypeManager.CreateInstance(IntPtr , JniHandleOwnership , Type ) --JavaLocationException Java.Lang.Error: Exception_WasThrown, Java.Lang.Error --- End of managed Java.Lang.Error stack trace --- java.lang.Error: Java callstack: at crc643df67da7b13bb6b1.TestInstrumentation_1.n_onStart(Native Method) at crc643df67da7b13bb6b1.TestInstrumentation_1.onStart(TestInstrumentation_1.java:35) at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:2189) --- End of managed Java.Lang.Error stack trace --- java.lang.Error: Java callstack: at crc643df67da7b13bb6b1.TestInstrumentation_1.n_onStart(Native Method) at crc643df67da7b13bb6b1.TestInstrumentation_1.onStart(TestInstrumentation_1.java:35) at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:2189) `Java.InteropTests.JniRuntimeJniValueManagerTests.CreateValue()`: System.NotSupportedException : Unable to activate instance of type Java.InteropTests.JavaLangRemappingTestObject from native handle 0x3432 (key_handle 0x9d3f1d1). ----> System.MissingMethodException : No constructor found for Java.InteropTests.JavaLangRemappingTestObject::.ctor(System.IntPtr, Android.Runtime.JniHandleOwnership) ----> Java.Interop.JavaLocationException : Exception_WasThrown, Java.Interop.JavaLocationException at Java.Interop.TypeManager.CreateInstance(IntPtr , JniHandleOwnership , Type ) at Android.Runtime.AndroidValueManager.CreatePeer(JniObjectReference& , JniObjectReferenceOptions , Type ) at Java.Interop.JavaPeerableValueMarshaler.CreateGenericValue(JniObjectReference& , JniObjectReferenceOptions , Type ) at Java.Interop.JniValueMarshaler`1[[Java.Interop.IJavaPeerable, Java.Interop, Version=7.0.0.0, Culture=neutral, PublicKeyToken=84e04ff9cfb79065]].CreateValue(JniObjectReference& , JniObjectReferenceOptions , Type ) at Java.Interop.JniRuntime.JniValueManager.CreateValue(JniObjectReference& , JniObjectReferenceOptions , Type ) at Java.InteropTests.JniRuntimeJniValueManagerTests.CreateValue() at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args) at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object , BindingFlags ) --MissingMethodException at Java.Interop.TypeManager.CreateProxy(Type , IntPtr , JniHandleOwnership ) at Java.Interop.TypeManager.CreateInstance(IntPtr , JniHandleOwnership , Type ) --JavaLocationException Java.Lang.Error: Exception_WasThrown, Java.Lang.Error --- End of managed Java.Lang.Error stack trace --- java.lang.Error: Java callstack: at crc643df67da7b13bb6b1.TestInstrumentation_1.n_onStart(Native Method) at crc643df67da7b13bb6b1.TestInstrumentation_1.onStart(TestInstrumentation_1.java:35) at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:2189) --- End of managed Java.Lang.Error stack trace --- java.lang.Error: Java callstack: at crc643df67da7b13bb6b1.TestInstrumentation_1.n_onStart(Native Method) at crc643df67da7b13bb6b1.TestInstrumentation_1.onStart(TestInstrumentation_1.java:35) at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:2189) etc., etc. (*Actually*, part of the problem is that `AndroidRuntime` doesn't support Java.Interop-style activation constructors *at all*… …and I don't want to deal with that right now, either.) It was borderline bananas to expect anything to work when `java/lang/Object` *wasn't* bound as `Java.Lang.Object, Mono.Android`. I'm frankly suprised there are *only* 23 failures, most of them duplicates of the same 8 failures, two of which are above. Update `` and related typemap code to special-case `Mono.Android.dll` so that it is processed *first*. This *should* ensure that `Java.Lang.Object` is *preferred* over any other possible binding of `java/lang/Object`. --- .../Tasks/GenerateJavaStubs.cs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs index 80f1f25b279..d96807f7d6f 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs @@ -213,7 +213,7 @@ void Run (XAAssemblyResolver res, bool useMarshalMethods) var scanner = new XAJavaTypeScanner (Log, cache) { ErrorOnCustomJavaObject = ErrorOnCustomJavaObject, }; - List allJavaTypes = scanner.GetJavaTypes (allTypemapAssemblies.Values, res); + List allJavaTypes = scanner.GetJavaTypes (GetAllTypemapAssemblies (allTypemapAssemblies.Values), res); var javaTypes = new List (); foreach (JavaType jt in allJavaTypes) { @@ -422,6 +422,19 @@ void MaybeAddAbiSpecifcAssembly (ITaskItem assembly, string fileName) } } + static ICollection GetAllTypemapAssemblies (ICollection assemblies) + { + // We need `Mono.Android.dll` to be *first*, so that in `acw-map.txt` the + // entry for `java/lang/Object` always resolves to `Java.Lang.Object, Mono.Android`. + var c = new List(assemblies); + var e = c.Find (item => string.Compare ("Mono.Android.dll", Path.GetFileName (item.ItemSpec), StringComparison.OrdinalIgnoreCase) == 0); + if (e != null) { + c.Remove (e); + c.Insert (0, e); + } + return c; + } + AssemblyDefinition LoadAssembly (string path, XAAssemblyResolver? resolver = null) { string pdbPath = Path.ChangeExtension (path, ".pdb"); From be2a63b5beb275424626f19a843981e24f7b0a79 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Thu, 1 Feb 2024 17:01:43 -0500 Subject: [PATCH 09/14] Process typemap entries for Mono.Android.dll first Context: https://github.com/xamarin/xamarin-android/blob/1858b55ed7ecced8ffb52a82a7151d639625e240/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs#L472 Context: https://github.com/xamarin/xamarin-android/blob/1858b55ed7ecced8ffb52a82a7151d639625e240/src/Xamarin.Android.Build.Tasks/Utilities/NativeTypeMappingData.cs#L24-L32 e38e3b74 was a valiant effort, but for naught, because the order of `allJavaTypes` within `` does not matter. Nor does the order of `GenerateJavaStubs.ResolvedAssemblies` and `GenerateJavaStubs.ResolvedUserAssemblies` matter. What matters is two things: 1. The `NativeTypeMappingData` constructor creates the typemap entries, checking for (and skipping) duplicate bindings, and it generates typemap entries in *in module order*. 2. Module order is determined by sorting the MVIDs of the assemblies. (Also, it's not *strictly* the MVID; it's the *byte representation* of the MVID, as the MVID is treated as a sequence of bytes.) We now turn to the assemblies in question [^0]: % ikdasm obj/Release/net9.0-android/android-arm64/linked/Mono.Android.dll | grep MVID // MVID: {0FE503F4-2262-4B58-91A7-6F50A0B71838} % ikdasm obj/Release/net9.0-android/android-x86/linked/Java.Interop-Tests.dll | grep MVID // MVID: {50B3412B-DF73-4BBB-9797-1BABC080BF56} Normal GUID output doesn't help; we need bytes. Enter our favorite C# REPL, and: var mono_android_dll = Guid.Parse("0FE503F4-2262-4B58-91A7-6F50A0B71838"); mono_android_dll.ToByteArray(); // == { 244, 3, 229, 15, 98, 34, 88, 75, 145, 167, 111, 80, 160, 183, 24, 56 } var java_interop_tests_dll = Guid.Parse("50B3412B-DF73-4BBB-9797-1BABC080BF56"); java_interop_tests_dll.ToByteArray(); // == { 43, 65, 179, 80, 115, 223, 187, 75, 151, 151, 27, 171, 192, 128, 191, 86 } `Java.Interop-Tests.dll` *always* sorts before `Mono.Android.dll` [^2]. "Revert" e38e3b74, and instead special-case `Mono.Android.dll` within `NativeTypeMappingData`: if `Mono.Android.dll` is a module, process it *first*, so that it has priority for binding `java/lang/Object`. Then process all the other modules. This feels both like a hack, *and* more reliable, so there we go. Additionally, I NEED MY PRINTF DEBUGGING. Plumb `Action logger` through `NativeTypeMappingData` and related types so that `NativeTypeMappingData` can print a message when a typemap "conflict" is encountered: Skipping typemap entry for `Java.InteropTests.JavaLangRemappingTestObject, Java.Interop-Tests`; `java/lang/Object` is already mapped. [^0]: Note that we're using linked output, so the MVIDs *will* vary from build to build, as well as their ordering [^1]. [^1]: Which makes this even more fun: *sometimes* things could work, sometimes not, all at the whim of the random number generator used to create the MVIDs. [^2]: But not quite the whole story, either: in local testing, *yes*, `Java.Interop-Tests.dll` consistently appears in `map_modules` before `Mono.Android.dll`. ***However***, *sometimes* `map_java` would map `java/lang/Object` to `Java.Lang.Object, Mono.Android` for e.g. arm64-v8a, while -- ***for the same build*** -- would map `java/lang/Object` to `Java.InteropTests.JavaLangRemappingTestRuntime, Java.Interop-Tests` for *everything else*: armeabi-v7a, x86, and x86_64. The `map_modules` tables were identical across ABIs. If it were *just* a matter of module sorting, then all ABIs would have the same typemaps, but they don't. I have no explanation for this yet. --- ...teCompressedAssembliesNativeSourceFiles.cs | 2 +- .../Tasks/GenerateJavaStubs.cs | 15 +---------- .../Tasks/GenerateJniRemappingNativeCode.cs | 4 +-- .../Tasks/GeneratePackageManagerJava.cs | 2 +- ...pplicationConfigNativeAssemblyGenerator.cs | 2 ++ ...ressedAssembliesNativeAssemblyGenerator.cs | 3 ++- .../JniRemappingAssemblyGenerator.cs | 6 +++-- .../LlvmIrGenerator/LlvmIrComposer.cs | 7 +++++ .../MarshalMethodsNativeAssemblyGenerator.cs | 4 ++- .../Utilities/NativeTypeMappingData.cs | 26 +++++++++++++++++-- .../Utilities/TypeMapGenerator.cs | 6 ++--- .../Utilities/TypeMappingAssemblyGenerator.cs | 8 +++++- ...TypeMappingDebugNativeAssemblyGenerator.cs | 3 ++- ...peMappingReleaseNativeAssemblyGenerator.cs | 3 ++- 14 files changed, 61 insertions(+), 30 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateCompressedAssembliesNativeSourceFiles.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateCompressedAssembliesNativeSourceFiles.cs index 48f849596e0..b3a7e2d3e4d 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateCompressedAssembliesNativeSourceFiles.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateCompressedAssembliesNativeSourceFiles.cs @@ -74,7 +74,7 @@ void GenerateCompressedAssemblySources () void Generate (IDictionary dict) { - var composer = new CompressedAssembliesNativeAssemblyGenerator (dict); + var composer = new CompressedAssembliesNativeAssemblyGenerator (dict, s => Log.LogDebugMessage (s)); LLVMIR.LlvmIrModule compressedAssemblies = composer.Construct (); foreach (string abi in SupportedAbis) { diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs index d96807f7d6f..80f1f25b279 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs @@ -213,7 +213,7 @@ void Run (XAAssemblyResolver res, bool useMarshalMethods) var scanner = new XAJavaTypeScanner (Log, cache) { ErrorOnCustomJavaObject = ErrorOnCustomJavaObject, }; - List allJavaTypes = scanner.GetJavaTypes (GetAllTypemapAssemblies (allTypemapAssemblies.Values), res); + List allJavaTypes = scanner.GetJavaTypes (allTypemapAssemblies.Values, res); var javaTypes = new List (); foreach (JavaType jt in allJavaTypes) { @@ -422,19 +422,6 @@ void MaybeAddAbiSpecifcAssembly (ITaskItem assembly, string fileName) } } - static ICollection GetAllTypemapAssemblies (ICollection assemblies) - { - // We need `Mono.Android.dll` to be *first*, so that in `acw-map.txt` the - // entry for `java/lang/Object` always resolves to `Java.Lang.Object, Mono.Android`. - var c = new List(assemblies); - var e = c.Find (item => string.Compare ("Mono.Android.dll", Path.GetFileName (item.ItemSpec), StringComparison.OrdinalIgnoreCase) == 0); - if (e != null) { - c.Remove (e); - c.Insert (0, e); - } - return c; - } - AssemblyDefinition LoadAssembly (string path, XAAssemblyResolver? resolver = null) { string pdbPath = Path.ChangeExtension (path, ".pdb"); diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJniRemappingNativeCode.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJniRemappingNativeCode.cs index b89ce87d26d..bd2cf4691b0 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJniRemappingNativeCode.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJniRemappingNativeCode.cs @@ -54,7 +54,7 @@ public override bool RunTask () void GenerateEmpty () { - Generate (new JniRemappingAssemblyGenerator (), typeReplacementsCount: 0); + Generate (new JniRemappingAssemblyGenerator (s => Log.LogDebugMessage (s)), typeReplacementsCount: 0); } void Generate () @@ -74,7 +74,7 @@ void Generate () } } - Generate (new JniRemappingAssemblyGenerator (typeReplacements, methodReplacements), typeReplacements.Count); + Generate (new JniRemappingAssemblyGenerator (typeReplacements, methodReplacements, s => Log.LogDebugMessage (s)), typeReplacements.Count); } void Generate (JniRemappingAssemblyGenerator jniRemappingComposer, int typeReplacementsCount) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs index 7c50f680dfa..42b1e2e5e66 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs @@ -392,7 +392,7 @@ void AddEnvironment () Log ); } else { - marshalMethodsAsmGen = new MarshalMethodsNativeAssemblyGenerator (assemblyCount, uniqueAssemblyNames); + marshalMethodsAsmGen = new MarshalMethodsNativeAssemblyGenerator (assemblyCount, uniqueAssemblyNames, s => Log.LogDebugMessage (s)); } LLVMIR.LlvmIrModule marshalMethodsModule = marshalMethodsAsmGen.Construct (); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs index 2dcbac7b0e4..4d82b9cc822 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs @@ -6,6 +6,7 @@ using Java.Interop.Tools.TypeNameMappings; using Microsoft.Build.Framework; using Microsoft.Build.Utilities; +using Microsoft.Android.Build.Tasks; using Xamarin.Android.Tasks.LLVMIR; namespace Xamarin.Android.Tasks @@ -172,6 +173,7 @@ sealed class XamarinAndroidBundledAssembly public bool MarshalMethodsEnabled { get; set; } public ApplicationConfigNativeAssemblyGenerator (IDictionary environmentVariables, IDictionary systemProperties, TaskLoggingHelper log) + : base (s => log.LogDebugMessage (s)) { if (environmentVariables != null) { this.environmentVariables = new SortedDictionary (environmentVariables, StringComparer.Ordinal); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/CompressedAssembliesNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/CompressedAssembliesNativeAssemblyGenerator.cs index e0b3740a453..efa6b47db2d 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/CompressedAssembliesNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/CompressedAssembliesNativeAssemblyGenerator.cs @@ -66,7 +66,8 @@ sealed class CompressedAssemblies StructureInfo compressedAssemblyDescriptorStructureInfo; StructureInfo compressedAssembliesStructureInfo; - public CompressedAssembliesNativeAssemblyGenerator (IDictionary assemblies) + public CompressedAssembliesNativeAssemblyGenerator (IDictionary assemblies, Action logger) + : base (logger) { this.assemblies = assemblies; } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/JniRemappingAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/JniRemappingAssemblyGenerator.cs index 93ab1cd7b31..25aa591dac9 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/JniRemappingAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/JniRemappingAssemblyGenerator.cs @@ -190,10 +190,12 @@ sealed class JniRemappingTypeReplacementEntry public int ReplacementMethodIndexEntryCount { get; private set; } = 0; - public JniRemappingAssemblyGenerator () + public JniRemappingAssemblyGenerator (Action logger) + : base (logger) {} - public JniRemappingAssemblyGenerator (List typeReplacements, List methodReplacements) + public JniRemappingAssemblyGenerator (List typeReplacements, List methodReplacements, Action logger) + : base (logger) { this.typeReplacementsInput = typeReplacements ?? throw new ArgumentNullException (nameof (typeReplacements)); this.methodReplacementsInput = methodReplacements ?? throw new ArgumentNullException (nameof (methodReplacements)); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrComposer.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrComposer.cs index 8db94269f32..af1f2278ce9 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrComposer.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrComposer.cs @@ -11,6 +11,13 @@ abstract class LlvmIrComposer { bool constructed; + protected readonly Action logger; + + protected LlvmIrComposer (Action logger) + { + this.logger = logger; + } + protected abstract void Construct (LlvmIrModule module); public LlvmIrModule Construct () diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs index f87cf118971..d548781cf35 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs @@ -243,7 +243,8 @@ public MarshalMethodAssemblyIndexValuePlaceholder (MarshalMethodInfo mmi, Assemb /// /// Constructor to be used ONLY when marshal methods are DISABLED /// - public MarshalMethodsNativeAssemblyGenerator (int numberOfAssembliesInApk, ICollection uniqueAssemblyNames) + public MarshalMethodsNativeAssemblyGenerator (int numberOfAssembliesInApk, ICollection uniqueAssemblyNames, Action logger) + : base (logger) { this.numberOfAssembliesInApk = numberOfAssembliesInApk; this.uniqueAssemblyNames = uniqueAssemblyNames ?? throw new ArgumentNullException (nameof (uniqueAssemblyNames)); @@ -255,6 +256,7 @@ public MarshalMethodsNativeAssemblyGenerator (int numberOfAssembliesInApk, IColl /// Constructor to be used ONLY when marshal methods are ENABLED /// public MarshalMethodsNativeAssemblyGenerator (int numberOfAssembliesInApk, ICollection uniqueAssemblyNames, IDictionary> marshalMethods, TaskLoggingHelper logger) + : base (s => logger.LogDebugMessage (s)) { this.numberOfAssembliesInApk = numberOfAssembliesInApk; this.uniqueAssemblyNames = uniqueAssemblyNames ?? throw new ArgumentNullException (nameof (uniqueAssemblyNames)); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/NativeTypeMappingData.cs b/src/Xamarin.Android.Build.Tasks/Utilities/NativeTypeMappingData.cs index be2d4ad71dc..0555dfde9e2 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/NativeTypeMappingData.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/NativeTypeMappingData.cs @@ -12,7 +12,7 @@ class NativeTypeMappingData public uint MapModuleCount { get; } public uint JavaTypeCount { get; } - public NativeTypeMappingData (TypeMapGenerator.ModuleReleaseData[] modules) + public NativeTypeMappingData (TypeMapGenerator.ModuleReleaseData[] modules, Action logger) { Modules = modules ?? throw new ArgumentNullException (nameof (modules)); @@ -21,15 +21,37 @@ public NativeTypeMappingData (TypeMapGenerator.ModuleReleaseData[] modules) var tempJavaTypes = new Dictionary (StringComparer.Ordinal); var moduleComparer = new TypeMapGenerator.ModuleUUIDArrayComparer (); + TypeMapGenerator.ModuleReleaseData? monoAndroid = null; foreach (TypeMapGenerator.ModuleReleaseData data in modules) { + if (data.AssemblyName == "Mono.Android") { + monoAndroid = data; + break; + } + } + + if (monoAndroid != null) { + ProcessModule (monoAndroid); + } + + foreach (TypeMapGenerator.ModuleReleaseData data in modules) { + if (data.AssemblyName == "Mono.Android") { + continue; + } + ProcessModule (data); + }; + + void ProcessModule (TypeMapGenerator.ModuleReleaseData data) + { int moduleIndex = Array.BinarySearch (modules, data, moduleComparer); if (moduleIndex < 0) throw new InvalidOperationException ($"Unable to map module with MVID {data.Mvid} to array index"); foreach (TypeMapGenerator.TypeMapReleaseEntry entry in data.Types) { entry.ModuleIndex = moduleIndex; - if (tempJavaTypes.ContainsKey (entry.JavaName)) + if (tempJavaTypes.ContainsKey (entry.JavaName)) { + logger ($"Skipping typemap entry for `{entry.ManagedTypeName}, {data.AssemblyName}`; `{entry.JavaName}` is already mapped."); continue; + } tempJavaTypes.Add (entry.JavaName, entry); } } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs index 3730176e751..d0fe2073b96 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs @@ -257,7 +257,7 @@ bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, L } GeneratedBinaryTypeMaps.Add (typeMapIndexPath); - var composer = new TypeMappingDebugNativeAssemblyGenerator (new ModuleDebugData ()); + var composer = new TypeMappingDebugNativeAssemblyGenerator (new ModuleDebugData (), logger); GenerateNativeAssembly (composer, composer.Construct (), outputDirectory); return true; @@ -289,7 +289,7 @@ bool GenerateDebugNativeAssembly (bool skipJniAddNativeMethodRegistrationAttribu PrepareDebugMaps (data); - var composer = new TypeMappingDebugNativeAssemblyGenerator (data); + var composer = new TypeMappingDebugNativeAssemblyGenerator (data, logger); GenerateNativeAssembly (composer, composer.Construct (), outputDirectory); return true; @@ -482,7 +482,7 @@ bool GenerateRelease (bool skipJniAddNativeMethodRegistrationAttributeScan, List module.Types = module.TypesScratch.Values.ToArray (); } - var composer = new TypeMappingReleaseNativeAssemblyGenerator (new NativeTypeMappingData (modules)); + var composer = new TypeMappingReleaseNativeAssemblyGenerator (new NativeTypeMappingData (modules, logger), logger); GenerateNativeAssembly (arch, composer, composer.Construct (), outputDirectory); } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingAssemblyGenerator.cs index e7e8d08a6a2..373f7249423 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingAssemblyGenerator.cs @@ -1,7 +1,13 @@ +using System; using Xamarin.Android.Tasks.LLVMIR; namespace Xamarin.Android.Tasks { abstract class TypeMappingAssemblyGenerator : LlvmIrComposer - {} + { + protected TypeMappingAssemblyGenerator (Action logger) + : base (logger) + { + } + } } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs index 2d9e3388475..cc1bc56e028 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs @@ -116,7 +116,8 @@ sealed class TypeMap List> managedToJavaMap; StructureInstance type_map; - public TypeMappingDebugNativeAssemblyGenerator (TypeMapGenerator.ModuleDebugData data) + public TypeMappingDebugNativeAssemblyGenerator (TypeMapGenerator.ModuleDebugData data, Action logger) + : base (logger) { this.data = data; diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs index a6e4fd465df..08e30ce5e84 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs @@ -176,7 +176,8 @@ sealed class ConstructionState ulong moduleCounter = 0; - public TypeMappingReleaseNativeAssemblyGenerator (NativeTypeMappingData mappingData) + public TypeMappingReleaseNativeAssemblyGenerator (NativeTypeMappingData mappingData, Action logger) + : base (logger) { this.mappingData = mappingData ?? throw new ArgumentNullException (nameof (mappingData)); javaNameHash32Comparer = new JavaNameHash32Comparer (); From feae564217211127668af3e82672aa6d744b9daa Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Fri, 2 Feb 2024 07:29:35 -0500 Subject: [PATCH 10/14] Cleanup: TaskLoggingHelper, not Action --- ...erateCompressedAssembliesNativeSourceFiles.cs | 2 +- .../Tasks/GenerateJavaStubs.cs | 2 +- .../Tasks/GenerateJniRemappingNativeCode.cs | 4 ++-- .../Tasks/GeneratePackageManagerJava.cs | 6 +++--- .../ApplicationConfigNativeAssemblyGenerator.cs | 7 ++----- ...ompressedAssembliesNativeAssemblyGenerator.cs | 6 ++++-- .../Utilities/JniRemappingAssemblyGenerator.cs | 10 ++++++---- .../Utilities/LlvmIrGenerator/LlvmIrComposer.cs | 8 +++++--- .../MarshalMethodsNativeAssemblyGenerator.cs | 12 +++++------- .../Utilities/NativeTypeMappingData.cs | 7 +++++-- .../Utilities/TypeMapGenerator.cs | 16 +++++++++------- .../Utilities/TypeMappingAssemblyGenerator.cs | 6 ++++-- .../TypeMappingDebugNativeAssemblyGenerator.cs | 6 ++++-- .../TypeMappingReleaseNativeAssemblyGenerator.cs | 6 ++++-- 14 files changed, 55 insertions(+), 43 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateCompressedAssembliesNativeSourceFiles.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateCompressedAssembliesNativeSourceFiles.cs index b3a7e2d3e4d..441299c527d 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateCompressedAssembliesNativeSourceFiles.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateCompressedAssembliesNativeSourceFiles.cs @@ -74,7 +74,7 @@ void GenerateCompressedAssemblySources () void Generate (IDictionary dict) { - var composer = new CompressedAssembliesNativeAssemblyGenerator (dict, s => Log.LogDebugMessage (s)); + var composer = new CompressedAssembliesNativeAssemblyGenerator (Log, dict); LLVMIR.LlvmIrModule compressedAssemblies = composer.Construct (); foreach (string abi in SupportedAbis) { diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs index 80f1f25b279..6a0444e872b 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs @@ -570,7 +570,7 @@ void SaveResource (string resource, string filename, string destDir, Func types, TypeDefinitionCache cache) { - var tmg = new TypeMapGenerator ((string message) => Log.LogDebugMessage (message), SupportedAbis); + var tmg = new TypeMapGenerator (Log, SupportedAbis); if (!tmg.Generate (Debug, SkipJniAddNativeMethodRegistrationAttributeScan, types, cache, TypemapOutputDirectory, GenerateNativeAssembly, out ApplicationConfigTaskState appConfState)) { throw new XamarinAndroidException (4308, Properties.Resources.XA4308); } diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJniRemappingNativeCode.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJniRemappingNativeCode.cs index bd2cf4691b0..53d73a27826 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJniRemappingNativeCode.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJniRemappingNativeCode.cs @@ -54,7 +54,7 @@ public override bool RunTask () void GenerateEmpty () { - Generate (new JniRemappingAssemblyGenerator (s => Log.LogDebugMessage (s)), typeReplacementsCount: 0); + Generate (new JniRemappingAssemblyGenerator (Log), typeReplacementsCount: 0); } void Generate () @@ -74,7 +74,7 @@ void Generate () } } - Generate (new JniRemappingAssemblyGenerator (typeReplacements, methodReplacements, s => Log.LogDebugMessage (s)), typeReplacements.Count); + Generate (new JniRemappingAssemblyGenerator (Log, typeReplacements, methodReplacements), typeReplacements.Count); } void Generate (JniRemappingAssemblyGenerator jniRemappingComposer, int typeReplacementsCount) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs index 42b1e2e5e66..101f70f84c6 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs @@ -386,13 +386,13 @@ void AddEnvironment () if (enableMarshalMethods) { marshalMethodsAsmGen = new MarshalMethodsNativeAssemblyGenerator ( + Log, assemblyCount, uniqueAssemblyNames, - marshalMethodsState?.MarshalMethods, - Log + marshalMethodsState?.MarshalMethods ); } else { - marshalMethodsAsmGen = new MarshalMethodsNativeAssemblyGenerator (assemblyCount, uniqueAssemblyNames, s => Log.LogDebugMessage (s)); + marshalMethodsAsmGen = new MarshalMethodsNativeAssemblyGenerator (Log, assemblyCount, uniqueAssemblyNames); } LLVMIR.LlvmIrModule marshalMethodsModule = marshalMethodsAsmGen.Construct (); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs index 4d82b9cc822..c4bdfc45eb0 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGenerator.cs @@ -136,7 +136,6 @@ sealed class XamarinAndroidBundledAssembly SortedDictionary ? environmentVariables; SortedDictionary ? systemProperties; - TaskLoggingHelper log; StructureInstance? application_config; List>? dsoCache; List>? xamarinAndroidBundledAssemblies; @@ -173,7 +172,7 @@ sealed class XamarinAndroidBundledAssembly public bool MarshalMethodsEnabled { get; set; } public ApplicationConfigNativeAssemblyGenerator (IDictionary environmentVariables, IDictionary systemProperties, TaskLoggingHelper log) - : base (s => log.LogDebugMessage (s)) + : base (log) { if (environmentVariables != null) { this.environmentVariables = new SortedDictionary (environmentVariables, StringComparer.Ordinal); @@ -182,8 +181,6 @@ public ApplicationConfigNativeAssemblyGenerator (IDictionary env if (systemProperties != null) { this.systemProperties = new SortedDictionary (systemProperties, StringComparer.Ordinal); } - - this.log = log; } protected override void Construct (LlvmIrModule module) @@ -324,7 +321,7 @@ List> InitDSOCache () continue; } - dsos.Add ((name, $"dsoName{dsos.Count.ToString (CultureInfo.InvariantCulture)}", ELFHelper.IsEmptyAOTLibrary (log, item.ItemSpec))); + dsos.Add ((name, $"dsoName{dsos.Count.ToString (CultureInfo.InvariantCulture)}", ELFHelper.IsEmptyAOTLibrary (Log, item.ItemSpec))); } var dsoCache = new List> (); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/CompressedAssembliesNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/CompressedAssembliesNativeAssemblyGenerator.cs index efa6b47db2d..6cb7f251f8a 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/CompressedAssembliesNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/CompressedAssembliesNativeAssemblyGenerator.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; +using Microsoft.Build.Utilities; + using Xamarin.Android.Tasks.LLVMIR; namespace Xamarin.Android.Tasks @@ -66,8 +68,8 @@ sealed class CompressedAssemblies StructureInfo compressedAssemblyDescriptorStructureInfo; StructureInfo compressedAssembliesStructureInfo; - public CompressedAssembliesNativeAssemblyGenerator (IDictionary assemblies, Action logger) - : base (logger) + public CompressedAssembliesNativeAssemblyGenerator (TaskLoggingHelper log, IDictionary assemblies) + : base (log) { this.assemblies = assemblies; } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/JniRemappingAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/JniRemappingAssemblyGenerator.cs index 25aa591dac9..dd6d0924fff 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/JniRemappingAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/JniRemappingAssemblyGenerator.cs @@ -2,6 +2,8 @@ using System.Collections.Generic; using System.Text; +using Microsoft.Build.Utilities; + using Xamarin.Android.Tasks.LLVMIR; namespace Xamarin.Android.Tasks @@ -190,12 +192,12 @@ sealed class JniRemappingTypeReplacementEntry public int ReplacementMethodIndexEntryCount { get; private set; } = 0; - public JniRemappingAssemblyGenerator (Action logger) - : base (logger) + public JniRemappingAssemblyGenerator (TaskLoggingHelper log) + : base (log) {} - public JniRemappingAssemblyGenerator (List typeReplacements, List methodReplacements, Action logger) - : base (logger) + public JniRemappingAssemblyGenerator (TaskLoggingHelper log, List typeReplacements, List methodReplacements) + : base (log) { this.typeReplacementsInput = typeReplacements ?? throw new ArgumentNullException (nameof (typeReplacements)); this.methodReplacementsInput = methodReplacements ?? throw new ArgumentNullException (nameof (methodReplacements)); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrComposer.cs b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrComposer.cs index af1f2278ce9..b250bf24085 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrComposer.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/LlvmIrGenerator/LlvmIrComposer.cs @@ -3,6 +3,8 @@ using System.IO.Hashing; using System.Text; +using Microsoft.Build.Utilities; + using Xamarin.Android.Tools; namespace Xamarin.Android.Tasks.LLVMIR @@ -11,11 +13,11 @@ abstract class LlvmIrComposer { bool constructed; - protected readonly Action logger; + protected readonly TaskLoggingHelper Log; - protected LlvmIrComposer (Action logger) + protected LlvmIrComposer (TaskLoggingHelper log) { - this.logger = logger; + this.Log = log ?? throw new ArgumentNullException (nameof (log)); } protected abstract void Construct (LlvmIrModule module); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs index d548781cf35..c30df45bb8e 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs @@ -228,7 +228,6 @@ public MarshalMethodAssemblyIndexValuePlaceholder (MarshalMethodInfo mmi, Assemb ICollection uniqueAssemblyNames; int numberOfAssembliesInApk; IDictionary> marshalMethods; - TaskLoggingHelper logger; StructureInfo marshalMethodsManagedClassStructureInfo; StructureInfo marshalMethodNameStructureInfo; @@ -243,8 +242,8 @@ public MarshalMethodAssemblyIndexValuePlaceholder (MarshalMethodInfo mmi, Assemb /// /// Constructor to be used ONLY when marshal methods are DISABLED /// - public MarshalMethodsNativeAssemblyGenerator (int numberOfAssembliesInApk, ICollection uniqueAssemblyNames, Action logger) - : base (logger) + public MarshalMethodsNativeAssemblyGenerator (TaskLoggingHelper log, int numberOfAssembliesInApk, ICollection uniqueAssemblyNames) + : base (log) { this.numberOfAssembliesInApk = numberOfAssembliesInApk; this.uniqueAssemblyNames = uniqueAssemblyNames ?? throw new ArgumentNullException (nameof (uniqueAssemblyNames)); @@ -255,13 +254,12 @@ public MarshalMethodsNativeAssemblyGenerator (int numberOfAssembliesInApk, IColl /// /// Constructor to be used ONLY when marshal methods are ENABLED /// - public MarshalMethodsNativeAssemblyGenerator (int numberOfAssembliesInApk, ICollection uniqueAssemblyNames, IDictionary> marshalMethods, TaskLoggingHelper logger) - : base (s => logger.LogDebugMessage (s)) + public MarshalMethodsNativeAssemblyGenerator (TaskLoggingHelper log, int numberOfAssembliesInApk, ICollection uniqueAssemblyNames, IDictionary> marshalMethods) + : base (log) { this.numberOfAssembliesInApk = numberOfAssembliesInApk; this.uniqueAssemblyNames = uniqueAssemblyNames ?? throw new ArgumentNullException (nameof (uniqueAssemblyNames)); this.marshalMethods = marshalMethods; - this.logger = logger ?? throw new ArgumentNullException (nameof (logger)); generateEmptyCode = false; defaultCallMarker = LlvmIrCallMarker.Tail; @@ -336,7 +334,7 @@ void Init () foreach (MarshalMethodInfo method in allMethods) { if (seenNativeSymbols.Contains (method.NativeSymbolName)) { - logger.LogDebugMessage ($"Removed MM duplicate '{method.NativeSymbolName}' (implemented: {method.Method.ImplementedMethod.FullName}; registered: {method.Method.RegisteredMethod.FullName}"); + Log.LogDebugMessage ($"Removed MM duplicate '{method.NativeSymbolName}' (implemented: {method.Method.ImplementedMethod.FullName}; registered: {method.Method.RegisteredMethod.FullName}"); continue; } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/NativeTypeMappingData.cs b/src/Xamarin.Android.Build.Tasks/Utilities/NativeTypeMappingData.cs index 0555dfde9e2..dfc6782c539 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/NativeTypeMappingData.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/NativeTypeMappingData.cs @@ -2,6 +2,9 @@ using System.Collections.Generic; using System.Linq; +using Microsoft.Build.Utilities; +using Microsoft.Android.Build.Tasks; + namespace Xamarin.Android.Tasks { class NativeTypeMappingData @@ -12,7 +15,7 @@ class NativeTypeMappingData public uint MapModuleCount { get; } public uint JavaTypeCount { get; } - public NativeTypeMappingData (TypeMapGenerator.ModuleReleaseData[] modules, Action logger) + public NativeTypeMappingData (TaskLoggingHelper log, TypeMapGenerator.ModuleReleaseData[] modules) { Modules = modules ?? throw new ArgumentNullException (nameof (modules)); @@ -49,7 +52,7 @@ void ProcessModule (TypeMapGenerator.ModuleReleaseData data) foreach (TypeMapGenerator.TypeMapReleaseEntry entry in data.Types) { entry.ModuleIndex = moduleIndex; if (tempJavaTypes.ContainsKey (entry.JavaName)) { - logger ($"Skipping typemap entry for `{entry.ManagedTypeName}, {data.AssemblyName}`; `{entry.JavaName}` is already mapped."); + log.LogDebugMessage ($"Skipping typemap entry for `{entry.ManagedTypeName}, {data.AssemblyName}`; `{entry.JavaName}` is already mapped."); continue; } tempJavaTypes.Add (entry.JavaName, entry); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs index d0fe2073b96..b54208a7d29 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs @@ -5,6 +5,8 @@ using System.Linq; using System.Text; +using Microsoft.Build.Utilities; + using Java.Interop.Tools.Cecil; using Mono.Cecil; using Microsoft.Android.Build.Tasks; @@ -133,7 +135,7 @@ public void AddKnownAssembly (TypeDefinition td) public string GetAssemblyName (TypeDefinition td) => td.Module.Assembly.FullName; } - Action logger; + TaskLoggingHelper log; Encoding outputEncoding; byte[] moduleMagicString; byte[] typemapIndexMagicString; @@ -141,9 +143,9 @@ public void AddKnownAssembly (TypeDefinition td) public IList GeneratedBinaryTypeMaps { get; } = new List (); - public TypeMapGenerator (Action logger, string[] supportedAbis) + public TypeMapGenerator (TaskLoggingHelper log, string[] supportedAbis) { - this.logger = logger ?? throw new ArgumentNullException (nameof (logger)); + this.log = log ?? throw new ArgumentNullException (nameof (log)); if (supportedAbis == null) throw new ArgumentNullException (nameof (supportedAbis)); this.supportedAbis = supportedAbis; @@ -257,7 +259,7 @@ bool GenerateDebugFiles (bool skipJniAddNativeMethodRegistrationAttributeScan, L } GeneratedBinaryTypeMaps.Add (typeMapIndexPath); - var composer = new TypeMappingDebugNativeAssemblyGenerator (new ModuleDebugData (), logger); + var composer = new TypeMappingDebugNativeAssemblyGenerator (log, new ModuleDebugData ()); GenerateNativeAssembly (composer, composer.Construct (), outputDirectory); return true; @@ -289,7 +291,7 @@ bool GenerateDebugNativeAssembly (bool skipJniAddNativeMethodRegistrationAttribu PrepareDebugMaps (data); - var composer = new TypeMappingDebugNativeAssemblyGenerator (data, logger); + var composer = new TypeMappingDebugNativeAssemblyGenerator (log, data); GenerateNativeAssembly (composer, composer.Construct (), outputDirectory); return true; @@ -443,7 +445,7 @@ void ProcessReleaseType (ReleaseGenerationState state, TypeDefinition td, Androi // This is disabled because it costs a lot of time (around 150ms per standard XF Integration app // build) and has no value for the end user. The message is left here because it may be useful to us // in our devloop at some point. - //logger ($"Warning: duplicate Java type name '{entry.JavaName}' in assembly '{moduleData.AssemblyName}' (new token: {entry.Token})."); + //log.LogDebugMessage ($"Warning: duplicate Java type name '{entry.JavaName}' in assembly '{moduleData.AssemblyName}' (new token: {entry.Token})."); moduleData.DuplicateTypes.Add (entry); } else { moduleData.TypesScratch.Add (entry.JavaName, entry); @@ -482,7 +484,7 @@ bool GenerateRelease (bool skipJniAddNativeMethodRegistrationAttributeScan, List module.Types = module.TypesScratch.Values.ToArray (); } - var composer = new TypeMappingReleaseNativeAssemblyGenerator (new NativeTypeMappingData (modules, logger), logger); + var composer = new TypeMappingReleaseNativeAssemblyGenerator (log, new NativeTypeMappingData (log, modules)); GenerateNativeAssembly (arch, composer, composer.Construct (), outputDirectory); } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingAssemblyGenerator.cs index 373f7249423..3427e49775e 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingAssemblyGenerator.cs @@ -1,12 +1,14 @@ using System; using Xamarin.Android.Tasks.LLVMIR; +using Microsoft.Build.Utilities; + namespace Xamarin.Android.Tasks { abstract class TypeMappingAssemblyGenerator : LlvmIrComposer { - protected TypeMappingAssemblyGenerator (Action logger) - : base (logger) + protected TypeMappingAssemblyGenerator (TaskLoggingHelper log) + : base (log) { } } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs index cc1bc56e028..3d047b206dc 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; +using Microsoft.Build.Utilities; + using Xamarin.Android.Tasks.LLVMIR; namespace Xamarin.Android.Tasks @@ -116,8 +118,8 @@ sealed class TypeMap List> managedToJavaMap; StructureInstance type_map; - public TypeMappingDebugNativeAssemblyGenerator (TypeMapGenerator.ModuleDebugData data, Action logger) - : base (logger) + public TypeMappingDebugNativeAssemblyGenerator (TaskLoggingHelper log, TypeMapGenerator.ModuleDebugData data) + : base (log) { this.data = data; diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs index 08e30ce5e84..ad1fe2dbf4b 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs @@ -4,6 +4,8 @@ using System.IO.Hashing; using System.Text; +using Microsoft.Build.Utilities; + using Xamarin.Android.Tasks.LLVMIR; namespace Xamarin.Android.Tasks @@ -176,8 +178,8 @@ sealed class ConstructionState ulong moduleCounter = 0; - public TypeMappingReleaseNativeAssemblyGenerator (NativeTypeMappingData mappingData, Action logger) - : base (logger) + public TypeMappingReleaseNativeAssemblyGenerator (TaskLoggingHelper log, NativeTypeMappingData mappingData) + : base (log) { this.mappingData = mappingData ?? throw new ArgumentNullException (nameof (mappingData)); javaNameHash32Comparer = new JavaNameHash32Comparer (); From 895682f2aa764532972db44c11cfb289a2a94c04 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Fri, 2 Feb 2024 07:45:04 -0500 Subject: [PATCH 11/14] Dump entire javac response file, not just source files. --- .../Tasks/JavaCompileToolTask.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/JavaCompileToolTask.cs b/src/Xamarin.Android.Build.Tasks/Tasks/JavaCompileToolTask.cs index 781fe96d73f..834659c991c 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/JavaCompileToolTask.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/JavaCompileToolTask.cs @@ -66,7 +66,6 @@ private void GenerateResponseFile () foreach (var file in JavaSourceFiles.Where (p => Path.GetExtension (p.ItemSpec) == ".java")) { var path = file.ItemSpec.Replace (@"\", @"\\"); sw.WriteLine (string.Format ("\"{0}\"", path)); - Log.LogDebugMessage ($"javac source: {path}"); } if (string.IsNullOrEmpty (StubSourceDirectory)) @@ -92,9 +91,12 @@ private void GenerateResponseFile () var path = file.Replace (@"\", @"\\").Normalize (NormalizationForm.FormC); sw.WriteLine (string.Format ("\"{0}\"", path)); - Log.LogDebugMessage ($"javac source: {path}"); } } + Log.LogDebugMessage ($"javac response file contents: {TemporarySourceListFile}"); + foreach (var line in File.ReadLines (TemporarySourceListFile)) { + Log.LogDebugMessage ($" {line}"); + } } } } From 6b1b6d31f99b9b64e694f5f76aba4409358433b4 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Fri, 2 Feb 2024 08:05:00 -0500 Subject: [PATCH 12/14] Re-enable retries. Reverts 8dc95b5ce7be9a77b93f174c5b9b77f3a3de98f2. --- build-tools/automation/yaml-templates/apk-instrumentation.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-tools/automation/yaml-templates/apk-instrumentation.yaml b/build-tools/automation/yaml-templates/apk-instrumentation.yaml index a60c98af275..7f8b03133bf 100644 --- a/build-tools/automation/yaml-templates/apk-instrumentation.yaml +++ b/build-tools/automation/yaml-templates/apk-instrumentation.yaml @@ -11,7 +11,7 @@ parameters: artifactFolder: "" useDotNet: true condition: succeeded() - retryCountOnTaskFailure: 0 + retryCountOnTaskFailure: 1 steps: - ${{ if eq(parameters.useDotNet, false) }}: From cb121ae6ac8eda6364cce9ae8baaca78d572efed Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Fri, 2 Feb 2024 08:32:16 -0500 Subject: [PATCH 13/14] Minimize patch diffs. --- .../Tasks/JavaCompileToolTask.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/JavaCompileToolTask.cs b/src/Xamarin.Android.Build.Tasks/Tasks/JavaCompileToolTask.cs index 834659c991c..1b61dd1397f 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/JavaCompileToolTask.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/JavaCompileToolTask.cs @@ -63,10 +63,8 @@ private void GenerateResponseFile () WriteOptionsToResponseFile (sw); // Include any user .java files if (JavaSourceFiles != null) - foreach (var file in JavaSourceFiles.Where (p => Path.GetExtension (p.ItemSpec) == ".java")) { - var path = file.ItemSpec.Replace (@"\", @"\\"); - sw.WriteLine (string.Format ("\"{0}\"", path)); - } + foreach (var file in JavaSourceFiles.Where (p => Path.GetExtension (p.ItemSpec) == ".java")) + sw.WriteLine (string.Format ("\"{0}\"", file.ItemSpec.Replace (@"\", @"\\"))); if (string.IsNullOrEmpty (StubSourceDirectory)) return; @@ -88,9 +86,8 @@ private void GenerateResponseFile () // Solution: // Since '\' is an escape character, we need to escape it. // [0] http://download.oracle.com/javase/1.4.2/docs/api/java/io/StreamTokenizer.html#quoteChar(int) - var path = file.Replace (@"\", @"\\").Normalize (NormalizationForm.FormC); sw.WriteLine (string.Format ("\"{0}\"", - path)); + file.Replace (@"\", @"\\").Normalize (NormalizationForm.FormC))); } } Log.LogDebugMessage ($"javac response file contents: {TemporarySourceListFile}"); From 92fc9e40989b4f179dc01a7cb025dfe8c17eba5a Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Fri, 2 Feb 2024 08:32:52 -0500 Subject: [PATCH 14/14] Back to java.interop/main! --- .gitmodules | 2 +- external/Java.Interop | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitmodules b/.gitmodules index 7900eb9c650..c9883e88b80 100644 --- a/.gitmodules +++ b/.gitmodules @@ -9,7 +9,7 @@ [submodule "external/Java.Interop"] path = external/Java.Interop url = https://github.com/xamarin/java.interop.git - branch = dev/jonp/ji-typemap-support + branch = main [submodule "external/lz4"] path = external/lz4 url = https://github.com/xamarin/lz4 diff --git a/external/Java.Interop b/external/Java.Interop index 1af1d9ab3d0..07c73009571 160000 --- a/external/Java.Interop +++ b/external/Java.Interop @@ -1 +1 @@ -Subproject commit 1af1d9ab3d0b3dee1eef5632ad7151aa3d513f3b +Subproject commit 07c73009571d3b5d36ae79d0d4d69a02062dd6e8