Add escape hatch for PublishAot without RuntimeIdentifier. Fixes #33414.#35366
Merged
dsplaisted merged 3 commits intoSep 18, 2023
Merged
Conversation
…et#33414. This adds an escape hatch for a sanity check when trying to use PublishAot without a RuntimeIdentifier, because the check is not valid when building universal apps for macOS and Mac Catalyst (the netX-macos and netX-maccatalyst target frameworks). When building such an app, the project file will set RuntimeIdentifiers (plural): <TargetFramework>net8.0-macos</TargetFramework> <RuntimeIdentifiers>osx-x64;osx-arm</RuntimeIdentifiers> and then during the build, the macOS SDK will run two inner builds, with RuntimeIdentifiers unset, and RuntimeIdentifier set to each of the rids (and at the end merge the result into a single app). The problem is that the outer build, where RuntimeIdentifiers is set, but RuntimeIdentifier isn't, triggers the sanity check, and that fails the build. I attempted to unset PublishAot for the outer build, but that doesn't work for two reasons: * It can't be unset if customers pass /p:PublishAot=true on the command line. * The package restore won't restore the NativeAOT packages nor include them in the build, and thus effectively NativeAOT is turned off for the inner builds as well, even if we set PublishAot=true for them. So instead add an escape hatch for the sanity check. Fixes dotnet#33414.
ghost
added
These new properties requires a downstream SDK to do the right thing, they won't work out-of-the box.
dalexsoto
approved these changes
Sep 18, 2023
nagilson
approved these changes
Sep 18, 2023
nagilson
left a comment
nagilson
left a comment
Member
There was a problem hiding this comment.
LGTM, thank you for making a PR with a fix. I'm sorry I did not get to the issue in a timely manner. This is a smart way to go about it.
Member
|
Keep in mind this will only go into .NET 9, you would need to target the 8.0-rc branches (new one is being created tonight/tomorrow morning) to get it in sooner. |
Member
|
/backport to release/8.0.1xx |
Contributor
|
Started backporting to release/8.0.1xx: https://github.com/dotnet/sdk/actions/runs/6228417168 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds an escape hatch for a sanity check when trying to use PublishAot without a RuntimeIdentifier, because the check is not valid when building universal apps for macOS and Mac Catalyst (the netX-macos and netX-maccatalyst target frameworks).
When building such an app, the project file will set RuntimeIdentifiers (plural):
and then during the build, the macOS SDK will run two inner builds, with RuntimeIdentifiers unset, and RuntimeIdentifier set to each of the rids (and at the end merge the result into a single app).
The problem is that the outer build, where RuntimeIdentifiers is set, but RuntimeIdentifier isn't, triggers the sanity check, and that fails the build.
I attempted to unset PublishAot for the outer build, but that doesn't work for two reasons:
So instead add an escape hatch for the sanity check.
Fixes #33414.