Add SDK bootstrap and sdkmanager wrapper

[rmarinho]

Summary

Adds SDK bootstrap and sdkmanager wrapper to Xamarin.Android.Tools.AndroidSdk, enabling programmatic Android SDK setup from scratch.

Closes #271

Architecture

Two-phase approach as requested in the issue:

1. Bootstrap: Download command-line tools from the manifest feed → SHA-1 verify → extract to cmdline-tools/<version>/
2. Manage: Use the extracted sdkmanager CLI for all package operations

API Surface

public class SdkManager : IDisposable
{
    // Construction — configurable logger
    public SdkManager(Action<TraceLevel, string>? logger = null);
    
    // Configuration
    public string ManifestFeedUrl { get; set; }       // default: aka.ms/AndroidManifestFeed/d18-0
    public SdkManifestSource ManifestSource { get; set; }  // Xamarin or Google
    public string? AndroidSdkPath { get; set; }
    public string? JavaSdkPath { get; set; }
    
    // Manifest
    public Task<IReadOnlyList<SdkManifestComponent>> GetManifestComponentsAsync(CancellationToken ct);
    
    // Bootstrap
    public Task BootstrapAsync(string targetPath, IProgress<SdkBootstrapProgress>? progress, CancellationToken ct);
    
    // Package Management (via sdkmanager CLI)
    public Task<(IReadOnlyList<SdkPackage> Installed, IReadOnlyList<SdkPackage> Available)> ListAsync(CancellationToken ct);
    public Task InstallAsync(IEnumerable<string> packages, bool acceptLicenses, CancellationToken ct);
    public Task UninstallAsync(IEnumerable<string> packages, CancellationToken ct);
    public Task UpdateAsync(CancellationToken ct);
    
    // License Management
    public Task AcceptLicensesAsync(CancellationToken ct);                              // Accept all
    public Task AcceptLicensesAsync(IEnumerable<string> licenseIds, CancellationToken ct);  // Accept specific
    public Task<IReadOnlyList<SdkLicense>> GetPendingLicensesAsync(CancellationToken ct);   // Get for UI presentation
    public bool AreLicensesAccepted();
    
    // Utility
    public string? FindSdkManagerPath();
    public void Dispose();
}

// Supporting types
public enum SdkManifestSource { Xamarin, Google }
public class SdkManifestComponent { Id, Revision, Path, DownloadUrl, Checksum, ... }
public class SdkLicense { Id, Text }
public class SdkPackage { Path, Version, Description, IsInstalled }
public class SdkBootstrapProgress { Phase, PercentComplete, Message }
public static class EnvironmentVariableNames { AndroidHome, AndroidSdkRoot, JavaHome, ... }

Key Design Decisions

• Manifest-driven: No hardcoded download URLs. Reads manifest feed XML to discover cmdline-tools with checksums
• Google manifest support: SdkManifestSource.Google option uses dl.google.com/android/repository/repository2-3.xml
• XmlReader parsing: Forward-only streaming parse for better performance (no DOM tree allocation)
• Version-based directory: Extracts to cmdline-tools/<version>/ instead of latest symlink
• License presentation API: GetPendingLicensesAsync() returns license text for IDE/CLI UI presentation
• sdkmanager CLI: After bootstrap, all operations delegate to the real sdkmanager
• IDisposable: Manages own HttpClient lifecycle
• netstandard2.0 compatible: Compiles on both netstandard2.0 and net9.0 for VS hosting
• CancellationToken support: All async operations properly propagate cancellation
• p/invoke chmod: Uses libc chmod directly instead of spawning processes on Unix
• ArrayPool buffers: Uses ArrayPool<byte>.Rent() for download buffers (NET5_0_OR_GREATER)
• EnvironmentVariableNames: Centralized constants for ANDROID_HOME, JAVA_HOME, etc.
• SHA-1 verification: Downloads verified against manifest checksums (throws for unknown checksum types)
• Zip Slip protection: Archive extraction validates paths to prevent directory traversal
• Cross-device fallback: Falls back to recursive copy when Directory.Move fails
• Platform validation: Throws PlatformNotSupportedException for unsupported OS/architecture

Review Feedback Addressed

