[dotnet] Detect when the RuntimeIdentifier is changed too late in the build - #26477
[dotnet] Detect when the RuntimeIdentifier is changed too late in the build#26477rolfbjarne wants to merge 3 commits into
Conversation
… build
'_SdkIsSimulator' is computed from the RuntimeIdentifier very early in the
build (in Xamarin.Shared.Sdk.props), and a lot of other things are derived
from it: the clang flags, the sysroot, and the intermediate and output
paths.
A '*.csproj.user' file is imported after that, so setting the
RuntimeIdentifier there results in a build that's partially configured for
the simulator and partially for device. The first sign of trouble is a
rather confusing linker error much later in the build:
ld: building for 'iOS-simulator', but linking in dylib (.../iossimulator-arm64/nativelibraries/libSystem.Globalization.Native.dylib) built for 'iOS'
It's also quite hard to figure out on your own, because neither
'rm -rf bin obj' nor 'dotnet nuget locals --clear' removes the
'*.csproj.user' file.
So detect the inconsistency in '_ValidateRuntimeIdentifier' instead, and
show an error that says what's actually wrong.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 57a20054-5162-48cd-97a2-58bceaa09807
…TooLate test Instead of writing a '*.csproj.user' file next to MySimpleApp and deleting it again afterwards (which leaves the repository dirty while the test is running, and leaves the file behind entirely if the test crashes), add a new 'RuntimeIdentifierInUserFile' test project with the '*.csproj.user' file checked in. '*.user' is in the root .gitignore, so add a .gitignore in the new test project directory to re-include the two files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 57a20054-5162-48cd-97a2-58bceaa09807
…sSimulator I originally skipped the validation when 'RuntimeIdentifiers' contained a simulator runtime identifier, on the theory that '_SdkIsSimulator' is computed from 'RuntimeIdentifiers' as well, and could thus legitimately disagree with 'RuntimeIdentifier'. That turns out not to be the case: * In the outer build of a multi-RID build 'RuntimeIdentifier' is empty, so '_ValidateRuntimeIdentifier' doesn't run at all. * In an inner build, 'RuntimeIdentifier' is one of the values in 'RuntimeIdentifiers', so if the latter is a list of simulator runtime identifiers, both are simulator values and there's no mismatch. * A 'RuntimeIdentifiers' with both device and simulator runtime identifiers already fails earlier, in '_RunRidSpecificBuild'. So the only thing the condition accomplished was to hide real instances of the very problem this validation is supposed to catch (such as a device 'RuntimeIdentifier' in a project that declares simulator 'RuntimeIdentifiers'). Remove it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 57a20054-5162-48cd-97a2-58bceaa09807
There was a problem hiding this comment.
Pull request overview
This PR improves build-time validation in the .NET for Apple platforms SDK by detecting when RuntimeIdentifier is modified after simulator/device-dependent configuration has already been computed, and emitting a clear, actionable error instead of failing later with confusing linker messages.
Changes:
- Add a
_ValidateRuntimeIdentifiercheck to detectRuntimeIdentifiersimulator/device mismatches introduced late in evaluation (commonly via*.csproj.user). - Add a regression unit test plus a dedicated test project that checks in
*.csproj.userfiles to reproduce the scenario reliably. - Introduce minimal test-project scaffolding (shared props/Makefiles/source) to support the unit test.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| dotnet/targets/Xamarin.Shared.Sdk.targets | Adds late-change detection for RuntimeIdentifier vs _SdkIsSimulator and a dedicated error message. |
| tests/dotnet/UnitTests/ProjectTest.cs | Adds a unit test asserting the new diagnostic when RuntimeIdentifier is changed via *.csproj.user. |
| tests/dotnet/RuntimeIdentifierInUserFile/.gitignore | Allows *.csproj.user to be checked in for the regression test. |
| tests/dotnet/RuntimeIdentifierInUserFile/Makefile | Test project wrapper makefile for shared dotnet test infrastructure. |
| tests/dotnet/RuntimeIdentifierInUserFile/Main.cs | Minimal app entry point to ensure platform assembly participation. |
| tests/dotnet/RuntimeIdentifierInUserFile/shared.csproj | Shared project configuration imported by platform-specific csproj files. |
| tests/dotnet/RuntimeIdentifierInUserFile/shared.mk | Shared makefile fragment to connect to common dotnet test rules. |
| tests/dotnet/RuntimeIdentifierInUserFile/iOS/Makefile | iOS-specific makefile including the shared mk. |
| tests/dotnet/RuntimeIdentifierInUserFile/iOS/RuntimeIdentifierInUserFile.csproj | iOS test app project targeting net$(BundledNETCoreAppTargetFrameworkVersion)-ios. |
| tests/dotnet/RuntimeIdentifierInUserFile/iOS/RuntimeIdentifierInUserFile.csproj.user | Checked-in user file that sets RuntimeIdentifier=ios-arm64 to trigger the mismatch. |
| tests/dotnet/RuntimeIdentifierInUserFile/tvOS/Makefile | tvOS-specific makefile including the shared mk. |
| tests/dotnet/RuntimeIdentifierInUserFile/tvOS/RuntimeIdentifierInUserFile.csproj | tvOS test app project targeting net$(BundledNETCoreAppTargetFrameworkVersion)-tvos. |
| tests/dotnet/RuntimeIdentifierInUserFile/tvOS/RuntimeIdentifierInUserFile.csproj.user | Checked-in user file that sets RuntimeIdentifier=tvos-arm64 to trigger the mismatch. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
This comment has been minimized.
This comment has been minimized.
🚀 [CI Build #29bb3ae] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 205 tests passed 🎉 Tests counts✅ assembly-processing: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
_SdkIsSimulatoris computed from the RuntimeIdentifier very early in the build (inXamarin.Shared.Sdk.props), and a lot of other things are derived from it: the clang flags, the sysroot, and the intermediate and output paths.A
*.csproj.userfile is imported after that, so setting the RuntimeIdentifier there results in a build that's partially configured for the simulator and partially for device. The first sign of trouble is a rather confusing linker error much later in the build:It's also quite hard to figure out on your own, because neither
rm -rf bin objnordotnet nuget locals --clearremoves the*.csproj.userfile.So detect the inconsistency in
_ValidateRuntimeIdentifierinstead, and show an error that says what's actually wrong.The test uses a dedicated test project with the
*.csproj.userfile checked in, so that the test doesn't have to create (and clean up) any files while running. Verified that the test fails without the fix (with the linker error above).This showed up in #26453, where it prevented the reporter from building at all.
🤖 Pull request created by Copilot