Use shared AdbRunner from android-tools for device listing by rmarinho · Pull Request #10916 · dotnet/android

rmarinho · 2026-03-11T17:42:42Z

Summary

Delegates the adb devices -l parsing, description building, and device/emulator merging logic from the GetAvailableAndroidDevices MSBuild task to the shared AdbRunner in Xamarin.Android.Tools.AndroidSdk (via the external/xamarin-android-tools submodule).

This removes ~200 lines of duplicated parsing/formatting/merging code from dotnet/android and consolidates it in the shared android-tools library where it can be reused by other consumers (e.g., the MAUI DevTools CLI).

Replaces #10880 — recreated under dev/ branch prefix so CI pipelines trigger.

Changes

Submodule update: external/xamarin-android-tools bumped to 1a26c0c (main branch). Includes:
- AdbRunner (Bump mono to mono-4.8.0-branch commit 8864f4c and install Mono.Btls.Interface #283) — shared ParseAdbDevicesOutput, BuildDeviceDescription, MergeDevicesAndEmulators, FormatDisplayName
- FormatDisplayName fix ([Mono.Android] API Level 25 enumification. #295) — preserves uppercase segments like XL, SE, FE
- CS8604 nullable fix ([build] Don't use config.guess and use "$HOST_CC -dumpmachine" instead #293)
- RS0026 fix ([aot] Rework construction and logging of AOT command #298) — split AcceptLicensesAsync overloads (was blocking CI)
- PublicAPI analyzer ([build] Install API-25 android.jar, Build API-25 bindings #294)
- Copilot instruction rules ([Xamarin.Android.Build.Tasks] Look in the AndroidSdk/extras folder for .aar files #296, Add API level 25 to known versions #297, [Xamarin.Android.Build.Tasks] Allow the cache'ing of the dex/jack compilation for support libraries. [DO NOT MERGE WIP] #299, [Xamarin.Android.NUnitLite] Generate NUnit 2 XML test results #301)
GetAvailableAndroidDevices.cs: Rewritten to delegate to AdbRunner.ParseAdbDevicesOutput(IEnumerable<string>), AdbRunner.BuildDeviceDescription, and AdbRunner.MergeDevicesAndEmulators instead of having its own parsing logic. Added ConvertToTaskItems to bridge AdbDeviceInfo -> ITaskItem.
GetAvailableAndroidDevicesTests.cs: Updated to use AdbRunner/AdbDeviceInfo directly instead of reflection. All tests preserved with equivalent coverage.

Commits (3)

4a0407b2c — Use shared AdbRunner from android-tools for device listing
af6af1ec8 — Update GetAvailableAndroidDevices.cs (hoist CreateTaskLogger() out of loop per @jonathanpeppers)
c8c34b869 — Bump xamarin-android-tools to 1a26c0c (latest main)

Review Feedback Addressed

Reviewer	Issue	Fix
@jonathanpeppers	`ParseAdbDevicesOutput` should take `IEnumerable<string>`	Done in android-tools #283 — avoids `string.Join` allocation
@jonathanpeppers	Add `Action<TraceLevel, string>` logger parameter	Done in android-tools #283 — routes debug messages through MSBuild logger
@jonathanpeppers	Hoist `CreateTaskLogger()` out of loop	Done in commit `af6af1ec8`

CI Status

Submodule at 1a26c0c which includes the RS0026 fix (#298). CI should pass.

Dependencies

android-tools Bump mono to mono-4.8.0-branch commit 8864f4c and install Mono.Btls.Interface #283 (AdbRunner) — ✅ MERGED
android-tools [Mono.Android] API Level 25 enumification. #295 (FormatDisplayName fix) — ✅ MERGED
android-tools [aot] Rework construction and logging of AOT command #298 (RS0026 fix) — ✅ MERGED
All included in submodule at 1a26c0c

cc @jonathanpeppers

Copilot

Pull request overview

Moves Android device/emulator listing in the GetAvailableAndroidDevices MSBuild task to use the shared AdbRunner implementation from the external/xamarin-android-tools submodule, reducing duplicated parsing/merging logic and aligning behavior across consumers.

Changes:

Updated external/xamarin-android-tools submodule to a commit that includes AdbRunner device parsing/merging and display-name formatting.
Rewrote GetAvailableAndroidDevices to delegate parsing/description building/merging to AdbRunner, adding a bridge method to emit ITaskItems.
Refactored tests to use AdbRunner/AdbDeviceInfo directly rather than invoking private task methods via reflection.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/Xamarin.Android.Build.Tasks/Tasks/GetAvailableAndroidDevices.cs	Delegates adb output parsing + device/emulator merge to `AdbRunner` and converts results into MSBuild `ITaskItem`s.
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/GetAvailableAndroidDevicesTests.cs	Updates tests to validate behavior via `AdbRunner` and `AdbDeviceInfo`, removing reflection-based testing.
external/xamarin-android-tools	Bumps submodule to a commit containing the shared `AdbRunner` logic used by the task/tests.

Comments suppressed due to low confidence (1)

external/xamarin-android-tools:1

The PR description notes this submodule commit causes RS0026 build failures (PublicAPI analyzer) until android-tools #298 is included. This submodule bump will keep CI red; consider updating the submodule to a commit that includes the #298 fix (or splitting this PR so the task change lands only once the submodule is buildable).

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-11T17:44:22Z

...droid.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/GetAvailableAndroidDevicesTests.cs

-			var result = task.FormatDisplayName ("emulator-5554", "pixel_9_pro_xl_api_36");
+			var result = AdbRunner.FormatDisplayName ("pixel_9_pro_xl_api_36");

 			Assert.AreEqual ("Pixel 9 Pro Xl API 36", result, "Should format complex names correctly");


The expected value appears inconsistent with the PR description (“preserves uppercase segments like XL, SE, FE”). If AdbRunner.FormatDisplayName now preserves XL, this assertion should expect Pixel 9 Pro XL API 36 to match the new behavior.

Suggested change

Assert.AreEqual ("Pixel 9 Pro Xl API 36", result, "Should format complex names correctly");

Assert.AreEqual ("Pixel 9 Pro XL API 36", result, "Should format complex names correctly");

The current assertion is correct. FormatDisplayName uses ToTitleCase which only preserves words that are already fully uppercase. Since the test input is pixel_9_pro_xl_api_36 (all lowercase), xl correctly becomes Xl (title-cased). Only if the input were Pixel_9_Pro_XL (uppercase XL) would it be preserved as XL.

The API part is correct because there's an ApiRegex post-processing step that converts Api → API after title-casing.

So: pixel_9_pro_xl_api_36 → Pixel 9 Pro Xl Api 36 → Pixel 9 Pro Xl API 36 ✅

jonathanpeppers

🤖 AI Review Summary

Verdict: ✅ LGTM

No issues found. This is a clean refactor that centralizes ~200 lines of duplicated parsing/formatting/merging logic into the shared AdbRunner library.

CI Status:

Linux ✅, Windows ✅ (public CI passing)
Mac ⏳ pending
Internal DevDiv build ❌ — likely the RS0026 AcceptLicensesAsync issue documented in the PR description (blocked on android-tools #298)

👍 Positive callouts:

Centralized duplication — Moves parsing, description building, and merging into Xamarin.Android.Tools.AndroidSdk where other consumers (MAUI DevTools CLI) can reuse it. This directly follows the review rule: "Centralize duplicate algorithms" (Postmortem #54).
Reflection eliminated — Tests previously used BindingFlags.NonPublic | BindingFlags.Instance reflection to access ParseAdbDevicesOutput. Now they call AdbRunner and ConvertToTaskItems directly. Much more maintainable.
ConvertToTaskItems is internal static — Good design: stateless conversion, testable without instantiating the full task, and internal visibility follows the convention for new helpers.
return !Log.HasLoggedErrors — ✅ Correct RunTask() pattern.
IReadOnlyList<AdbDeviceInfo> parameter — ✅ Follows the IReadOnlyList<T> return type convention.
CreateTaskLogger() hoisted out of loop — Good catch from earlier review feedback, avoids repeated allocation.
Mono formatting — Tabs, spaces before parens, IsNullOrEmpty() extensions, #nullable enable — all correct.

Review generated by android-reviewer from review guidelines.

Refactor GetAvailableAndroidDevices MSBuild task to delegate parsing and merging logic to AdbRunner from Xamarin.Android.Tools.AndroidSdk (via the xamarin-android-tools submodule at d3c269d). - Remove local Regex-based parsing, DeviceType enum, and helper methods - Use AdbRunner.ParseAdbDevicesOutput, BuildDeviceDescription, MergeDevicesAndEmulators, FormatDisplayName, MapAdbStateToStatus - Update tests to use AdbRunner APIs and AdbDeviceInfo/AdbDeviceType Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

jonathanpeppers · 2026-03-11T21:46:24Z

...droid.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/GetAvailableAndroidDevicesTests.cs

+			var result = AdbRunner.FormatDisplayName ("pixel_9_pro_xl_api_36");

 			Assert.AreEqual ("Pixel 9 Pro Xl API 36", result, "Should format complex names correctly");


Wait, I thought we fixed this? It says Xl.

If it only preserves casing of XL, like AdbRunner.FormatDisplayName ("pixel_9_pro_XL_api_36") do we have at least one test for that?

Right but we didn t change the test right? I was also confused, so you are saying we should have a special case for XL ?

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

rmarinho requested review from grendello, jonathanpeppers and simonrozsival as code owners March 11, 2026 17:42

Copilot AI review requested due to automatic review settings March 11, 2026 17:42

Copilot AI reviewed Mar 11, 2026

View reviewed changes

jonathanpeppers reviewed Mar 11, 2026

View reviewed changes

rmarinho and others added 2 commits March 11, 2026 20:10

Update GetAvailableAndroidDevices.cs

af6af1e

rmarinho force-pushed the dev/rumar/use-shared-adb-runner branch from 8c20e70 to e158bf1 Compare March 11, 2026 20:12

jonathanpeppers reviewed Mar 11, 2026

View reviewed changes

Bump xamarin-android-tools to 1a26c0c (latest main)

c8c34b8

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

rmarinho force-pushed the dev/rumar/use-shared-adb-runner branch from e158bf1 to c8c34b8 Compare March 12, 2026 09:01

jonathanpeppers approved these changes Mar 12, 2026

View reviewed changes

jonathanpeppers merged commit f715c14 into main Mar 12, 2026
6 checks passed

jonathanpeppers deleted the dev/rumar/use-shared-adb-runner branch March 12, 2026 13:39

jonathanpeppers added the copilot `copilot-cli` or other AIs were used to author this label Mar 12, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use shared AdbRunner from android-tools for device listing#10916

Use shared AdbRunner from android-tools for device listing#10916
jonathanpeppers merged 3 commits intomainfrom
dev/rumar/use-shared-adb-runner

rmarinho commented Mar 11, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

rmarinho Mar 11, 2026

Uh oh!

jonathanpeppers left a comment

Uh oh!

jonathanpeppers Mar 11, 2026 •

edited

Loading

Uh oh!

rmarinho Mar 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	Assert.AreEqual ("Pixel 9 Pro Xl API 36", result, "Should format complex names correctly");
	Assert.AreEqual ("Pixel 9 Pro XL API 36", result, "Should format complex names correctly");

		var result = AdbRunner.FormatDisplayName ("pixel_9_pro_xl_api_36");

		Assert.AreEqual ("Pixel 9 Pro Xl API 36", result, "Should format complex names correctly");

Conversation

rmarinho commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Commits (3)

Review Feedback Addressed

CI Status

Dependencies

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

rmarinho Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers left a comment

Choose a reason for hiding this comment

🤖 AI Review Summary

Uh oh!

jonathanpeppers Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rmarinho Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rmarinho commented Mar 11, 2026 •

edited

Loading

jonathanpeppers Mar 11, 2026 •

edited

Loading