Merge dotnet/android-tools into external/xamarin-android-tools with full history

.github/copilot-instructions.md

@@ -18,6 +18,17 @@
 - **Run tests:** `dotnet-local.cmd test bin/TestDebug/net9.0/Xamarin.Android.Build.Tests.dll --filter Name~TestName`
 - **Device tests:** `dotnet-local.cmd test bin/TestDebug/MSBuildDeviceIntegration/net9.0/MSBuildDeviceIntegration.dll`
 
+### external/xamarin-android-tools/
+
+The in-tree Android tools libraries include SDK/JDK discovery (`AndroidSdkInfo`, `JdkInfo`, SDK manifest parsing, `AdbRunner`, `EmulatorRunner`) and MSBuild task infrastructure (`AndroidTask`, `AndroidToolTask`, `AsyncTask`, `Files`, `ProcessUtils`, `FileUtil`, `MemoryStreamPool`). They target `netstandard2.0` and/or modern .NET, so verify API availability across all target frameworks before using newer BCL APIs. Prefer `ANDROID_HOME` for new Android SDK environment handling; `ANDROID_SDK_ROOT` is deprecated and should only remain for compatibility.
+
+Useful focused checks:
+```sh
+dotnet build external/xamarin-android-tools/Xamarin.Android.Tools.sln
+dotnet test external/xamarin-android-tools/tests/Xamarin.Android.Tools.AndroidSdk-Tests/Xamarin.Android.Tools.AndroidSdk-Tests.csproj
+dotnet test external/xamarin-android-tools/tests/Microsoft.Android.Build.BaseTasks-Tests/Microsoft.Android.Build.BaseTasks-Tests.csproj
+```
+
 ## Critical Rules
 
 **Never use `git commit --amend`:** Always create new commits. The user will squash or fixup as needed.

.github/skills/android-reviewer/SKILL.md

@@ -9,7 +9,7 @@ description: >-
 
 # Android PR Reviewer
 
-Review PRs against guidelines distilled from past reviews by senior maintainers of dotnet/android.
+Review PRs against guidelines distilled from past reviews by senior maintainers of dotnet/android, including the in-tree shared tooling under `external/xamarin-android-tools/`.
 
 ## Review Mindset
 
@@ -70,7 +70,7 @@ Based on the file types identified in step 2, read the appropriate rule files fr
 
 **Conditionally load based on changed file types:**
 - `references/csharp-rules.md` — When any `.cs` files changed. Covers nullable, async, error handling, performance, and code organization.
-- `references/msbuild-rules.md` — When `.targets`, `.props`, `.projitems`, or `.csproj` files changed, or when MSBuild task C# files changed (e.g., files under `src/Xamarin.Android.Build.Tasks/`).
+- `references/msbuild-rules.md` — When `.targets`, `.props`, `.projitems`, or `.csproj` files changed, or when MSBuild task C# files changed (e.g., files under `src/Xamarin.Android.Build.Tasks/` or `external/xamarin-android-tools/src/Microsoft.Android.Build.BaseTasks/`).
 - `references/native-rules.md` — When `.c`, `.cpp`, `.h`, or `.hpp` files changed. Covers memory management, C++ best practices, symbol visibility, and platform-specific code.
 - `references/interop-rules.md` — When both C# and native files changed, when the diff contains P/Invoke or JNI interop code (e.g., `DllImport`, `[Register]` attribute changes, `JNIEnv` calls, `[MarshalAs]`, `[StructLayout]`, `JniObjectReference`, `JniPeerMembers`, `JniTransition`), or when files under `external/Java.Interop/`, `src/Mono.Android/`, or `src/native/` changed.
 - `references/testing-rules.md` — When test files changed (e.g., files under `tests/`, `**/Tests/`, or test project directories).

.github/skills/android-reviewer/references/csharp-rules.md

@@ -4,6 +4,15 @@ General C# guidance applicable to any .NET repository.
 
 ---
 
+## Target Framework Compatibility
+
+| Check | What to look for |
+|-------|-----------------|
+| **Oldest TFM must compile** | Code under `external/xamarin-android-tools/` may target `netstandard2.0` and modern .NET. Verify every API and overload against the oldest target framework; common traps include cancellation-token overloads such as `HttpContent.ReadAsStringAsync(CancellationToken)`, modern `ProcessStartInfo.ArgumentList` usage without the existing fallback helpers, and newer language/BCL features that need `#if` guards or polyfills. |
+| **Prefer existing compatibility helpers** | Use repository helpers such as `ProcessUtils`, `FileUtil`, and nullable extension methods instead of direct modern-BCL calls when they provide `netstandard2.0` fallbacks or better annotations. |
+
+---
+
 ## Nullable Reference Types
 
 | Check | What to look for |
@@ -53,6 +62,8 @@ General C# guidance applicable to any .NET repository.
 |-------|-----------------|
 | **Avoid unnecessary allocations** | Don't create intermediate collections when LINQ chaining or a single list would do. Char arrays for `string.Split()` should be `static readonly` fields. |
 | **ArrayPool for large buffers** | Buffers ≥ 1 KB should use `ArrayPool<byte>.Shared.Rent()` with `try`/`finally` return. Large allocations go to the LOH and are expensive to GC. |
+| **Use existing pools for repeated streams/buffers** | In Android tools code, prefer existing pooling helpers such as `MemoryStreamPool`/`ObjectPool<T>` for repeated temporary streams or buffers instead of allocating new instances in hot paths. If mutable buffers are reused with single-caller assumptions, document the thread-safety invariant. |
+| **Static `HttpClient`** | `HttpClient` instances should be `static readonly` fields, not per-instance objects that are repeatedly constructed or disposed. Per-instance clients can exhaust sockets; inject clients only when there is a real caller requirement. |
 | **`HashSet.Add()` already handles duplicates** | Calling `.Contains()` before `.Add()` does the hash lookup twice. Just call `.Add()`. (Postmortem `#41`) |
 | **Don't wrap a value in an interpolated string** | `$"{someString}"` creates an unnecessary `string.Format` call when `someString` is already a string. (Postmortem `#42`) |
 | **Consider allocations when choosing types** | `Stopwatch` is heap-allocated; `DateTime`/`ValueStopwatch` is a struct. On hot paths or startup, prefer value types. (Postmortem `#39`) |

.github/skills/android-reviewer/references/repo-conventions.md

@@ -40,10 +40,13 @@ and merge conflicts.
 | Check | What to look for |
 |-------|-----------------|
 | **Use existing utilities** | Check `MonoAndroidHelper`, `FileUtil`, `PathUtil`, `ITaskItemExtensions`, and other utilities before writing new helpers. Duplicating existing logic is the most expensive AI pattern. |
+| **Use Android tools utilities** | In `external/xamarin-android-tools/`, process execution should go through `ProcessUtils`, file extraction/download/checksum/path helpers through `FileUtil`, and repeated buffers/streams through `ObjectPool<T>` or `MemoryStreamPool` where applicable. |
 | **`Log.LogDebugMessage` for diagnostics** | Use `Log.LogDebugMessage(…)` for verbose/debug output, not `Console.WriteLine` or `Debug.WriteLine`. Don't spam logcat with messages that fire on every type lookup miss. (Postmortem `#9`) |
+| **Android tools logger delegate** | SDK/JDK discovery helpers in `external/xamarin-android-tools/` commonly use `Action<TraceLevel, string>? logger` (see `AndroidSdkInfo.DefaultConsoleLogger`) so callers can route diagnostics into MSBuild or IDE logs. Don't replace this with `Console.WriteLine` or `Debug.WriteLine`. |
 | **Return `IReadOnlyList<T>`** | Public methods should return `IReadOnlyList<T>` or `IReadOnlyCollection<T>` instead of mutable `List<T>`. |
 | **Prefer C# pattern matching** | Use `is`, `switch` expressions, and property patterns instead of `if`/`else` type-check chains. |
 | **Structured args, not string interpolation** | Process arguments should be `IEnumerable<string>` or use `ArgumentList`, not a single interpolated string. |
