From 63dcf363df40c5210ea1a6fffb6c909d751037c5 Mon Sep 17 00:00:00 2001 From: Jonathan Pobst Date: Mon, 30 Sep 2024 11:18:00 -1000 Subject: [PATCH 1/7] [generator] Fix StackOverflow when copying DIM from package-protected interfaces. --- .../DefaultInterfaceMethodsTests.cs | 36 +++++++++++++++++++ .../Xamarin.Test.IExtendedInterface.cs | 4 +-- .../Xamarin.Test.IExtendedInterface.cs | 4 +-- .../InterfaceGen.cs | 5 +-- 4 files changed, 43 insertions(+), 6 deletions(-) diff --git a/tests/generator-Tests/Unit-Tests/DefaultInterfaceMethodsTests.cs b/tests/generator-Tests/Unit-Tests/DefaultInterfaceMethodsTests.cs index 9b94acd88..7269620a0 100644 --- a/tests/generator-Tests/Unit-Tests/DefaultInterfaceMethodsTests.cs +++ b/tests/generator-Tests/Unit-Tests/DefaultInterfaceMethodsTests.cs @@ -514,5 +514,41 @@ public void CompatVirtualMethod_Interface () Assert.True (writer.ToString ().NormalizeLineEndings ().Contains ("catch (Java.Lang.NoSuchMethodError) { throw new Java.Lang.AbstractMethodError (__id); }".NormalizeLineEndings ()), $"was: `{writer}`"); } + + [Test] + public void FixDefaultInterfaceMethodStackOverflow () + { + // The bug was that this causes a stack overflow, but this also tests + // that the OverriddenInterfaceMethod is now correctly set in the interface method. + var xml = """ + + + + + + + + + + + """; + + var gens = ParseApiDefinition (xml); + + foreach (var iface in gens.OfType ()) { + generator.Context.ContextTypes.Push (iface); + generator.WriteType (iface, string.Empty, new GenerationInfo ("", "", "MyAssembly")); + generator.Context.ContextTypes.Pop (); + } + + var klass1 = gens.Single (g => g.Name == "IInternalConfigurator"); + var klass2 = gens.Single (g => g.Name == "IConfigurator"); + + Assert.AreEqual (1, klass1.Methods.Count); + Assert.AreEqual (1, klass2.Methods.Count); + + Assert.AreNotSame (klass1.Methods [0], klass2.Methods [0]); + Assert.AreSame (klass1.Methods [0].OverriddenInterfaceMethod, klass2.Methods [0]); + } } } diff --git a/tests/generator-Tests/expected.ji/AccessModifiers/Xamarin.Test.IExtendedInterface.cs b/tests/generator-Tests/expected.ji/AccessModifiers/Xamarin.Test.IExtendedInterface.cs index 3ea50e6bd..83d01f24a 100644 --- a/tests/generator-Tests/expected.ji/AccessModifiers/Xamarin.Test.IExtendedInterface.cs +++ b/tests/generator-Tests/expected.ji/AccessModifiers/Xamarin.Test.IExtendedInterface.cs @@ -11,7 +11,7 @@ public partial interface IExtendedInterface : IJavaPeerable { [global::Java.Interop.JniMethodSignature ("extendedMethod", "()V")] void ExtendedMethod (); - // Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/interface[@name='BaseInterface']/method[@name='baseMethod' and count(parameter)=0]" + // Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/interface[@name='ExtendedInterface']/method[@name='baseMethod' and count(parameter)=0]" [global::Java.Interop.JniMethodSignature ("baseMethod", "()V")] void BaseMethod (); @@ -46,7 +46,7 @@ public unsafe void BaseMethod () { const string __id = "baseMethod.()V"; try { - _members_xamarin_test_BaseInterface.InstanceMethods.InvokeAbstractVoidMethod (__id, this, null); + _members_xamarin_test_ExtendedInterface.InstanceMethods.InvokeAbstractVoidMethod (__id, this, null); } finally { } } diff --git a/tests/generator-Tests/expected.xaji/AccessModifiers/Xamarin.Test.IExtendedInterface.cs b/tests/generator-Tests/expected.xaji/AccessModifiers/Xamarin.Test.IExtendedInterface.cs index 04ab7fd4f..b569b5075 100644 --- a/tests/generator-Tests/expected.xaji/AccessModifiers/Xamarin.Test.IExtendedInterface.cs +++ b/tests/generator-Tests/expected.xaji/AccessModifiers/Xamarin.Test.IExtendedInterface.cs @@ -12,7 +12,7 @@ public partial interface IExtendedInterface : IJavaObject, IJavaPeerable { [Register ("extendedMethod", "()V", "GetExtendedMethodHandler:Xamarin.Test.IExtendedInterfaceInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null")] void ExtendedMethod (); - // Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/interface[@name='BaseInterface']/method[@name='baseMethod' and count(parameter)=0]" + // Metadata.xml XPath method reference: path="/api/package[@name='xamarin.test']/interface[@name='ExtendedInterface']/method[@name='baseMethod' and count(parameter)=0]" [Register ("baseMethod", "()V", "GetBaseMethodHandler:Xamarin.Test.IExtendedInterfaceInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null")] void BaseMethod (); @@ -95,7 +95,7 @@ public unsafe void BaseMethod () { const string __id = "baseMethod.()V"; try { - _members_xamarin_test_BaseInterface.InstanceMethods.InvokeAbstractVoidMethod (__id, this, null); + _members_xamarin_test_ExtendedInterface.InstanceMethods.InvokeAbstractVoidMethod (__id, this, null); } finally { } } diff --git a/tools/generator/Java.Interop.Tools.Generator.ObjectModel/InterfaceGen.cs b/tools/generator/Java.Interop.Tools.Generator.ObjectModel/InterfaceGen.cs index f86750e54..7f36d0cff 100644 --- a/tools/generator/Java.Interop.Tools.Generator.ObjectModel/InterfaceGen.cs +++ b/tools/generator/Java.Interop.Tools.Generator.ObjectModel/InterfaceGen.cs @@ -69,8 +69,9 @@ public override void FixupAccessModifiers (CodeGenerationOptions opt) var baseType = opt.SymbolTable.Lookup (implementedInterface); if (baseType is InterfaceGen interfaceGen && interfaceGen.RawVisibility != "public") { // Copy over "private" methods - interfaceGen.Methods.Where (m => !Methods.Contains (m)).ToList ().ForEach (Methods.Add); - + foreach (var method in interfaceGen.Methods.ToList ()) + if (!Methods.Any (m => m.Matches (method))) + Methods.Add (method.Clone (this)); } else { break; } From 12c9c824f01c17c36f517bbdbd92f88da60ecbff Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 9 Oct 2024 13:45:27 -0400 Subject: [PATCH 2/7] Add test around interface method inheritance Context: 1adb7964a2033c83c298c070f2d1ab896d92671b Context: https://github.com/dotnet/java-interop/issues/1183 While reviewing #1261, I noticed this change: ```diff diff --git a/tests/generator-Tests/expected.ji/AccessModifiers/Xamarin.Test.IExtendedInterface.cs b/tests/generator-Tests/expected.ji/AccessModifiers/Xamarin.Test.IExtendedInterface.cs index 3ea50e6bd..83d01f24a 100644 --- a/tests/generator-Tests/expected.ji/AccessModifiers/Xamarin.Test.IExtendedInterface.cs +++ b/tests/generator-Tests/expected.ji/AccessModifiers/Xamarin.Test.IExtendedInterface.cs @@ -46,7 +46,7 @@ public unsafe void BaseMethod () { const string __id = "baseMethod.()V"; try { - _members_xamarin_test_BaseInterface.InstanceMethods.InvokeAbstractVoidMethod (__id, this, null); + _members_xamarin_test_ExtendedInterface.InstanceMethods.InvokeAbstractVoidMethod (__id, this, null); } finally { } } ``` and went "hmm". Recall and consider 1adb7964: if we invoke the interface method on the "wrong" declaring type, things break. This is why 1adb7964 updated interface invokers to only invoke methods upon their declared interfaces. The concern comes around *non-`public`* interface method invocation: /* package */ interface PackagePrivateInterface { void m(); } public interface PublicInterface extends PackagePrivateInterface { } With commit 63dcf363, attempts to invoke `m()` are made upon `PublicInterface`, not `PackagePrivateInterface`. Is this a problem? Begin partially implementing #1183, adding a new `build-tools/Java.Interop.Sdk` (not quite a) "project", which has an `Sdk.props` and `Sdk.targets` file, which combined add support for: * A `@(JavaCompile)` build action, for Java source. * A `@(JavaReference)` build action, for Java libraries. * As a pre-Compile step, files with `%(JavaCompile.Bind)`=True or `%(JavaReference.Bind)`=True will be automatically bound, via `class-parse` + `generator`. * As a post-Compile step, the assembly will be processed with `jcw-gen` to generate Java Callable Wrappers, which will be compiled into `$(AssemblyName).jar`. Then, update `tests/Java.Base-Tests` to use this new functionality, adding a test case to hit the "invoke method declared in a private interface upon the public interface" scenario. Result: it works! Of partial interest is `IInterfaceMethodInheritanceInvoker`: internal partial class IInterfaceMethodInheritanceInvoker : global::Java.Lang.Object, IInterfaceMethodInheritance { static readonly JniPeerMembers _members_net_dot_jni_test_BaseInterface = new JniPeerMembers ("net/dot/jni/test/BaseInterface", typeof (IInterfaceMethodInheritanceInvoker)); static readonly JniPeerMembers _members_net_dot_jni_test_InterfaceMethodInheritance = new JniPeerMembers ("net/dot/jni/test/InterfaceMethodInheritance", typeof (IInterfaceMethodInheritanceInvoker)); static readonly JniPeerMembers _members_net_dot_jni_test_InternalInterface = new JniPeerMembers ("net/dot/jni/test/InternalInterface", typeof (IInterfaceMethodInheritanceInvoker)); static readonly JniPeerMembers _members_net_dot_jni_test_PublicInterface = new JniPeerMembers ("net/dot/jni/test/PublicInterface", typeof (IInterfaceMethodInheritanceInvoker)); } Of these four fields, two are for internal types: `_members_net_dot_jni_test_BaseInterface` and `_members_net_dot_jni_test_InternalInterface`. Fortunately those types aren't otherwise used. Of concern, though, is that the constructor invocation *does* result in a `JNIEnv::FindClass()` invocation, meaning these bindings would look up (ostensibly) "private" types, which could change! This presents a compatibility concern: if (when?) those type names change, then the generated bindings will break. TODO: * Test this puppy in dotnet/android. Just because "it works" on Desktop JDK doesn't mean it does *on Android*. * Update `generator` output to *not* emit the `static readonly JniPeerMembers` fields for internal types. --- build-tools/Java.Interop.Sdk/Sdk/Sdk.props | 6 + build-tools/Java.Interop.Sdk/Sdk/Sdk.targets | 218 ++++++++++++++++++ tests/Java.Base-Tests/Java.Base-Tests.csproj | 11 + tests/Java.Base-Tests/Java.Base-Tests.targets | 47 ---- .../InterfaceMethodInheritanceTests.cs | 26 +++ .../test/HasInterfaceMethodInheritance.java | 26 +++ .../jni/test/InterfaceMethodInheritance.java | 9 + .../net/dot/jni/test/PublicInterface.java | 9 + 8 files changed, 305 insertions(+), 47 deletions(-) create mode 100644 build-tools/Java.Interop.Sdk/Sdk/Sdk.props create mode 100644 build-tools/Java.Interop.Sdk/Sdk/Sdk.targets create mode 100644 tests/Java.Base-Tests/Java.Base/InterfaceMethodInheritanceTests.cs create mode 100644 tests/Java.Base-Tests/java/net/dot/jni/test/HasInterfaceMethodInheritance.java create mode 100644 tests/Java.Base-Tests/java/net/dot/jni/test/InterfaceMethodInheritance.java create mode 100644 tests/Java.Base-Tests/java/net/dot/jni/test/PublicInterface.java diff --git a/build-tools/Java.Interop.Sdk/Sdk/Sdk.props b/build-tools/Java.Interop.Sdk/Sdk/Sdk.props new file mode 100644 index 000000000..ff58e1577 --- /dev/null +++ b/build-tools/Java.Interop.Sdk/Sdk/Sdk.props @@ -0,0 +1,6 @@ + + + ; + : + + diff --git a/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets b/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets new file mode 100644 index 000000000..f68f3b2e6 --- /dev/null +++ b/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets @@ -0,0 +1,218 @@ + + + + + True + + + True + + + + + $(OutputPath)$(AssemblyName).jar + + + + + _BuildJavaCompileForManagedBinding; + _GenerateApiDescription; + _GenerateManagedBinding; + _CleanupManagedBinding; + + + _JavaCreateJcws; + _JavaCreateOutputJar; + + + + + + + + + + + <_GeneratorPath>$(UtilityOutputFullPath)generator.dll + <_JavaIntermediateDir>$(IntermediateOutputPath)_ji\ + <_JavaManagedBindingInput>$(_JavaIntermediateDir)o.jar + <_JavaManagedBindingDir>$(_JavaIntermediateDir)mcw\ + <_JavaJcwClassesDir>$(_JavaIntermediateDir)classes\ + <_JavaJcwSourcesDir>$(_JavaIntermediateDir)java\ + + + + + <_JavaCompileForBindingInputs + Condition=" '%(JavaCompile.Bind)' == 'True' " + Include="@(JavaCompile)" + /> + + + + + + <_JavacRefs Include="$(ToolOutputFullPath)java-interop.jar" /> + <_JavacRefs Include="@(JavaReference)" /> + <_JavacRefsWithForwardSlash Include="@(_JavacRefs->Replace('%5c', '/'))" /> + + + + + + <_ClassesDir>$(_JavaIntermediateDir)\bound-classes + <_ResponseFile>$(_JavaIntermediateDir)r.txt + <_Classpath>@(_JavacRefsWithForwardSlash, '$(JavaPathSeparator)') + + + + <_Response Include="-classpath" /> + <_Response Include=""$(_Classpath)"" /> + <_Response Include="@(_JavaCompileForBindingInputs->Replace('%5c', '/'))" /> + + + + + + + + + + + + <_ClassParseInputs + Condition=" Exists($(_JavaManagedBindingInput))" + Include="$(_JavaManagedBindingInput)" + /> + <_ClassParseInputs + Condition=" '%(JavaReference.Bind)' == 'True' " + Include="@(JavaReference)" + /> + + + + + + + <_ClassParse>"$(UtilityOutputFullPath)class-parse.dll" + <_Inputs>@(_ClassParseInputs, ' ') + <_Output>"-o=$(_JavaManagedBindingDir)api.xml" + + + + + + + + "$(_GeneratorPath)" + <_GenFlags>--public --global + <_Out>-o "$(_JavaManagedBindingDir)" + <_Codegen>--codegen-target=JavaInterop1 + <_Assembly>"--assembly=$(AssemblyName)" + <_TypeMap>--type-map-report=$(_JavaManagedBindingDir)type-mapping.txt + <_Api>$(_JavaManagedBindingDir)api.xml + <_Dirs>--enumdir=$(_JavaManagedBindingDir) + <_FullIntermediateOutputPath>$([System.IO.Path]::GetFullPath('$(_JavaManagedBindingDir)')) + <_LangFeatures>--lang-features=nullable-reference-types,default-interface-methods,nested-interface-types,interface-constants + + + + <_RefAsmDir Include="@(ReferencePathWithRefAssemblies->'%(RootDir)%(Directory).'->Distinct())" /> + <_Lib Include="@(_RefAsmDir->'-L "%(Identity)"')" /> + <_JavaBaseRef Include="@(ReferencePathWithRefAssemblies)" + Condition=" '%(FileName)' == 'Java.Base' " + /> + <_Ref Include="@(_JavaBaseRef->'-r "%(FullPath)"')" /> + + + + + + + + + + + $(DefineConstants);$([System.String]::Copy('$(_GeneratedDefineConstants)').Replace ('%24(DefineConstants);', '')) + + + + + + + + + + + <_RefAsmDirs Include="@(ReferencePathWithRefAssemblies->'%(RootDir)%(Directory).'->Distinct())" /> + + + <_JcwGen>"$(UtilityOutputFullPath)/jcw-gen.dll" + <_Target>--codegen-target JavaInterop1 + <_Output>-o "$(_JavaJcwSourcesDir)" + <_Libpath>@(_RefAsmDirs->'-L "%(Identity)"', ' ') + + + + + + + + <_JavaGeneratedJcwSource Include="$(_JavaJcwSourcesDir)**\*.java" /> + + + + + + + + <_ResponseFile>$(_JavaIntermediateDir)r.txt + <_Classpath>@(_JavacRefsWithForwardSlash, '$(JavaPathSeparator)') + + + <_Source Include="@(JavaCompile->Replace('%5c', '/'))" /> + <_Source Include="@(_JavaGeneratedJcwSource->Replace('%5c', '/'))" /> + + + + + + + + diff --git a/tests/Java.Base-Tests/Java.Base-Tests.csproj b/tests/Java.Base-Tests/Java.Base-Tests.csproj index 78111a2d7..e1f4b2bc4 100644 --- a/tests/Java.Base-Tests/Java.Base-Tests.csproj +++ b/tests/Java.Base-Tests/Java.Base-Tests.csproj @@ -8,10 +8,12 @@ false + $(TestOutputFullPath) + $(OutputPath)java.base-tests.jar @@ -26,6 +28,15 @@ + + + + + + + + + diff --git a/tests/Java.Base-Tests/Java.Base-Tests.targets b/tests/Java.Base-Tests/Java.Base-Tests.targets index 3312b41b6..8c119d541 100644 --- a/tests/Java.Base-Tests/Java.Base-Tests.targets +++ b/tests/Java.Base-Tests/Java.Base-Tests.targets @@ -1,49 +1,2 @@ - - - <_BuildJavaBaseTestsJarInputs Include="$(TargetPath)" /> - <_BuildJavaBaseTestsJarInputs Include="$(MSBuildThisFileFullPath)" /> - <_BuildJavaBaseTestsJarInputs Include="$(MSBuildThisFileFullPath)" /> - <_BuildJavaBaseTestsJarInputs Include="java\**\*.java" /> - - - - - - - - <_RefAsmDirs Include="@(ReferencePathWithRefAssemblies->'%(RootDir)%(Directory).'->Distinct())" /> - - - <_JcwGen>"$(UtilityOutputFullPath)/jcw-gen.dll" - <_Target>--codegen-target JavaInterop1 - <_Output>-o "$(IntermediateOutputPath)/java" - <_Libpath>@(_RefAsmDirs->'-L "%(Identity)"', ' ') - - - - - - - <_JcwSource Include="$(IntermediateOutputPath)java\**\*.java;java\**\*.java" /> - - - - - - - <_JcwSourceReal Include="$(IntermediateOutputPath)java\**\*.java;java\**\*.java" /> - - - - - diff --git a/tests/Java.Base-Tests/Java.Base/InterfaceMethodInheritanceTests.cs b/tests/Java.Base-Tests/Java.Base/InterfaceMethodInheritanceTests.cs new file mode 100644 index 000000000..dc49dfd27 --- /dev/null +++ b/tests/Java.Base-Tests/Java.Base/InterfaceMethodInheritanceTests.cs @@ -0,0 +1,26 @@ +using System; + +using Java.Interop; + +using NUnit.Framework; + +namespace Java.BaseTests { + + [TestFixture] + public class InterfaceMethodInheritanceTests : JavaVMFixture { + + [Test] + public void InterfaceMethod () + { + using var iface = global::Net.Dot.Jni.Test.HasInterfaceMethodInheritance.Create (); + var m = iface!.M (); + Assert.AreEqual ("HasInterfaceMethodInheritance.m", m); + var n = iface!.N (); + Assert.AreEqual ("HasInterfaceMethodInheritance.n", n); + var o = iface!.O (); + Assert.AreEqual ("HasInterfaceMethodInheritance.o", o); + var p = iface!.P (); + Assert.AreEqual ("HasInterfaceMethodInheritance.p", p); + } + } +} diff --git a/tests/Java.Base-Tests/java/net/dot/jni/test/HasInterfaceMethodInheritance.java b/tests/Java.Base-Tests/java/net/dot/jni/test/HasInterfaceMethodInheritance.java new file mode 100644 index 000000000..ffb2824c0 --- /dev/null +++ b/tests/Java.Base-Tests/java/net/dot/jni/test/HasInterfaceMethodInheritance.java @@ -0,0 +1,26 @@ +package net.dot.jni.test; + +public class HasInterfaceMethodInheritance implements InterfaceMethodInheritance { + private HasInterfaceMethodInheritance() { + } + + public static InterfaceMethodInheritance create() { + return new HasInterfaceMethodInheritance(); + } + + public String m() { + return "HasInterfaceMethodInheritance.m"; + } + + public String n() { + return "HasInterfaceMethodInheritance.n"; + } + + public String o() { + return "HasInterfaceMethodInheritance.o"; + } + + public String p() { + return "HasInterfaceMethodInheritance.p"; + } +} diff --git a/tests/Java.Base-Tests/java/net/dot/jni/test/InterfaceMethodInheritance.java b/tests/Java.Base-Tests/java/net/dot/jni/test/InterfaceMethodInheritance.java new file mode 100644 index 000000000..e27d3fa50 --- /dev/null +++ b/tests/Java.Base-Tests/java/net/dot/jni/test/InterfaceMethodInheritance.java @@ -0,0 +1,9 @@ +package net.dot.jni.test; + +/* package */ interface BaseInterface { + String m(); +} + +public interface InterfaceMethodInheritance extends BaseInterface, PublicInterface { + String n(); +} diff --git a/tests/Java.Base-Tests/java/net/dot/jni/test/PublicInterface.java b/tests/Java.Base-Tests/java/net/dot/jni/test/PublicInterface.java new file mode 100644 index 000000000..9ce0a1f98 --- /dev/null +++ b/tests/Java.Base-Tests/java/net/dot/jni/test/PublicInterface.java @@ -0,0 +1,9 @@ +package net.dot.jni.test; + +/* package */ interface InternalInterface { + String o(); +} + +public interface PublicInterface extends InternalInterface { + String p(); +} From 850b06d841d606a99508cba18237c7c565d76f0b Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 9 Oct 2024 19:57:50 -0400 Subject: [PATCH 3/7] Fix unit test failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 12c9c824 didn't work properly in a "from clean" build: git clean -xdff git submodule foreach --recursive git clean -xdff git submodule update --init --recursive dotnet build -c Release -t:Prepare *.sln dotnet build -c Release *.sln dotnet test bin/TestRelease-net8.0/Java.Base-Tests.dll failed with: Failed InterfaceMethod [71 ms] Error Message: Java.Interop.JavaException : net/dot/jni/test/HasInterfaceMethodInheritance ----> Java.Interop.JavaException : net.dot.jni.test.HasInterfaceMethodInheritance Stack Trace: at Java.Interop.JniEnvironment.Types.TryFindClass(String classname, Boolean throwOnError) in /Users/runner/work/1/s/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs:line 89 at Java.Interop.JniEnvironment.Types.FindClass(String classname) in /Users/runner/work/1/s/src/Java.Interop/Java.Interop/JniEnvironment.Types.cs:line 37 … The cause of the failure is that `java.base-tests.jar` did not exist. `java.base-tests.jar` did not exist because `_JavaCreateJcws` & co didn't execute! didn't execute, as per the build log: Skipping target "_JavaCreateOutputJar" because it has no inputs. It had no inputs because `$(TargetPath)` didn't exist, and it didn't exist because `$(TargetPath)` (typically) exists in `$(OutputPath)`, and doesn't exist until after the `Build` target. `_JavaCreateOutputJar`, meanwhile, was executing after `CoreCompile`. *An* assembly existed, but it was in `$(IntermediateOutputPath)`, not `$(OutputPath)`, and thus `$(TargetPath)` did not yet exist. Rename the `JavaCreateJavaCallableWrappers` target to `JavaCreateOutputJar`, as I like `JavaCreateOutputJar` better, and update `AfterTargets` so that it happens after Build. This ensures that `$(TargetPath)` exists when `_JavaCreateOutputJar` is executed, which in turn ensures that `java.base-tests.jar` is created. This should allow unit tests to complete successfully. --- build-tools/Java.Interop.Sdk/Sdk/Sdk.targets | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets b/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets index f68f3b2e6..04f4ac93d 100644 --- a/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets +++ b/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets @@ -20,10 +20,10 @@ _GenerateManagedBinding; _CleanupManagedBinding; - + _JavaCreateJcws; _JavaCreateOutputJar; - + - + From 8b36274113a95e20950ed07fdcc8b2f433d4702c Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 9 Oct 2024 20:57:57 -0400 Subject: [PATCH 4/7] [jnimarshalmethod-gen] Support resolving types from active assembly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For reasons I'm not going to investigate, if an interface type is defined in the assembly that `jnimarshalmethod-gen` is processing, `Assembly.Location` will be the empty string, which causes an error: % dotnet bin/Release-net8.0/jnimarshalmethod-gen.dll bin/TestRelease-net8.0/Java.Base-Tests.dll -v -v --keeptemp … error JM4006: jnimarshalmethod-gen: Unable to process assembly 'bin/TestRelease-net8.0/Java.Base-Tests.dll' Name can not be empty System.ArgumentException: Name can not be empty at Mono.Cecil.AssemblyNameReference.Parse(String fullName) at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Resolve(String fullName, ReaderParameters parameters) in /Users/runner/work/1/s/src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/DirectoryAssemblyResolver.cs:line 261 at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.Resolve(String fullName) in /Users/runner/work/1/s/src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/DirectoryAssemblyResolver.cs:line 256 at Java.Interop.Tools.Cecil.DirectoryAssemblyResolver.GetAssembly(String fileName) in /Users/runner/work/1/s/src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/DirectoryAssemblyResolver.cs:line 251 at Xamarin.Android.Tools.JniMarshalMethodGenerator.Extensions.NeedsMarshalMethod(MethodDefinition md, DirectoryAssemblyResolver resolver, TypeDefinitionCache cache, MethodInfo method, String& name, String& methodName, String& signature) in /Users/runner/work/1/s/tools/jnimarshalmethod-gen/App.cs:line 790 at Xamarin.Android.Tools.JniMarshalMethodGenerator.App.CreateMarshalMethodAssembly(String path) in /Users/runner/work/1/s/tools/jnimarshalmethod-gen/App.cs:line 538 at Xamarin.Android.Tools.JniMarshalMethodGenerator.App.ProcessAssemblies(List`1 assemblies) in /Users/runner/work/1/s/tools/jnimarshalmethod-gen/App.cs:line 285 Update `App.NeedsMarshalMethod()` so that when the assembly Location is not present, `DirectoryAssemblyResolver.Resolve(assemblyName)` is instead used. This prevents the `ArgumentException`. --- build-tools/Java.Interop.Sdk/Sdk/Sdk.targets | 2 +- tools/jnimarshalmethod-gen/App.cs | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets b/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets index 04f4ac93d..ab6cfecc9 100644 --- a/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets +++ b/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets @@ -160,7 +160,7 @@ $(DefineConstants);$([System.String]::Copy('$(_GeneratedDefineConstants)').Replace ('%24(DefineConstants);', '')) - + diff --git a/tools/jnimarshalmethod-gen/App.cs b/tools/jnimarshalmethod-gen/App.cs index 6771fc452..7479706e3 100644 --- a/tools/jnimarshalmethod-gen/App.cs +++ b/tools/jnimarshalmethod-gen/App.cs @@ -786,8 +786,12 @@ public static bool NeedsMarshalMethod (this MethodDefinition md, DirectoryAssemb if (iface.IsGenericType) continue; + var location = iface.Assembly.Location; + var ad = string.IsNullOrEmpty (location) + ? resolver.Resolve (iface.Assembly.GetName ().Name) + : resolver.GetAssembly (location); + var ifaceMap = method.DeclaringType.GetInterfaceMap (iface); - var ad = resolver.GetAssembly (iface.Assembly.Location); var id = ad.MainModule.GetType (iface.GetCecilName ()); if (id == null) { From 611cc3f353606f84227a824e813f1b478067169b Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 9 Oct 2024 21:15:44 -0400 Subject: [PATCH 5/7] Attempt to fix Windows build. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows is failing to build the solution! error BG0000: System.InvalidOperationException: A .xml file must be specified. at Xamarin.Android.Binder.CodeGeneratorOptions.Parse(String[] args) in D:\a\_work\1\s\tools\generator\CodeGeneratorOptions.cs:line 193 at Xamarin.Android.Binder.CodeGenerator.Main(String[] args) in D:\a\_work\1\s\tools\generator\CodeGenerator.cs:line 29 ##[error]build-tools\Java.Interop.Sdk\Sdk\Sdk.targets(147,5): Error MSB3073: The command "dotnet "D:\a\_work\1\s\bin\Release-net8.0\generator.dll" --public --global -o "obj\\Release-net8.0\_ji\mcw\" -L "D:\a\_work\1\s\src\Java.Base\bin\Release\ref\." -L "D:\a\_work\1\s\bin\Release-net8.0\ref\." -L "D:\a\_work\1\s\bin\TestRelease-net8.0\ref\." -L "C:\hostedtoolcache\windows\dotnet\packs\Microsoft.NETCore.App.Ref\8.0.7\ref\net8.0\." -L "C:\Users\cloudtest\.nuget\packages\microsoft.testplatform.testhost\17.5.0-preview-20221003-04\lib\netcoreapp3.1\." -L "C:\Users\cloudtest\.nuget\packages\microsoft.codecoverage\17.5.0-preview-20221003-04\lib\netcoreapp3.1\." -L "C:\Users\cloudtest\.nuget\packages\mono.options\6.12.0.148\lib\netstandard2.0\." -L "C:\Users\cloudtest\.nuget\packages\newtonsoft.json\13.0.1\lib\netstandard2.0\." -L "C:\Users\cloudtest\.nuget\packages\nuget.frameworks\5.11.0\lib\netstandard2.0\." -L "C:\Users\cloudtest\.nuget\packages\nunit\3.13.2\lib\netstandard2.0\." -L "D:\a\_work\1\s\external\xamarin-android-tools\bin\Release\net6.0\ref\." -r "D:\a\_work\1\s\src\Java.Base\bin\Release\ref\Java.Base.dll" --codegen-target=JavaInterop1 "--assembly=Java.Base-Tests" --type-map-report=obj\\Release-net8.0\_ji\mcw\type-mapping.txt --lang-features=nullable-reference-types,default-interface-methods,nested-interface-types,interface-constants --enumdir=obj\\Release-net8.0\_ji\mcw\ obj\\Release-net8.0\_ji\mcw\api.xml " exited with code 1. D:\a\_work\1\s\build-tools\Java.Interop.Sdk\Sdk\Sdk.targets(147,5): error MSB3073: The command "dotnet "D:\a\_work\1\s\bin\Release-net8.0\generator.dll" --public --global -o "obj\\Release-net8.0\_ji\mcw\" -L "D:\a\_work\1\s\src\Java.Base\bin\Release\ref\." -L "D:\a\_work\1\s\bin\Release-net8.0\ref\." -L "D:\a\_work\1\s\bin\TestRelease-net8.0\ref\." -L "C:\hostedtoolcache\windows\dotnet\packs\Microsoft.NETCore.App.Ref\8.0.7\ref\net8.0\." -L "C:\Users\cloudtest\.nuget\packages\microsoft.testplatform.testhost\17.5.0-preview-20221003-04\lib\netcoreapp3.1\." -L "C:\Users\cloudtest\.nuget\packages\microsoft.codecoverage\17.5.0-preview-20221003-04\lib\netcoreapp3.1\." -L "C:\Users\cloudtest\.nuget\packages\mono.options\6.12.0.148\lib\netstandard2.0\." -L "C:\Users\cloudtest\.nuget\packages\newtonsoft.json\13.0.1\lib\netstandard2.0\." -L "C:\Users\cloudtest\.nuget\packages\nuget.frameworks\5.11.0\lib\netstandard2.0\." -L "C:\Users\cloudtest\.nuget\packages\nunit\3.13.2\lib\netstandard2.0\." -L "D:\a\_work\1\s\external\xamarin-android-tools\bin\Release\net6.0\ref\." -r "D:\a\_work\1\s\src\Java.Base\bin\Release\ref\Java.Base.dll" --codegen-target=JavaInterop1 "--assembly=Java.Base-Tests" --type-map-report=obj\\Release-net8.0\_ji\mcw\type-mapping.txt --lang-features=nullable-reference-types,default-interface-methods,nested-interface-types,interface-constants --enumdir=obj\\Release-net8.0\_ji\mcw\ obj\\Release-net8.0\_ji\mcw\api.xml " exited with code 1. [D:\a\_work\1\s\tests\Java.Base-Tests\Java.Base-Tests.csproj] My guess is that because I have `$(_JavaManagedBindingDir)` set to end with `.`, it's "escaping" anything that follows it, e.g. `-o "$(_JavaManagedBindingDir)"` becomes `-o "obj\Release-net8.0\_ji\mcw\"`., and `\"` escapes the `"`. Plausible result: the entire command-line no longer makes sense. Repeat the often repeated "I can't find a good way to trim the trailing `\`" solution by appending a `.` to the end of `$(_JavaManagedBindingDir)` whenever `"` follows; that is, replace: $(_JavaManagedBindingDir)" with $(_JavaManagedBindingDir)." Additionally, ensure that all usage of `$(_JavaManagedBindingDir)` is quoted, in case it ever contains a space. Does It Build™? --- build-tools/Java.Interop.Sdk/Sdk/Sdk.targets | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets b/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets index ab6cfecc9..8d43e7f3c 100644 --- a/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets +++ b/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets @@ -126,12 +126,12 @@ "$(_GeneratorPath)" <_GenFlags>--public --global - <_Out>-o "$(_JavaManagedBindingDir)" + <_Out>-o "$(_JavaManagedBindingDir)." <_Codegen>--codegen-target=JavaInterop1 <_Assembly>"--assembly=$(AssemblyName)" <_TypeMap>--type-map-report=$(_JavaManagedBindingDir)type-mapping.txt <_Api>$(_JavaManagedBindingDir)api.xml - <_Dirs>--enumdir=$(_JavaManagedBindingDir) + <_Dirs>"--enumdir=$(_JavaManagedBindingDir)." <_FullIntermediateOutputPath>$([System.IO.Path]::GetFullPath('$(_JavaManagedBindingDir)')) <_LangFeatures>--lang-features=nullable-reference-types,default-interface-methods,nested-interface-types,interface-constants From 22c45cd90e684501edc1375e341f9235b0023105 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 9 Oct 2024 21:52:49 -0400 Subject: [PATCH 6/7] Continue attempting to fix Windows build. Same but different as 611cc3f3, this time with jcw-gen. Replace `"$(_JavaJcwSourcesDir)"` with `"$(_JavaJcwSourcesDir)."`. --- build-tools/Java.Interop.Sdk/Sdk/Sdk.targets | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets b/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets index 8d43e7f3c..f0f470348 100644 --- a/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets +++ b/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets @@ -178,7 +178,7 @@ <_JcwGen>"$(UtilityOutputFullPath)/jcw-gen.dll" <_Target>--codegen-target JavaInterop1 - <_Output>-o "$(_JavaJcwSourcesDir)" + <_Output>-o "$(_JavaJcwSourcesDir)." <_Libpath>@(_RefAsmDirs->'-L "%(Identity)"', ' ') From e07276ba62a54680d24373550e89ae2e80ac487d Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Wed, 9 Oct 2024 22:05:48 -0400 Subject: [PATCH 7/7] =?UTF-8?q?=E2=80=A6and=20again=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- build-tools/Java.Interop.Sdk/Sdk/Sdk.targets | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets b/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets index f0f470348..92ef90e2a 100644 --- a/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets +++ b/build-tools/Java.Interop.Sdk/Sdk/Sdk.targets @@ -210,9 +210,9 @@ Lines="@(_Source)" Overwrite="True" /> - + - +