Skip to content

Validate custom install locations before downloading distributions - #41628

Draft
Ben Hillis (benhillis) with Copilot wants to merge 2 commits into
masterfrom
copilot/validate-location-path-existence
Draft

Ben Hillis (benhillis) with Copilot wants to merge 2 commits into
masterfrom
copilot/validate-location-path-existence

Conversation

Copilot AI commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Summary of the Pull Request

wsl --install --location deferred path creation until after downloading the distribution. Custom locations are now created and validated before payload acquisition, failing immediately when unusable.

PR Checklist

  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

  • Pre-creates custom install directories before distribution download.
  • Removes newly created empty directories when installation fails.
  • Preserves existing directories and service-side path conflict validation.

Validation Steps Performed

  • Added regression coverage verifying an invalid location fails before network download begins.

Copilot AI lite review requested due to automatic review settings September 16, 2026 20:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Co-authored-by: benhillis <17727402+benhillis@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 16, 2026 20:52
Copilot AI changed the title [WIP] Add validation for target directory existence before downloading Validate custom install locations before downloading distributions Sep 16, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved directory cleanup and path validation issues remain, along with a localization gap and missing regression coverage.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

src/windows/common/WslInstall.cpp:294

  • Failed to create install location: %ls is a newly introduced English string on the wsl --install error path, but it has no localization resource. Use a Message... helper with the path/error details as placeholders and THROW_HR_WITH_USER_ERROR so failures from this preflight validation follow the localized error handling used elsewhere in this component.
        THROW_IF_WIN32_ERROR_MSG(error.value(), "Failed to create install location: %ls", location->c_str());

src/windows/common/WslInstall.cpp:294

  • create_directories treats an already-existing path as successful with no error even when it is a regular file. Because directoryCreated is false in that case, this proceeds to download and the service only fails later when it tries to install beneath the file, so the preflight still accepts an unusable --location. Check is_directory (and propagate its error) after this call before starting acquisition.
        directoryCreated = std::filesystem::create_directories(*location, error);
        THROW_IF_WIN32_ERROR_MSG(error.value(), "Failed to create install location: %ls", location->c_str());

test/windows/UnitTests.cpp:5683

  • This test only exercises the error path where create_directories fails immediately because g_testDistroPath is a file. It does not cover the new directoryCreated == true branch (successful creation followed by download/import failure), so it would not catch the incomplete cleanup cases above. Add a nested, otherwise-valid location and assert that the newly created directories are removed after failure.
                auto [output, error] = LxsstuLaunchWslAndCaptureOutput(
                    std::format(L"--install invalid-location --no-launch --location \"{}\"", invalidLocation.native()), -1);
                VERIFY_IS_TRUE(output.find(L"Downloading:") == std::wstring::npos);
                VERIFY_IS_FALSE(error.empty());
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment on lines +297 to +301
auto directoryCleanup = wil::scope_exit_log(WI_DIAGNOSTICS_INFO, [&]() {
if (directoryCreated)
{
LOG_IF_WIN32_BOOL_FALSE(RemoveDirectoryW(location->c_str()));
}

This branch has not been deployed

No deployments
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.

wsl --install --location downloads distribution before validating target directory existence

3 participants