[msbuild] Stop building universal macOS 27 apps by default. Fixes #25675. - #26636
rolfbjarne wants to merge 3 commits into
Conversation
Stop creating universal Release apps by default when SupportedOSPlatformVersion is macOS 27.0 or later, matching Xcode 27 behavior. Keep explicit runtime identifiers and older deployment targets unchanged, and cover the version boundary. Fixes #25675 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Apply the macOS 27 runtime identifier behavior to Mac Catalyst as well. Release builds targeting version 27.0 or later now default to the host architecture, while earlier targets remain universal and explicit runtime identifiers remain unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Fix the omitted-version evaluation path and add corresponding test and documentation coverage.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates macOS and Mac Catalyst Release RID defaults to use the host architecture for deployment targets 27.0+.
Changes:
- Updates MSBuild RID selection.
- Adds 27.0 boundary tests.
- Documents the revised defaults.
File summaries
| File | Summary |
|---|---|
tests/dotnet/UnitTests/ProjectTest.cs |
Adds boundary coverage; unpinned SDK-supplied versions remain uncovered. |
dotnet/targets/Xamarin.Shared.Sdk.props |
Updates version-aware RID selection; evaluation order misses omitted versions. |
docs/configuration-properties.md |
Documents the new defaults; the unset case is not described. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Lite
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Use TargetPlatformVersion when SupportedOSPlatformVersion has not yet been populated by the .NET SDK, so unpinned macOS and Mac Catalyst 27 projects also default to the host architecture. Add regression coverage for both unpinned platforms and clarify the effective-version behavior in the documentation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Defer RID selection until the effective deployment target is available and add explicit RID preservation tests.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
dotnet/targets/Xamarin.Shared.Sdk.props:190
- ❌ bug — This fallback is evaluated from
Sdk.propsbefore the project body and later imports are evaluated. For example, a project can explicitly setSupportedOSPlatformVersionto 12.0/15.0 (tests/dotnet/Net11_0SimpleApp/macOS/Net11_0SimpleApp.csproj:5), but this code sees it as empty and uses the SDKTargetPlatformVersion(27.0) instead. Its Release build then selects the host-architecture RID rather than preserving the universal default for the earlier deployment target. Defer this RID decision until the effective project property is available, and add a regression case for a project-file value.
<_EffectiveSupportedOSPlatformVersion Condition="'$(SupportedOSPlatformVersion)' != ''">$(SupportedOSPlatformVersion)</_EffectiveSupportedOSPlatformVersion>
<_EffectiveSupportedOSPlatformVersion Condition="'$(_EffectiveSupportedOSPlatformVersion)' == ''">$(TargetPlatformVersion)</_EffectiveSupportedOSPlatformVersion>
<_DefaultToUniversalDesktopApp Condition="'$(Configuration)' == 'Release' And ('$(_PlatformName)' == 'macOS' Or '$(_PlatformName)' == 'MacCatalyst') And $([MSBuild]::VersionLessThan($(_EffectiveSupportedOSPlatformVersion), '27.0'))">true</_DefaultToUniversalDesktopApp>
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
| [TestCase (ApplePlatform.MacOSX, "26.0", true)] | ||
| [TestCase (ApplePlatform.MacOSX, "27.0", false)] | ||
| [TestCase (ApplePlatform.MacOSX, null, false)] | ||
| [TestCase (ApplePlatform.MacCatalyst, "26.0", true)] | ||
| [TestCase (ApplePlatform.MacCatalyst, "27.0", false)] | ||
| [TestCase (ApplePlatform.MacCatalyst, null, false)] | ||
| public void DefaultDesktopReleaseRuntimeIdentifiers (ApplePlatform platform, string? supportedOSPlatformVersion, bool expectUniversal) |
✅ 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 |
🔥 [CI Build #b0cbaac] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 1 tests failed, 263 tests passed. Failures❌ dotnettests tests (macOS)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Sonoma (14): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
Release builds for macOS and Mac Catalyst currently default to universal apps when no runtime identifier is specified.
Match Xcode 27 behavior by defaulting to the host architecture when SupportedOSPlatformVersion is 27.0 or later. Keep the existing universal default for earlier deployment targets, and preserve explicitly configured runtime identifiers.
Add unit coverage for the 27.0 boundary on both platforms and document the updated defaults.
Fixes #25675.
🤖 Pull request created by Copilot