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.

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

Switch the order of AOT SDK and framework assemblies #88703

Merged
merged 1 commit into from
Jul 12, 2023

Conversation

SingleAccretion
Copy link
Contributor

When you have built a CoreCLR CoreLib, it "poisons" the build s.t. running libraries tests with NAOT becomes impossible.

This is because $(IlcFrameworkPath) draws from a shared framework that contains the CoreCLR CoreLib, while it should use the CoreLib from the AOT SDK.

This targeted change changes the order in which these two assemblies get passed to ILC and allows one to use "/p:TestNativeAot=true" without additional workarounds.

This may be a bit controversial as it has the potential to hide problems with bad build states, so I am putting this up more for discussion.

When you have built a CoreCLR CoreLib, it "poisons" the build
s.t. running libraries tests with NAOT becomes impossible.

This is because $(IlcFrameworkPath) draws from a shared framework
that contains the CoreCLR CoreLib, while it should use the CoreLib
from the AOT SDK.

This targeted change changes the order in which these two assemblies
get passed to ILC and allows one to use "/p:TestNativeAot=true"
without additional workarounds.
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jul 11, 2023
@ghost
Copy link

ghost commented Jul 11, 2023

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

When you have built a CoreCLR CoreLib, it "poisons" the build s.t. running libraries tests with NAOT becomes impossible.

This is because $(IlcFrameworkPath) draws from a shared framework that contains the CoreCLR CoreLib, while it should use the CoreLib from the AOT SDK.

This targeted change changes the order in which these two assemblies get passed to ILC and allows one to use "/p:TestNativeAot=true" without additional workarounds.

This may be a bit controversial as it has the potential to hide problems with bad build states, so I am putting this up more for discussion.

Author: SingleAccretion
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -

Copy link
Member

@MichalStrehovsky MichalStrehovsky left a comment

Choose a reason for hiding this comment

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

I think this is a good change. The existing ordering was already arbitrary so changing the arbitrary order to something that can fix an issue people could run into in their inner loop is fine by me. Thanks!

@MichalStrehovsky MichalStrehovsky marked this pull request as ready for review July 12, 2023 02:11
@MichalStrehovsky MichalStrehovsky merged commit 5464296 into dotnet:main Jul 12, 2023
103 of 106 checks passed
@SingleAccretion SingleAccretion deleted the Tgts branch July 12, 2023 11:53
@ghost ghost locked as resolved and limited conversation to collaborators Aug 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-NativeAOT-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants