[msbuild] Show a better error for invalid SupportedOSPlatformVersion values. Fixes #23626 - #26488
Conversation
…formVersion values containing whitespace A value with an embedded newline (e.g. from an XML formatting mistake) would previously pass Version.TryParse successfully, but later break the line-based custom-linker-options.txt file, producing a confusing 'No equals sign' error. Validate up front and report a clear error instead. Fixes #23626 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 655532d6-e35b-427a-bf9e-3ada6eb79ec6
There was a problem hiding this comment.
Pull request overview
This PR improves the MSBuild UX for invalid SupportedOSPlatformVersion / MinSupportedOSPlatformVersion values by failing early with a targeted diagnostic when those properties contain whitespace (notably newlines), preventing downstream linker-option parsing errors like “No equals sign”.
Changes:
- Validate
SupportedOSPlatformVersionandMinSupportedOSPlatformVersionfor embedded whitespace before using them during app manifest compilation. - Add a new localized error string (
E7187) for the whitespace validation failure. - Add a regression test covering a newline-prefixed
SupportedOSPlatformVersionvalue.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/CompileAppManifestTaskTests.cs | Adds a regression test asserting the new error message for whitespace in SupportedOSPlatformVersion. |
| msbuild/Xamarin.MacDev.Tasks/Tasks/CompileAppManifest.cs | Adds early validation to catch whitespace-containing version values and emit a clear error instead of producing broken downstream option files. |
| msbuild/Xamarin.Localization.MSBuild/MSBStrings.resx | Introduces the new E7187 error string used by the task. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| if (value.Any (char.IsWhiteSpace)) { | ||
| Log.LogError (MSBStrings.E7187 /* The value '{0}' for the property '{1}' is not a valid version number, because it contains whitespace. */, value, propertyName); | ||
| return false; | ||
| } |
| task.SupportedOSPlatformVersion = "\n13.0"; | ||
|
|
||
| ExecuteTask (task, expectedErrorCount: 1); | ||
| Assert.That (Engine.Logger.ErrorEvents [0].Message, Is.EqualTo ("The value '\n13.0' for the property 'SupportedOSPlatformVersion' is not a valid version number, because it contains whitespace.")); | ||
| } |
✅ 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 #45951d3] Test results 🔥Test results❌ Tests failed on VSTS: test results 1 tests crashed, 1 tests failed, 201 tests passed. Failures❌ windows tests🔥 Failed catastrophically on VSTS: test results - windows (no summary found). Html Report (VSDrops) Download ❌ Tests on macOS Tahoe (26) tests1 tests failed, 4 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ 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 |
Validate SupportedOSPlatformVersion and MinSupportedOSPlatformVersion before writing linker options.
This prevents embedded whitespace such as a newline from producing a confusing linker "No equals sign" error.
Fixes #23626
🤖 Pull request created by Copilot