[msbuild] Add a XamarinTask.SdkDevPath property and deduplicate a lot of code in child classes.#24523
Conversation
… of code in child classes.
There was a problem hiding this comment.
Pull request overview
This pull request consolidates the SdkDevPath property by moving it from multiple child task classes to the base XamarinTask class, eliminating code duplication. Additionally, it standardizes error messaging by using the new MSBStrings.E7169 error message format.
Changes:
- Moved
SdkDevPathproperty from child classes to the baseXamarinTaskclass - Added validation logic in the
SdkDevPathgetter to log an error if the property is not set - Updated
DetectSdkLocationto use the new pattern withnewkeyword and pass-through to base class - Added new error message
E7169for property validation
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| XamarinTask.cs | Added SdkDevPath property with validation logic to base class |
| XcodeTool.cs | Removed duplicate SdkDevPath property declaration |
| XcodeCompilerToolTask.cs | Removed custom SdkDevPath property and updated usage to access backing field |
| XcodeBuildTask.cs | Removed duplicate SdkDevPath property declaration |
| ScnTool.cs | Removed duplicate SdkDevPath property declaration |
| OptimizeImage.cs | Removed duplicate SdkDevPath property declaration |
| MetalLib.cs | Removed duplicate SdkDevPath property declaration |
| Metal.cs | Removed duplicate SdkDevPath property declaration |
| MergeAppBundles.cs | Removed duplicate SdkDevPath property declaration |
| LinkNativeCode.cs | Removed duplicate SdkDevPath property declaration |
| InstallNameTool.cs | Removed duplicate SdkDevPath property declaration |
| GetMlaunchArguments.cs | Removed duplicate SdkDevPath property declaration |
| DetectSdkLocation.cs | Updated to expose base class property as output with new keyword |
| DSymUtil.cs | Removed duplicate SdkDevPath property declaration |
| CoreMLCompiler.cs | Removed custom SdkDevPath property implementation |
| CompileSceneKitAssets.cs | Removed duplicate SdkDevPath property declaration |
| CompileNativeCode.cs | Removed duplicate SdkDevPath property declaration |
| AlTool.cs | Removed duplicate SdkDevPath property declaration |
| AOTCompile.cs | Removed duplicate SdkDevPath property declaration |
| MSBStrings.resx | Added new error message E7169 for property validation |
Comments suppressed due to low confidence (1)
msbuild/Xamarin.MacDev.Tasks/Tasks/XcodeCompilerToolTask.cs:212
- The code is directly accessing the protected field
sdkDevPathinstead of using the propertySdkDevPath. This is inconsistent with other task classes (e.g., DSymUtil, LinkNativeCode) that passSdkDevPathto ExecuteAsync. ChangesdkDevPathtoSdkDevPathto use the property accessor.
var rv = ExecuteAsync (tool, args, sdkDevPath, environment: environment).Result;
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.
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.
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.
✅ [CI Build #2955eba] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #2955eba] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ [CI Build #2955eba] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #2955eba] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #2955eba] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #2955eba] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
💻 [CI Build #2955eba] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #2955eba] Tests on macOS arm64 - Mac Tahoe (26) passed 💻✅ All tests on macOS arm64 - Mac Tahoe (26) passed. Pipeline on Agent |
✅ 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.
🚀 [CI Build #2955eba] Test results 🚀Test results✅ All tests passed on VSTS: test results. 🎉 All 117 tests passed 🎉 Tests counts✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
No description provided.