-
Notifications
You must be signed in to change notification settings - Fork 564
[One .NET] simplify native library inclusion #5386
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.IO; | ||
| using Microsoft.Build.Framework; | ||
|
|
||
| namespace Xamarin.Android.Tasks | ||
| { | ||
| /// <summary> | ||
| /// Processes .so files coming from @(ResolvedFileToPublish). | ||
| /// * Checks if ABI is valid | ||
| /// * Fixes up libmonodroid.so based on $(AndroidIncludeDebugSymbols) | ||
| /// </summary> | ||
| public class ProcessNativeLibraries : AndroidTask | ||
| { | ||
| public override string TaskPrefix => "PRNL"; | ||
|
|
||
| /// <summary> | ||
| /// Assumed to be .so files only | ||
| /// </summary> | ||
| public ITaskItem [] InputLibraries { get; set; } | ||
|
|
||
| public bool IncludeDebugSymbols { get; set; } | ||
|
|
||
| [Output] | ||
| public ITaskItem [] OutputLibraries { get; set; } | ||
|
|
||
| public override bool RunTask () | ||
| { | ||
| if (InputLibraries == null || InputLibraries.Length == 0) | ||
| return true; | ||
|
|
||
| var output = new List<ITaskItem> (InputLibraries.Length); | ||
|
|
||
| foreach (var library in InputLibraries) { | ||
| var abi = MonoAndroidHelper.GetNativeLibraryAbi (library); | ||
| if (string.IsNullOrEmpty (abi)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. …especially when I look at this: Presumably if Which dovetails back with the warning message: if an unsupported As-is, the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't believe the Do you think the existing checks here needs to be changed? I think it will already check |
||
| var packageId = library.GetMetadata ("NuGetPackageId"); | ||
| if (!string.IsNullOrEmpty (packageId)) { | ||
| Log.LogCodedWarning ("XA4301", library.ItemSpec, 0, Properties.Resources.XA4301_ABI_NuGet, library.ItemSpec, packageId); | ||
| } else { | ||
| Log.LogCodedWarning ("XA4301", library.ItemSpec, 0, Properties.Resources.XA4301_ABI, library.ItemSpec); | ||
| } | ||
| continue; | ||
| } | ||
| // Both libmono-android.debug.so and libmono-android.release.so are in InputLibraries. | ||
| // Use IncludeDebugSymbols to determine which one to include. | ||
| // We may eventually have files such as `libmono-android-checked+asan.release.so` as well. | ||
| var fileName = Path.GetFileNameWithoutExtension (library.ItemSpec); | ||
| if (fileName.StartsWith ("libmono-android", StringComparison.Ordinal)) { | ||
| if (fileName.EndsWith (".debug", StringComparison.Ordinal)) { | ||
| if (!IncludeDebugSymbols) | ||
| continue; | ||
| library.SetMetadata ("ArchiveFileName", "libmonodroid.so"); | ||
| } else if (fileName.EndsWith (".release", StringComparison.Ordinal)) { | ||
| if (IncludeDebugSymbols) | ||
| continue; | ||
| library.SetMetadata ("ArchiveFileName", "libmonodroid.so"); | ||
| } | ||
| } | ||
| output.Add (library); | ||
| } | ||
|
|
||
| OutputLibraries = output.ToArray (); | ||
|
|
||
| return !Log.HasLoggedErrors; | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How are we going to handle
_AndroidNativeLibraryForFastDevin the .net 6 world? This is used when we fast deploy most native libraries to the override directory rather than include them in the apk.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully some test will fail here, as this was removed for .NET 6:
I'll need to add an equivalent for these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
libxamarin-debug-app-helper.sois in our runtime packs and will end up as a@(FrameworkNativeLibrary), and those items aren't fast deployed. Before fb727f0, these files were hard coded in the<BuildApk/>task.It looks like only
@(AndroidNativeLibrary)items are added to@(_AndroidNativeLibraryForFastDev): https://github.com/xamarin/monodroid/blob/27736a7ffc48d606ab45598f761e873f8572f46a/tools/msbuild/Xamarin.Android.Common.Debugging.targets#L224.Maybe we should look at fast deploying
@(FrameworkNativeLibrary)in a future PR? If we try to fast deploy both customer libs and framework libs we'll need to rework: https://github.com/xamarin/xamarin-android/blob/aaa37c3df94490ee9befb8381f4dd7aa18226355/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs#L589As for the three
$(_AndroidNativeLibraryForFastDev)items explicitly mentioned here, two files are used for native static code analysis (which I don't think we ever need to put into customer apps). The other one is for the interpreter, and I am not sure if that is available in .NET 6? I am not sure if we need to worry about any of these in a .NET 6 context?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
certain native libs still need to be in the apk even when fast deploying. specifically monodroid.so and its immediate dependencies. This is because it is loaded as part of the runtime startup. So we can't just fast dev everything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latest changes shouldn't change the current logic for .NET 6, except the block:
Instead of silently ignoring, you get a
XA4301warning now.The tests we have for Fast Deployment should be passing, so I don't think there is actually anything broken?