Skip to content

Commit c6fc83d

Browse files
rmarinhoCopilotjonathanpeppers
authored
Add JDK installation support (Microsoft OpenJDK) (#274)
Fixes: #270 Adds JDK installation support to `Xamarin.Android.Tools.AndroidSdk` using the **Microsoft Build of OpenJDK** (https://www.microsoft.com/openjdk). ## New Files - **`JdkInstaller.cs`** — Discovers, downloads, installs, validates, and removes Microsoft OpenJDK - **`DownloadUtils.cs`** — Shared helpers: download with progress, SHA-256 checksum verification, Zip Slip-safe ZIP extraction, tar.gz extraction, checksum file fetching/parsing - **`Models/JdkVersionInfo.cs`** — Version metadata (major, minor, patch, build, platform, architecture, URL, checksum URL) - **`Models/JdkInstallPhase.cs`** — `enum` for progress reporting phases (Downloading, Verifying, Extracting, etc.) - **`Models/JdkInstallProgress.cs`** — `record` combining phase, percentage, and message for `IProgress<T>` callbacks - **`IsExternalInit.cs`** — Enables `init` accessors on netstandard2.0 - **`.github/copilot-instructions.md`** — Repository-specific Copilot context ## Modified Files - **`FileUtil.cs`** — Added `TryDeleteFile`, `TryDeleteDirectory`, `MoveWithRollback`, `CommitMove`, `IsTargetPathWritable`, `IsUnderDirectory`, `GetInstallerExtension`, `GetArchiveExtension` - **`ProcessUtils.cs`** — Added `CreateProcessStartInfo` (uses `ArgumentList` on net5+, `JoinArguments` fallback), `IsElevated` (uses `Environment.IsPrivilegedProcess` on net7+, P/Invoke fallback) - **`.csproj`** — Added `System.Buffers` 4.5.1 (netstandard2.0), `IsExternalInit.cs` exclusion on modern TFMs ## API Surface ```csharp public class JdkInstaller : IDisposable { public JdkInstaller(Action<string>? logger = null); public static int RecommendedMajorVersion { get; } // 21 public static IReadOnlyList<int> SupportedVersions { get; } // [21] public Task<IReadOnlyList<JdkVersionInfo>> DiscoverAsync(CancellationToken ct = default); public Task InstallAsync(int majorVersion, string targetPath, IProgress<JdkInstallProgress>? progress = null, CancellationToken ct = default); public bool IsValid(string jdkPath); public bool Remove(string jdkPath); public void Dispose(); } // Helpers used by JdkInstaller (not on JdkInstaller itself): // FileUtil.IsTargetPathWritable(string, Action<TraceLevel, string>) // ProcessUtils.IsElevated() ``` ## Key Design Decisions - **Microsoft OpenJDK only** — downloads from `https://aka.ms/download-jdk` with SHA-256 checksum verification - **JDK 21 only** — `RecommendedMajorVersion` = 21, `SupportedVersions` = `[21]`. Array allows future additions - **Owns its `HttpClient`** — no injection; `Dispose()` cleans it up - **Elevated install mode** — when `ProcessUtils.IsElevated()` is true, uses platform-native silent installers (`.msi` via `msiexec /qn`, `.pkg` via `/usr/sbin/installer`). When not elevated, extracts archive to `targetPath` - **Safe directory replacement** — `MoveWithRollback`: rename existing → temp, move new in place, delete temp via `CommitMove`. Avoids Delete/Move race on Windows - **Zip Slip protection** — ZIP extraction validates that all entry paths resolve under the destination directory - **`ArrayPool<byte>` download buffer** — `ArrayPool<byte>.Shared.Rent` for the 80 KB download buffer, returned in `finally` - **Cross-platform P/Invoke fallbacks** — `IsUserAnAdmin()` (shell32) on Windows, `geteuid()` (libc) on Unix, behind `#if !NET5_0_OR_GREATER` - **`NullProgress` pattern** — avoids null checks throughout install flow - **`netstandard2.0` + `net10.0`** — conditional compilation for `ArgumentList`, `Environment.IsPrivilegedProcess`, `ReadAsStringAsync`/`ReadAsStreamAsync` overloads. Private helpers encapsulate `#if` blocks ## Tests 66 unit tests across 5 test files (all target `net10.0`): | Test File | Count | Coverage | |--------------------------|-------|------------------------------------------------------------------------------------------| | `JdkInstallerTests.cs` | 19 | `IsValid`, `DiscoverAsync`, `InstallAsync`, `IsTargetPathWritable`, `Remove`, properties | | `DownloadUtilsTests.cs` | 20 | `ParseChecksumFile` (12 cases), `VerifyChecksum` (4), `ExtractZipSafe` + Zip Slip (4) | | `FileUtilTests.cs` | 15 | `MoveWithRollback` (4), `IsUnderDirectory` (11 including cross-platform path cases) | | `ProcessUtilsTests.cs` | 7 | `CreateProcessStartInfo` (6), `IsElevated` smoke (1) | | `JdkVersionInfoTests.cs` | 5 | Constructor, defaults, `ToString`, mutable properties | ## Review Feedback Addressed JDK 21 only (removed 17), removed `HttpClient` injection, `IDisposable`, `CancellationToken` pattern, path normalization, validation cleanup, `ArgumentList` for tar, absolute `/usr/bin/tar`, null comparison pattern, one type per file, `record` type, checksum errors throw, test `TearDown` dispose, CI skip, elevated install mode, Zip Slip protection, `ArrayPool` buffer, extracted constants (`BufferSize`, `BytesPerMB`, `WhitespaceChars`), private `ReadAsStreamAsync`/`ReadAsStringAsync` helpers, cross-platform test paths, comprehensive unit test coverage Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
1 parent 062cc29 commit c6fc83d

File tree

15 files changed

+1567
-2
lines changed

15 files changed

+1567
-2
lines changed

‎.github/copilot-instructions.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,21 @@ dotnet test tests/Microsoft.Android.Build.BaseTasks-Tests/Microsoft.Android.Buil
2424

2525
Output: `bin\$(Configuration)\` (redistributables), `bin\Test$(Configuration)\` (tests). `$(DotNetTargetFrameworkVersion)` = `10.0` in `Directory.Build.props`. Versioning: `nuget.version` has `major.minor`; patch = git commit count since file changed.
2626

27+
## Android Environment Variables
28+
29+
Per the [official Android docs](https://developer.android.com/tools/variables#envar):
30+
31+
- **`ANDROID_HOME`** — the canonical variable for the Android SDK root path. Use this everywhere.
32+
- **`ANDROID_SDK_ROOT`** — **deprecated**. Do not introduce new usages. Existing code may still read it for backward compatibility but always prefer `ANDROID_HOME`.
33+
- **`ANDROID_USER_HOME`** — user-level config/AVD storage (defaults to `~/.android`).
34+
- **`ANDROID_EMULATOR_HOME`** — emulator config (defaults to `$ANDROID_USER_HOME`).
35+
- **`ANDROID_AVD_HOME`** — AVD data (defaults to `$ANDROID_USER_HOME/avd`).
36+
37+
When setting environment variables for SDK tools (e.g. `sdkmanager`, `avdmanager`), set `ANDROID_HOME`. The `EnvironmentVariableNames` class in this repo defines the constants.
38+
2739
## Conventions
2840

41+
- **One type per file**: each public class, struct, enum, or interface must be in its own `.cs` file named after the type (e.g. `JdkVersionInfo` → `JdkVersionInfo.cs`). Do not combine multiple top-level types in a single file.
2942
- [Mono Coding Guidelines](http://www.mono-project.com/community/contributing/coding-guidelines/): tabs, K&R braces, `PascalCase` public members.
3043
- Nullable enabled in `AndroidSdk`. `NullableAttributes.cs` excluded on `net10.0+`.
3144
- Strong-named via `product.snk`. In the AndroidSdk project, tests use `InternalsVisibleTo` with full public key (`Properties/AssemblyInfo.cs`).

‎src/Xamarin.Android.Tools.AndroidSdk/DownloadUtils.cs‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Buffers;
6+
using System.Diagnostics;
7+
using System.IO;
8+
using System.IO.Compression;
9+
using System.Net.Http;
10+
using System.Security.Cryptography;
11+
using System.Threading;
12+
using System.Threading.Tasks;
13+
14+
namespace Xamarin.Android.Tools
15+
{
16+
/// <summary>
17+
/// Shared helpers for downloading files, verifying checksums, and extracting archives.
18+
/// </summary>
19+
static class DownloadUtils
20+
{
21+
const int BufferSize = 81920;
22+
const long BytesPerMB = 1024 * 1024;
23+
static readonly char[] WhitespaceChars = [' ', '\t', '\n', '\r'];
24+
25+
static Task<Stream> ReadAsStreamAsync (HttpContent content, CancellationToken cancellationToken)
26+
{
27+
#if NET5_0_OR_GREATER
28+
return content.ReadAsStreamAsync (cancellationToken);
29+
#else
30+
return content.ReadAsStreamAsync ();
31+
#endif
32+
}
33+
34+
static Task<string> ReadAsStringAsync (HttpContent content, CancellationToken cancellationToken)
35+
{
36+
#if NET5_0_OR_GREATER
37+
return content.ReadAsStringAsync (cancellationToken);
38+
#else
39+
return content.ReadAsStringAsync ();
40+
#endif
41+
}
42+
43+
/// <summary>Downloads a file from the given URL with optional progress reporting.</summary>
44+
public static async Task DownloadFileAsync (HttpClient client, string url, string destinationPath, long expectedSize, IProgress<(double percent, string message)>? progress, CancellationToken cancellationToken)
45+
{
46+
using var response = await client.GetAsync (url, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait (false);
47+
response.EnsureSuccessStatusCode ();
48+
49+
var totalBytes = response.Content.Headers.ContentLength ?? expectedSize;
50+
51+
using var contentStream = await ReadAsStreamAsync (response.Content, cancellationToken).ConfigureAwait (false);
52+
53+
var dirPath = Path.GetDirectoryName (destinationPath);
54+
if (!string.IsNullOrEmpty (dirPath))
55+
Directory.CreateDirectory (dirPath);
56+
57+
using var fileStream = new FileStream (destinationPath, FileMode.Create, FileAccess.Write, FileShare.None, BufferSize, useAsync: true);
58+
59+
var buffer = ArrayPool<byte>.Shared.Rent (BufferSize);
60+
try {
61+
long totalRead = 0;
62+
int bytesRead;
63+
64+
while ((bytesRead = await contentStream.ReadAsync (buffer, 0, buffer.Length, cancellationToken).ConfigureAwait (false)) > 0) {
65+
await fileStream.WriteAsync (buffer, 0, bytesRead, cancellationToken).ConfigureAwait (false);
66+
totalRead += bytesRead;
67+
68+
if (progress is not null && totalBytes > 0) {
69+
var pct = (double) totalRead / totalBytes * 100;
70+
progress.Report ((pct, $"Downloaded {totalRead / BytesPerMB} MB / {totalBytes / BytesPerMB} MB"));
71+
}
72+
}
73+
}
74+
finally {
75+
ArrayPool<byte>.Shared.Return (buffer);
76+
}
77+
}
78+
79+
/// <summary>Verifies a file's SHA-256 hash against an expected value.</summary>
80+
public static void VerifyChecksum (string filePath, string expectedChecksum)
81+
{
82+
using var sha256 = SHA256.Create ();
83+
using var stream = File.OpenRead (filePath);
84+
85+
var hash = sha256.ComputeHash (stream);
86+
var actual = BitConverter.ToString (hash).Replace ("-", "").ToLowerInvariant ();
87+
88+
if (!string.Equals (actual, expectedChecksum, StringComparison.OrdinalIgnoreCase))
89+
throw new InvalidOperationException ($"Checksum verification failed. Expected: {expectedChecksum}, Actual: {actual}");
90+
}
91+
92+
/// <summary>Extracts a ZIP archive with Zip Slip protection.</summary>
93+
public static void ExtractZipSafe (string archivePath, string destinationPath, CancellationToken cancellationToken)
94+
{
95+
using var archive = ZipFile.OpenRead (archivePath);
96+
var fullExtractRoot = Path.GetFullPath (destinationPath);
97+
98+
foreach (var entry in archive.Entries) {
99+
cancellationToken.ThrowIfCancellationRequested ();
100+
101+
if (string.IsNullOrEmpty (entry.Name))
102+
continue;
103+
104+
var destinationFile = Path.GetFullPath (Path.Combine (fullExtractRoot, entry.FullName));
105+
106+
// Zip Slip protection
107+
if (!FileUtil.IsUnderDirectory (destinationFile, fullExtractRoot)) {
108+
throw new InvalidOperationException ($"Archive entry '{entry.FullName}' would extract outside target directory.");
109+
}
110+
111+
var entryDir = Path.GetDirectoryName (destinationFile);
112+
if (!string.IsNullOrEmpty (entryDir))
113+
Directory.CreateDirectory (entryDir);
114+
115+
entry.ExtractToFile (destinationFile, overwrite: true);
116+
}
117+
}
118+
119+
/// <summary>Extracts a tar.gz archive using the system tar command.</summary>
120+
public static async Task ExtractTarGzAsync (string archivePath, string destinationPath, Action<TraceLevel, string> logger, CancellationToken cancellationToken)
121+
{
122+
var psi = ProcessUtils.CreateProcessStartInfo ("/usr/bin/tar", "-xzf", archivePath, "-C", destinationPath);
123+
124+
using var stdout = new StringWriter ();
125+
using var stderr = new StringWriter ();
126+
var exitCode = await ProcessUtils.StartProcess (psi, stdout: stdout, stderr: stderr, cancellationToken).ConfigureAwait (false);
127+
128+
if (exitCode != 0) {
129+
var errorOutput = stderr.ToString ();
130+
logger (TraceLevel.Error, $"tar extraction failed (exit code {exitCode}): {errorOutput}");
131+
throw new IOException ($"Failed to extract archive '{archivePath}': {errorOutput}");
132+
}
133+
}
134+
135+
/// <summary>Fetches a SHA-256 checksum from a remote URL, returning null on failure.</summary>
136+
public static async Task<string?> FetchChecksumAsync (HttpClient httpClient, string checksumUrl, string label, Action<TraceLevel, string> logger, CancellationToken cancellationToken)
137+
{
138+
try {
139+
using var response = await httpClient.GetAsync (checksumUrl, cancellationToken).ConfigureAwait (false);
140+
response.EnsureSuccessStatusCode ();
141+
var content = await ReadAsStringAsync (response.Content, cancellationToken).ConfigureAwait (false);
142+
var checksum = ParseChecksumFile (content);
143+
logger (TraceLevel.Verbose, $"{label}: checksum={checksum}");
144+
return checksum;
145+
}
146+
catch (OperationCanceledException) {
147+
throw;
148+
}
149+
catch (Exception ex) {
150+
logger (TraceLevel.Warning, $"Could not fetch checksum for {label}: {ex.Message}");
151+
return null;
152+
}
153+
}
154+
155+
/// <summary>Parses "hash filename" or just "hash" from .sha256sum.txt content.</summary>
156+
public static string? ParseChecksumFile (string content)
157+
{
158+
if (string.IsNullOrWhiteSpace (content))
159+
return null;
160+
161+
var trimmed = content.Trim ();
162+
var end = trimmed.IndexOfAny (WhitespaceChars);
163+
return end >= 0 ? trimmed.Substring (0, end) : trimmed;
164+
}
165+
}
166+
}

‎src/Xamarin.Android.Tools.AndroidSdk/FileUtil.cs‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Diagnostics;
23
using System.IO;
34
using System.Runtime.InteropServices;
45

@@ -51,6 +52,135 @@ public static void SystemRename (string sourceFile, string destFile)
5152
}
5253
}
5354

55+
/// <summary>Deletes a file if it exists, logging any failure instead of throwing.</summary>
56+
internal static void TryDeleteFile (string path, Action<TraceLevel, string> logger)
57+
{
58+
if (!File.Exists (path))
59+
return;
60+
try { File.Delete (path); }
61+
catch (Exception ex) { logger (TraceLevel.Warning, $"Could not delete '{path}': {ex.Message}"); }
62+
}
63+
64+
/// <summary>Recursively deletes a directory if it exists, logging any failure instead of throwing.</summary>
65+
internal static void TryDeleteDirectory (string path, string label, Action<TraceLevel, string> logger)
66+
{
67+
if (!Directory.Exists (path))
68+
return;
69+
try { Directory.Delete (path, recursive: true); }
70+
catch (Exception ex) { logger (TraceLevel.Warning, $"Could not clean up {label} at '{path}': {ex.Message}"); }
71+
}
72+
73+
/// <summary>Moves a directory to the target path, backing up any existing directory and restoring on failure.</summary>
74+
internal static void MoveWithRollback (string sourcePath, string targetPath, Action<TraceLevel, string> logger)
75+
{
76+
string? backupPath = null;
77+
if (Directory.Exists (targetPath)) {
78+
backupPath = targetPath + $".old-{Guid.NewGuid ():N}";
79+
Directory.Move (targetPath, backupPath);
80+
}
81+
82+
var parentDir = Path.GetDirectoryName (targetPath);
83+
if (!string.IsNullOrEmpty (parentDir))
84+
Directory.CreateDirectory (parentDir);
85+
86+
try {
87+
Directory.Move (sourcePath, targetPath);
88+
}
89+
catch (Exception ex) {
90+
logger (TraceLevel.Error, $"Failed to move to '{targetPath}': {ex.Message}");
91+
if (backupPath is not null && Directory.Exists (backupPath)) {
92+
try {
93+
if (Directory.Exists (targetPath))
94+
Directory.Delete (targetPath, recursive: true);
95+
Directory.Move (backupPath, targetPath);
96+
logger (TraceLevel.Warning, $"Restored previous directory from backup '{backupPath}'.");
97+
}
98+
catch (Exception restoreEx) {
99+
logger (TraceLevel.Error, $"Failed to restore from backup: {restoreEx.Message}");
100+
}
101+
}
102+
throw;
103+
}
104+
105+
// Delete backup only after move and caller validation succeed
106+
if (backupPath is not null)
107+
TryDeleteDirectory (backupPath, "old backup", logger);
108+
}
109+
110+
/// <summary>Deletes a backup created by MoveWithRollback. Call after validation succeeds.</summary>
111+
internal static void CommitMove (string targetPath, Action<TraceLevel, string> logger)
112+
{
113+
// Find and clean up any leftover backup directories
114+
var parentDir = Path.GetDirectoryName (targetPath);
115+
if (string.IsNullOrEmpty (parentDir) || !Directory.Exists (parentDir))
116+
return;
117+
118+
var dirName = Path.GetFileName (targetPath);
119+
foreach (var dir in Directory.GetDirectories (parentDir, $"{dirName}.old-*")) {
120+
TryDeleteDirectory (dir, "old backup", logger);
121+
}
122+
}
123+
124+
/// <summary>Checks if the target path is writable by probing write access on the nearest existing ancestor.</summary>
125+
/// <remarks>
126+
/// Follows the same pattern as dotnet/sdk WorkloadInstallerFactory.CanWriteToDotnetRoot:
127+
/// probe with File.Create + DeleteOnClose, only catch UnauthorizedAccessException.
128+
/// See https://github.com/dotnet/sdk/blob/db01067a9c4b67dc1806956393ec63b032032166/src/Cli/dotnet/Commands/Workload/Install/WorkloadInstallerFactory.cs
129+
/// </remarks>
130+
internal static bool IsTargetPathWritable (string targetPath, Action<TraceLevel, string> logger)
131+
{
132+
if (string.IsNullOrEmpty (targetPath))
133+
return false;
134+
135+
try {
136+
targetPath = Path.GetFullPath (targetPath);
137+
}
138+
catch {
139+
return false;
140+
}
141+
142+
try {
143+
// Walk up to the nearest existing ancestor directory
144+
var testDir = targetPath;
145+
while (!string.IsNullOrEmpty (testDir) && !Directory.Exists (testDir))
146+
testDir = Path.GetDirectoryName (testDir);
147+
148+
if (string.IsNullOrEmpty (testDir))
149+
return false;
150+
151+
var testFile = Path.Combine (testDir, Path.GetRandomFileName ());
152+
using (File.Create (testFile, 1, FileOptions.DeleteOnClose)) { }
153+
return true;
154+
}
155+
catch (UnauthorizedAccessException) {
156+
logger (TraceLevel.Warning, $"Target path '{targetPath}' is not writable.");
157+
return false;
158+
}
159+
}
160+
161+
/// <summary>Checks if a path is under a given directory.</summary>
162+
internal static bool IsUnderDirectory (string path, string directory)
163+
{
164+
if (string.IsNullOrEmpty (directory) || string.IsNullOrEmpty (path))
165+
return false;
166+
if (path.Equals (directory, StringComparison.OrdinalIgnoreCase))
167+
return true;
168+
return path.StartsWith (directory + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase);
169+
}
170+
171+
// Returns .msi (Windows), .pkg (macOS), or null (Linux)
172+
internal static string? GetInstallerExtension ()
173+
{
174+
if (OS.IsWindows) return ".msi";
175+
if (OS.IsMac) return ".pkg";
176+
return null;
177+
}
178+
179+
internal static string GetArchiveExtension ()
180+
{
181+
return OS.IsWindows ? ".zip" : ".tar.gz";
182+
}
183+
54184
[DllImport ("libc", SetLastError=true)]
55185
static extern int rename (string old, string @new);
56186
}

‎src/Xamarin.Android.Tools.AndroidSdk/IsExternalInit.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
namespace System.Runtime.CompilerServices
5+
{
6+
// Polyfill for netstandard2.0 to support C# 9+ records and init-only properties.
7+
internal static class IsExternalInit { }
8+
}

0 commit comments

Comments
 (0)