Feedback	Implementation
Rename ManifestComponent	→ SdkManifestComponent
Support Google manifest	Added SdkManifestSource enum
Remove HttpClient parameter	SdkManager manages own client
Use XmlReader	Replaced XDocument with streaming XmlReader
Use version dir not "latest"	Extracts to cmdline-tools/<version>/
ArrayPool for buffers	Conditional on NET5_0_OR_GREATER
Validate checksumType	Throws NotSupportedException for non-sha1
p/invoke for chmod	Uses libc chmod directly
Remove #region	All regions removed
Log exceptions	No empty catch blocks
License presentation API	GetPendingLicensesAsync + AcceptLicensesAsync(ids)
EnvironmentVariableNames	Centralized constants

Tests

Comprehensive unit tests covering:

• Manifest parsing (XML → SdkManifestComponent)
• License output parsing
• sdkmanager --list output parsing
• Path resolution and configuration
• Error handling

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

[Copilot]

Pull request overview

This pull request adds SDK bootstrap and sdkmanager wrapper functionality to Xamarin.Android.Tools.AndroidSdk, enabling programmatic Android SDK setup from scratch. The implementation uses a two-phase approach: first, download and extract command-line tools from a manifest feed with SHA-1 verification, then use the extracted sdkmanager CLI for package management operations.

Changes:

• Added SdkManager class with manifest parsing, bootstrap, and package management capabilities
• Added supporting types: ManifestComponent, SdkPackage, SdkBootstrapProgress, and SdkBootstrapPhase
• Added comprehensive unit tests covering manifest parsing, sdkmanager output parsing, path resolution, and configuration

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.

File	Description
src/Xamarin.Android.Tools.AndroidSdk/SdkManager.cs	Core implementation providing manifest-driven bootstrap, HTTP downloads with SHA-1 verification, and sdkmanager CLI wrapper for package operations
tests/Xamarin.Android.Tools.AndroidSdk-Tests/SdkManagerTests.cs	Unit tests covering manifest/output parsing, path resolution, configuration, and error handling

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[rmarinho]

Suggestions from related work

I've been working on related tooling (#281-#284) and noticed a few patterns that might be useful:

1. Manifest Caching for Offline/CI

Our implementation includes manifest caching for offline scenarios:

public AndroidManifestFeedParser(HttpClient? httpClient = null, string? cacheDirectory = null)
{
    _cacheDirectory = cacheDirectory;
    // ...
}

// On successful load, cache for offline use
if (!string.IsNullOrEmpty(_cacheDirectory))
    SaveToCache(manifestContent);

// On network failure, try cache
manifestContent = LoadFromCache();

This helps CI pipelines that may have intermittent network access.

2. Separate Tool Runners

For testability and reuse, we split the tool-running logic into a base class that can be shared:

• AndroidToolRunner - base process execution
• AndroidEnvironmentHelper - JAVA_HOME/ANDROID_HOME setup

This allows other runners (AvdManager, Adb, Emulator) to reuse the same patterns.

Would be happy to collaborate on merging these approaches! The PRs #281-#284 are designed to complement this work.

[rmarinho]

Review Response: PR #275 (SdkManager)

Thank you for the review! Here's my analysis:

✅ ACCEPTED

1. IDisposable for HttpClient (Accept)

Class owns HttpClient but doesn't implement IDisposable.

Agree. The class conditionally owns an HttpClient and should implement IDisposable to properly dispose it when ownsHttpClient is true.

2. CancellationToken in GetStringAsync (Accept)

CancellationToken not passed to HTTP call.

Agree. Should use:

using var response = await httpClient.GetAsync(ManifestFeedUrl, cancellationToken);
response.EnsureSuccessStatusCode();
var xml = await response.Content.ReadAsStringAsync();

3. ReadAsStreamAsync cancellation (Accept with modification)

No token support in netstandard2.0.

Agree. The pattern of registering cancellationToken to dispose the response, combined with token-aware ReadAsync in the download loop, is the appropriate approach for netstandard2.0.

4. Package parameter validation (Accept)

InstallAsync doesn't validate null/empty packages.

Agree. Should throw ArgumentException for null/empty package collections to avoid generating malformed sdkmanager invocations with confusing errors.

5. Cross-device Directory.Move (Accept)

May fail on Linux if temp and target on different filesystems.

Agree. This is a real scenario on Linux where /tmp may be a tmpfs while the SDK target is on a real filesystem. The pattern of trying Directory.Move then falling back to recursive copy + delete is appropriate.

---

⚠️ MODIFIED

6. Missing test coverage (Partially accept)

UpdateAsync and AcceptLicensesAsync lack tests.

Partially agree. Adding negative-path tests (no sdkmanager → throws) is worthwhile and straightforward. Full positive-path tests requiring a real Android SDK or heavy mocking are lower priority - the sdkmanager execution logic is already tested via InstallAsync/UninstallAsync tests.

