dotnet / dotnet/sdk

dotnetup: Add hookable/hideable progress target for background predownloads

Open
#53,818 0 comments 0 reactions 0 assignees View on GitHub
Area-dotnetup
Dominant language
C#
Stars
3.2k
Forks
1.3k
PR merge metrics
PR metrics pending

Description

## Problem

When the walkthrough kicks off a background predownload via `InstallerOrchestratorSingleton.PredownloadToCacheAsync`, it uses a `NullProgressTarget` that silently swallows all progress. If the download takes a long time and the user answers the walkthrough prompts quickly, `RunPrimaryInstall` blocks on `predownloadTask?.GetAwaiter().GetResult()` with zero UI feedback — making it look like the app hung.

Reference: [PR #53464 discussion (dsplaisted)](https://github.com/dotnet/sdk/pull/53464#discussion_r3061497905)

> "What if a download takes a long time and the user answers the mode question quickly? Then I think it will need to wait for the download progress here, but there won't be any UI progress, so it will look like it hung."

## Current Behavior

- `PredownloadToCacheAsync` creates a `LazyProgressReporter(new NullProgressTarget())` — all progress updates are discarded.
- `RunPrimaryInstall` synchronously waits on the predownload task before calling the real install.
- During the wait there is no spinner, progress bar, or any console output.

## Desired Behavior

When the predownload is still running at the point we need its result, the user should see progress feedback. Two potential approaches:

### Option A: Switchable / hookable progress target

Create an `IProgressTarget` implementation that starts hidden (like `NullProgressTarget`) but can be "activated" later to forward progress to a real target (e.g., `SpectreProgressTarget`). API sketch:

```csharp
public class SwitchableProgressTarget : IProgressTarget
{
private volatile IProgressTarget _inner = new NullProgressTarget();

///
/// Activates visible progress. Any tasks already tracked begin rendering.
///
public void Activate(IProgressTarget visibleTarget)
{
_inner = visibleTarget;
// Transfer any already-tracked tasks to the new target
}

public IProgressReporter CreateProgressReporter() => _inner.CreateProgressReporter();
}
```

The walkthrough would call `Activate(new SpectreProgressTarget())` just before the synchronous wait, so progress appears only if the user is actually waiting.

### Option B: Hidden-with-reveal progress target

A single `IProgressTarget` that renders nothing until `Show()` is called, at which point it begins rendering in-place (e.g., a Spectre progress bar or a simple "Downloading..." message).

```csharp
public class HideableProgressTarget : IProgressTarget
{
private bool _visible;

public void Show() { _visible = true; /* begin rendering tracked tasks */ }
public IProgressReporter CreateProgressReporter() => new HideableReporter(this);
}
```

### Option C: Simplest — just show a message

Instead of a full progress target switch, `RunPrimaryInstall` could render a "Waiting for download to complete..." message (or a Spectre spinner) around the `GetAwaiter().GetResult()` call if the task is still incomplete. This avoids changes to the progress target hierarchy entirely.

## Acceptance Criteria

- [ ] If the predownload is still in progress when the walkthrough finishes prompts, the user sees some indication that work is happening (spinner, progress bar, or status message).
- [ ] If the predownload finishes before the prompts, no extra UI is shown.
- [ ] No regressions to `--noninteractive` / `--no-progress` behavior.
- [ ] Thread safety is maintained — the predownload runs on a background thread.

## Relevant Code

- [`InstallerOrchestratorSingleton.cs`](../dotnetup/InstallerOrchestratorSingleton.cs) — `PredownloadToCacheAsync` uses `NullProgressTarget`
- [`WalkthroughWorkflows.cs`](../dotnetup/Commands/Walkthrough/WalkthroughWorkflows.cs) — `RunPrimaryInstall` waits on the predownload task
- [`IProgressTarget.cs`](../Microsoft.Dotnet.Installation/IProgressTarget.cs) — `NullProgressTarget` definition
- [`LazyProgressReporter.cs`](../Microsoft.Dotnet.Installation/LazyProgressReporter.cs) — deferred reporter creation
- [`SpectreProgressTarget.cs`](../dotnetup/SpectreProgressTarget.cs) — visible Spectre-based implementation
- [`NonUpdatingProgressTarget.cs`](../dotnetup/NonUpdatingProgressTarget.cs) — text-only progress (prints "Downloading...")

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.