-
Notifications
You must be signed in to change notification settings - Fork 512
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 NativeAOT on macOS #18524
Conversation
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
46feaff
to
47307df
Compare
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
I wonder what the Also, it seems that UPD: #18530 |
The tests are failing with the following error:
Logs can be found here: https://gist.github.com/mandel-macaque/93cc94e2258904f9abc9c9b7b3ddbc06 |
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.
We should get to the bottom of the failing tests, but code looks good.
Thanks! I will check these tests locally... (likely not until next week, I am at remote location with spotty internet connection) |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Azure Pipelines successfully started running 1 pipeline(s). |
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.
The monotouch-test failures are unrelated, but the dotnettests ones aren't. The macOS/mac catalyst template tests are failing:
I investigated a bit:
This is rather puzzling, I wouldn't have expected any of the changes here to have such an effect. |
Thanks for digging into the dotnettests tests. I am still puzzled by it but I think I finally managed to run some of these tests locally, so I can investigate. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
💻 [PR Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻✅ All tests on macOS M1 - Mac Ventura (13.0) passed. Pipeline on Agent |
💻 [CI Build] Windows Integration Tests passed 💻✅ All Windows Integration Tests passed. Pipeline on Agent |
💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻✅ All tests on macOS M1 - Mac Big Sur (11.5) passed. Pipeline on Agent |
📚 [PR Build] Artifacts 📚Packages generatedView packagesPipeline on Agent |
✅ API diff for current PR / commitLegacy Xamarin (No breaking changes)
NET (empty diffs)
✅ API diff vs stableLegacy Xamarin (No breaking changes).NET (No breaking changes)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
🚀 [CI Build] Test results 🚀Test results✅ All tests passed on VSTS: simulator tests. 🎉 All 233 tests passed 🎉 Tests counts✅ bcl: All 69 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
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.
❤️
I believe some of these changes aren't necessary anymore now that I've added support for the [Preserve] attribute (so test suites can be fully linked for NativeAOT), so I've created a different PR (#18765) to test this hypothesis. I'll analyze the results next Tuesday. |
Should we also try to cleanup this code within this PR? xamarin-macios/src/Foundation/NSObject2.cs Lines 294 to 315 in c024223
#if !NET
[MethodImplAttribute (MethodImplOptions.InternalCall)]
extern static void RegisterToggleRef (NSObject obj, IntPtr handle, bool isCustomType);
#endif // !NET
// ...
static void RegisterToggleReference (NSObject obj, IntPtr handle, bool isCustomType)
{
#if NET
Runtime.RegisterToggleReferenceCoreCLR (obj, handle, isCustomType);
#else
RegisterToggleRef (obj, handle, isCustomType);
#endif
} |
@ivanpovazan That's not equivalent. We still use MonoVM on .NET 8 ;) |
My mistake, I misinterpreted what Why I was mentioning this part of the code is that we get: |
Yeah, I would love to fix that warning but it's not that trivial. Definitely worth a separate PR. |
Superseded by #18765. |
This PR implements a workaround for the following build warning: ``` ILC: Method '[Microsoft.iOS]Foundation.NSObject.RegisterToggleRef(NSObject,native int,bool)' will always throw because: Invalid IL or CLR metadata in 'Void Foundation.NSObject.RegisterToggleRef(Foundation.NSObject, IntPtr, Boolean)' ``` Ref #18524
No description provided.