Report an undetermined WSL version instead of asserting WSL2 - #19158
Report an undetermined WSL version instead of asserting WSL2#19158Adam Ratzman (adamint) wants to merge 2 commits into
Conversation
The WSL check classified the version by parsing a kernel major version out of /proc/version and treating anything >= 4 as WSL2. WSL1 has no kernel of its own and always reports a fixed 4.4.0 compatibility banner, so that comparison classified every real WSL1 system as WSL2. The check then emitted a green "WSL2 environment detected" row, which meant the limited-container-support warning the check exists to surface never fired for the users who needed it. The same method also collapsed two distinct unknowns into confident answers. A missing, unreadable, or unrecognized /proc/version fell through to WSL2 and passed, while a Microsoft banner whose version could not be parsed fell through to WSL1 and told the user to upgrade to a version they may already run. Classify from the markers each version actually writes: WSL2 kernels carry a "WSL2" suffix, and WSL1 carries the 4.4.0 "-Microsoft" compatibility banner. Anything else, including a custom kernel configured through .wslconfig, is now reported as a distinct warning naming the file that could not be classified, so doctor never asserts an environment it was unable to observe. The banner read moves behind a constructor seam so the classification can be tested without the host being WSL. Reverting the classification fails 7 of the 15 new tests, including a real WSL1 banner reported as WSL2 and an unreadable banner reported as pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19158Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19158" |
There was a problem hiding this comment.
Pull request overview
Corrects aspire doctor WSL detection to avoid misreporting WSL1 or unknown environments as WSL2.
Changes:
- Adds three-state WSL classification and actionable warnings.
- Introduces injectable kernel-banner reading.
- Adds 15 focused regression tests.
Show a summary per file
| File | Description |
|---|---|
src/Aspire.Cli/Utils/EnvironmentChecker/WslEnvironmentCheck.cs |
Implements marker-based WSL classification. |
tests/Aspire.Cli.Tests/Commands/WslEnvironmentCheckTests.cs |
Covers WSL1, WSL2, unknown, and non-WSL scenarios. |
Review details
Suppressed comments (2)
src/Aspire.Cli/Utils/EnvironmentChecker/WslEnvironmentCheck.cs:159
- The WSL1 matcher is broader than the fixed marker it is intended to recognize: it accepts every
4.4.xrelease, while the separate check on line 132 allowsMicrosoftto occur anywhere in compiler/build metadata. For example,Linux version 4.4.1-custom (Microsoft@builder)is reported as WSL1 instead of unknown. Match the complete4.4.0-<build>-Microsoftrelease token so custom/native kernels are not given a confident WSL1 warning.
[GeneratedRegex(@"Linux\s+version\s+4\.4\.", RegexOptions.IgnoreCase)]
src/Aspire.Cli/Utils/EnvironmentChecker/WslEnvironmentCheck.cs:121
- This searches the entire
/proc/versionpayload, not the kernel release token described above. Build metadata follows that token, so a native/custom banner such asLinux version 6.1.0-custom (root@WSL2-builder) ...is classified as WSL2 and produces a passing row even though no WSL2 kernel marker was observed. Restrict the match to the release token afterLinux version.
if (procVersion.Contains("WSL2", StringComparison.OrdinalIgnoreCase))
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Review details
Suppressed comments (1)
src/Aspire.Cli/Utils/EnvironmentChecker/WslEnvironmentCheck.cs:123
- Microsoft's WSL kernel release notes list the 4.19 WSL2 releases as
4.19.84-microsoft-standardthrough4.19.128-microsoft-standard; those banners do not contain the literalWSL2. This condition therefore classifies genuine early WSL2 installations asUnknownand emits an unnecessary warning/upgrade command. Recognize themicrosoft-standardmarker as WSL2 as well, and cover an actual 4.19 banner in the regression cases.
// WSL 2 runs a genuine Microsoft-built kernel whose release string carries a "WSL2" marker:
// Linux version 5.15.90.1-microsoft-standard-WSL2 (oe-user@oe-host) (...) #1 SMP ...
if (procVersion.Contains("WSL2", StringComparison.OrdinalIgnoreCase))
{
return WslVersion.Wsl2;
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Description
aspire doctor'swslcheck classified the WSL version by parsing a kernel major version out of/proc/versionand treating anything>= 4as WSL2:That premise is wrong. WSL1 has no kernel of its own and always reports a fixed 4.4.0 compatibility banner:
So
majorVersionis 4, the comparison succeeds, and every real WSL1 system was classified as WSL2. The check then emitted a greenWSL2 environment detectedrow, which means the "WSL1 detected - limited container support" warning never appeared.The same method collapsed two distinct unknowns into confident answers:
/proc/versionfell through toreturn 2and reported pass. WSL injectsWSL_DISTRO_NAMEinto every distribution shell, so the check could know it was in WSL while being unable to know which version.return 1and reported WSL1, telling the user to runwsl --set-version <distro> 2for a version they may already be running.Fix
Classify from the markers each version actually writes rather than from an ordinal comparison. WSL2 kernels carry a
WSL2suffix (5.15.90.1-microsoft-standard-WSL2); WSL1 carries the 4.4.0-Microsoftcompatibility banner; anything else isUnknown.The outcome is three-state rather than two. Doctor never asserts an environment it was unable to observe, because a passing row tells the user to stop investigating.
The banner read moves behind a constructor seam, mirroring the existing
OperatingSystemCheck(Func<OperatingSystemDetails>)test constructor in the same folder, so classification is testable without the host being WSL.Also in this change
Two adjacent doctor checks had the same blank-string problem, and the new tests cover the behavior:
ContainerRuntimeCheck.GetConfiguredRuntimenow treats blankASPIRE_CONTAINER_RUNTIMEas unset, so it can fall back toDOTNET_ASPIRE_CONTAINER_RUNTIMEinstead of honoring whitespace as a configured runtime name.DevCertsChecknow treats blankSSL_CERT_DIRas unset when building its fix command, so the generated command does not preserve an empty path as though it were user configuration.Verification
Aspire.Cli.Tests— full run: 4832 total, 0 failed, 0 warnings. The WSL check previously had no test coverage; it now has 15 tests.The tests are load-bearing. Restoring the original
majorVersion >= 4classification fails 7 of 15:DetermineWslVersion_ReportsWsl1_ForRealWsl1BannerWsl1Wsl2CheckAsync_ReportsWarning_ForWsl1WarningPassCheckAsync_ReportsWarning_WhenBannerIsUnreadableButDistroNameIsSetWarningPassDetermineWslVersion_ReportsUnknown_WhenBannerIsUnavailableUnknownWsl2DetermineWslVersion_ReportsUnknown_WhenBannerIsBlankUnknownWsl2DetermineWslVersion_ReportsUnknown_ForCustomKernelWithoutMarkersUnknownWsl2DetermineWslVersion_ReportsUnknown_WhenMicrosoftBannerHasNoRecognizableVersionUnknownWsl1The first row is the user-visible defect; the last is the opposite failure mode, where a probe that could not determine an answer produced a confident warning.
Reference for the WSL1/WSL2 kernel distinction: https://learn.microsoft.com/windows/wsl/compare-versions
Checklist