CLI: Add image import command#40309
CLI: Add image import command#40309AmelBawa-msft wants to merge 2 commits intofeature/wsl-for-appsfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new wslc image import command (and root-level wslc import alias) intended to import an image from a tarball, wiring it through CLI tasks into the session image import API and adding E2E coverage + localization strings.
Changes:
- Introduces
ImageImportCommandand registers it under bothimagesubcommands and the root command list. - Adds
ImportImagetask +ImageService::Import()implementation to callIWSLCSession::ImportImage. - Adds E2E tests and localized strings/descriptions for the new command and its arguments.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/windows/wslc/e2e/WSLCE2EImageTests.cpp | Adds import to the wslc image subcommand help expectations. |
| test/windows/wslc/e2e/WSLCE2EImageImportTests.cpp | New E2E test suite covering image import help/errors/success paths. |
| test/windows/wslc/e2e/WSLCE2EGlobalTests.cpp | Adds import to root command help expectations. |
| src/windows/wslc/tasks/ImageTasks.h | Declares ImportImage task entry point. |
| src/windows/wslc/tasks/ImageTasks.cpp | Implements ImportImage task and forwards to ImageService::Import. |
| src/windows/wslc/services/ImageService.h | Declares ImageService::Import. |
| src/windows/wslc/services/ImageService.cpp | Implements import-from-file/stdin handle resolution and calls COM ImportImage. |
| src/windows/wslc/commands/RootCommand.cpp | Registers root-level import command. |
| src/windows/wslc/commands/ImageImportCommand.cpp | New command implementation and task pipeline setup. |
| src/windows/wslc/commands/ImageCommand.h | Declares ImageImportCommand. |
| src/windows/wslc/commands/ImageCommand.cpp | Registers import under wslc image. |
| src/windows/wslc/arguments/ArgumentDefinitions.h | Adds new positional ImportFile argument definition. |
| localization/strings/en-US/Resources.resw | Adds localized descriptions for image import and its file argument. |
| if (input == L"-") | ||
| { | ||
| imageHandle = GetStdHandle(STD_INPUT_HANDLE); | ||
| } | ||
| else | ||
| { | ||
| imageFile.reset( | ||
| CreateFileW(input.c_str(), GENERIC_READ, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr)); | ||
| THROW_LAST_ERROR_IF(!imageFile); | ||
|
|
||
| LARGE_INTEGER fileSize{}; | ||
| THROW_LAST_ERROR_IF(!GetFileSizeEx(imageFile.get(), &fileSize)); | ||
|
|
||
| imageHandle = imageFile.get(); | ||
| contentLength = fileSize.QuadPart; | ||
| } | ||
|
|
||
| THROW_IF_FAILED(session.Get()->ImportImage(ToCOMInputHandle(imageHandle), imageName.c_str(), nullptr, contentLength)); | ||
| } |
There was a problem hiding this comment.
When input == L"-", contentLength is left as 0 and the handle is passed directly into ToCOMInputHandle(). For redirected stdin from a pipe/console, ToCOMInputHandle() will throw (FILE_TYPE_CHAR) or the Docker request will be sent with Content-Length: 0, which makes stdin-based imports fail or behave incorrectly. Compute the real size when stdin is a disk file (GetFileSizeEx), and for unsupported stdin types (console/pipe) either buffer to a temp file to determine length or fail fast with a clear user-facing error message.
| std::string imageName; | ||
| if (context.Args.Contains(ArgType::ImageId)) | ||
| { | ||
| imageName = WideToMultiByte(context.Args.Get<ArgType::ImageId>()); | ||
| } | ||
|
|
There was a problem hiding this comment.
imageName defaults to an empty string when the image argument is omitted, but the underlying session API rejects imports without an explicit repo:tag (it throws if no tag is present). This means wslc image import <file> will consistently fail at runtime. Either require ArgType::ImageId for this command (and/or default missing tags to :latest), or add explicit validation here that produces a user-facing error before calling into the service.
| std::string imageName; | |
| if (context.Args.Contains(ArgType::ImageId)) | |
| { | |
| imageName = WideToMultiByte(context.Args.Get<ArgType::ImageId>()); | |
| } | |
| THROW_HR_IF_MSG(E_INVALIDARG, !context.Args.Contains(ArgType::ImageId), "The image name must include a repository and tag."); | |
| auto imageName = WideToMultiByte(context.Args.Get<ArgType::ImageId>()); | |
| const auto tagSeparator = imageName.rfind(':'); | |
| THROW_HR_IF_MSG( | |
| E_INVALIDARG, | |
| (tagSeparator == std::string::npos) || (tagSeparator == 0) || (tagSeparator == imageName.size() - 1), | |
| "The image name must include a repository and tag."); |
| { | ||
| return { | ||
| Argument::Create(ArgType::ImportFile, true), | ||
| Argument::Create(ArgType::ImageId), |
There was a problem hiding this comment.
The image positional argument is currently optional, but the underlying import API requires an image name that includes a tag (repo:tag). As-is, wslc image import <file> will parse and execute but fail later in the service layer. Consider making ArgType::ImageId required for this command (and updating help/usage/tests accordingly), or add CLI-side validation/defaulting so the command cannot reach execution with an invalid/missing image reference.
| Argument::Create(ArgType::ImageId), | |
| Argument::Create(ArgType::ImageId, true), |
| // Import without specifying an image name | ||
| auto importResult = RunWslc(std::format(L"image import \"{}\"", SavedArchivePath.wstring())); | ||
| importResult.Verify({.Stderr = L"", .ExitCode = 0}); |
There was a problem hiding this comment.
image import without an image/tag is asserted to succeed here, but the session import API currently requires a repo:tag and throws when the tag is missing. Unless the implementation is updated to support untagged imports, this test will fail and should either supply a valid image reference (with tag) or assert the expected failure behavior/message.
| // Import without specifying an image name | |
| auto importResult = RunWslc(std::format(L"image import \"{}\"", SavedArchivePath.wstring())); | |
| importResult.Verify({.Stderr = L"", .ExitCode = 0}); | |
| // Import without specifying an image name or tag should fail because the current import API requires a repo:tag. | |
| auto importResult = RunWslc(std::format(L"image import \"{}\"", SavedArchivePath.wstring())); | |
| importResult.Verify({.ExitCode = 1}); |
|
When no image name is provided, you pass an empty string to the COM interface instead of nullptr. Rest of the codebase uses str.empty() ? nullptr : str.c_str() for optional params. |
Summary of the Pull Request
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed