Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Xamarin.Android.Build.Tasks] fix detection of "Android libraries" #8904

Merged
merged 1 commit into from
Apr 26, 2024
Merged

[Xamarin.Android.Build.Tasks] fix detection of "Android libraries" #8904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildWithLibraryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,11 +269,7 @@ public void ProjectDependencies ([Values(true, false)] bool projectReference)
};
libB.Sources.Clear ();
libB.Sources.Add (new BuildItem.Source ("Foo.cs") {
TextContent = () => @"public class Foo {
jonpryor marked this conversation as resolved.
Show resolved Hide resolved
public Foo () {
var bar = new Bar();
}
}",
TextContent = () => "public class Foo : Bar { }",
});

var libC = new XamarinAndroidLibraryProject () {
Expand All @@ -283,7 +279,7 @@ public Foo () {
};
libC.Sources.Clear ();
libC.Sources.Add (new BuildItem.Source ("Bar.cs") {
TextContent = () => "public class Bar { }",
TextContent = () => "public class Bar : Java.Lang.Object { }",
});
libC.Sources.Add (new BuildItem ("EmbeddedResource", "Foo.resx") {
TextContent = () => InlineData.ResxWithContents ("<data name=\"CancelButton\"><value>Cancel</value></data>")
Expand All @@ -309,8 +305,8 @@ public Foo () {
ProjectName = "AppA",
IsRelease = true,
Sources = {
new BuildItem.Source ("Bar.cs") {
TextContent = () => "public class Bar : Foo { }",
new BuildItem.Source ("Baz.cs") {
TextContent = () => "public class Baz : Foo { }",
},
new BuildItem ("EmbeddedResource", "Foo.resx") {
TextContent = () => InlineData.ResxWithContents ("<data name=\"CancelButton\"><value>Cancel</value></data>")
Expand All @@ -321,6 +317,10 @@ public Foo () {
}
};
appA.AddReference (libB);
if (!projectReference) {
// @(ProjectReference) implicits adds this reference. For `class Baz : Foo : Bar`:
appA.OtherBuildItems.Add (new BuildItem.Reference ($@"..\{libC.ProjectName}\bin\Release\{libC.TargetFramework}\{libC.ProjectName}.dll"));
}
var appBuilder = CreateApkBuilder (Path.Combine (path, appA.ProjectName));
Assert.IsTrue (appBuilder.Build (appA), $"{appA.ProjectName} should succeed");

Expand All @@ -332,6 +332,17 @@ public Foo () {
helper.AssertContainsEntry ($"assemblies/{libC.ProjectName}.dll");
helper.AssertContainsEntry ($"assemblies/es/{appA.ProjectName}.resources.dll");
helper.AssertContainsEntry ($"assemblies/es/{libC.ProjectName}.resources.dll");

var intermediate = Path.Combine (Root, appBuilder.ProjectDirectory, appA.IntermediateOutputPath);
var dexFile = Path.Combine (intermediate, "android", "bin", "classes.dex");
FileAssert.Exists (dexFile);

// NOTE: the crc hashes here might change one day, but if we used [Android.Runtime.Register("")]
// LibraryB.dll would have a reference to Mono.Android.dll, which invalidates the test.
string className = "Lcrc6414a4b78410c343a2/Bar;";
Assert.IsTrue (DexUtils.ContainsClass (className, dexFile, AndroidSdkPath), $"`{dexFile}` should include `{className}`!");
className = "Lcrc646d2d82b4d8b39bd8/Foo;";
Assert.IsTrue (DexUtils.ContainsClass (className, dexFile, AndroidSdkPath), $"`{dexFile}` should include `{className}`!");
}

[Test]
Expand Down
7 changes: 6 additions & 1 deletion src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,13 @@ public static void LogWarning (object log, string msg, params object [] args)
#if MSBUILD
public static bool IsMonoAndroidAssembly (ITaskItem assembly)
{
// 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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still support building agains MonoAndoid libraries, I think it will say MonoAndroid for old Xamarin.Android NuGet packages.

return true;

var tpi = assembly.GetMetadata ("TargetPlatformIdentifier");
if (tpi.IndexOf ("Android", StringComparison.OrdinalIgnoreCase) != -1)
return true;

var hasReference = assembly.GetMetadata ("HasMonoAndroidReference");
Expand Down