-
Notifications
You must be signed in to change notification settings - Fork 516
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
Add support for using NativeAOT. #17374
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
3e23d85
to
190a850
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
FWIW here are adaptations that work for building a MAUI template app with NativeAOT on a iOS device |
@rolfbjarne we should consider adding something like this in the native linker configuration: dotnet/runtime#86707 (comment) |
I've filed #18332 to look into this. |
190a850
to
53de0d6
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
e5eab3a
to
4b99217
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…and use them ourselves.
…hen using NativeAOT (and warn if someone asks otherwise).
…get the correct BCL libraries.
This is because NativeAOT contains swift code, and we'd have to add code to embed the Swift libraries in any apps that target early OS versions. We could eventually implement this, but let's wait and see if there's a demand first.
This matches how NativeAOT works for other platforms.
… build when using NativeAOT.
…7 tests. We had to bump NUnitLite and Touch.Unit's TargetFramework properties to net8.0-* (otherwise projects using NativeAOT fails if these projects are referenced), which means they can't be used from .NET 7 (aka our .NET 7 tests). So just don't reference NUnitLite and Touch.Unit in this test.
… waiting for a dotnet/runtime fix.
…, we need a few upstream fixes.
We can fix this better once this fix reaches us: dotnet/runtime#87813 because then we can set the ICU data file at build time (to a relative path).
7e1482c
to
b4e3361
Compare
💻 [CI Build] Windows Integration Tests passed 💻✅ All Windows Integration Tests passed. Pipeline on Agent |
📚 [PR Build] Artifacts 📚Packages generatedView packagesPipeline on Agent |
❌ [PR Build] Tests on macOS M1 - Mac Ventura (13.0) failed ❌Failed tests are:
Pipeline on Agent |
❌ [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) failed ❌Failed tests are:
Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)
✅ API diff vs stable.NET (No breaking changes)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
🔥 [CI Build] Test results 🔥Test results❌ Tests failed on VSTS: simulator tests 0 tests crashed, 1 tests failed, 91 tests passed. Failures❌ dotnettests tests [attempt 2]
Html Report (VSDrops) Download Successes
Pipeline on Agent |
return IntPtr.Zero; | ||
|
||
string location; | ||
if (IsNativeAOT) { |
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.
When I read this, it raises some questions to me: what is wrong with asm?.Location
in the IsNativeAOT
case?
Docs say that this is a path unless it was loaded from a byte array in which case it will be ""
. Is that the case here? Because if it is, maybe the test should be if it's empty string which would catch more cases maybe?
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.
Assembly.Location doesn't return anything meaningful when using NativeAOT, because there's no assembly on disk.
Additionally, the NativeAOT toolchain will show a warning if Assembly.Location is called, so we have to avoid it if we want to be warning-free (these warnings will show up for every customer as well, so it's much better to just avoid the warning instead of having the code path that copes with whatever it returns + the warning.
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.
NativeAOT doesn't have the assemblies as files, similar to the single-file CoreCLR deployment mode. They are all linked together into the same executable.
Here's one place where it is documented: https://learn.microsoft.com/en-us/dotnet/core/deploying/single-file/overview?tabs=cli#api-incompatibility
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.
FWIW, here is another related workaround for NativeAOT which explains the limitations in the PR description: xamarin/Touch.Unit#116 (comment)
Test failures are unrelated. |
Add support for using NativeAOT on all our platforms.
This contains numerous changes in a lot of places to add support for NativeAOT:
And it all pretty much consists of special-casing NativeAOT everywhere we need to.
Note: NativeAOT doesn't work on macOS yet, because a dotnet/runtime fix is required, and thus the corresponding test variations for monotouch-test have been commented out.
This PR is best reviewed commit-by-commit.
This contributes towards #17339.