[Xamarin.Android.Build.Tasks] fix detection of "Android libraries"#8904
Merged
jonpryor merged 1 commit intodotnet:mainfrom Apr 26, 2024
Merged
Conversation
Fixes: dotnet/maui#18819 The sample creates a problem such as: * Create a .NET Maui 8 class library with a base class inheriting from Java.Lang.Object: `MauiLib1` * `BaseClass` * `DerivedClass1` : No error when creating an instance * Create a .NET Maui 8 mobile app with a derived class of the base class in step 1: `MauiApp1` * `DerivedClass2` : No error when creating an instance * Create a .Net Maui 8 class library with a derived class of the base class in step 1: `MauiLib2` * `DerivedClass3` * Create an instance of all 3 derived class in mobile app in step 2. * `NotSupportedException` creating `DerivedClass3` Only System.NotSupportedException Message=Cannot create instance of type 'MauiLib2.DerivedClass3': no Java peer type found. at Java.Interop.JniPeerMembers.JniInstanceMethods..ctor(Type declaringType) in /Users/runner/work/1/s/xamarin-android/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 22 at Java.Interop.JniPeerMembers.JniInstanceMethods.GetConstructorsForType(Type declaringType) in /Users/runner/work/1/s/xamarin-android/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 77 at Java.Interop.JniPeerMembers.JniInstanceMethods.StartCreateInstance(String constructorSignature, Type declaringType, JniArgumentValue* parameters) in /Users/runner/work/1/s/xamarin-android/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 139 at Java.Lang.Object..ctor() in /Users/runner/work/1/s/xamarin-android/src/Mono.Android/obj/Release/net8.0/android-34/mcw/Java.Lang.Object.cs:line 37 at MauiLib1.BaseClass..ctor() at MauiLib2.DerivedClass..ctor() in C:\Project\Maui\MauiApp1\MauiLib2\Platforms\Android\DerivedClass.cs:line 7 at MauiApp1.App..ctor(IServiceProvider serviceProvider) in C:\Project\Maui\MauiApp1\MauiApp1\App.xaml.cs:line 18 I could reproduce this problem in an existing `ProjectDependencies()` test, by making some existing classes `Java.Lang.Object` and asserting their JCWs are in the final `classes.dex` file. The fix here appears to be our detection of "Android libraries" in: var tfi = assembly.GetMetadata ("TargetFrameworkIdentifier"); if (string.Compare (tfi, "MonoAndroid", StringComparison.OrdinalIgnoreCase) == 0) return true; This looks like it would only support Xamarin.Android class libraries, as the metadata says: TargetPlatformIdentifier = Android The only reason this works *at all*, is because we also look for `Mono.Android.dll` assembly references: which is more of a fallback. Update the code to look for `Android` in the `TargetPlatformIdentifier`, and use `string.IndexOf()` for both checks.
dellis1972
approved these changes
Apr 25, 2024
jonpryor
reviewed
Apr 25, 2024
| // NOTE: look for both MonoAndroid and Android | ||
| var tfi = assembly.GetMetadata ("TargetFrameworkIdentifier"); | ||
| if (string.Compare (tfi, "MonoAndroid", StringComparison.OrdinalIgnoreCase) == 0) | ||
| if (tfi.IndexOf ("Android", StringComparison.OrdinalIgnoreCase) != -1) |
Contributor
There was a problem hiding this comment.
At this point in time (2024), this looks like it'll work. However, this doesn't quite feel "future-proof", as it will succeed for any string that contains Android: AreAndroidsHuman, HarmonyOSIsMaybeAndroidCompatible, etc.
Requiring that this be == 0 will require that things start with Android, which might still be problematic, but "feels" nominally safer.
Member
Author
There was a problem hiding this comment.
We still support building agains MonoAndoid libraries, I think it will say MonoAndroid for old Xamarin.Android NuGet packages.
jonpryor
reviewed
Apr 25, 2024
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildWithLibraryTests.cs
Show resolved
Hide resolved
Contributor
Fixes: https://github.com/dotnet/maui/issues/18819
Context: https://github.com/kernshen/NoJavaPeer.git
Context: https://github.com/xamarin/xamarin-android/pull/4225#issuecomment-583139797
An assembly's assembly references do not include transitive
dependencies. Given:
// Mono.Android.dll
namespace Java.Lang {
public partial class Object {}
}
// MauiLib1.dll
namespace MauiLib1 {
public class BaseClass : Java.Lang.Object {}
}
// MauiLib2.dll
namespace MauiLib2 {
public class DerivedClass3 : MauiLib1.BaseClass {}
}
then the assembly references for `MauiLib1.dll` will include
`Mono.Android.dll` (it directly references a type from it),
while the assembly references for `MauiLib2.dll` will include
`MauiLib1.dll` (it directly references a type from it) *but*
`MauiLib2.dll` *will not* have an assembly reference to
`Mono.Android.dll`.
This is how things have worked since .NET Framework 1.0. This should
not be surprising.
[As part of the .NET for Android][0] [`SignAndroidPackage`][1] target,
Java Callable Wrappers (JCWs) need to be emitted for all
`Java.Lang.Object` subclasses. This in turn requires
*loading all assemblies* to *find* the `Java.Lang.Object` subclasses.
As a performance optimization, we only load assemblies which we
believed could contain `Java.Lang.Object` subclasses:
1. Assemblies with `'%(TargetFrameworkIdentifier)' == 'MonoAndroid'`,
which is "carry over" from how Xamarin.Android did things,
and works if a .NET for Android project references a
Xamarin.Android project.
2. Assemblies with an assembly reference to `Mono.Android.dll`.
Assemblies with transitive dependencies were caught by (1)…
in Xamarin.Android.
With .NET for Android, that is no longer the case:
`%(TargetFrameworkIdentifier)` is now always `.NETCoreApp`. This in
turn meant that the only assemblies that could be used to generate
Java Callable Wrappers were those which directly referenced
`Mono.Android.dll`!
Enter com/dotnet/maui#18819 and kernshen/NoJavaPeer, which contains
MAUI and .NET for Android solutions with the above transitive reference
structure:
1. `MauiLib1.dll` / `AndroidLib1.dll` references `Mono.Android.dll`,
exports `BaseClass`
2. `MauiLib2.dll` / `AndroidLib2.dll` references `*Lib1.dll` *and not*
`Mono.Android.dll`, exports `DerivedClass3` which inherits `BaseClass`.
3. App project attempts to instantiate `DerivedClass3`.
The result: a runtime exception:
Only System.NotSupportedException Message=Cannot create instance of type 'MauiLib2.DerivedClass3': no Java peer type found.
at Java.Interop.JniPeerMembers.JniInstanceMethods..ctor(Type declaringType) in /Users/runner/work/1/s/xamarin-android/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 22
at Java.Interop.JniPeerMembers.JniInstanceMethods.GetConstructorsForType(Type declaringType) in /Users/runner/work/1/s/xamarin-android/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 77
at Java.Interop.JniPeerMembers.JniInstanceMethods.StartCreateInstance(String constructorSignature, Type declaringType, JniArgumentValue* parameters) in /Users/runner/work/1/s/xamarin-android/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 139
at Java.Lang.Object..ctor() in /Users/runner/work/1/s/xamarin-android/src/Mono.Android/obj/Release/net8.0/android-34/mcw/Java.Lang.Object.cs:line 37
at MauiLib1.BaseClass..ctor()
at MauiLib2.DerivedClass..ctor() in C:\Project\Maui\MauiApp1\MauiLib2\Platforms\Android\DerivedClass.cs:line 7
at MauiApp1.App..ctor(IServiceProvider serviceProvider) in C:\Project\Maui\MauiApp1\MauiApp1\App.xaml.cs:line 18
The exception occurs because there is no JCW for `DerivedClass3`, and
there isn't a JCW for `DerivedClass3` because `MauiLib2.dll` was not
processed at all, because it had no assembly reference to
`Mono.Android.dll`.
As a workaround, update `MauiLib2.dll` to contain an assembly reference
to `Mono.Android.dll`.
*Fix* this scenario by updating
`MonoAndroidHelper.IsMonoAndroidAssembly()` to use consider these
to be .NET for Android assemblies:
1. Assemblies with `%(TargetFrameworkIdentifier)` *containing*
`Android`. (This doesn't actually change anything; it's a
simplification.)
2. Assemblies with `%(TargetPlatformIdentifier)` *containing*
`Android`.
*This* causes `MauiLib2.dll` to be treated as a .NET for Android
assembly, fixing the bug.
3. Assemblies with an assembly reference to `Mono.Android.dll`.
The addition of check (2) allows assemblies with only transitive
(non-) references to `Mono.Android.dll` to be properly considered,
allowing JCWs to be emitted for types within them.
Update the `BuildWithLibraryTests.ProjectDependencies()` unit test to
better check for this scenario.
[0]: https://github.com/xamarin/xamarin-android/wiki/Blueprint#after-build
[1]: https://learn.microsoft.com/en-us/dotnet/android/building-apps/build-targets#signandroidpackage |
jonathanpeppers
added a commit
that referenced
this pull request
Apr 26, 2024
…8904) Fixes: dotnet/maui#18819 Context: https://github.com/kernshen/NoJavaPeer.git Context: #4225 (comment) An assembly's assembly references do not include transitive dependencies. Given: // Mono.Android.dll namespace Java.Lang { public partial class Object {} } // MauiLib1.dll namespace MauiLib1 { public class BaseClass : Java.Lang.Object {} } // MauiLib2.dll namespace MauiLib2 { public class DerivedClass3 : MauiLib1.BaseClass {} } then the assembly references for `MauiLib1.dll` will include `Mono.Android.dll` (it directly references a type from it), while the assembly references for `MauiLib2.dll` will include `MauiLib1.dll` (it directly references a type from it) *but* `MauiLib2.dll` *will not* have an assembly reference to `Mono.Android.dll`. This is how things have worked since .NET Framework 1.0. This should not be surprising. [As part of the .NET for Android][0] [`SignAndroidPackage`][1] target, Java Callable Wrappers (JCWs) need to be emitted for all `Java.Lang.Object` subclasses. This in turn requires *loading all assemblies* to *find* the `Java.Lang.Object` subclasses. As a performance optimization, we only load assemblies which we believed could contain `Java.Lang.Object` subclasses: 1. Assemblies with `'%(TargetFrameworkIdentifier)' == 'MonoAndroid'`, which is "carry over" from how Xamarin.Android did things, and works if a .NET for Android project references a Xamarin.Android project. 2. Assemblies with an assembly reference to `Mono.Android.dll`. Assemblies with transitive dependencies were caught by (1)… in Xamarin.Android. With .NET for Android, that is no longer the case: `%(TargetFrameworkIdentifier)` is now always `.NETCoreApp`. This in turn meant that the only assemblies that could be used to generate JCWs were those which directly referenced `Mono.Android.dll`! Enter dotnet/maui#18819 and kernshen/NoJavaPeer, which contains MAUI and .NET for Android solutions with the above transitive reference structure: 1. `MauiLib1.dll` / `AndroidLib1.dll` references `Mono.Android.dll`, exports `BaseClass` 2. `MauiLib2.dll` / `AndroidLib2.dll` references `*Lib1.dll` *and not* `Mono.Android.dll`; exports `DerivedClass3` which inherits `BaseClass`. 3. App project attempts to instantiate `DerivedClass3`. The result: a runtime exception: Only System.NotSupportedException Message=Cannot create instance of type 'MauiLib2.DerivedClass3': no Java peer type found. at Java.Interop.JniPeerMembers.JniInstanceMethods..ctor(Type declaringType) in /Users/runner/work/1/s/xamarin-android/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 22 at Java.Interop.JniPeerMembers.JniInstanceMethods.GetConstructorsForType(Type declaringType) in /Users/runner/work/1/s/xamarin-android/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 77 at Java.Interop.JniPeerMembers.JniInstanceMethods.StartCreateInstance(String constructorSignature, Type declaringType, JniArgumentValue* parameters) in /Users/runner/work/1/s/xamarin-android/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 139 at Java.Lang.Object..ctor() in /Users/runner/work/1/s/xamarin-android/src/Mono.Android/obj/Release/net8.0/android-34/mcw/Java.Lang.Object.cs:line 37 at MauiLib1.BaseClass..ctor() at MauiLib2.DerivedClass..ctor() in C:\Project\Maui\MauiApp1\MauiLib2\Platforms\Android\DerivedClass.cs:line 7 at MauiApp1.App..ctor(IServiceProvider serviceProvider) in C:\Project\Maui\MauiApp1\MauiApp1\App.xaml.cs:line 18 The exception occurs because there is no JCW for `DerivedClass3`, and there isn't a JCW for `DerivedClass3` because `MauiLib2.dll` was not processed at all, because it had no assembly reference to `Mono.Android.dll`. As a workaround, update `MauiLib2.dll` to contain an assembly reference to `Mono.Android.dll`. *Fix* this scenario by updating `MonoAndroidHelper.IsMonoAndroidAssembly()` to consider these to be .NET for Android assemblies: 1. Assemblies with `%(TargetFrameworkIdentifier)` *containing* `Android`. (This doesn't actually change anything; it's a simplification.) 2. Assemblies with `%(TargetPlatformIdentifier)` *containing* `Android`. *This* causes `MauiLib2.dll` to be treated as a .NET for Android assembly, fixing the bug. 3. Assemblies with an assembly reference to `Mono.Android.dll`. The addition of check (2) allows assemblies with only transitive (non-) references to `Mono.Android.dll` to be properly considered, allowing JCWs to be emitted for types within them. Update the `BuildWithLibraryTests.ProjectDependencies()` unit test to better check for this scenario. [0]: https://github.com/xamarin/xamarin-android/wiki/Blueprint#after-build [1]: https://learn.microsoft.com/en-us/dotnet/android/building-apps/build-targets#signandroidpackage
jonathanpeppers
added a commit
that referenced
this pull request
Apr 29, 2024
…8904) Fixes: dotnet/maui#18819 Context: https://github.com/kernshen/NoJavaPeer.git Context: #4225 (comment) An assembly's assembly references do not include transitive dependencies. Given: // Mono.Android.dll namespace Java.Lang { public partial class Object {} } // MauiLib1.dll namespace MauiLib1 { public class BaseClass : Java.Lang.Object {} } // MauiLib2.dll namespace MauiLib2 { public class DerivedClass3 : MauiLib1.BaseClass {} } then the assembly references for `MauiLib1.dll` will include `Mono.Android.dll` (it directly references a type from it), while the assembly references for `MauiLib2.dll` will include `MauiLib1.dll` (it directly references a type from it) *but* `MauiLib2.dll` *will not* have an assembly reference to `Mono.Android.dll`. This is how things have worked since .NET Framework 1.0. This should not be surprising. [As part of the .NET for Android][0] [`SignAndroidPackage`][1] target, Java Callable Wrappers (JCWs) need to be emitted for all `Java.Lang.Object` subclasses. This in turn requires *loading all assemblies* to *find* the `Java.Lang.Object` subclasses. As a performance optimization, we only load assemblies which we believed could contain `Java.Lang.Object` subclasses: 1. Assemblies with `'%(TargetFrameworkIdentifier)' == 'MonoAndroid'`, which is "carry over" from how Xamarin.Android did things, and works if a .NET for Android project references a Xamarin.Android project. 2. Assemblies with an assembly reference to `Mono.Android.dll`. Assemblies with transitive dependencies were caught by (1)… in Xamarin.Android. With .NET for Android, that is no longer the case: `%(TargetFrameworkIdentifier)` is now always `.NETCoreApp`. This in turn meant that the only assemblies that could be used to generate JCWs were those which directly referenced `Mono.Android.dll`! Enter dotnet/maui#18819 and kernshen/NoJavaPeer, which contains MAUI and .NET for Android solutions with the above transitive reference structure: 1. `MauiLib1.dll` / `AndroidLib1.dll` references `Mono.Android.dll`, exports `BaseClass` 2. `MauiLib2.dll` / `AndroidLib2.dll` references `*Lib1.dll` *and not* `Mono.Android.dll`; exports `DerivedClass3` which inherits `BaseClass`. 3. App project attempts to instantiate `DerivedClass3`. The result: a runtime exception: Only System.NotSupportedException Message=Cannot create instance of type 'MauiLib2.DerivedClass3': no Java peer type found. at Java.Interop.JniPeerMembers.JniInstanceMethods..ctor(Type declaringType) in /Users/runner/work/1/s/xamarin-android/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 22 at Java.Interop.JniPeerMembers.JniInstanceMethods.GetConstructorsForType(Type declaringType) in /Users/runner/work/1/s/xamarin-android/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 77 at Java.Interop.JniPeerMembers.JniInstanceMethods.StartCreateInstance(String constructorSignature, Type declaringType, JniArgumentValue* parameters) in /Users/runner/work/1/s/xamarin-android/external/Java.Interop/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods.cs:line 139 at Java.Lang.Object..ctor() in /Users/runner/work/1/s/xamarin-android/src/Mono.Android/obj/Release/net8.0/android-34/mcw/Java.Lang.Object.cs:line 37 at MauiLib1.BaseClass..ctor() at MauiLib2.DerivedClass..ctor() in C:\Project\Maui\MauiApp1\MauiLib2\Platforms\Android\DerivedClass.cs:line 7 at MauiApp1.App..ctor(IServiceProvider serviceProvider) in C:\Project\Maui\MauiApp1\MauiApp1\App.xaml.cs:line 18 The exception occurs because there is no JCW for `DerivedClass3`, and there isn't a JCW for `DerivedClass3` because `MauiLib2.dll` was not processed at all, because it had no assembly reference to `Mono.Android.dll`. As a workaround, update `MauiLib2.dll` to contain an assembly reference to `Mono.Android.dll`. *Fix* this scenario by updating `MonoAndroidHelper.IsMonoAndroidAssembly()` to consider these to be .NET for Android assemblies: 1. Assemblies with `%(TargetFrameworkIdentifier)` *containing* `Android`. (This doesn't actually change anything; it's a simplification.) 2. Assemblies with `%(TargetPlatformIdentifier)` *containing* `Android`. *This* causes `MauiLib2.dll` to be treated as a .NET for Android assembly, fixing the bug. 3. Assemblies with an assembly reference to `Mono.Android.dll`. The addition of check (2) allows assemblies with only transitive (non-) references to `Mono.Android.dll` to be properly considered, allowing JCWs to be emitted for types within them. Update the `BuildWithLibraryTests.ProjectDependencies()` unit test to better check for this scenario. [0]: https://github.com/xamarin/xamarin-android/wiki/Blueprint#after-build [1]: https://learn.microsoft.com/en-us/dotnet/android/building-apps/build-targets#signandroidpackage
grendello
added a commit
that referenced
this pull request
May 6, 2024
* release/8.0.2xx: [Mono.Android] fix potential leak of RunnableImplementors (#8900) Bump to xamarin/monodroid@21aed734 (#8905) [Xamarin.Android.Build.Tasks] fix detection of "Android libraries" (#8904) [ci] Do not use @self annotation for templates (#8783) Bump to xamarin/xamarin-android-tools/release/8.0.1xx@d50747cb (#8892) [Xamarin.Android.Build.Tasks] Remove "Xamarin" from messages (#8884) [ci] Use NuGetAuthenticate@1 (#8877) [ci] Migrate to the 1ES template (#8876)
grendello
added a commit
that referenced
this pull request
May 7, 2024
* main: Update README (#8913) Bumps to xamarin/Java.Interop/main@4e893bf (#8924) Bump to dotnet/installer@fa261b952d 9.0.100-preview.5.24253.16 (#8921) [Mono.Android] Dispose of the `RunnableImplementor` on error (#8907) Bump NDK to r26d (#8868) Bump to dotnet/installer@d301a122c4 9.0.100-preview.5.24229.2 (#8912) Localized file check-in by OneLocBuild Task (#8910) LEGO: Merge pull request 8909 [api-merge] Add `removed-since` info (#8897) Bump to xamarin/monodroid@9ca6d9f6 (#8895) [Xamarin.Android.Build.Tasks] fix detection of "Android libraries" (#8904)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: dotnet/maui#18819
The sample creates a problem such as:
Create a .NET Maui 8 class library with a base class inheriting from Java.Lang.Object:
MauiLib1BaseClass*DerivedClass1: No error when creating an instanceCreate a .NET Maui 8 mobile app with a derived class of the base class in step 1:
MauiApp1DerivedClass2: No error when creating an instanceCreate a .Net Maui 8 class library with a derived class of the base class in step 1:
MauiLib2*DerivedClass3* Create an instance of all 3 derived class in mobile app in step 2. *NotSupportedExceptioncreatingDerivedClass3I could reproduce this problem in an existing
ProjectDependencies()test, by making some existing classesJava.Lang.Objectand asserting their JCWs are in the finalclasses.dexfile.The fix here appears to be our detection of "Android libraries" in:
This looks like it would only support Xamarin.Android class libraries, as the metadata says:
The only reason this works at all, is because we also look for
Mono.Android.dllassembly references: which is more of a fallback.Update the code to look for
Androidin theTargetPlatformIdentifier, and usestring.IndexOf()for both checks.