Skip to content

CLI: Relative volume path support, improved error detection and messaging, more volume tests#40193

Open
dkbennett wants to merge 5 commits intofeature/wsl-for-appsfrom
user/dkbennett/relativevolumepaths
Open

CLI: Relative volume path support, improved error detection and messaging, more volume tests#40193
dkbennett wants to merge 5 commits intofeature/wsl-for-appsfrom
user/dkbennett/relativevolumepaths

Conversation

@dkbennett
Copy link
Copy Markdown
Member

@dkbennett dkbennett commented Apr 15, 2026

Summary of the Pull Request

This primarily adds support for relative volume paths in volume specifications.
It also adds:

  • Localization to the volume error messages
  • Empty container path check
  • Invalid windows host path check
  • Docker named volume name check

This also removes all known cases of "Unspecified error. E_FAIL" messages that could occur with volume specifier strings due to invalid input being passed into the service. The CLI now checks for the obvious error conditions and provides a better user message in the case of bad input.

Tests are also updated to reflect any changes in messaging and behavior as a result of the new empty checks and relative path.

image

PR Checklist

  • Closes: Link to issue #xxx
  • 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

  • Added localization strings for volume error messages in the CLI
  • Added empty container check and better error messaging.
  • Added relative path checks using Windows API for resolving the path and checking for names.
  • Added docker named volume name check based on docker's regex for valid names. This will be used for adding in named volume support.

Currently named volumes can be detected but are ignored and treated as relative paths until named volume support is completed.

Relative paths function with forward and backward slashes and validates against the Windows API for valid path. The paths provided for the host are not required to exist on disk, but they are required to be a valid resolved name.

Validation Steps Performed

  • Many unit tests updated for invalid input checks and valid input checks.
  • New unit tests for verifying relative path name resolution.
  • New E2E test for relative path read/write
  • Updated E2E tests for unsupported & supported conditions and updated error messages.
  • Also manual validation of the behavior.

@dkbennett dkbennett requested a review from a team as a code owner April 15, 2026 20:42
Copilot AI review requested due to automatic review settings April 15, 2026 20:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds CLI-side validation and relative host-path handling for --volume specs to avoid opaque E_FAIL errors, while updating tests and localized error messages accordingly.

Changes:

  • Add relative host-path resolution for volume mounts and validate path/name inputs.
  • Localize volume-spec error messages and improve error detection (empty container path, invalid Windows host path).
  • Expand/adjust unit + E2E tests to cover the new parsing rules and messaging.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp Adds E2E coverage for relative host-path volumes; updates expected stderr strings for invalid specs.
test/windows/wslc/WSLCVolumeMountUnitTests.cpp Updates parse tests for relative paths and adds named-volume name validation tests.
test/windows/wslc/CommandLineTestCases.h Updates CLI parsing testcases to use ./ volume host syntax.
src/windows/wslc/services/ContainerModel.h Exposes VolumeMount::IsValidNamedVolumeName.
src/windows/wslc/services/ContainerModel.cpp Implements named-volume name validation, adds relative path resolution + improved/localized validation errors.
localization/strings/en-US/Resources.resw Adds localized strings for the new volume validation messages/usage text.

Comment thread src/windows/wslc/services/ContainerModel.cpp
Comment thread src/windows/wslc/services/ContainerModel.cpp
Comment thread src/windows/wslc/services/ContainerModel.cpp Outdated
Comment thread src/windows/wslc/services/ContainerModel.cpp Outdated
Comment thread localization/strings/en-US/Resources.resw
Comment thread localization/strings/en-US/Resources.resw Outdated
Comment thread test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 15, 2026 20:55
Comment thread src/windows/wslc/services/ContainerModel.cpp Outdated
Comment thread src/windows/wslc/services/ContainerModel.cpp Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Comment thread src/windows/wslc/services/ContainerModel.cpp Outdated
Comment thread src/windows/wslc/services/ContainerModel.cpp
Comment thread src/windows/wslc/services/ContainerModel.cpp
Comment thread test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp Outdated
Comment thread test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp
Comment thread src/windows/wslc/services/ContainerModel.cpp
// This can be either an existing named volume or a new named volume that will be created.
if (VolumeMount::IsValidNamedVolumeName(rawHostPath))
{
// TODO: Handle named volume reference in the request.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for adding this block. I will investigate what is needed to wire this up

// Returns true if the given string is a valid Docker named volume name.
// Based on Docker's named volume validation: ^[a-zA-Z0-9][a-zA-Z0-9_.-]{1,}$
// Source: https://github.com/moby/moby/blob/master/volume/validate.go
bool VolumeMount::IsValidNamedVolumeName(const std::wstring& name)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks for adding this validation and the original reference!

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.

5 participants