-
Notifications
You must be signed in to change notification settings - Fork 554
[msbuild/dotnet] Add support for listing the devices and simulators available to run on. #24279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: net11.0
Are you sure you want to change the base?
Conversation
|
@jonathanpeppers I think the spec needs to be updated so that the correct RuntimeIdentifier is set, depending on what the user chose - maybe the returned item group with the devices can have a 'RuntimeIdentifier' metadata that, if specified, is set for the subsequent build/run operation? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for listing available devices and simulators for iOS and tvOS apps, enabling device selection via the dotnet run --device command. The implementation follows the .NET SDK's device selection specification for MAUI-style device management.
Key Changes:
- New
GetAvailableDevicesMSBuild task that queriesdevicectlandsimctlto retrieve available physical devices and simulators - New
ComputeAvailableDevicestarget that can be invoked to list all compatible devices - Property rename from
_DeviceNametoDevicefor specifying target devices
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/common/JsonExtensions.cs | New utility class with JSON parsing extension methods for processing devicectl/simctl output |
| tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/GetAvailableDevicesTest.cs | Comprehensive test suite covering device filtering scenarios (platform, OS version, device family) |
| msbuild/Xamarin.MacDev.Tasks/Tasks/GetAvailableDevices.cs | Main task implementation that queries devices, applies filters, and returns available devices |
| msbuild/Xamarin.Shared/Xamarin.Shared.targets | Moved _AppBundleManifestPath property definition to allow access by GetAvailableDevices |
| msbuild/Xamarin.MacDev.Tasks/Xamarin.MacDev.Tasks.csproj | Added JsonExtensions.cs to the project |
| dotnet/targets/Xamarin.Shared.Sdk.targets | Added ComputeAvailableDevices target and Device property with backward compatibility for _DeviceName |
| tests/common/shared-dotnet.mk | Updated to use Device property instead of deprecated _DeviceName |
| docs/building-apps/build-targets.md | Documentation for the new ComputeAvailableDevices target |
| docs/building-apps/build-properties.md | Documentation for the new Device property |
| Target that queries available devices and simulators. | ||
| This target is called by 'dotnet run' to support device selection. | ||
| Returns @(Devices) items with metadata: ??????? |
Copilot
AI
Nov 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment states "Returns @(Devices) items with metadata: ???????" which appears to be a placeholder that was not filled in. The metadata should be documented here (Description, Type, OSVersion, UDID) similar to what's in the build-targets.md documentation.
| Returns @(Devices) items with metadata: ??????? | |
| Returns @(Devices) items with metadata: Description, Type, OSVersion, UDID |
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.
| <DeviceName Condition="'$(DeviceName)' == ''">$(_DeviceName)</DeviceName> | ||
| <!-- Try to keep _DeviceName working for a while yet --> | ||
| <Device Condition="'$(Device)' == ''">$(_DeviceName)</Device> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need to keep $(DeviceName) around, it's public? Maybe it's not really used/documented.
| Condition="'$(IsMacEnabled)' == 'true'" | ||
| SessionId="$(BuildSessionId)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a question... Will this work on Windows? Ideally, it could, but that could be future 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.
700ce36 to
2ccb1f6
Compare
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.
…mulators available to run on. This consists of two parts: * Add an MSBuild target that lists all the available devices (`ComputeAvailableDevices`), by returning them in a `$(Devices)` item group. * Add support for the `$(Device)` property to specify the device or simulator to use. Regarding the device list, we'll filter the returned list by: * Platform (don't return an Apple TV simulator for an iOS app). * Minimum OS version (not return an iOS 18.0 device when the app's minimum OS version is 26.0). * Only devices that are actually available, as reported by `devicectl` or `simctl`. References: * dotnet/android#10576 * https://github.com/dotnet/sdk/blob/2b9fc02a265c735f2132e4e3626e94962e48bdf5/documentation/specs/dotnet-run-for-maui.md Fixes #23995.
2ccb1f6 to
6823a2b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
| [Output] | ||
| public ITaskItem [] DiscardedDevices { get; set; } = Array.Empty<ITaskItem> (); | ||
|
|
||
| [Output] |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The RuntimeIdentifier property has the [Output] attribute but is being used as an input parameter. Output parameters should only be set by the task, not read as input. This property should not have the [Output] attribute if it's meant to be an input parameter for filtering devices.
| [Output] |
| } | ||
| } | ||
|
|
||
| Version.TryParse (productVersion, out var minimumOSVersion); |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential null reference exception: Version.TryParse may leave minimumOSVersion as null if parsing fails, but it's used directly on line 271 without a null check. Consider using a default value like 'new Version (0, 0)' if parsing fails.
| Version.TryParse (productVersion, out var minimumOSVersion); | |
| var minimumOSVersion = new Version (0, 0); | |
| Version.TryParse (productVersion, out minimumOSVersion); |
| System.Threading.Tasks.Task<IEnumerable<DeviceInfo>> RunSimCtlAsync () | ||
| { | ||
| var doc = ExecuteCtlToJsonAsync ("simctl", "list", "--json").Result; |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using .Result on an async task in a non-async method can cause deadlocks. The RunSimCtlAsync method should be declared as async and use 'await' instead of '.Result'. The method signature on line 277 should be 'async System.Threading.Tasks.Task<IEnumerable> RunSimCtlAsync ()' and line 279 should use 'await ExecuteCtlToJsonAsync'.
| Target that queries available devices and simulators. | ||
| This target is called by 'dotnet run' to support device selection. | ||
| Returns @(Devices) items with metadata: ??????? |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incomplete documentation: The comment on line 150 says "Returns @(Devices) items with metadata: ???????" which indicates the metadata documentation is incomplete. The metadata fields should be documented here as they are in the build-targets.md file (Description, Type, OSVersion, UDID, RuntimeIdentifier).
| Returns @(Devices) items with metadata: ??????? | |
| Returns @(Devices) items with metadata: Description, Type, OSVersion, UDID, RuntimeIdentifier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dotnet run will only use %(RuntimeIdentifier), but I put all kinds of other metadata that Android had mainly for debugging purposes.
✅ [CI Build #6823a2b] Build passed (Build packages) ✅Pipeline on Agent |
✅ [PR Build #6823a2b] Build passed (Detect API changes) ✅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 |
✅ [CI Build #6823a2b] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #6823a2b] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #6823a2b] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
❌ [CI Build #6823a2b] Tests on macOS arm64 - Mac Tahoe (26) failed ❌Failed tests are:
Pipeline on Agent |
🔥 [CI Build #6823a2b] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 4 tests failed, 113 tests passed. Failures❌ linker tests1 tests failed, 43 tests passed.Failed tests
Html Report (VSDrops) Download ❌ monotouch tests (macOS)1 tests failed, 8 tests passed.Failed tests
Html Report (VSDrops) Download ❌ msbuild tests1 tests failed, 1 tests passed.Failed tests
Html Report (VSDrops) Download ❌ xcframework tests1 tests failed, 3 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
jonathanpeppers
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good! There is a possible exception the JSON parsing can throw, but might not happen in real life.
| Target that queries available devices and simulators. | ||
| This target is called by 'dotnet run' to support device selection. | ||
| Returns @(Devices) items with metadata: ??????? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dotnet run will only use %(RuntimeIdentifier), but I put all kinds of other metadata that Android had mainly for debugging purposes.
| await ExecuteAsync (Log, "xcrun", arguments, sdkDevPath: SdkDevPath, cancellationToken: cancellationTokenSource!.Token); | ||
| return File.ReadAllText (tmpfile); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the file is long, you could avoid making a large string if JsonDocument opened a Stream instead.
Do you know how long the task takes? If it's fast, probably doesn't matter.
|
|
||
| public static string? GetStringProperty (this JsonElement? element, string propertyName) | ||
| { | ||
| return GetNullableProperty (element, propertyName)?.GetString (); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only thing I've run into is if you got values like JsonValueKind.True, JsonValueKind.Number, JsonValueKind.Null, I think it will throw InvalidOperationException:
I had to do stuff like this in the past:
static string? GetJsonValueAsString (JsonElement element) =>
element.ValueKind switch {
JsonValueKind.String => element.GetString (),
JsonValueKind.True => "true",
JsonValueKind.False => "false",
JsonValueKind.Number or JsonValueKind.Object or JsonValueKind.Array => element.GetRawText (),
_ => null, // Null or Undefined
};
This consists of two parts:
ComputeAvailableDevices), by returning them in a$(Devices)item group.$(Device)property to specify the device or simulator to use.Regarding the device list, we'll filter the returned list by:
devicectlorsimctl.References:
$(Device)andComputeAvailableDevicesMSBuild target android#10576Fixes #23995.