-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Consolidate <NativeAotSupported definitions #103273
Conversation
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
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.
Looks good otherwise, thanks!
<!-- Decide if we're going to do the NativeAOT builds --> | ||
<PropertyGroup> | ||
<!-- disable on Mono, for now --> | ||
<SupportsNativeAotComponents Condition="'$(SupportsNativeAotComponents)' == '' and '$(RuntimeFlavor)' == 'Mono'">false</SupportsNativeAotComponents> |
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.
Cc @lambdageek - we might be losing this exclusion, but that doesn't seem harmful on its own.
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.
yea it's ok. I don't think there's anything that depends on compile-native.proj
when we're building Mono, so this check was probably redundant
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 think in an earlier iteration of the cdacreader build infrastructure I had compile-native as a separate subset and it was getting pulled in from some mono cross-compiler scenarios. But now it's only pulled in from coreclr's runtime-prereqs.proj, which shouldn't be triggered by any mono builds
Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
I have subsequent changes on top of this to consolidate LocateNativeCompiler targets (scattered in multiple places). When this is merged, I'll open a next PR. |
I want to give @lambdageek at least a chance to see this since I'm not directly involved in compile-native and might not know about all the workflows (I don't know why it excludes runtimeflavor mono). (We're all probably in different time zones) |
/azp run runtime-extra-platforms,runtime-ioslike |
Azure Pipelines successfully started running 2 pipeline(s). |
I think we had a bad interaction between #103332 and #103273. Windows x86 with native AOT got broken in Preview 5 (#86573 (comment)).
I think we had a bad interaction between #103332 and #103273. Windows x86 with native AOT got broken in Preview 5 (#86573 (comment)).
This was enabled in #103273 but needs more work to actually build. Official builds are failing.
This was enabled in #103273 but needs more work to actually build. Official builds are failing.
This reverts commit b86c463.
* Revert "Do not set UseNativeAotForComponents for arm32 MUSL (#103469)" This reverts commit f1f0750. * Revert "Set UseNativeAotForComponents to false on bionic (#103454)" This reverts commit d901213. * Revert "Try fixing x86 Windows legs (#103411)" This reverts commit 6927fea. * Revert "Consolidate <NativeAotSupported definitions (#103273)" This reverts commit b86c463.
…03273)" (dotnet#103497)" This reverts commit 7e9cab2.
No description provided.