Repository navigation
[dotnet] Set a default PublishRuntimeIdentifier again. Fixes #24547 - #26013
rolfbjarne merged 3 commits into
Conversation
We had to stop setting `PublishRuntimeIdentifier` because the SDK appends it to `RuntimeIdentifiers`, which confuses our build (we use `RuntimeIdentifiers` to mean "build for all these RIDs", not just "restore all these RIDs"), and we ended up with both `RuntimeIdentifier` and `RuntimeIdentifiers` set. The SDK now has an escape hatch for this behavior (`AppendPublishRuntimeIdentifierToRuntimeIdentifiers=false`), so re-enable setting `PublishRuntimeIdentifier` for iOS/tvOS and opt out of appending it to `RuntimeIdentifiers`. Also add a regression test. Ref: dotnet/sdk#54291 Fixes #24547 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Re-enables a default PublishRuntimeIdentifier for iOS/tvOS publishes, leveraging a new .NET SDK escape hatch to prevent the publish RID from being appended into RuntimeIdentifiers (which this repo treats as “build for all these RIDs”, and which can otherwise trigger RuntimeIdentifier/RuntimeIdentifiers clash validation errors).
Changes:
- Re-enable default
PublishRuntimeIdentifiervalues (ios-arm64/tvos-arm64) in the shared SDK props. - Set
AppendPublishRuntimeIdentifierToRuntimeIdentifiers=falseto avoid the SDK mutatingRuntimeIdentifiers. - Add a unit test regression check that
PublishRuntimeIdentifieris set but not appended toRuntimeIdentifiers.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/dotnet/UnitTests/PostBuildTest.cs | Adds regression coverage validating PublishRuntimeIdentifier defaults don’t leak into RuntimeIdentifiers. |
| dotnet/targets/Xamarin.Shared.Sdk.props | Restores default publish RID behavior for iOS/tvOS and opts out of SDK RID-appending behavior. |
| <PublishRuntimeIdentifier Condition="'$(_PlatformName)' == 'tvOS'">tvos-arm64</PublishRuntimeIdentifier> | ||
| --> | ||
| <!-- Disable the default PublishRuntimeIdentifier behavior: https://github.com/dotnet/sdk/pull/52566 --> | ||
| <UseDefaultPublishRuntimeIdentifier>false</UseDefaultPublishRuntimeIdentifier> | ||
| <!-- Don't append PublishRuntimeIdentifier to RuntimeIdentifiers: we use RuntimeIdentifiers to mean "build for all these RIDs", and appending the publish RID confuses our build: https://github.com/dotnet/sdk/pull/54291 / https://github.com/dotnet/macios/issues/24547 --> | ||
| <AppendPublishRuntimeIdentifierToRuntimeIdentifiers>false</AppendPublishRuntimeIdentifierToRuntimeIdentifiers> |
There was a problem hiding this comment.
Fixed in 6c34ce6. The two escape hatches (UseDefaultPublishRuntimeIdentifier=false and AppendPublishRuntimeIdentifierToRuntimeIdentifiers=false) are now set in an unconditional property group, so they apply even when a RuntimeIdentifier/RuntimeIdentifiers is specified explicitly. Only our default PublishRuntimeIdentifier value stays guarded by _XamarinUsingDefaultRuntimeIdentifier. Added a regression test covering the explicit-RID case.
…pecify a RuntimeIdentifier. The default PublishRuntimeIdentifier we set was overriding an explicit RuntimeIdentifier during publish (the SDK sets RuntimeIdentifier = PublishRuntimeIdentifier when publishing). This turned simulator builds into device builds, which broke the NativeAOT simulator tests (they set an explicit simulator RuntimeIdentifier + PublishAot/_IsPublishing). Guard the default so it only applies when the user didn't specify a RuntimeIdentifier. We can't check '$(RuntimeIdentifier)' == '' because we've already computed a default RuntimeIdentifier by this point, so we check the _XamarinUsingDefaultRuntimeIdentifier property instead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.
…ntifier behavior. The escape hatches UseDefaultPublishRuntimeIdentifier=false and AppendPublishRuntimeIdentifierToRuntimeIdentifiers=false must be set unconditionally, regardless of whether a RuntimeIdentifier or RuntimeIdentifiers was specified. A previous iteration gated these together with our default PublishRuntimeIdentifier behind _XamarinUsingDefaultRuntimeIdentifier. That meant builds that specified an explicit RuntimeIdentifier (or a plural RuntimeIdentifiers) lost the escape hatches, so the SDK computed a default PublishRuntimeIdentifier (the host portable RID) and appended it to RuntimeIdentifiers. For MacCatalyst 'link all' this produced the clash "Building for all the runtime identifiers 'maccatalyst-x64;osx-arm64' at the same time isn't possible". Split into two property groups: the escape hatches are now unconditional, while our default PublishRuntimeIdentifier (ios-arm64/tvos-arm64) stays guarded by _XamarinUsingDefaultRuntimeIdentifier so it doesn't clobber an explicit RuntimeIdentifier. Also add a regression test asserting the escape hatches are set even when a RuntimeIdentifier is specified. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
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.
🚀 [CI Build #6c34ce6] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 256 tests passed 🎉 Tests counts✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Sonoma (14): All 5 tests passed. [attempt 2] Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
Now that dotnet/sdk#54291 has merged and is available in our bundled SDK, we can set a default
PublishRuntimeIdentifieragain.Background
We previously had to stop setting
PublishRuntimeIdentifierbecause the .NET SDK appends it toRuntimeIdentifiers. We useRuntimeIdentifiersto mean "build for all these RIDs" (not just "restore for these RIDs"), so the appended publish RID left bothRuntimeIdentifierandRuntimeIdentifiersset at the same time, which our build rejects with a validation error.Change
PublishRuntimeIdentifier(ios-arm64/tvos-arm64).RuntimeIdentifiersvia the new escape hatchAppendPublishRuntimeIdentifierToRuntimeIdentifiers=false(added in Add escape hatch for PublishRuntimeIdentifier being appended to RuntimeIdentifiers by default sdk#54291).PublishRuntimeIdentifieris set but is not appended toRuntimeIdentifiers.Validated with a clean build; the new test passes for iOS and tvOS.
Fixes #24547
🤖 Pull request created by Copilot