Skip to content

Add SDK bootstrap and sdkmanager wrapper#275

Open
rmarinho wants to merge 27 commits intomainfrom
feature/sdk-manager
Open

Add SDK bootstrap and sdkmanager wrapper#275
rmarinho wants to merge 27 commits intomainfrom
feature/sdk-manager

Conversation

@rmarinho
Copy link
Member

@rmarinho rmarinho commented Feb 20, 2026

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:

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

File Structure

SdkManager is split into focused partial classes for maintainability:

  • SdkManager.cs - Core class, properties, constructor, dispose (~84 lines)
  • SdkManager.Manifest.cs - Manifest download and XML parsing (~141 lines)
  • SdkManager.Bootstrap.cs - SDK bootstrap/download/extract flow (~175 lines)
  • SdkManager.Packages.cs - Package list/install/uninstall/update + output parsing (~228 lines)
  • SdkManager.Licenses.cs - License accept/parse/check operations (~246 lines)
  • SdkManager.Process.cs - Process execution, elevation, download, checksum, env config (~307 lines)

Supporting files:

  • EnvironmentVariableNames.cs - Centralized constants for ANDROID_HOME, JAVA_HOME, etc.
  • FileUtil.cs - Extended with chmod, recursive copy, executable permissions, elevation check
  • Models/Sdk/*.cs - Data models: SdkPackage, SdkLicense, SdkManifestComponent, SdkBootstrapProgress
  • .github/copilot-instructions.md - Updated coding conventions
  • SdkManagerTests.cs - 534 lines of tests

Key Features

  • Manifest-driven bootstrap: XmlReader-based parsing with platform/arch filtering
  • ANDROID_USER_HOME: Sets ~/.android so sdkmanager writes install properties in user-writable location
  • Elevated execution (Windows): Detects admin-protected paths, re-launches via UAC. Read-only ops skip elevation.
  • Zip Slip protection: Entry-by-entry extraction with path traversal validation
  • Rollback on failure: Backup existing installation, restore on extract/move failure
  • Cross-device fallback: Directory.Move falls back to recursive copy
  • SHA-1 checksum verification of downloaded archives
  • p/invoke chmod with process fallback for Unix executable permissions
  • ArrayPool buffers (net5.0+) for download operations
  • netstandard2.0 + net10.0 compatible, full CancellationToken support
  • ANDROID_SDK_ROOT deprecated per Android docs - only ANDROID_HOME is set
  • File utilities consolidated in FileUtil.cs - no duplication across domain classes

Tests

89 tests pass (4 skipped). Covers manifest parsing, list output parsing, license parsing, error cases.

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

Copilot AI review requested due to automatic review settings February 20, 2026 17:01

This comment was marked as resolved.

@rmarinho rmarinho force-pushed the feature/sdk-manager branch from 10b4527 to 1a94a8d Compare February 20, 2026 17:16
@rmarinho rmarinho added the copilot `copilot-cli` or other AIs were used to author this label Feb 20, 2026
@rmarinho

This comment was marked as resolved.

@rmarinho

This comment was marked as resolved.

@rmarinho

This comment was marked as outdated.

@rmarinho

This comment was marked as outdated.

@rmarinho

This comment was marked as resolved.

@rmarinho

This comment was marked as outdated.

@rmarinho

This comment was marked as resolved.

This comment was marked as resolved.

@rmarinho

This comment was marked as outdated.

@rmarinho

This comment was marked as resolved.

Copy link
Contributor

Copilot AI left a 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 11 out of 11 changed files in this pull request and generated 9 comments.

Copy link
Contributor

Copilot AI commented Feb 25, 2026

@rmarinho I've opened a new pull request, #288, to work on those changes. Once the pull request is ready, I'll request review from you.

rmarinho and others added 4 commits February 26, 2026 16:20
Split the 1200-line SdkManager.cs into focused partial class files:
- SdkManager.cs: Core class, properties, constructor, dispose (~95 lines)
- SdkManager.Manifest.cs: Manifest download and parsing (~141 lines)
- SdkManager.Bootstrap.cs: SDK bootstrap/download/extract (~188 lines)
- SdkManager.Packages.cs: Package list/install/uninstall/update (~224 lines)
- SdkManager.Licenses.cs: License accept/parse/check (~220 lines)
- SdkManager.Process.cs: Process execution, elevation, download (~310 lines)
- SdkManager.FileUtils.cs: File permissions, copy, elevation check (~108 lines)

No behavioral changes - pure structural refactor.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move IsElevated, Chmod, CopyDirectoryRecursive, and SetExecutablePermissions
to the shared FileUtil class for reuse across JdkInstaller and SdkManager.
Delete SdkManager.FileUtils.cs partial and update callers.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add ThrowIfDisposed() calls to all public async methods in SdkManager
- Add explicit AndroidSdkPath null check in AcceptLicensesAsync(licenseIds)
- Fix null-coalescing for Environment.GetFolderPath in RequiresElevation
- Wrap cancellation registration dispose in try-catch for safety

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace empty catch blocks with proper exception logging
- Add SdkManagerTimeout property (default 30min) replacing hardcoded 10min
- Log exception details in RequiresElevation write test
- Log license check non-zero exits at Verbose level

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rmarinho rmarinho force-pushed the feature/sdk-manager branch from 39bfbd5 to e3d82d8 Compare February 26, 2026 16:27
rmarinho and others added 8 commits February 26, 2026 16:35
- Prefer internal over public for new APIs
- Always use ProcessUtils instead of System.Diagnostics.Process
- Use ArgumentList.Add() instead of string interpolation for args
- Use FileUtil for file operations
- Concise XML docs, no restating method names
- netstandard2.0 workarounds for cancellation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add TryDeleteFiles batch helper to FileUtil
- Use FileUtil.TryDeleteFiles/TryDeleteDirectory in SdkManager cleanup
- Replace direct Process.Start in SetExecutablePermissions with ProcessUtils
- Add XML doc remarks explaining why elevated flow uses Process.Start directly
- Simplify RunSdkManagerElevatedAsync temp file management
- Use single GUID for all temp files in elevated flow

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Check exitCode from sdkmanager --licenses
- Only treat non-zero as non-fatal when output matches known 'already accepted' cases
- Throw InvalidOperationException for genuine failures (missing Java, permissions, etc.)

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

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace duplicated path-writability check with existing FileUtil helper.
Direct Process.Start in elevated path is intentional (UAC requires UseShellExecute).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add environment variables and UseShellExecute support to ProcessUtils.StartProcess
- Replace SdkManager.VerifyChecksum with DownloadUtils.VerifyChecksum
- Replace SdkManager.DownloadFileAsync with DownloadUtils.DownloadFileAsync
- Refactor RunSdkManagerElevatedAsync to use ProcessUtils
- Remove ~60 lines of duplicated process/download code

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e AndroidEnvironmentHelper, reorder methods

- Replace raw ProcessStartInfo with ProcessUtils.CreateProcessStartInfo in SdkManager.Process and Licenses
- Merge two VerifyChecksum overloads into single method with default parameter
- Restore AndroidEnvironmentHelper (consumed by maui DevTools CLI)
- Move public AreLicensesAccepted before internal/private methods in Licenses

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

This comment was marked as resolved.

rmarinho and others added 8 commits February 26, 2026 21:54
…ckupPath, fix XML docs

- Change RunSdkManagerAsync to accept string[] instead of single string
- Pass packages as individual args to avoid ArgumentList quoting issues
- Escape arguments in elevated cmd.exe script to prevent injection
- Fix httpClient field indentation in SdkManager.cs
- Null-guard backupPath in Bootstrap.cs cleanup
- Split duplicate XML doc blocks in ProcessUtils.cs
- Split cmd.exe args properly: /c and script path as separate args

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

- Validate package arguments reject shell-special characters (&|><^)
- Apply sanitization to env vars and arguments in elevated script

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Replace async ProcessUtils.StartProcess().Result with synchronous
Process.Start/WaitForExit to avoid thread-pool starvation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace single 'echo y|' with a for-loop that generates 100 'y'
responses, matching the non-elevated path's continuous feeding behavior.
This ensures multi-license operations complete successfully.

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

- Extract RequireSdkManagerPath() combining ThrowIfDisposed + FindSdkManagerPath
- Extract ThrowOnSdkManagerFailure() to consolidate error logging + throw pattern
- Simplify GetEnvironmentVariables() with collection initializer
- Remove verbose XML doc comments on private methods
- Net reduction: 39 lines across 4 files

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

Copilot AI left a 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 19 out of 19 changed files in this pull request and generated 7 comments.

var script = $"""
@echo off
{envBlock}
{licenseInput}"{sdkManagerPath}" {escapedArgs} > "{stdoutFile}" 2> "{stderrFile}"
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the generated elevated .cmd script, "{sdkManagerPath}" {escapedArgs} will typically point to sdkmanager.bat. Calling a .bat from within another .cmd file without call terminates the wrapper script early, so the echo %ERRORLEVEL% > "{exitCodeFile}" line never runs and failures can be reported as success. Prefix the sdkmanager invocation with call (or invoke it via cmd /c) so the wrapper continues and captures the real exit code.

Suggested change
{licenseInput}"{sdkManagerPath}" {escapedArgs} > "{stdoutFile}" 2> "{stderrFile}"
{licenseInput}call "{sdkManagerPath}" {escapedArgs} > "{stdoutFile}" 2> "{stderrFile}"

Copilot uses AI. Check for mistakes.
Comment on lines 263 to 268
await Task.Run (() => {
if (!process.WaitForExit ((int) timeout.TotalMilliseconds)) {
KillProcess (process);
throw new TimeoutException ($"Process timed out after {timeout.TotalMinutes} minutes.");
}
}, cancellationToken).ConfigureAwait (false);
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StartShellExecuteProcessAsync() passes the CancellationToken to Task.Run, but if cancellation occurs while waiting, the task can be canceled without killing the spawned process. This can leave an elevated cmd.exe (and child sdkmanager) running after the caller cancels. Consider registering the token to KillProcess(process) (similar to StartProcess) and ensure cancellation triggers process termination before returning/throwing.

Suggested change
await Task.Run (() => {
if (!process.WaitForExit ((int) timeout.TotalMilliseconds)) {
KillProcess (process);
throw new TimeoutException ($"Process timed out after {timeout.TotalMinutes} minutes.");
}
}, cancellationToken).ConfigureAwait (false);
using (cancellationToken.Register (() => KillProcess (process))) {
await Task.Run (() => {
if (!process.WaitForExit ((int) timeout.TotalMilliseconds)) {
KillProcess (process);
throw new TimeoutException ($"Process timed out after {timeout.TotalMinutes} minutes.");
}
}).ConfigureAwait (false);
}

Copilot uses AI. Check for mistakes.
rmarinho and others added 3 commits February 26, 2026 23:14
…nownVersions, use ProcessUtils in FileUtil

- Fix double-enumeration in InstallAsync/UninstallAsync by materializing once
- Derive ApiLevelToVersionMap from AndroidVersions.KnownVersions instead of hardcoding
- Use ProcessUtils.StartProcess in SetExecutablePermissions instead of raw Process
- Reword copilot-instructions to clarify ProcessUtils wraps ArgumentList

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Rename to SetExecutablePermissionsAsync with CancellationToken support
- Remove .GetAwaiter().GetResult() blocking call
- Properly await ProcessUtils.StartProcess in bootstrap flow

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The sdkmanager shell script uses save()/eval which concatenates
individually-quoted arguments into a single string. On macOS/Linux,
pass arguments as a single Arguments string instead of ArgumentList
to work around this shell script bug.

Also fix double-enumeration in UninstallAsync by materializing
the packages collection upfront.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

copilot `copilot-cli` or other AIs were used to author this enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add SDK bootstrap and sdkmanager wrapper (move from android-platform-support)

4 participants