WSLC SDK install fallback uses repair#40908
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts the WSLC SDK’s temporary “fallback to GitHub” installation path (used when Windows Update finds no applicable WSL package update) to use repair semantics, avoiding an incorrect “update not needed” short-circuit caused by comparing against the SDK binary’s wsl::shared::PackageVersion.
Changes:
- Switch the fallback
install::UpdatePackage(...)call toRepair=trueso the GitHub install path does not skip due to version filtering. - Add an inline comment documenting why pre-release builds + repair semantics are required in this SDK-driven fallback.
| } | ||
| wsl::windows::common::install::UpdatePackage(true, false, false); | ||
| // Use pre-release builds and repair semantics (required since this function uses the SDK binary version as a filter). | ||
| wsl::windows::common::install::UpdatePackage(true, true, false); |
There was a problem hiding this comment.
Could we add test coverage for this ? We could do something similar to this test and validate that "a package" is installed (either via DCAT, or injected via the GH URL override).
There was a problem hiding this comment.
For DCAT, we should be able to expect that short of publishing a version incongruent with its actual build number, no available package is newer than anywhere a test is running. However, the change here to force repair means that the code will attempt to install whatever latest version is presented to it. I suppose that an option for the test is to:
- expect no DCAT updates, but at least it ran the search
- return a url to the local server that doesn't exist
- have the test expect the install attempt that fails with a 404-type error
There was a problem hiding this comment.
We could also return a local server URL that actually serves package. If we uninstall it before running the test, we can validate that it was reinstalled once complete
There was a problem hiding this comment.
Although on top of all of that, the API isn't easily testable as we can't request what should be installed. If it can create a session manager, it won't attempt to update. Honestly that seems like a righteous change to the API, as currently you can:
- Detect missing components (
WslcGetMissingComponents) - Install missing components (
WslcInstallWithDependencies) - Determine the installed WSL version (
WslcGetVersion)
But you cannot attempt to update WSL after you have ensured that it is present but found that the installed version is too low for your scenario (3.0.0 adds support for feature X that you need, but 2.9.2 is installed).
Adding a WslcComponentFlags parameter to WslcInstallWithDependencies would allow that. The caller would decide what should be attempted, not us with an effective repeat of WslcGetMissingComponents. I think it would be wise then to also add an options parameter to provide the repair semantics (and potentially more later), as right now we force a DCAT update by overwriting the version.
On top of allowing all of that, it makes it much easier to test as we can directly request that code path be exercised rather than forcing a failed attempt to create a session manager.
There was a problem hiding this comment.
Followup PR for no-update test and API changes.
Summary of the Pull Request
Use repair semantics in the temporary WSLC SDK install API fallback to GitHub.
PR Checklist
Detailed Description of the Pull Request / Additional comments
Switch to repair semantics for this fallback path to prevent the SDK's version from being used as the current WSL install version:
WSL/src/windows/common/install.cpp
Line 89 in f0ba469
Also passes along the "we don't own the process" flag to
DownloadFileto prevent progress output.Validation Steps Performed
Manually confirmed that the latest GH release is installed when no DCAT is available.