Use shared SimctlOutputParser from Xamarin.MacDev for simctl JSON parsing#24856
Use shared SimctlOutputParser from Xamarin.MacDev for simctl JSON parsing#24856
Conversation
Refactor GetAvailableDevices.RunSimCtlAsync to use the shared SimctlOutputParser from the Xamarin.MacDev submodule instead of inline JSON parsing. This: - Reuses tested parsing logic (65+ unit tests in macios-devtools) - Shares the parser across both MSBuild tasks and the MAUI CLI tool - Keeps devicetypes parsing inline (not yet in shared parser) - Preserves all MSBuild metadata, filtering, and discarding logic Depends on dotnet/macios-devtools PR #158 being merged to main. The submodule currently points to feature/simulator-management. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Refactor GetAvailableDevices to use all three shared parsers: - SimctlOutputParser.ParseDevices: simulator device extraction - SimctlOutputParser.ParseRuntimes: runtime extraction - SimctlOutputParser.ParseDeviceTypes: devicetypes extraction - DeviceCtlOutputParser.ParseDevices: physical device extraction This eliminates all inline JSON parsing (System.Text.Json and Xamarin.Utils JsonExtensions no longer needed in this file). The only remaining code is MSBuild-specific TaskItem mapping, platform filtering, RuntimeIdentifier computation, and CanRunArm64. Net result: -45 lines, zero inline JSON parsing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix NU1605 package downgrade error: Xamarin.MacDev references System.Text.Json 9.0.4, while Xamarin.MacDev.Tasks pinned 8.0.5. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- XamarinTask: match ICustomLogger nullable signatures (Exception?, object?[]) - GetAvailableDevices: add using Xamarin.Utils for ApplePlatform - Update submodule to latest (6a2d99b) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
AppleSdkSettings was marked [Obsolete] in macios-devtools PR #140 (XcodeLocator). Existing usages in Sdks.cs, CompileAppManifest.cs, and DetectSdkLocation.cs are intentional pending full migration. TreatWarningsAsErrors=true promotes the warning to an error. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Point submodule from feature/simulator-management branch to main, which now includes the merged simulator management APIs. Co-authored-by: Copilot <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.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
This looks related to the PR:
|
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.
|
The same problem exists, there's a number of failures like this:
|
…devtools-simctl-parser
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 #ad93a11] Build failed (Build macOS tests) 🔥Build failed for the job 'Build macOS tests' (with job status 'Failed') Pipeline on Agent |
🔥 [CI Build #ad93a11] Test results 🔥Test results❌ Tests failed on VSTS: test results 5 tests crashed, 55 tests failed, 76 tests passed. Failures❌ dotnettests tests (iOS)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ dotnettests tests (MacCatalyst)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ dotnettests tests (macOS)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ dotnettests tests (tvOS)1 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ introspection tests5 tests failed, 1 tests passed.Failed tests
Html Report (VSDrops) Download ❌ linker tests44 tests failed, 0 tests passed.Failed tests
Html Report (VSDrops) Download ❌ msbuild tests1 tests failed, 1 tests passed.Failed tests
Html Report (VSDrops) Download ❌ windows tests1 tests failed, 2 tests passed.Failed tests
Html Report (VSDrops) Download ❌ Tests on macOS Monterey (12) tests🔥 Failed catastrophically on VSTS: test results - mac_monterey (no summary found). Html Report (VSDrops) Download ❌ Tests on macOS Ventura (13) tests🔥 Failed catastrophically on VSTS: test results - mac_ventura (no summary found). Html Report (VSDrops) Download ❌ Tests on macOS Sonoma (14) tests🔥 Failed catastrophically on VSTS: test results - mac_sonoma (no summary found). Html Report (VSDrops) Download ❌ Tests on macOS Sequoia (15) tests🔥 Failed catastrophically on VSTS: test results - mac_sequoia (no summary found). Html Report (VSDrops) Download ❌ Tests on macOS Tahoe (26) tests🔥 Failed catastrophically on VSTS: test results - mac_tahoe (no summary found). Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download macOS testsPipeline on Agent |
…devtools-simctl-parser
Two issues fixed: 1. System.Text.Json 9.0.4 was being ILMerged into the netstandard2.0 tasks assembly. Internal types like PooledByteBufferWriter from 9.0.4 are incompatible when loaded in .NET 10 runtime (which has its own STJ with different base type signatures), causing TypeLoadExceptions. Fixed by excluding System.Text.Json and System.Text.Encodings.Web from ILMerge - these are available at runtime from the host framework or as side-by-side files in the task output directory. 2. The new SimctlOutputParser from Xamarin.MacDev now correctly extracts OS version from simctl runtime identifiers (e.g. 'iOS-26-0' -> '26.0') for unavailable devices. Updated test expectations accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Revert the ILMerge exclusion approach and instead downgrade System.Text.Json from 9.0.4 to 8.0.5 in both the tasks project and the Xamarin.MacDev submodule. Version 9.0.4 internal types (PooledByteBufferWriter) are incompatible when ILMerged into a netstandard2.0 assembly and loaded in .NET 10 runtime. Version 8.0.5 does not have this issue. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Refactors the GetAvailableDevices MSBuild task to rely on shared simctl/devicectl JSON parsing from Xamarin.MacDev model/parsers, updating tests to match the resulting metadata.
Changes:
- Replace inline
simctlJSON traversal withSimctlOutputParser.Parse*calls and typed model objects. - Replace inline
devicectlJSON extraction withDeviceCtlOutputParser.ParseDevices. - Update
GetAvailableDevicestests to reflectOSVersionmetadata values coming from the shared parser(s).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/GetAvailableDevicesTest.cs | Updates expected OSVersion metadata for certain discarded devices. |
| msbuild/Xamarin.MacDev.Tasks/Xamarin.MacDev.Tasks.csproj | Adds CS0618 suppression (and retains existing package refs). |
| msbuild/Xamarin.MacDev.Tasks/Tasks/XamarinTask.cs | Adjusts ICustomLogger explicit interface implementation signatures for nullable/params. |
| msbuild/Xamarin.MacDev.Tasks/Tasks/GetAvailableDevices.cs | Switches to shared parsers/models for simctl and devicectl JSON parsing while keeping device filtering/business logic. |
| var runtimePlatform = hasRuntime ? runtimeInfo!.Platform : string.Empty; | ||
| var runtimeVersion = hasRuntime ? runtimeInfo!.Version : device.OSVersion; | ||
| var supportedArchitectures = hasRuntime ? runtimeInfo!.SupportedArchitectures : new List<string> (); |
There was a problem hiding this comment.
Avoid using the null-forgiving operator here. TryGetValue flow analysis already ensures runtimeInfo is non-null when hasRuntime is true, so the ! is unnecessary and conflicts with the repo’s nullable guidance. Prefer restructuring to only use runtimeInfo inside the if (hasRuntime) block (or declare runtimeInfo as non-nullable) and remove the ! usages.
| var runtimePlatform = hasRuntime ? runtimeInfo!.Platform : string.Empty; | |
| var runtimeVersion = hasRuntime ? runtimeInfo!.Version : device.OSVersion; | |
| var supportedArchitectures = hasRuntime ? runtimeInfo!.SupportedArchitectures : new List<string> (); | |
| var runtimePlatform = ""; | |
| var runtimeVersion = device.OSVersion; | |
| var supportedArchitectures = new List<string> (); | |
| if (hasRuntime) { | |
| runtimePlatform = runtimeInfo.Platform; | |
| runtimeVersion = runtimeInfo.Version; | |
| supportedArchitectures = runtimeInfo.SupportedArchitectures; | |
| } |
| <NoWarn>$(NoWarn);NU1701</NoWarn> <!-- Package 'Microsoft.Build 16.10.0' was restored using '.NETFramework,Version=v4.6.1, .NETFramework,Version=v4.6.2, .NETFramework,Version=v4.7, .NETFramework,Version=v4.7.1, .NETFramework,Version=v4.7.2, .NETFramework,Version=v4.8, .NETFramework,Version=v4.8.1' instead of the project target framework '.NETStandard,Version=v2.0'. This package may not be fully compatible with your project. --> | ||
| <NoWarn>$(NoWarn);MSB3277</NoWarn> <!-- warning MSB3277: Found conflicts between different versions of "System.Reflection.Metadata" that could not be resolved. --> | ||
| <NoWarn>$(NoWarn);8002</NoWarn> <!-- Referenced projects aren't signed: this doesn't matter, because we use ILMerge to merge into a single assembly which we sign --> | ||
| <NoWarn>$(NoWarn);CS0618</NoWarn> <!-- AppleSdkSettings is [Obsolete] in favor of XcodeLocator; suppress until migration is complete --> |
There was a problem hiding this comment.
Project-wide suppression of CS0618 will hide any other newly introduced [Obsolete] usages in this project. Since the obsolete warning is currently coming from a small number of AppleSdkSettings call sites, prefer scoping the suppression to those specific files/lines (e.g., #pragma warning disable CS0618 around the calls) or suppressing only where needed, so other obsolete warnings still surface.
| <NoWarn>$(NoWarn);CS0618</NoWarn> <!-- AppleSdkSettings is [Obsolete] in favor of XcodeLocator; suppress until migration is complete --> |
| <NoWarn>$(NoWarn);NU1701</NoWarn> <!-- Package 'Microsoft.Build 16.10.0' was restored using '.NETFramework,Version=v4.6.1, .NETFramework,Version=v4.6.2, .NETFramework,Version=v4.7, .NETFramework,Version=v4.7.1, .NETFramework,Version=v4.7.2, .NETFramework,Version=v4.8, .NETFramework,Version=v4.8.1' instead of the project target framework '.NETStandard,Version=v2.0'. This package may not be fully compatible with your project. --> | ||
| <NoWarn>$(NoWarn);MSB3277</NoWarn> <!-- warning MSB3277: Found conflicts between different versions of "System.Reflection.Metadata" that could not be resolved. --> | ||
| <NoWarn>$(NoWarn);8002</NoWarn> <!-- Referenced projects aren't signed: this doesn't matter, because we use ILMerge to merge into a single assembly which we sign --> | ||
| <NoWarn>$(NoWarn);CS0618</NoWarn> <!-- AppleSdkSettings is [Obsolete] in favor of XcodeLocator; suppress until migration is complete --> |
There was a problem hiding this comment.
PR description mentions bumping System.Text.Json to 9.0.4, but this project file still references System.Text.Json 8.0.5. Either update the description, or include the version bump in the actual changes if it’s still required to address NU1605 from the submodule dependency graph.
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.
…devtools-simctl-parser
The Xamarin.MacDev submodule renamed its assembly from 'Xamarin.MacDev' to 'Xamarin.Apple.Tools.MaciOS'. Update the ILMerge targets to use the new assembly filename so that it gets correctly merged into Xamarin.MacDev.Tasks.dll. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🔥 [PR Build #02cf72a] Build failed (Detect API changes) 🔥Build failed for the job 'Detect API changes' (with job status 'Failed') Pipeline on Agent |
|
🔥 Unable to find the contents for the comment: D:\a\1\s\change-detection\results\gh-comment.md does not exist :fire Pipeline on Agent |
🔥 [PR Build #02cf72a] Build failed (Build packages) 🔥Build failed for the job 'Build packages' (with job status 'Failed') Pipeline on Agent |
Summary
This PR refactors
GetAvailableDevices.RunSimCtlAsyncto use the sharedSimctlOutputParserfrom theXamarin.MacDevsubmodule (dotnet/macios-devtools) instead of inline JSON parsing.Replaces #24812 — recreated under
dev/branch prefix so CI pipelines trigger.What this PR does
SimctlOutputParserand model classes (main branch, SHA10a0c3c)RunSimCtlAsyncwith:SimctlOutputParser.ParseDevices(json)— typedSimulatorDeviceInfoobjectsSimctlOutputParser.ParseRuntimes(json)— typedSimulatorRuntimeInfoobjectsSimctlOutputParser.ParseDeviceTypes(json)— typedSimulatorDeviceTypeInfoobjectsDeviceCtlOutputParser.ParseDevices(json)— typed physical device infoBuild fixes included
System.Text.Jsonto 9.0.4 (NU1605 fix from submodule dependency)AppleSdkSettingsobsolete warningsDependencies
mainStatus
xamarin-macios-cimain pipeline passed earliercc @rolfbjarne