+| **Android SDK environment variables** | In `external/xamarin-android-tools/`, use `EnvironmentVariableNames.AndroidHome`/`ANDROID_HOME` for new SDK-root behavior. `ANDROID_SDK_ROOT` is deprecated by Android and should only be read for backward compatibility. |
 | **Method names must reflect behavior** | If `CreateFoo()` sometimes returns an existing instance, rename it `GetOrCreateFoo()` or `GetFoo()`. (Postmortem `#4`) |
 | **Choose collision-proof names** | Types and constants that could collide with user code or Android concepts need disambiguating prefixes (e.g., `__Xamarin.Android.Resource.Designer` with a `__` prefix). (Postmortem `#2`) |
 | **Don't assume transitive assembly references** | An assembly containing an `Activity` subclass does not necessarily reference `Mono.Android.dll` directly — the reference may be transitive. Skipping assemblies based on direct reference checks can break user code. (Postmortem `#64`) |
@@ -93,6 +96,7 @@ and merge conflicts.
 | Check | What to look for |
 |-------|-----------------|
 | **XmlReader over LINQ XML** | For forward-only XML parsing (manifests, config files), prefer `XmlReader` — it's streaming and allocation-free. `XElement`/`XDocument` builds a full DOM tree. |
+| **SDK manifest parsing stays streaming** | Android SDK repository manifests can be large; keep `external/xamarin-android-tools/` manifest parsing on streaming `XmlReader`-style paths unless there is measured evidence a DOM is acceptable. |
 | **p/invoke over process spawn** | For single syscalls like `chmod`, use `[DllImport("libc")]` instead of spawning a child process. Process creation is orders of magnitude more expensive. |
 | **Use `Files.CopyIfStringChanged()`** | Don't write to a file if the content hasn't changed — it breaks incremental builds by updating timestamps. (Postmortem `#53`) |
 | **Don't remove caches without measurement** | If a cache (like `TypeDefinitionCache`) had a measured perf win, removing it requires proving the replacement provides equivalent caching. (Postmortem `#57`) |

.github/skills/android-reviewer/references/security-rules.md

@@ -11,6 +11,7 @@ I/O, archives, or process execution.
 |-------|-----------------|
 | **Zip Slip protection** | Archive extraction must validate that every entry path, after `Path.GetFullPath()`, resolves under the destination directory. Never use `ZipFile.ExtractToDirectory()` for untrusted archives without entry-by-entry validation. |
 | **Path traversal** | `StartsWith()` checks on paths must normalize with `Path.GetFullPath()` first. A path like `C:\Program Files\..\Users\evil` bypasses naive prefix checks. Also check for directory boundary issues (`C:\Program FilesX` matching `C:\Program Files`). |
+| **Mandatory checksum verification** | Downloads or archive installs in Android tools code must not proceed unverified when checksum/hash data is expected but missing or mismatched. Fail closed with an actionable error. |
 
 ---
 

.github/skills/android-reviewer/references/testing-rules.md

@@ -16,6 +16,7 @@ Guidance for test code. The repo-specific conventions (e.g., `BaseTest`,
 | **Test assertions must be specific** | `Assert.IsNotNull(result)` or `Assert.IsTrue(success)` don't tell you what went wrong. Prefer `Assert.AreEqual(expected, actual)` or NUnit constraints (`Assert.That` with `Does.Contain`, `Is.EqualTo`, etc.) for richer failure messages. |
 | **Deterministic test data** | Tests should not depend on system locale, timezone, or current date. Use explicit `CultureInfo.InvariantCulture` and hardcoded dates when testing formatting. |
 | **Test edge cases** | Empty collections, null inputs, boundary values, concurrent calls, and very large inputs should all be considered. If the PR only tests the happy path, suggest edge cases. |
+| **Android tools SDK/JDK fixtures** | Tests under `external/xamarin-android-tools/tests/` commonly build isolated fake SDK/JDK layouts and platform-specific tool scripts (`.bat` on Windows, shell scripts on Unix). Keep these fixtures self-contained and cleaned up in setup/teardown rather than depending on the developer machine's installed SDK/JDK. |
 | **Generator tests must include Invoker types** | Tests for generated binding code (under `external/Java.Interop/tools/generator/` and related test projects) should verify both the interface/class output and the `*Invoker` type behavior. Invoker codegen has historically had subtle bugs with default interface methods and virtual dispatch. |
 | **JVM-dependent tests** | Tests that require a running JVM must be in projects that configure the JVM environment (e.g., `Java.Interop-Tests`). Verify that test classes requiring a JVM are not placed in unit-test-only projects, where they will silently skip or fail with obscure errors. |
 | **Expected codegen output tests** | Generator tests that compare against expected output files should be updated when the expected format changes. Stale expected-output files cause spurious test failures that mask real regressions. |

.gitmodules

@@ -22,10 +22,6 @@
     path = external/robin-map
     url = https://github.com/xamarin/robin-map
     branch = master
-[submodule "external/xamarin-android-tools"]
-    path = external/xamarin-android-tools
-    url = https://github.com/dotnet/android-tools
-    branch = main
 [submodule "external/xxHash"]
     path = external/xxHash
     url = https://github.com/Cyan4973/xxHash.git

build-tools/automation/azure-pipelines-public.yaml

@@ -429,6 +429,20 @@ stages:
       - ImageOverride -equals ACES_VM_SharedPool_Tahoe
       os: macOS
 
+# Android Tools Tests Stage
+- template: /build-tools/automation/yaml-templates/stage-xamarin-android-tools-tests.yaml@self
+  parameters:
+    windowsPool:
+      name: $(NetCorePublicPoolName)
+      demands:
+      - ImageOverride -equals $(WindowsPoolImageNetCorePublic)
+      os: windows
+    macPool:
+      name: AcesShared
+      demands:
+      - ImageOverride -equals ACES_VM_SharedPool_Tahoe
+      os: macOS
+
 # MAUI Tests Stage
 - stage: maui_tests
   displayName: MAUI Tests

build-tools/automation/azure-pipelines.yaml

@@ -110,6 +110,17 @@ extends:
           image: $(WindowsPoolImage1ESPT)
           os: windows
 
+    - template: /build-tools/automation/yaml-templates/stage-xamarin-android-tools-tests.yaml@self
+      parameters:
+        # The internal Xamarin.Android pipeline uses 1ES Pipeline Templates, which
+        # require Windows jobs to run on a 1ES-hosted pool. macOS is fine on the
+        # hosted Azure Pipelines pool as long as `os: macOS` is set (which is now
+        # the template default).
+        windowsPool:
+          name: MAUI-1ESPT
+          image: $(WindowsPoolImage1ESPT)
+          os: windows
+
     - stage: maui_tests
       displayName: MAUI Tests
       dependsOn: mac_build

build-tools/automation/yaml-templates/stage-xamarin-android-tools-tests.yaml

@@ -0,0 +1,135 @@
+# xamarin-android-tools tests stage
+#
+# Mirrors the build + test jobs from the upstream dotnet/android-tools
+# azure-pipelines.yaml so that the Xamarin.Android.Tools build + unit tests
+# continue to run as part of dotnet/android CI now that android-tools has been
+# merged in-tree under external/xamarin-android-tools/.
+#
+# The upstream pipeline also packed a NuGet and published build artifacts;
+# those are intentionally dropped here (the binaries aren't consumed from this
+# stage -- dotnet/android's own build produces the shipping assemblies). Only
+# logs + test results are surfaced.
+#
+# Referenced from both:
+#   - build-tools/automation/azure-pipelines.yaml         (official / 1ES)
+#   - build-tools/automation/azure-pipelines-public.yaml  (public PR validation)
+
+parameters:
+- name: stageName
+  type: string
+  default: xamarin_android_tools_tests
+- name: dependsOn
+  type: object
+  default: []
+- name: condition
+  type: string
+  default: succeeded()
+- name: windowsPool
+  type: object
+  # NOTE: This default is a Microsoft-hosted Azure Pipelines pool, which is NOT
+  # allowed by 1ES Pipeline Templates in Official mode. Callers that extend the
+  # 1ES Official template (e.g. build-tools/automation/azure-pipelines.yaml) must
+  # override this with a 1ES-hosted pool like `MAUI-1ESPT`.
+  default:
+    name: Azure Pipelines
+    vmImage: $(HostedWinImage)
+    os: windows
+- name: macPool
+  type: object
+  default:
+    name: Azure Pipelines
+    vmImage: $(HostedMacImage)
+    os: macOS
+
+stages:
+- stage: ${{ parameters.stageName }}
+  displayName: Android Tools Tests
+  dependsOn: ${{ parameters.dependsOn }}
+  condition: ${{ parameters.condition }}
+  variables:
+    DotNetCoreVersion: 10.0.x
+    XatSourceDirectory: $(System.DefaultWorkingDirectory)/external/xamarin-android-tools
+    # When built in-tree, external/xamarin-android-tools.override.props imports
+    # dotnet/android's root Directory.Build.props, which pins
+    # DotNetTargetFrameworkVersion=11.0 and sets AndroidToolsDisableMultiTargeting=true
+    # (so the product build gets a single netstandard2.0 assembly). Upstream
+    # dotnet/android-tools CI instead builds standalone: multi-targeting
+    # netstandard2.0 + net10.0, with the test projects running on net10.0. Its
+    # ProcessUtils tests assert ProcessStartInfo.ArgumentList, which only exists on
+    # the net10.0 (NET5_0_OR_GREATER) build -- the netstandard2.0 build takes the
+    # string-Arguments path. To reproduce upstream's green test configuration we
+    # re-enable multi-targeting and force net10.0 via these global properties.
+    XatBuildProperties: -p:AndroidToolsDisableMultiTargeting=false -p:DotNetTargetFrameworkVersion=10.0
+  jobs:
+
+  # Check - "Xamarin.Android (Android Tools Tests Windows - .NET)"
+  - job: xamarin_android_tools_windows
+    displayName: Windows - .NET
+    pool: ${{ parameters.windowsPool }}
+    timeoutInMinutes: 30
+    workspace:
+      clean: all
+    steps:
+    - checkout: self
+      clean: true
+
+    - task: UseDotNet@2
+      displayName: Use .NET Core $(DotNetCoreVersion)
+      inputs:
+        version: $(DotNetCoreVersion)
+
+    - task: DotNetCoreCLI@2
+      displayName: Build Xamarin.Android.Tools.sln
+      inputs:
+        command: build
+        projects: $(XatSourceDirectory)/Xamarin.Android.Tools.sln
+        arguments: -c Debug $(XatBuildProperties) -bl:$(Build.ArtifactStagingDirectory)/build-windows.binlog
+
+    - task: DotNetCoreCLI@2
+      displayName: Run Tests
+      inputs:
+        command: test
+        projects: $(XatSourceDirectory)/bin/TestDebug/**/*-Tests.dll
+
+    - task: PublishBuildArtifacts@1
+      displayName: Upload build logs
+      condition: always()
+      inputs:
+        pathToPublish: $(Build.ArtifactStagingDirectory)
+        artifactName: android-tools-logs-windows
+
+  # Check - "Xamarin.Android (Android Tools Tests Mac - .NET)"
+  - job: xamarin_android_tools_mac
+    displayName: Mac - .NET
+    pool: ${{ parameters.macPool }}
+    timeoutInMinutes: 30
+    workspace:
+      clean: all
+    steps:
+    - checkout: self
+      clean: true
+
+    - task: UseDotNet@2
+      displayName: Use .NET Core $(DotNetCoreVersion)
+      inputs:
+        version: $(DotNetCoreVersion)
+
+    - task: DotNetCoreCLI@2
+      displayName: Build Xamarin.Android.Tools.sln
+      inputs:
+        command: build
+        projects: $(XatSourceDirectory)/Xamarin.Android.Tools.sln
+        arguments: -c Debug $(XatBuildProperties) -bl:$(Build.ArtifactStagingDirectory)/build-mac.binlog
+
+    - task: DotNetCoreCLI@2
+      displayName: Run Tests
+      inputs:
+        command: test
+        projects: $(XatSourceDirectory)/bin/TestDebug/**/*-Tests.dll
+
+    - task: PublishBuildArtifacts@1
+      displayName: Upload build logs
+      condition: always()
+      inputs:
+        pathToPublish: $(Build.ArtifactStagingDirectory)
+        artifactName: android-tools-logs-mac