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

ResolveReadyToRunCompilers: map non-portable rids when targetOS is determined. #28380

Merged
merged 2 commits into from
Oct 12, 2022

Conversation

tmds
Copy link
Member

@tmds tmds commented Oct 7, 2022

@tmds tmds force-pushed the non_portable_rid branch from 44ab0e6 to aea2bba Compare October 7, 2022 09:03
new[] { "linux", "linux-musl", "osx", "win" },
out _);

// For source-build, allow the bootstrap SDK rid to be unknown to the runtime repo graph.
Copy link
Member Author

Choose a reason for hiding this comment

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

After the initial bootstrap using Microsoft's SDK (e.g. linux-x64) further builds of source-build can use a non-portable SDK that was previously built (e.g. fedora.36-x64).

The runtime repo overwrites the BundledRuntimeIdentifierGraphFile to either the one it includes in its sources, or the one that it generates.

These graph files may not know the rid from the bootstrap SDK (e.g. fedora.36-x64).
(It will be known to the graph that is in the SDK.)

Rather than make the BundledRuntimeIdentifierGraphFile also use the SDK graph in some cases, I've opted to special case it here under _targetRuntimeIdentifier == _hostRuntimeIdentifier.

@build-analysis build-analysis bot mentioned this pull request Oct 7, 2022
2 tasks
Copy link
Member

@AntonLapounov AntonLapounov left a comment

Choose a reason for hiding this comment

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

Looks reasonable. Thank you!

@tmds
Copy link
Member Author

tmds commented Oct 11, 2022

The CI failure on Windows doesn't look related to this change.

@AntonLapounov
Copy link
Member

All CI jobs must be green to merge a PR. I have restarted the failed leg.

@tmds
Copy link
Member Author

tmds commented Oct 12, 2022

CI is all green now.

@ViktorHofer ViktorHofer merged commit 71e047e into dotnet:main Oct 12, 2022
@ViktorHofer
Copy link
Member

Thanks a lot @tmds

@tmds
Copy link
Member Author

tmds commented Nov 9, 2022

@AntonLapounov @ViktorHofer can you initiate a backport of this PR to 7.0?

@ViktorHofer
Copy link
Member

@AntonLapounov can you do so please and fill out the servicing template?

@AntonLapounov
Copy link
Member

Do we use a /backport command or do it manually in SDK? I guess we could use the template from #28380. I know only that this is needed for source-build.

@ViktorHofer
Copy link
Member

There is no backport command here, you would need to do it manually. Usually in the SDK, changes are applied to the lowest applicable branch and then bots will auto-flow them into the newer branches.

tmds added a commit to tmds/sdk that referenced this pull request Nov 14, 2022
…termined. (dotnet#28380)

* ResolveReadyToRunCompilers: map non-portable rids when targetOS is determined.

* For source-build, allow using a rid that is not in the graph if it matches the host rid.
AntonLapounov pushed a commit that referenced this pull request Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

crossgen2 doesn't resolve compilers for non-portable rids
3 participants