Skip to content

Preserve explicit AndroidSdkHome when directory doesn't exist yet - #91

Merged
Redth merged 1 commit into
Redth:mainfrom
dalexsoto:fix/preserve-explicit-sdk-home
Mar 29, 2026
Merged

Redth merged 1 commit into
Redth:mainfrom
dalexsoto:fix/preserve-explicit-sdk-home

Conversation

@dalexsoto

Copy link
Copy Markdown
Collaborator

Problem

When calling AndroidSdkManager.Acquire() or SdkManager.DownloadSdk() with a target directory that doesn't exist yet (the typical fresh-install scenario), the operation fails with:

Android SDK Directory was not specified.

Root Cause

Both SdkTool and AndroidSdkManager constructors pass the user-specified path through SdkLocator.Locate(), which only returns paths where Directory.Exists() is true. When the target directory doesn't exist yet:

  1. Locate() discards the user's path
  2. It may return a different SDK found via env vars or known paths (e.g. ~/.android)
  3. Or it returns nothing, leaving AndroidSdkHome as null
  4. DownloadSdk() then throws because it has no target directory

Fix

When a path is explicitly provided via SdkToolOptions.AndroidSdkHome or the AndroidSdkManager(DirectoryInfo) constructor, honor it directly instead of passing it through Locate(). Auto-discovery via SdkLocator is only used when no path is specified.

Also removes a redundant re-assignment in the obsolete SdkTool(DirectoryInfo?) constructor that was overwriting the result from the delegated constructor.

Tests

Added SdkTool_PreserveHome_Tests with 6 tests covering:

  • SdkManager preserves home when directory doesn't exist
  • AndroidSdkManager preserves home when directory doesn't exist
  • DownloadSdk doesn't throw "not specified" for non-existent directory
  • Both types still work correctly with existing directories
  • Auto-discovery still works when no path is specified

When a user specifies an explicit AndroidSdkHome directory (e.g. for
Acquire/DownloadSdk), the SdkLocator.Locate() call discards it because
PathLocator only returns paths where Directory.Exists() is true. This
causes AndroidSdkHome to be null or resolve to a different SDK,
making Acquire() fail with:
  'Android SDK Directory was not specified.'

Honor the explicitly provided path directly instead of passing it
through Locate(). Only auto-discover via SdkLocator when no path
is specified.

Also remove the redundant re-assignment in the obsolete
SdkTool(DirectoryInfo?) constructor which was overwriting the result
from the delegated SdkToolOptions constructor.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dalexsoto
dalexsoto force-pushed the fix/preserve-explicit-sdk-home branch from 38e134b to 6bdd0e0 Compare March 27, 2026 20:24
@dalexsoto
dalexsoto requested a review from Redth March 27, 2026 20:26
@Redth
Redth merged commit 9cee679 into Redth:main Mar 29, 2026
6 checks passed
dalexsoto added a commit to Redth/MAUI.Sherpa that referenced this pull request Mar 31, 2026
## Problem

The Doctor's 'Fix: Android SDK' action failed with:
  'Android SDK Directory was not specified.'

Even after fixing the upstream AndroidSdk.Tools library (PR #91), a
second issue remained: after successfully acquiring the SDK to
~/android-sdk, restarting the app caused the Doctor to pick up
~/.android instead — a user config directory, not an SDK installation.

## Root Cause

Two bugs contributed to the broken experience:

### 1. Upstream: SdkLocator discarded non-existent paths (fixed in AndroidSdk 0.35.1)

AndroidSdkManager and SdkTool constructors passed the user-specified
target directory through SdkLocator.Locate(), which only returns paths
where Directory.Exists() is true. When acquiring a fresh SDK, the
target directory doesn't exist yet, so:
  - Locate() discarded the user's path
  - It returned either null or a different directory (e.g. ~/.android)
  - DownloadSdk() then threw because it had no valid target

This was fixed upstream in Redth/AndroidSdk.Tools#91 and released
in AndroidSdk 0.35.1.

### 2. Local: Acquired SDK path was never persisted

After AcquireSdkAsync successfully installed the SDK to ~/android-sdk:
  - The in-memory _sdkManager was updated correctly
  - SdkPathChanged event was NOT fired (missing invoke)
  - The path was NOT saved to secure storage

On app restart, AndroidSdkSettingsService.InitializeAsync() found no
saved custom path and fell back to DetectSdkAsync(), which auto-
discovered ~/.android (a config directory created during the fix
process) instead of ~/android-sdk (the actual SDK installation).

## Changes

### AndroidSdkService.cs
- Fire SdkPathChanged event after successful SDK acquisition so that
  listeners (device watchers, UI pages) are notified immediately

### DoctorService.cs
- Accept optional IAndroidSdkSettingsService via constructor injection
- After a successful 'install-android-sdk' fix action, persist the
  acquired SDK path via SetCustomSdkPathAsync() so it survives app
  restarts and is not overridden by auto-detection

### MauiSherpa.Core.csproj
- Update AndroidSdk from 0.33.0 to 0.35.1 (includes upstream fix)
- Update AndroidSdk.Adbd from 0.33.0 to 0.35.1

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dalexsoto added a commit to Redth/MAUI.Sherpa that referenced this pull request Mar 31, 2026
## Problem

The Doctor's 'Fix: Android SDK' action failed with:
  'Android SDK Directory was not specified.'

Even after fixing the upstream AndroidSdk.Tools library (PR #91), a
second issue remained: after successfully acquiring the SDK to
~/android-sdk, restarting the app caused the Doctor to pick up
~/.android instead — a user config directory, not an SDK installation.

## Root Cause

Two bugs contributed to the broken experience:

### 1. Upstream: SdkLocator discarded non-existent paths (fixed in AndroidSdk 0.35.1)

AndroidSdkManager and SdkTool constructors passed the user-specified
target directory through SdkLocator.Locate(), which only returns paths
where Directory.Exists() is true. When acquiring a fresh SDK, the
target directory doesn't exist yet, so:
  - Locate() discarded the user's path
  - It returned either null or a different directory (e.g. ~/.android)
  - DownloadSdk() then threw because it had no valid target

This was fixed upstream in Redth/AndroidSdk.Tools#91 and released
in AndroidSdk 0.35.1.

### 2. Local: Acquired SDK path was never persisted

After AcquireSdkAsync successfully installed the SDK to ~/android-sdk:
  - The in-memory _sdkManager was updated correctly
  - SdkPathChanged event was NOT fired (missing invoke)
  - The path was NOT saved to secure storage

On app restart, AndroidSdkSettingsService.InitializeAsync() found no
saved custom path and fell back to DetectSdkAsync(), which auto-
discovered ~/.android (a config directory created during the fix
process) instead of ~/android-sdk (the actual SDK installation).

## Changes

### AndroidSdkService.cs
- Fire SdkPathChanged event after successful SDK acquisition so that
  listeners (device watchers, UI pages) are notified immediately

### DoctorService.cs
- Accept optional IAndroidSdkSettingsService via constructor injection
- After a successful 'install-android-sdk' fix action, persist the
  acquired SDK path via SetCustomSdkPathAsync() so it survives app
  restarts and is not overridden by auto-detection

### MauiSherpa.Core.csproj
- Update AndroidSdk from 0.33.0 to 0.35.1 (includes upstream fix)
- Update AndroidSdk.Adbd from 0.33.0 to 0.35.1

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants