Remove the dotnetSdkInstallationEnabled feature flag#14811
Remove the dotnetSdkInstallationEnabled feature flag#14811Copilot wants to merge 4 commits intorelease/13.2from
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14811Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14811" |
There was a problem hiding this comment.
Pull request overview
This PR removes the CLI’s automatic .NET SDK installation capability (and its feature flag), simplifying the SDK “installer” into a pure availability check while keeping the CLI fail-fast behavior when the required SDK isn’t present.
Changes:
- Removed
DotNetSdkInstallationEnabledfeature flag (CLI + VS Code extension schemas) and deleted embedded dotnet-install scripts/workflow. - Simplified
IDotNetSdkInstaller/DotNetSdkInstallerto only check for SDK availability (no install path). - Updated call sites and tests to reflect the new check-only behavior and telemetry outcomes.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/Utils/SdkInstallHelperTests.cs | Updates tests for check-only flow and revised telemetry tags. |
| tests/Aspire.Cli.Tests/TestServices/TestDotNetSdkInstaller.cs | Updates test stub to the new CheckAsync tuple signature. |
| tests/Aspire.Cli.Tests/Templating/DotNetTemplateFactoryTests.cs | Adjusts template tests for updated CheckAsync return shape. |
| tests/Aspire.Cli.Tests/DotNetSdkInstallerTests.cs | Removes install-script expectations and updates checks to new tuple signature. |
| tests/Aspire.Cli.Tests/Commands/SdkInstallerTests.cs | Updates command tests to use the new CheckAsync tuple signature. |
| src/Aspire.Cli/Utils/SdkInstallResult.cs | Prunes enum values to reflect check-only outcomes. |
| src/Aspire.Cli/Utils/SdkInstallHelper.cs | Removes install UX; now only checks and displays an error on failure. |
| src/Aspire.Cli/Utils/SdkCheckResult.cs | Prunes enum values to reflect check-only outcomes. |
| src/Aspire.Cli/Utils/EnvironmentChecker/DotNetSdkCheck.cs | Updates to the new CheckAsync tuple signature. |
| src/Aspire.Cli/Templating/DotNetTemplateFactory.cs | Calls the simplified EnsureSdkInstalledAsync signature. |
| src/Aspire.Cli/Resources/dotnet-install.sh | Deleted (no longer embedding installer scripts). |
| src/Aspire.Cli/Resources/dotnet-install.ps1 | Deleted (no longer embedding installer scripts). |
| src/Aspire.Cli/Projects/DotNetAppHostProject.cs | Updates SDK check calls prior to run/publish. |
| src/Aspire.Cli/NuGet/NuGetPackagePrefetcher.cs | Updates to the new CheckAsync tuple signature. |
| src/Aspire.Cli/KnownFeatures.cs | Removes the dotnetSdkInstallationEnabled feature definition/metadata. |
| src/Aspire.Cli/DotNet/IDotNetSdkInstaller.cs | Removes install API; CheckAsync now returns a 3-tuple. |
| src/Aspire.Cli/DotNet/DotNetSdkInstaller.cs | Deletes installation logic and simplifies constructor/dependencies. |
| src/Aspire.Cli/DotNet/DotNetCliRunner.cs | Removes DOTNET_ROLL_FORWARD=LatestMajor injection tied to auto-install. |
| src/Aspire.Cli/Commands/InitCommand.cs | Updates SDK check call site for new helper signature. |
| src/Aspire.Cli/Commands/ExecCommand.cs | Updates SDK check call site for new helper signature. |
| src/Aspire.Cli/Commands/AddCommand.cs | Updates SDK check call site for new helper signature. |
| src/Aspire.Cli/Aspire.Cli.csproj | Removes embedded resource entries for installer scripts. |
| extension/schemas/aspire-settings.schema.json | Removes dotnetSdkInstallationEnabled setting. |
| extension/schemas/aspire-global-settings.schema.json | Removes dotnetSdkInstallationEnabled setting. |
| .github/workflows/update-dotnet-install-scripts.yml | Deleted (no longer maintaining embedded scripts). |
Comments suppressed due to low confidence (2)
tests/Aspire.Cli.Tests/Utils/SdkInstallHelperTests.cs:92
- The test
EnsureSdkInstalledAsync_WhenSdkMissing_DisplaysErrorno longer asserts that an error was actually displayed; it only checks telemetry tags. This makes the test name/intent misleading and leaves the user-facing error path unverified.
Set interactionService.DisplayErrorCallback to capture the message and assert it contains the minimum required version and the detected version.
public async Task EnsureSdkInstalledAsync_WhenSdkMissing_DisplaysError()
{
using var fixture = new TelemetryFixture();
var sdkInstaller = new TestDotNetSdkInstaller
{
CheckAsyncCallback = _ => (false, "8.0.100", "9.0.302")
};
var interactionService = new TestConsoleInteractionService();
var result = await SdkInstallHelper.EnsureSdkInstalledAsync(
sdkInstaller,
interactionService,
fixture.Telemetry);
Assert.False(result);
Assert.NotNull(fixture.CapturedActivity);
var tags = fixture.CapturedActivity.Tags.ToDictionary(t => t.Key, t => t.Value);
Assert.Equal("8.0.100", tags[TelemetryConstants.Tags.SdkDetectedVersion]);
Assert.Equal("9.0.302", tags[TelemetryConstants.Tags.SdkMinimumRequiredVersion]);
Assert.Equal("not_installed", tags[TelemetryConstants.Tags.SdkCheckResult]);
Assert.Equal("not_installed", tags[TelemetryConstants.Tags.SdkInstallResult]);
}
src/Aspire.Cli/Utils/SdkInstallResult.cs:19
SdkInstallResultis now documented as an “availability check”, but the type name still implies an installation result. With installation removed, this mismatch is easy to misread when maintaining telemetry/tagging code.
Consider renaming the enum to reflect the new behavior (or at least clarify in the XML doc that the name is retained for historical/telemetry reasons).
/// <summary>
/// Result of an SDK availability check.
/// </summary>
internal enum SdkInstallResult
{
/// <summary>
/// A valid SDK was already installed.
/// </summary>
AlreadyInstalled,
/// <summary>
/// The SDK is missing or does not meet the minimum required version.
/// </summary>
NotInstalled
| protected override async Task<int> ExecuteAsync(ParseResult parseResult, CancellationToken cancellationToken) | ||
| { | ||
| // Check if the .NET SDK is available | ||
| if (!await SdkInstallHelper.EnsureSdkInstalledAsync(_sdkInstaller, InteractionService, _features, Telemetry, _hostEnvironment, cancellationToken)) | ||
| if (!await SdkInstallHelper.EnsureSdkInstalledAsync(_sdkInstaller, InteractionService, Telemetry, cancellationToken)) | ||
| { | ||
| return ExitCodeConstants.SdkNotInstalled; | ||
| } |
There was a problem hiding this comment.
After removing the SDK-install UX, _hostEnvironment and _features are no longer used anywhere in this class (only assigned). With EnforceCodeStyleInBuild + TreatWarningsAsErrors, this typically triggers IDE0051 (unused private members) and breaks the build.
Remove the unused fields (and, if possible, the now-unneeded constructor parameters) to avoid analyzer failures.
|
|
||
| // For C#, we need the .NET SDK | ||
| if (!await SdkInstallHelper.EnsureSdkInstalledAsync(_sdkInstaller, InteractionService, _features, Telemetry, _hostEnvironment, cancellationToken)) | ||
| if (!await SdkInstallHelper.EnsureSdkInstalledAsync(_sdkInstaller, InteractionService, Telemetry, cancellationToken)) | ||
| { | ||
| return ExitCodeConstants.SdkNotInstalled; | ||
| } |
There was a problem hiding this comment.
InitCommand still stores _hostEnvironment and _features, but they are no longer referenced after the EnsureSdkInstalledAsync signature change. Given this repo sets EnforceCodeStyleInBuild=true and TreatWarningsAsErrors=true, unused private fields commonly produce IDE0051 and can fail CI.
Remove the unused fields (and consider dropping the corresponding constructor parameters if they’re no longer needed) to keep the build clean.
| // Only set the environment variable if it's not already set by the user | ||
| if (!finalEnv.ContainsKey("DOTNET_ROLL_FORWARD")) | ||
| { | ||
| finalEnv["DOTNET_ROLL_FORWARD"] = "LatestMajor"; |
🎬 CLI E2E Test RecordingsThe following terminal recordings are available for commit
📹 Recordings uploaded automatically from CI run #22532455799 |
|
This looks like an issue with JSON formatting: |
… logic - Remove DotNetSdkInstallationEnabled from KnownFeatures.cs - Remove dotnetSdkInstallationEnabled from VS Code extension schemas - Simplify IDotNetSdkInstaller: remove InstallAsync, drop ForceInstall from CheckAsync - Simplify DotNetSdkInstaller: remove InstallAsync, install helpers, forceInstall logic, unused constructor params - Simplify SdkInstallHelper.EnsureSdkInstalledAsync: remove features/hostEnvironment params - Prune SdkInstallResult and SdkCheckResult enums - Remove DOTNET_ROLL_FORWARD=LatestMajor injection from DotNetCliRunner - Delete dotnet-install.sh, dotnet-install.ps1, update-dotnet-install-scripts.yml - Remove embedded resource entries from Aspire.Cli.csproj - Update all callers and tests Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
Remove unused _hostEnvironment fields and constructor parameters from ExecCommand and InitCommand. Rename HighestInstalledVersion to HighestDetectedVersion in TestDotNetSdkInstaller to match IDotNetSdkInstaller.CheckAsync. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
b2a84e0 to
ab6ba2a
Compare
Description
Users are responsible for acquiring .NET SDK. The CLI still fails fast when the required SDK version is absent, but no longer offers automatic download/installation.
Feature flag removal
DotNetSdkInstallationEnabledfromKnownFeatures.csand both VS Code extension schemasSDK installer simplification
IDotNetSdkInstaller: removedInstallAsync;CheckAsyncreturns(Success, HighestDetectedVersion, MinimumRequiredVersion)— droppedForceInstallDotNetSdkInstaller: removedInstallAsync, all install helpers,forceInstall/alwaysInstallSdklogic, private SDK dir check, unused constructor params (executionContext,dotNetCliRunner,logger)SdkInstallHelper.EnsureSdkInstalledAsync: removedfeaturesandhostEnvironmentparams; now just checks and displays error on failureSdkInstallResult/SdkCheckResult: pruned toAlreadyInstalled/NotInstalledRuntime behavior
DOTNET_ROLL_FORWARD=LatestMajorinjection fromDotNetCliRunner(only needed to support auto-installed SDKs)Deleted files
src/Aspire.Cli/Resources/dotnet-install.shsrc/Aspire.Cli/Resources/dotnet-install.ps1.github/workflows/update-dotnet-install-scripts.ymlChecklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue:Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
centralus-2.in.applicationinsights.azure.com/home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests --internal-msbuild-node /home/REDACTED/.local/share/fbbd10cf7c5c45b9a1742c769b8bd30c/.p --filter-not-trait category=failing --filter-not-trait quarantined=true --filter-not-trait outerloop=true(dns block)/usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests.deps.json /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Microsoft.DotNet.RemoteExecutor.dll Aspire.Cli.Tests, Version=42.42.42.42, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51 Aspire.Cli.Tests.CliSmokeTests+<>c <LocaleOverrideReturnsExitCode>b__4_0 /tmp/bfvrwdhr.gc1 invalid-locale False ASPIRE_LOCALE_OVERRIDE(dns block)/usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Aspire.Cli.Tests.deps.json /home/REDACTED/work/aspire/aspire/artifacts/bin/Aspire.Cli.Tests/Debug/net10.0/Microsoft.DotNet.RemoteExecutor.dll Aspire.Cli.Tests, Version=42.42.42.42, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51 Aspire.Cli.Tests.CliSmokeTests+<>c <LocaleOverrideReturnsExitCode>b__4_0 /tmp/bfvrwdhr.gc1(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
This section details on the original issue you should resolve
<issue_title>Remove the dotnetSdkInstallationEnabled feature flag</issue_title>
<issue_description>We've decided that the user is responsible to acquiring .NET for C# based apphosts.</issue_description>
<agent_instructions>Users are responsible for acquiring .NET for C# apphosts. The CLI still fails fast when the required SDK version is absent, but no longer offers automatic download and installation.
Changes
Feature flag removal
DotNetSdkInstallationEnabledfromKnownFeatures.csand both VS Code extension schemasSDK installer simplification
IDotNetSdkInstaller: removedInstallAsync;CheckAsyncnow returns(Success, HighestDetectedVersion, MinimumRequiredVersion)(droppedForceInstall)DotNetSdkInstaller: removedInstallAsync, all install helpers,forceInstall/alwaysInstallSdklogic, private SDK dir check, and unused constructor params (executionContext,dotNetCliRunner,logger)SdkInstallHelper.EnsureSdkInstalledAsync: removedfeaturesandhostEnvironmentparams; now just checks availability and displays an error on failureSdkInstallResult/SdkCheckResult: pruned toAlreadyInstalled/NotInstalledonlyRuntime behavior
DOTNET_ROLL_FORWARD=LatestMajorinjection fromDotNetCliRunner(was needed only to support auto-installed SDKs)Deleted files
src/Aspire.Cli/Resources/dotnet-install.shsrc/Aspire.Cli/Resources/dotnet-install.ps1.github/workflows/update-dotnet-install-scripts.ymlTests: updated
TestDotNetSdkInstaller,SdkInstallHelperTests,DotNetSdkInstallerTests, andSdkInstallerTeststo reflect the simplified interface.Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue:💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.