---

Summary

5 of 6 suggestions accepted. The cross-device move fix is particularly important for Linux reliability.

[rmarinho]

Review Status: Already Implemented ✅

After reviewing the code, I found that all accepted feedback items are already implemented:

Suggestion	Status	Evidence
IDisposable for HttpClient	✅ Done	Line 120: class SdkManager : IDisposable
CancellationToken in HTTP calls	✅ Done	Line 177-180: Uses GetAsync(url, cancellationToken) pattern
Package parameter validation	✅ Done	Line 528: `if (packages == null
Cross-device Directory.Move	✅ Done	Lines 390-397: Try Move, catch IOException, fallback to recursive copy

The code already addresses all the review concerns properly. Nice work!

[rmarinho]

Implemented: p/invoke for chmod

Addressed Jonathan's feedback in commit 14db704:

`csharp
[DllImport ("libc", SetLastError = true)]
static extern int chmod (string pathname, int mode);

static bool Chmod (string path, int mode)
{
try {
return chmod (path, mode) == 0;
}
catch {
return false;
}
}
`

The implementation:

1. Uses p/invoke for efficiency (avoids spawning processes)
2. Falls back to process if p/invoke fails (e.g., on systems without libc)
3. Mode 0x1ED = 0755 octal (rwxr-xr-x)

This avoids the overhead of creating a new process for each file in the bin directory.

[rmarinho]

Jonathan's Review Feedback - Addressed ✅

Commit c52a6b5 addresses all review feedback:

Feedback	Status	Implementation
Rename ManifestComponent → SdkManifestComponent	✅	Renamed to avoid AndroidManifest.xml confusion
Support Google manifest as option	✅	Added SdkManifestSource enum + GoogleManifestFeedUrl constant
Use ArrayPool for buffer	✅	Conditional #if NET5_0_OR_GREATER for ArrayPool (netstandard2.0 doesn't have System.Buffers)
Validate checksumType parameter	✅	Throws NotSupportedException for non-sha1 types
Throw for unsupported OS/arch	✅	Added PlatformNotSupportedException for unknown platforms
Remove #region directives	✅	All regions removed
Log exceptions in catch blocks	✅	Added logging to all previously empty catch blocks
p/invoke for chmod	✅	Already done in commit 14db704

API Changes

New enum:

public enum SdkManifestSource { Xamarin, Google }

New property:

public SdkManifestSource ManifestSource { get; set; }

Renamed type:

• ManifestComponent → SdkManifestComponent

Questions from Review

Q: Why pass in HttpClient?
A: Enterprise/IDE scenarios often have proxy/auth requirements. Visual Studio provides a pre-configured HttpClient that handles corporate network auth. Making this optional (with internal fallback) supports both simple CLI usage and complex IDE integration.

Q: Should we use XmlReader instead of XElement?
A: XElement is simpler and the manifest is small (~50KB). The overhead is negligible for this use case. Could revisit if profiling shows it matters.

Q: Use version number instead of 'latest' for directory?
A: Good point - will consider in follow-up. Current approach matches existing sdkmanager behavior on macOS.

Q: API to present licenses to user?
A: AcceptLicensesAsync auto-accepts by sending 'y' to stdin. For UI presentation, consumers can:

1. Run sdkmanager --licenses manually first
2. Use the LicenseAsync() method to get license text (future enhancement)

[jonathanpeppers]

If this fails, should we just let the exception happen?

The problem is you won't be able to use sdkmanager if it fails, so seems like it should error?

[rmarinho]

Additional Feedback Addressed ✅

Commit fd485a2 addresses remaining items:

Feedback	Status
Use version number instead of 'latest'	✅ Now extracts to cmdline-tools/<version>/ (e.g., 19.0)
Clean up System.IO.Path duplication	✅ Using Path directly (using System.IO; at top)
Remove random empty lines	✅ Cleaned up

Behavior Change

Before: Bootstrap extracts to cmdline-tools/latest/
After: Bootstrap extracts to cmdline-tools/<version>/ (e.g., cmdline-tools/19.0/)

FindSdkManagerPath() now searches versioned directories first (sorted descending), then falls back to latest for backward compatibility with existing installations.

Still Pending (lower priority)

• XmlReader instead of XElement: The manifest is small (~50KB) and parsing is fast. Would add complexity for minimal benefit. Can revisit if profiling shows it matters.
• License presentation API: Current AcceptLicensesAsync auto-accepts. For UI presentation, consumers can run sdkmanager --licenses separately or we can add a GetLicensesAsync() method in a follow-up.

[rmarinho]

Implemented: XmlReader + License Presentation API ✅

Commit 5d54af9 addresses the remaining items:

1. XmlReader instead of XElement

Replaced \XDocument.Parse()\ / \XElement\ with forward-only \XmlReader:

\\csharp
using var reader = XmlReader.Create (stringReader, new XmlReaderSettings { IgnoreWhitespace = true });
while (reader.Read ()) {
if (reader.NodeType == XmlNodeType.Element && reader.Depth == 1) {
var revision = reader.GetAttribute ("revision");
// ...
}
}
\\

Benefits:

• No DOM tree allocation
• Forward-only streaming parse
• Lower memory usage for large manifests

2. License Presentation API

New methods for IDEs/CLI tools to present licenses before accepting:

\\csharp
// Get pending licenses with full text
var licenses = await sdkManager.GetPendingLicensesAsync();
foreach (var license in licenses) {
Console.WriteLine($"License: {license.Id}");
Console.WriteLine(license.Text);
if (UserAccepts()) {
// Accept specific licenses
await sdkManager.AcceptLicensesAsync(new[] { license.Id });
}
}
\\

New types:

• \SdkLicense\ class with \Id\ and \Text\ properties
• \GetPendingLicensesAsync()\ - Get licenses needing acceptance
• \AcceptLicensesAsync(IEnumerable)\ - Accept specific licenses by ID

[rmarinho]

Addressed in commit 8e8f59c

chmod failure (r2842575105): ✅

• SetExecutablePermissions now throws InvalidOperationException on failure instead of silently continuing
• If chmod fails, sdkmanager won't work, so we should error out

ANDROID_SDK_ROOT deprecated (r2842566684): ✅

• Removed ANDROID_SDK_ROOT from ConfigureEnvironment
• Now only sets ANDROID_HOME per Android docs

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The XML documentation comment has incorrect indentation. It should start at the same indentation level as the method it documents, without the leading whitespace and tab.

Suggested change

	/// <summary>
	/// <summary>

[Copilot]

The SdkManager class implements IDisposable and owns an HttpClient that needs disposal. However, the test creates SdkManager instances in SetUp but never disposes them. Consider adding a TearDown method that calls manager?.Dispose() to ensure proper resource cleanup after each test.

Suggested change

	[TearDown]
	public void TearDown ()
	{
	manager?.Dispose ();
	manager = null;
	}

[Copilot]

The documentation states "This project uses xUnit" and provides xUnit-specific guidance (lines 185-193), but the actual test project uses NUnit as evidenced by the test attributes [TestFixture], [Test], and [SetUp] in SdkManagerTests.cs, as well as the NUnit package reference in the csproj file. This inconsistency should be corrected to accurately reflect that NUnit is the testing framework used in this project.

Suggested change

	- **This project uses xUnit** - use xUnit for all new tests
	### xUnit
	- Packages: `Microsoft.NET.Test.Sdk`, `xunit`, `xunit.runner.visualstudio`
	- No class attribute; use `[Fact]`
	- Parameterized tests: `[Theory]` with `[InlineData]`
	- Setup/teardown: constructor and `IDisposable`
	- **This project uses NUnit** - use NUnit for all new tests
	### NUnit
	- Packages: `Microsoft.NET.Test.Sdk`, `NUnit`, `NUnit3TestAdapter`
	- Class attribute: `[TestFixture]`; test methods: `[Test]`
	- Parameterized tests: `[TestCase]` (or `[TestCaseSource]` for more complex cases)
	- Setup/teardown: `[SetUp]` / `[TearDown]`

[Copilot]

There is a contradiction in the formatting guidelines. Line 39 states that "File-scoped namespaces (must use traditional namespace X { })" under "NOT available (C# 10+)", but line 53 states "Prefer file-scoped namespace declarations". Since this project uses C# 9.0, file-scoped namespaces are not available, so line 53 should be corrected to indicate traditional namespace blocks should be used.

Suggested change

	- Prefer file-scoped namespace declarations and single-line using directives
	- Use traditional `namespace X { }` blocks for namespaces and prefer single-line using directives

[Copilot]

The XML documentation comment has incorrect indentation. It should start at the same indentation level as the method it documents, without the leading whitespace and tab. The comment should be /// <summary> not /// <summary> with leading whitespace.