From 6d8ba57cd2f39e89a3700caf44c0499d46ccb184 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Fri, 20 Feb 2026 05:10:02 -0800 Subject: [PATCH 01/14] First cut at start/stop/run/create --- src/windows/wslc/CMakeLists.txt | 14 ++- src/windows/wslc/arguments/Argument.h | 3 + .../wslc/arguments/ArgumentDefinitions.h | 34 ++++- src/windows/wslc/arguments/ArgumentParser.cpp | 10 +- src/windows/wslc/arguments/ArgumentTypes.h | 2 +- .../wslc/arguments/ArgumentValidation.cpp | 62 ++++++++++ .../wslc/arguments/ArgumentValidation.h | 26 ++++ .../wslc/commands/ContainerCommand.cpp | 6 +- src/windows/wslc/commands/ContainerCommand.h | 60 +++++++++ .../wslc/commands/ContainerCreateCommand.cpp | 64 ++++++++++ .../wslc/commands/ContainerRunCommand.cpp | 84 +++++++++++++ .../wslc/commands/ContainerStartCommand.cpp | 61 +++++++++ .../wslc/commands/ContainerStopCommand.cpp | 98 +++++++++++++++ src/windows/wslc/commands/RootCommand.cpp | 6 +- src/windows/wslc/core/Command.cpp | 7 +- src/windows/wslc/core/ExecutionContextData.h | 4 + src/windows/wslc/services/ContainerModel.h | 18 +++ .../wslc/services/PullImageCallback.cpp | 116 ++++++++++++++++++ src/windows/wslc/services/PullImageCallback.h | 46 +++++++ src/windows/wslc/tasks/ContainerTasks.cpp | 83 +++++++++++++ src/windows/wslc/tasks/ContainerTasks.h | 5 + test/windows/wslc/CommandLineTestCases.h | 11 ++ .../windows/wslc/WSLCCLIArgumentUnitTests.cpp | 8 +- .../wslc/WSLCCLIExecutionUnitTests.cpp | 13 ++ test/windows/wslc/WSLCCLIParserUnitTests.cpp | 18 +-- 25 files changed, 828 insertions(+), 31 deletions(-) create mode 100644 src/windows/wslc/arguments/ArgumentValidation.cpp create mode 100644 src/windows/wslc/arguments/ArgumentValidation.h create mode 100644 src/windows/wslc/commands/ContainerCreateCommand.cpp create mode 100644 src/windows/wslc/commands/ContainerRunCommand.cpp create mode 100644 src/windows/wslc/commands/ContainerStartCommand.cpp create mode 100644 src/windows/wslc/commands/ContainerStopCommand.cpp create mode 100644 src/windows/wslc/services/PullImageCallback.cpp create mode 100644 src/windows/wslc/services/PullImageCallback.h diff --git a/src/windows/wslc/CMakeLists.txt b/src/windows/wslc/CMakeLists.txt index 46c7ef023..9f68867ff 100644 --- a/src/windows/wslc/CMakeLists.txt +++ b/src/windows/wslc/CMakeLists.txt @@ -13,6 +13,7 @@ set(HEADERS arguments/ArgumentTypes.h arguments/Argument.h arguments/ArgumentParser.h + arguments/ArgumentValidation.h core/Command.h core/CLIExecutionContext.h core/ExecutionContextData.h @@ -25,6 +26,7 @@ set(HEADERS services/ContainerModel.h services/ContainerService.h services/ImageService.h + services/PullImageCallback.h services/SessionModel.h services/SessionService.h tasks/Task.h @@ -37,14 +39,20 @@ set(SOURCES core/Main.cpp arguments/Argument.cpp arguments/ArgumentParser.cpp + arguments/ArgumentValidation.cpp commands/RootCommand.cpp commands/ContainerCommand.cpp + commands/ContainerCreateCommand.cpp commands/ContainerListCommand.cpp + commands/ContainerRunCommand.cpp + commands/ContainerStartCommand.cpp + commands/ContainerStopCommand.cpp commands/DiagCommand.cpp commands/DiagListCommand.cpp services/ConsoleService.cpp services/ContainerService.cpp services/ImageService.cpp + services/PullImageCallback.cpp services/SessionModel.cpp services/SessionService.cpp tasks/DiagTasks.cpp @@ -54,7 +62,11 @@ set(SOURCES # Object library for WSLC components. # Used to build the executable and also unit testing components. add_library(wslclib OBJECT ${SOURCES} ${HEADERS}) -set_target_properties(wslclib PROPERTIES FOLDER windows) +set_target_properties(wslclib PROPERTIES +# UNITY_BUILD ON +# UNITY_BUILD_BATCH_SIZE 0 # 0 means CMake decides automatically + FOLDER windows +) target_include_directories(wslclib PUBLIC ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/core diff --git a/src/windows/wslc/arguments/Argument.h b/src/windows/wslc/arguments/Argument.h index 20d929729..9606a0414 100644 --- a/src/windows/wslc/arguments/Argument.h +++ b/src/windows/wslc/arguments/Argument.h @@ -93,6 +93,9 @@ struct Argument return m_countLimit; } + // Validates this argument's value in the provided args + void Validate(const ArgMap& execArgs) const; + private: ArgType m_argType; std::wstring m_name; diff --git a/src/windows/wslc/arguments/ArgumentDefinitions.h b/src/windows/wslc/arguments/ArgumentDefinitions.h index f235e9b3a..430b8859c 100644 --- a/src/windows/wslc/arguments/ArgumentDefinitions.h +++ b/src/windows/wslc/arguments/ArgumentDefinitions.h @@ -34,16 +34,40 @@ Module Name: // clang-format off #define WSLC_ARGUMENTS(_) \ _(All, "all", L"a", Kind::Flag, L"Show all regardless of state.") \ +_(Attach, "attach", L"a", Kind::Flag, Localization::WSLCCLI_AttachArgDescription()) \ +_(CIDFile, "cidfile", NO_ALIAS, Kind::Value, L"Write the container ID to the provided path.") \ _(Command, "command", NO_ALIAS, Kind::Positional, L"The command to run") \ -_(ContainerId, "container-id", NO_ALIAS, Kind::Positional, L"Specify the target container by its ID") \ +_(ContainerId, "container-id", NO_ALIAS, Kind::Positional, Localization::WSLCCLI_ContainerIdArgDescription()) \ +_(Detach, "detach", L"d", Kind::Flag, L"Run container in detached mode") \ +_(DNS, "dns", NO_ALIAS, Kind::Value, L"IP address of the DNS nameserver in resolv.conf") \ +_(DNSDomain, "dns-domain", NO_ALIAS, Kind::Value, L"Set the default DNS Domain") \ +_(DNSOption, "dns-option", NO_ALIAS, Kind::Value, L"Set DNS options") \ +_(DNSSearch, "dns-search", NO_ALIAS, Kind::Value, L"Set DNS search domains") \ +_(Entrypoint, "entrypoint", NO_ALIAS, Kind::Value, L"Specifies the container init process executable") \ +_(Env, "env", L"e", Kind::Value, L"Key=Value pairs for environment variables") \ +_(EnvFile, "env-file", NO_ALIAS, Kind::Value, L"File containing key=value pairs of env variables") \ _(Format, "format", NO_ALIAS, Kind::Value, L"Output formatting (json or table) (Default:table)") \ -_(ForwardArgs, "forwardargs", NO_ALIAS, Kind::Forward, L"Args to pass along") \ +_(ForwardArgs, "arguments", NO_ALIAS, Kind::Forward, L"Arguments to pass to container's init process") \ +_(GroupId, "groupid", NO_ALIAS, Kind::Value, L"Group Id for the process") \ _(Help, "help", WSLC_CLI_HELP_ARG, Kind::Flag, Localization::WSLCCLI_HelpArgDescription()) \ +_(ImageId, "image", NO_ALIAS, Kind::Positional, L"Image name") \ _(Info, "info", NO_ALIAS, Kind::Flag, Localization::WSLCCLI_InfoArgDescription()) \ _(Interactive, "interactive", L"i", Kind::Flag, Localization::WSLCCLI_InteractiveArgDescription()) \ -_(Publish, "publish", L"p", Kind::Value, L"Publish port") \ +_(Name, "name", NO_ALIAS, Kind::Value, L"Name of the container") \ +_(NoDNS, "no-dns", NO_ALIAS, Kind::Flag, L"No configuration of DNS in the container") \ +_(Progress, "progress", NO_ALIAS, Kind::Value, L"Progress type (format: none|ansi) (default: ansi)") \ +_(Publish, "publish", L"p", Kind::Value, L"Publish a port from a container to host") \ +_(Pull, "pull", NO_ALIAS, Kind::Value, L"Image pull policy (always|missing|never) (default:never)") \ _(Quiet, "quiet", L"q", Kind::Flag, L"Outputs the container IDs only") \ -_(Remove, "remove", L"rm", Kind::Flag, L"Remove the container after execution") \ +_(Remove, "remove", L"rm", Kind::Flag, L"Remove the container after it stops") \ +_(Scheme, "scheme", NO_ALIAS, Kind::Value, L"Use this scheme for registry connection") \ _(SessionId, "session", NO_ALIAS, Kind::Value, Localization::WSLCCLI_SessionIdArgDescription()) \ -_(Verbose, "verbose", L"v", Kind::Flag, L"Output verbose details") +_(Signal, "signal", L"s", Kind::Value, L"Signal to send (default: SIGKILL)") \ +_(Time, "time", L"t", Kind::Value, L"Time in seconds to wait before executing (default 5)") \ +_(TMPFS, "tmpfs", NO_ALIAS, Kind::Value, L"Mount tmpfs to the container at the given path") \ +_(TTY, "tty", L"t", Kind::Flag, L"Open a TTY with the container process.") \ +_(User, "user", L"u", Kind::Value, L"User ID for the process (name|uid|uid:gid)") \ +_(Verbose, "verbose", L"v", Kind::Flag, L"Output verbose details") \ +_(Virtual, "virtualization", NO_ALIAS, Kind::Value, L"Expose virtualization capabilities to the container") \ +_(Volume, "volume", NO_ALIAS, Kind::Value, L"Bind mount a volume to the container") \ // clang-format on diff --git a/src/windows/wslc/arguments/ArgumentParser.cpp b/src/windows/wslc/arguments/ArgumentParser.cpp index 230415389..81ad7edea 100644 --- a/src/windows/wslc/arguments/ArgumentParser.cpp +++ b/src/windows/wslc/arguments/ArgumentParser.cpp @@ -177,7 +177,9 @@ ParseArgumentsStateMachine::State ParseArgumentsStateMachine::ProcessAnchoredPos } // If we haven't reached the limit for the anchor positional, treat this as another anchor positional. - if (m_executionArgs.Count(m_anchorPositional.value().Type()) < m_anchorPositional.value().Limit()) + // Anchors with a -1 limit will never be full and therefore will always treat subsequent positionals as anchors. + if ((m_executionArgs.Count(m_anchorPositional.value().Type()) < m_anchorPositional.value().Limit()) || + (m_anchorPositional.value().Limit() < 0)) { // validate that we dont have any invalid argument specifiers. if (!currArg.empty() && currArg[0] == WSLC_CLI_ARG_ID_CHAR) @@ -218,11 +220,11 @@ ParseArgumentsStateMachine::State ParseArgumentsStateMachine::ProcessAnchoredPos // currArg is the first forwarded argument // All the rest of the args are forward args. - std::vector forwardedArgs; - forwardedArgs.push_back(std::wstring{currArg}); + std::vector forwardedArgs; + forwardedArgs.emplace_back(string::WideToMultiByte(std::wstring{currArg})); while (m_invocationItr != m_invocation.end()) { - forwardedArgs.push_back(std::wstring{*m_invocationItr}); + forwardedArgs.emplace_back(string::WideToMultiByte(*m_invocationItr)); ++m_invocationItr; } diff --git a/src/windows/wslc/arguments/ArgumentTypes.h b/src/windows/wslc/arguments/ArgumentTypes.h index 43638107a..8ddfc083f 100644 --- a/src/windows/wslc/arguments/ArgumentTypes.h +++ b/src/windows/wslc/arguments/ArgumentTypes.h @@ -74,7 +74,7 @@ namespace details { template <> struct KindToType { - using type = std::vector; + using type = std::vector; }; template diff --git a/src/windows/wslc/arguments/ArgumentValidation.cpp b/src/windows/wslc/arguments/ArgumentValidation.cpp new file mode 100644 index 000000000..93dfbd14d --- /dev/null +++ b/src/windows/wslc/arguments/ArgumentValidation.cpp @@ -0,0 +1,62 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + ArgumentValidation.cpp + +Abstract: + + Implementation of the Argument Validation. + +--*/ +#include "Argument.h" +#include "ArgumentTypes.h" +#include "Exceptions.h" +#include "ArgumentValidation.h" +#include "ContainerModel.h" +#include + +namespace wsl::windows::wslc { +// Common argument validation that occurs across multiple commands. +void Argument::Validate(const ArgMap& execArgs) const +{ + switch (m_argType) + { + case ArgType::Signal: + validation::ValidateWSLASignal(execArgs.Get(), m_name); + break; + + case ArgType::Time: + validation::ValidateUInteger(execArgs.Get(), m_name); + break; + + default: + break; + } +} +} // namespace wsl::windows::wslc + +namespace wsl::windows::wslc::validation { + +void ValidateWSLASignal(const std::wstring& value, const std::wstring& argName) +{ + if (!models::SignalMap.contains(value)) + { + throw ArgumentException(L"Invalid " + argName + L" argument value: " + value); + } +} + +void ValidateUInteger(const std::wstring& value, const std::wstring& argName) +{ + try + { + [[maybe_unused]] auto intValue = std::stoul(value); + } + catch (const std::exception&) + { + throw ArgumentException(L"Invalid " + argName + L" argument value: " + value); + } +} +} // namespace wsl::windows::wslc::validation \ No newline at end of file diff --git a/src/windows/wslc/arguments/ArgumentValidation.h b/src/windows/wslc/arguments/ArgumentValidation.h new file mode 100644 index 000000000..87f12b3fb --- /dev/null +++ b/src/windows/wslc/arguments/ArgumentValidation.h @@ -0,0 +1,26 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + ArgumentValidation.cpp + +Abstract: + + Declaration of Argument Validation functions. + +--*/ +#pragma once +#include "ArgumentTypes.h" +#include "Exceptions.h" + +using namespace wsl::windows::wslc; +using namespace wsl::windows::wslc::argument; + +namespace wsl::windows::wslc::validation { + +void ValidateWSLASignal(const std::wstring& value, const std::wstring& argName); +void ValidateUInteger(const std::wstring& value, const std::wstring& argName); + +} // namespace wsl::windows::wslc::validation \ No newline at end of file diff --git a/src/windows/wslc/commands/ContainerCommand.cpp b/src/windows/wslc/commands/ContainerCommand.cpp index c7c8d87c2..fe471ff97 100644 --- a/src/windows/wslc/commands/ContainerCommand.cpp +++ b/src/windows/wslc/commands/ContainerCommand.cpp @@ -21,8 +21,12 @@ namespace wsl::windows::wslc { std::vector> ContainerCommand::GetCommands() const { std::vector> commands; - commands.reserve(1); + commands.reserve(5); commands.push_back(std::make_unique(FullName())); + commands.push_back(std::make_unique(FullName())); + commands.push_back(std::make_unique(FullName())); + commands.push_back(std::make_unique(FullName())); + commands.push_back(std::make_unique(FullName())); return commands; } diff --git a/src/windows/wslc/commands/ContainerCommand.h b/src/windows/wslc/commands/ContainerCommand.h index f6a4f1590..6dbbfcafc 100644 --- a/src/windows/wslc/commands/ContainerCommand.h +++ b/src/windows/wslc/commands/ContainerCommand.h @@ -32,6 +32,21 @@ struct ContainerCommand final : public Command void ExecuteInternal(CLIExecutionContext& context) const override; }; +// Create Command +struct ContainerCreateCommand final : public Command +{ + constexpr static std::wstring_view CommandName = L"create"; + ContainerCreateCommand(const std::wstring& parent) : Command(CommandName, parent) + { + } + std::vector GetArguments() const override; + std::wstring ShortDescription() const override; + std::wstring LongDescription() const override; + +protected: + void ExecuteInternal(CLIExecutionContext& context) const override; +}; + // List Command struct ContainerListCommand final : public Command { @@ -47,4 +62,49 @@ struct ContainerListCommand final : public Command void ValidateArgumentsInternal(const ArgMap& execArgs) const override; void ExecuteInternal(CLIExecutionContext& context) const override; }; + +// Run Command +struct ContainerRunCommand final : public Command +{ + constexpr static std::wstring_view CommandName = L"run"; + ContainerRunCommand(const std::wstring& parent) : Command(CommandName, parent) + { + } + std::vector GetArguments() const override; + std::wstring ShortDescription() const override; + std::wstring LongDescription() const override; + +protected: + void ExecuteInternal(CLIExecutionContext& context) const override; +}; + +// Start Command +struct ContainerStartCommand final : public Command +{ + constexpr static std::wstring_view CommandName = L"start"; + ContainerStartCommand(const std::wstring& parent) : Command(CommandName, parent) + { + } + std::vector GetArguments() const override; + std::wstring ShortDescription() const override; + std::wstring LongDescription() const override; + +protected: + void ExecuteInternal(CLIExecutionContext& context) const override; +}; + +// Stop Command +struct ContainerStopCommand final : public Command +{ + constexpr static std::wstring_view CommandName = L"stop"; + ContainerStopCommand(const std::wstring& parent) : Command(CommandName, parent) + { + } + std::vector GetArguments() const override; + std::wstring ShortDescription() const override; + std::wstring LongDescription() const override; + +protected: + void ExecuteInternal(CLIExecutionContext& context) const override; +}; } // namespace wsl::windows::wslc \ No newline at end of file diff --git a/src/windows/wslc/commands/ContainerCreateCommand.cpp b/src/windows/wslc/commands/ContainerCreateCommand.cpp new file mode 100644 index 000000000..caf32b156 --- /dev/null +++ b/src/windows/wslc/commands/ContainerCreateCommand.cpp @@ -0,0 +1,64 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + ContainerCreateCommand.cpp + +Abstract: + + Implementation of command execution logic. + +--*/ + +#include "precomp.h" +#include "ContainerModel.h" +#include "ContainerCommand.h" +#include "ContainerService.h" +#include "TablePrinter.h" +#include "CLIExecutionContext.h" +#include "ExecutionContextData.h" +#include "ContainerTasks.h" +#include "Task.h" + +using wsl::windows::common::wslutil::PrintMessage; +using wsl::windows::wslc::models::ContainerInformation; +using wsl::windows::wslc::services::ContainerService; +using namespace wsl::shared; +using namespace wsl::windows::wslc::execution; +using namespace wsl::windows::wslc::task; + +namespace wsl::windows::wslc { +// Container Create Command +std::vector ContainerCreateCommand::GetArguments() const +{ + return { + Argument::Create(ArgType::ImageId, true), Argument::Create(ArgType::Command), Argument::Create(ArgType::ForwardArgs), + Argument::Create(ArgType::CIDFile), Argument::Create(ArgType::DNS), Argument::Create(ArgType::DNSDomain), + Argument::Create(ArgType::DNSOption), Argument::Create(ArgType::DNSSearch), Argument::Create(ArgType::Entrypoint), + Argument::Create(ArgType::Env, false, -1), Argument::Create(ArgType::EnvFile), Argument::Create(ArgType::GroupId), + Argument::Create(ArgType::Interactive), Argument::Create(ArgType::Name), Argument::Create(ArgType::NoDNS), + Argument::Create(ArgType::Progress), Argument::Create(ArgType::Remove), Argument::Create(ArgType::Scheme), + Argument::Create(ArgType::SessionId), Argument::Create(ArgType::TMPFS), Argument::Create(ArgType::TTY), + Argument::Create(ArgType::User), Argument::Create(ArgType::Volume), Argument::Create(ArgType::Virtual), + }; +} + +std::wstring ContainerCreateCommand::ShortDescription() const +{ + return {L"Create a container."}; +} + +std::wstring ContainerCreateCommand::LongDescription() const +{ + return { + L"Creates a container. By default, the container is created in the background; use --detach to create in the " + L"foreground."}; +} + +void ContainerCreateCommand::ExecuteInternal(CLIExecutionContext& context) const +{ + context << CreateSession << SetCreateContainerOptionsFromArgs << CreateContainer; +} +} // namespace wsl::windows::wslc \ No newline at end of file diff --git a/src/windows/wslc/commands/ContainerRunCommand.cpp b/src/windows/wslc/commands/ContainerRunCommand.cpp new file mode 100644 index 000000000..5f9885183 --- /dev/null +++ b/src/windows/wslc/commands/ContainerRunCommand.cpp @@ -0,0 +1,84 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + ContainerRunCommand.cpp + +Abstract: + + Implementation of command execution logic. + +--*/ + +#include "precomp.h" +#include "ContainerModel.h" +#include "ContainerCommand.h" +#include "ContainerService.h" +#include "TablePrinter.h" +#include "CLIExecutionContext.h" +#include "ExecutionContextData.h" +#include "ContainerTasks.h" +#include "Task.h" + +#include +#include +#include + +using wsl::windows::common::wslutil::PrintMessage; +using wsl::windows::wslc::models::ContainerInformation; +using wsl::windows::wslc::services::ContainerService; +using namespace wsl::shared; +using namespace wsl::windows::wslc::execution; +using namespace wsl::windows::wslc::task; + +namespace wsl::windows::wslc { +// Container Run Command +std::vector ContainerRunCommand::GetArguments() const +{ + return { + Argument::Create(ArgType::ImageId, true), + Argument::Create(ArgType::Command), + Argument::Create(ArgType::ForwardArgs), + Argument::Create(ArgType::CIDFile), + Argument::Create(ArgType::Detach), + Argument::Create(ArgType::DNS), + Argument::Create(ArgType::DNSDomain), + Argument::Create(ArgType::DNSOption), + Argument::Create(ArgType::DNSSearch), + Argument::Create(ArgType::Entrypoint), + Argument::Create(ArgType::Env, std::nullopt, -1), + Argument::Create(ArgType::EnvFile), + Argument::Create(ArgType::Interactive), + Argument::Create(ArgType::Name), + Argument::Create(ArgType::NoDNS), + Argument::Create(ArgType::Progress), + Argument::Create(ArgType::Publish, std::nullopt, -1), + Argument::Create(ArgType::Pull), + Argument::Create(ArgType::Remove), + Argument::Create(ArgType::Scheme), + Argument::Create(ArgType::SessionId), + Argument::Create(ArgType::TMPFS), + Argument::Create(ArgType::TTY), + Argument::Create(ArgType::User), + Argument::Create(ArgType::Volume), + Argument::Create(ArgType::Virtual), + }; +} + +std::wstring ContainerRunCommand::ShortDescription() const +{ + return {L"Run a container."}; +} + +std::wstring ContainerRunCommand::LongDescription() const +{ + return {L"Runs a container. By default, the container is started in the background; use --detach to run in the foreground."}; +} + +void ContainerRunCommand::ExecuteInternal(CLIExecutionContext& context) const +{ + context << CreateSession << SetRunContainerOptionsFromArgs << RunContainer; +} +} // namespace wsl::windows::wslc \ No newline at end of file diff --git a/src/windows/wslc/commands/ContainerStartCommand.cpp b/src/windows/wslc/commands/ContainerStartCommand.cpp new file mode 100644 index 000000000..b20e664e6 --- /dev/null +++ b/src/windows/wslc/commands/ContainerStartCommand.cpp @@ -0,0 +1,61 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + ContainerStartCommand.cpp + +Abstract: + + Implementation of command execution logic. + +--*/ + +#include "precomp.h" +#include "ContainerModel.h" +#include "ContainerCommand.h" +#include "ContainerService.h" +#include "TablePrinter.h" +#include "CLIExecutionContext.h" +#include "ExecutionContextData.h" +#include "ContainerTasks.h" +#include "Task.h" + +using wsl::windows::common::string::WideToMultiByte; +using wsl::windows::common::wslutil::PrintMessage; +using wsl::windows::wslc::models::ContainerInformation; +using wsl::windows::wslc::services::ContainerService; +using namespace wsl::shared; +using namespace wsl::windows::wslc::execution; +using namespace wsl::windows::wslc::task; + +namespace wsl::windows::wslc { +// Container Start Command +std::vector ContainerStartCommand::GetArguments() const +{ + return { + Argument::Create(ArgType::ContainerId, true), + Argument::Create(ArgType::Attach), // NYI + Argument::Create(ArgType::Interactive), // NYI + Argument::Create(ArgType::SessionId), // NYI + }; +} + +std::wstring ContainerStartCommand::ShortDescription() const +{ + return {L"Start a container."}; +} + +std::wstring ContainerStartCommand::LongDescription() const +{ + return { + L"Starts a container. Provides options to attach to the container's stdout and stderr streams and could be interactive " + L"to keep stdin open."}; +} + +void ContainerStartCommand::ExecuteInternal(CLIExecutionContext& context) const +{ + context << CreateSession << StartContainer; +} +} // namespace wsl::windows::wslc \ No newline at end of file diff --git a/src/windows/wslc/commands/ContainerStopCommand.cpp b/src/windows/wslc/commands/ContainerStopCommand.cpp new file mode 100644 index 000000000..a64c9a467 --- /dev/null +++ b/src/windows/wslc/commands/ContainerStopCommand.cpp @@ -0,0 +1,98 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + ContainerStopCommand.cpp + +Abstract: + + Implementation of command execution logic. + +--*/ + +#include "precomp.h" +#include "ContainerModel.h" +#include "ContainerCommand.h" +#include "ContainerService.h" +#include "TablePrinter.h" +#include "CLIExecutionContext.h" +#include "ExecutionContextData.h" +#include "ContainerTasks.h" +#include "Task.h" + +#include +#include +#include + +using wsl::windows::common::wslutil::PrintMessage; +using wsl::windows::wslc::models::ContainerInformation; +using wsl::windows::wslc::services::ContainerService; +using namespace wsl::shared; +using namespace wsl::windows::wslc::execution; +using namespace wsl::windows::wslc::task; +using namespace wsl::windows::wslc::models; +using namespace wsl::windows::wslc::services; + +namespace wsl::windows::wslc { +// Container Stop Command +std::vector ContainerStopCommand::GetArguments() const +{ + return { + Argument::Create(ArgType::ContainerId, std::nullopt, -1), + Argument::Create(ArgType::All), + Argument::Create(ArgType::SessionId), + Argument::Create(ArgType::Signal, std::nullopt, std::nullopt, L"Signal to send (default: SIGTERM)"), + Argument::Create(ArgType::Time), + }; +} + +std::wstring ContainerStopCommand::ShortDescription() const +{ + return {L"Stop a container."}; +} + +std::wstring ContainerStopCommand::LongDescription() const +{ + return { + L"Stops a container. By default, the container is stopped in the background; use --detach to stop in the foreground."}; +} + +void ContainerStopCommand::ExecuteInternal(CLIExecutionContext& context) const +{ + context << CreateSession; + + auto containersToStop = context.Args.GetAll(); + if (context.Args.Contains(ArgType::All)) + { + // All overwrites any specified container IDs. + containersToStop.clear(); + context << GetContainers; + const auto& allContainers = context.Data.Get(); + for (const auto& container : allContainers) + { + if (container.State == WSLA_CONTAINER_STATE::WslaContainerStateRunning) + { + containersToStop.emplace_back(string::MultiByteToWide(container.Name)); + } + } + } + + StopContainerOptions options; + if (context.Args.Contains(ArgType::Signal)) + { + options.Signal = SignalMap.at(context.Args.Get()); + } + + if (context.Args.Contains(ArgType::Time)) + { + options.Timeout = std::stoul(context.Args.Get()); + } + + for (const auto& id : containersToStop) + { + ContainerService::Stop(context.Data.Get(), string::WideToMultiByte(id), options); + } +} +} // namespace wsl::windows::wslc \ No newline at end of file diff --git a/src/windows/wslc/commands/RootCommand.cpp b/src/windows/wslc/commands/RootCommand.cpp index f4f97c7d3..7b49656eb 100644 --- a/src/windows/wslc/commands/RootCommand.cpp +++ b/src/windows/wslc/commands/RootCommand.cpp @@ -23,10 +23,14 @@ namespace wsl::windows::wslc { std::vector> RootCommand::GetCommands() const { std::vector> commands; - commands.reserve(3); + commands.reserve(7); commands.push_back(std::make_unique(FullName())); commands.push_back(std::make_unique(FullName())); commands.push_back(std::make_unique(FullName())); + commands.push_back(std::make_unique(FullName())); + commands.push_back(std::make_unique(FullName())); + commands.push_back(std::make_unique(FullName())); + commands.push_back(std::make_unique(FullName())); return commands; } diff --git a/src/windows/wslc/core/Command.cpp b/src/windows/wslc/core/Command.cpp index d0c5556dd..33942c48f 100644 --- a/src/windows/wslc/core/Command.cpp +++ b/src/windows/wslc/core/Command.cpp @@ -349,10 +349,15 @@ void Command::ValidateArguments(ArgMap& execArgs) const throw CommandException(Localization::WSLCCLI_RequiredArgumentError(arg.Name())); } - if (arg.Limit() < execArgs.Count(arg.Type())) + if ((arg.Limit() > 0) && (arg.Limit() < execArgs.Count(arg.Type()))) { throw CommandException(Localization::WSLCCLI_TooManyArgumentsError(arg.Name())); } + + if (execArgs.Contains(arg.Type())) + { + arg.Validate(execArgs); + } } ValidateArgumentsInternal(execArgs); diff --git a/src/windows/wslc/core/ExecutionContextData.h b/src/windows/wslc/core/ExecutionContextData.h index 5862abaf1..a4506cc94 100644 --- a/src/windows/wslc/core/ExecutionContextData.h +++ b/src/windows/wslc/core/ExecutionContextData.h @@ -33,6 +33,8 @@ enum class Data : size_t { Session, Containers, + CreateContainerOptions, + RunContainerOptions, Max }; @@ -45,6 +47,8 @@ namespace details { DEFINE_DATA_MAPPING(Session, wsl::windows::wslc::models::Session); DEFINE_DATA_MAPPING(Containers, std::vector); + DEFINE_DATA_MAPPING(CreateContainerOptions, wsl::windows::wslc::models::ContainerCreateOptions); + DEFINE_DATA_MAPPING(RunContainerOptions, wsl::windows::wslc::models::ContainerRunOptions); } // namespace details struct DataMap : wsl::windows::wslc::EnumBasedVariantMap diff --git a/src/windows/wslc/services/ContainerModel.h b/src/windows/wslc/services/ContainerModel.h index 8f78c2298..f9270ba2d 100644 --- a/src/windows/wslc/services/ContainerModel.h +++ b/src/windows/wslc/services/ContainerModel.h @@ -17,8 +17,26 @@ Module Name: #include #include #include +#include +#include namespace wsl::windows::wslc::models { + +// Map of signal names to WSLASignal enum values +inline static const std::unordered_map SignalMap = { + {L"SIGHUP", WSLASignalSIGHUP}, {L"SIGINT", WSLASignalSIGINT}, {L"SIGQUIT", WSLASignalSIGQUIT}, + {L"SIGILL", WSLASignalSIGILL}, {L"SIGTRAP", WSLASignalSIGTRAP}, {L"SIGABRT", WSLASignalSIGABRT}, + {L"SIGIOT", WSLASignalSIGIOT}, {L"SIGBUS", WSLASignalSIGBUS}, {L"SIGFPE", WSLASignalSIGFPE}, + {L"SIGKILL", WSLASignalSIGKILL}, {L"SIGUSR1", WSLASignalSIGUSR1}, {L"SIGSEGV", WSLASignalSIGSEGV}, + {L"SIGUSR2", WSLASignalSIGUSR2}, {L"SIGPIPE", WSLASignalSIGPIPE}, {L"SIGALRM", WSLASignalSIGALRM}, + {L"SIGTERM", WSLASignalSIGTERM}, {L"SIGTKFLT", WSLASignalSIGTKFLT}, {L"SIGCHLD", WSLASignalSIGCHLD}, + {L"SIGCONT", WSLASignalSIGCONT}, {L"SIGSTOP", WSLASignalSIGSTOP}, {L"SIGTSTP", WSLASignalSIGTSTP}, + {L"SIGTTIN", WSLASignalSIGTTIN}, {L"SIGTTOU", WSLASignalSIGTTOU}, {L"SIGURG", WSLASignalSIGURG}, + {L"SIGXCPU", WSLASignalSIGXCPU}, {L"SIGXFSZ", WSLASignalSIGXFSZ}, {L"SIGVTALRM", WSLASignalSIGVTALRM}, + {L"SIGPROF", WSLASignalSIGPROF}, {L"SIGWINCH", WSLASignalSIGWINCH}, {L"SIGIO", WSLASignalSIGIO}, + {L"SIGPOLL", WSLASignalSIGPOLL}, {L"SIGPWR", WSLASignalSIGPWR}, {L"SIGSYS", WSLASignalSIGSYS}, +}; + struct ContainerCreateOptions { bool TTY = false; diff --git a/src/windows/wslc/services/PullImageCallback.cpp b/src/windows/wslc/services/PullImageCallback.cpp new file mode 100644 index 000000000..177b6b7af --- /dev/null +++ b/src/windows/wslc/services/PullImageCallback.cpp @@ -0,0 +1,116 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + PullImageCallback.cpp + +Abstract: + + This file contains the PullImageCallback Implementation. + +--*/ + +#include "precomp.h" +#include "PullImageCallback.h" +#include "ImageService.h" +#include "TablePrinter.h" +#include + +namespace wsl::windows::wslc::services { +using namespace wsl::shared; + +ChangeTerminalMode::ChangeTerminalMode(HANDLE console, bool cursorVisible) +{ + m_console = console; + THROW_IF_WIN32_BOOL_FALSE(GetConsoleCursorInfo(console, &m_originalCursorInfo)); + CONSOLE_CURSOR_INFO newCursorInfo = m_originalCursorInfo; + newCursorInfo.bVisible = cursorVisible; + THROW_IF_WIN32_BOOL_FALSE(SetConsoleCursorInfo(console, &newCursorInfo)); +} + +ChangeTerminalMode::~ChangeTerminalMode() +{ + LOG_IF_WIN32_BOOL_FALSE(SetConsoleCursorInfo(m_console, &m_originalCursorInfo)); +} + +auto PullImageCallback::MoveToLine(SHORT line) +{ + if (line > 0) + { + wprintf(L"\033[%iA", line); + } + + return wil::scope_exit([line = line]() { + if (line > 1) + { + wprintf(L"\033[%iB", line - 1); + } + }); +} + +HRESULT PullImageCallback::OnProgress(LPCSTR status, LPCSTR id, ULONGLONG current, ULONGLONG total) +{ + try + { + if (id == nullptr || *id == '\0') // Print all 'global' statuses on their own line + { + wprintf(L"%hs\n", status); + m_currentLine++; + return S_OK; + } + + auto info = Info(); + + auto it = m_statuses.find(id); + if (it == m_statuses.end()) + { + // If this is the first time we see this ID, create a new line for it. + m_statuses.emplace(id, m_currentLine); + wprintf(L"%ls\n", GenerateStatusLine(status, id, current, total, info).c_str()); + m_currentLine++; + } + else + { + auto revert = MoveToLine(m_currentLine - it->second); + wprintf(L"%ls\n", GenerateStatusLine(status, id, current, total, info).c_str()); + } + + return S_OK; + } + CATCH_RETURN(); +} + +CONSOLE_SCREEN_BUFFER_INFO PullImageCallback::Info() +{ + CONSOLE_SCREEN_BUFFER_INFO info{}; + THROW_IF_WIN32_BOOL_FALSE(GetConsoleScreenBufferInfo(GetStdHandle(STD_OUTPUT_HANDLE), &info)); + return info; +} + +std::wstring PullImageCallback::GenerateStatusLine(LPCSTR status, LPCSTR id, ULONGLONG current, ULONGLONG total, const CONSOLE_SCREEN_BUFFER_INFO& info) +{ + std::wstring line; + if (total != 0) + { + line = std::format(L"{} '{}': {}%", status, id, current * 100 / total); + } + else if (current != 0) + { + line = std::format(L"{} '{}': {}s", status, id, current); + } + else + { + line = std::format(L"{} '{}'", status, id); + } + + // Erase any previously written char on that line. + while (line.size() < info.dwSize.X) + { + line += L' '; + } + + return line; +} +} // namespace wsl::windows::wslc::services diff --git a/src/windows/wslc/services/PullImageCallback.h b/src/windows/wslc/services/PullImageCallback.h new file mode 100644 index 000000000..d56c1cc8d --- /dev/null +++ b/src/windows/wslc/services/PullImageCallback.h @@ -0,0 +1,46 @@ +/*++ + +Copyright (c) Microsoft. All rights reserved. + +Module Name: + + PullImageCallback.h + +Abstract: + + This file contains the PullImageCallback definition + +--*/ +#pragma once +#include "SessionService.h" + +namespace wsl::windows::wslc::services { + +class ChangeTerminalMode +{ +public: + NON_COPYABLE(ChangeTerminalMode); + NON_MOVABLE(ChangeTerminalMode); + ChangeTerminalMode(HANDLE console, bool cursorVisible); + ~ChangeTerminalMode(); + +private: + HANDLE m_console{}; + CONSOLE_CURSOR_INFO m_originalCursorInfo{}; +}; + +class DECLSPEC_UUID("7A1D3376-835A-471A-8DC9-23653D9962D0") PullImageCallback + : public Microsoft::WRL::RuntimeClass, IProgressCallback, IFastRundown> +{ +public: + auto MoveToLine(SHORT line); + HRESULT OnProgress(LPCSTR status, LPCSTR id, ULONGLONG current, ULONGLONG total) override; + +private: + static CONSOLE_SCREEN_BUFFER_INFO Info(); + std::wstring GenerateStatusLine(LPCSTR status, LPCSTR id, ULONGLONG current, ULONGLONG total, const CONSOLE_SCREEN_BUFFER_INFO& info); + std::map m_statuses; + SHORT m_currentLine = 0; + ChangeTerminalMode m_terminalMode{GetStdHandle(STD_OUTPUT_HANDLE), false}; +}; +} // namespace wsl::windows::wslc::services \ No newline at end of file diff --git a/src/windows/wslc/tasks/ContainerTasks.cpp b/src/windows/wslc/tasks/ContainerTasks.cpp index 0c6317211..f1f672fd4 100644 --- a/src/windows/wslc/tasks/ContainerTasks.cpp +++ b/src/windows/wslc/tasks/ContainerTasks.cpp @@ -14,19 +14,48 @@ Module Name: #include "Argument.h" #include "CLIExecutionContext.h" #include "ContainerModel.h" +#include "ImageModel.h" #include "ContainerService.h" #include "SessionModel.h" #include "SessionService.h" #include "Task.h" #include "ContainerTasks.h" +#include "PullImageCallback.h" #include #include using namespace wsl::windows::wslc::execution; using namespace wsl::windows::wslc::services; using namespace wsl::windows::wslc::models; +using namespace wsl::windows::common; namespace wsl::windows::wslc::task { + +namespace { + void PopulateCommonContainerOptionsFromArgs(CLIExecutionContext& context, ContainerCreateOptions& options) + { + if (context.Args.Contains(ArgType::Name)) + { + options.Name = string::WideToMultiByte(context.Args.Get()); + } + + if (context.Args.Contains(ArgType::TTY)) + { + options.TTY = true; + } + + if (context.Args.Contains(ArgType::Interactive)) + { + options.Interactive = true; + } + + if (context.Args.Contains(ArgType::ForwardArgs)) + { + options.Arguments = context.Args.Get(); + } + } +} // anonymous namespace + void CreateSession(CLIExecutionContext& context) { std::optional options = std::nullopt; @@ -44,4 +73,58 @@ void GetContainers(CLIExecutionContext& context) auto& session = context.Data.Get(); context.Data.Add(ContainerService::List(session)); } + +void StartContainer(CLIExecutionContext& context) +{ + WI_ASSERT(context.Data.Contains(Data::Session)); + WI_ASSERT(context.Args.Contains(ArgType::ContainerId)); + const auto& id = string::WideToMultiByte(context.Args.Get()); + ContainerService::Start(context.Data.Get(), id); +} + +void SetCreateContainerOptionsFromArgs(CLIExecutionContext& context) +{ + ContainerCreateOptions options; + PopulateCommonContainerOptionsFromArgs(context, options); + context.Data.Add(std::move(options)); +} + +void SetRunContainerOptionsFromArgs(CLIExecutionContext& context) +{ + ContainerRunOptions options; + PopulateCommonContainerOptionsFromArgs(context, options); + if (context.Args.Contains(ArgType::Detach)) + { + options.Detach = true; + } + + context.Data.Add(std::move(options)); +} + +void CreateContainer(CLIExecutionContext& context) +{ + WI_ASSERT(context.Data.Contains(Data::Session)); + WI_ASSERT(context.Args.Contains(ArgType::ImageId)); + WI_ASSERT(context.Data.Contains(Data::CreateContainerOptions)); + PullImageCallback callback; + auto result = ContainerService::Create( + context.Data.Get(), + string::WideToMultiByte(context.Args.Get()), + context.Data.Get(), + &callback); + wslutil::PrintMessage(wsl::shared::string::MultiByteToWide(result.Id)); +} + +void RunContainer(CLIExecutionContext& context) +{ + WI_ASSERT(context.Data.Contains(Data::Session)); + WI_ASSERT(context.Args.Contains(ArgType::ImageId)); + WI_ASSERT(context.Data.Contains(Data::RunContainerOptions)); + PullImageCallback callback; + ContainerService::Run( + context.Data.Get(), + string::WideToMultiByte(context.Args.Get()), + context.Data.Get(), + &callback); +} } // namespace wsl::windows::wslc::task diff --git a/src/windows/wslc/tasks/ContainerTasks.h b/src/windows/wslc/tasks/ContainerTasks.h index 0ab4dd2f5..d7631eaf2 100644 --- a/src/windows/wslc/tasks/ContainerTasks.h +++ b/src/windows/wslc/tasks/ContainerTasks.h @@ -19,4 +19,9 @@ using wsl::windows::wslc::execution::CLIExecutionContext; namespace wsl::windows::wslc::task { void CreateSession(CLIExecutionContext& context); void GetContainers(CLIExecutionContext& context); +void StartContainer(CLIExecutionContext& context); +void SetCreateContainerOptionsFromArgs(CLIExecutionContext& context); +void SetRunContainerOptionsFromArgs(CLIExecutionContext& context); +void CreateContainer(CLIExecutionContext& context); +void RunContainer(CLIExecutionContext& context); } // namespace wsl::windows::wslc::task diff --git a/test/windows/wslc/CommandLineTestCases.h b/test/windows/wslc/CommandLineTestCases.h index cb2ab7363..f2341f3bc 100644 --- a/test/windows/wslc/CommandLineTestCases.h +++ b/test/windows/wslc/CommandLineTestCases.h @@ -42,6 +42,17 @@ COMMAND_LINE_TEST_CASE(L"container list -qa", L"list", true) COMMAND_LINE_TEST_CASE(L"container list --format json", L"list", true) COMMAND_LINE_TEST_CASE(L"container list --format table", L"list", true) COMMAND_LINE_TEST_CASE(L"container list --format badformat", L"list", false) +COMMAND_LINE_TEST_CASE(L"run ubuntu", L"run", true) +COMMAND_LINE_TEST_CASE(L"container run ubuntu bash -c 'echo Hello World'", L"run", true) +COMMAND_LINE_TEST_CASE(L"container run ubuntu", L"run", true) +COMMAND_LINE_TEST_CASE(L"container run -it --name foo ubuntu", L"run", true) +COMMAND_LINE_TEST_CASE(L"stop --all", L"stop", true) +COMMAND_LINE_TEST_CASE(L"container stop -a --signal SIGKILL", L"stop", true) +COMMAND_LINE_TEST_CASE(L"container stop -a --signal sigkill", L"stop", false) +COMMAND_LINE_TEST_CASE(L"start cont", L"start", true) +COMMAND_LINE_TEST_CASE(L"container start cont", L"start", true) +COMMAND_LINE_TEST_CASE(L"create ubuntu:latest", L"create", true) +COMMAND_LINE_TEST_CASE(L"container create --name foo ubuntu", L"create", true) // Error cases COMMAND_LINE_TEST_CASE(L"invalid command", L"", false) diff --git a/test/windows/wslc/WSLCCLIArgumentUnitTests.cpp b/test/windows/wslc/WSLCCLIArgumentUnitTests.cpp index 0c6315bc8..77402378f 100644 --- a/test/windows/wslc/WSLCCLIArgumentUnitTests.cpp +++ b/test/windows/wslc/WSLCCLIArgumentUnitTests.cpp @@ -76,7 +76,7 @@ class WSLCCLIArgumentUnitTests args.Add(argType, std::wstring(L"test")); break; case Kind::Forward: - args.Add(argType, std::vector{L"forward1", L"forward2"}); + args.Add(argType, std::vector{"forward1", "forward2"}); break; case Kind::Flag: args.Add(argType, true); @@ -112,7 +112,7 @@ class WSLCCLIArgumentUnitTests VERIFY_IS_TRUE(argsContainer.Contains(ArgType::Help)); argsContainer.Add(std::wstring(L"test")); VERIFY_IS_TRUE(argsContainer.Contains(ArgType::ContainerId)); - argsContainer.Add(std::vector{L"test1", L"test2"}); + argsContainer.Add(std::vector{"test1", "test2"}); VERIFY_IS_TRUE(argsContainer.Contains(ArgType::ForwardArgs)); // Verify basic retrieval @@ -121,8 +121,8 @@ class WSLCCLIArgumentUnitTests auto retrievedString = argsContainer.Get(); VERIFY_ARE_EQUAL(retrievedString, std::wstring(L"test")); auto retrievedStringSet = argsContainer.Get(); - VERIFY_ARE_EQUAL(retrievedStringSet[0], std::wstring(L"test1")); - VERIFY_ARE_EQUAL(retrievedStringSet[1], std::wstring(L"test2")); + VERIFY_ARE_EQUAL(retrievedStringSet[0], std::string("test1")); + VERIFY_ARE_EQUAL(retrievedStringSet[1], std::string("test2")); // Verify multimap functionality and Runtime Add argsContainer.Add(ArgType::Publish, std::wstring(L"test1")); diff --git a/test/windows/wslc/WSLCCLIExecutionUnitTests.cpp b/test/windows/wslc/WSLCCLIExecutionUnitTests.cpp index 0db55a61b..8cca63011 100644 --- a/test/windows/wslc/WSLCCLIExecutionUnitTests.cpp +++ b/test/windows/wslc/WSLCCLIExecutionUnitTests.cpp @@ -83,6 +83,19 @@ class WSLCCLIExecutionUnitTests dataMap.Add(std::move(containers)); handled = true; } + else if (dataType == Data::CreateContainerOptions) + { + wsl::windows::wslc::models::ContainerCreateOptions options; + dataMap.Add(std::move(options)); + handled = true; + } + else if (dataType == Data::RunContainerOptions) + { + wsl::windows::wslc::models::ContainerRunOptions options; + dataMap.Add(std::move(options)); + handled = true; + } + if (!handled) { VERIFY_FAIL(L"Unhandled Data type in test"); diff --git a/test/windows/wslc/WSLCCLIParserUnitTests.cpp b/test/windows/wslc/WSLCCLIParserUnitTests.cpp index b1dccdbb5..10b434ba6 100644 --- a/test/windows/wslc/WSLCCLIParserUnitTests.cpp +++ b/test/windows/wslc/WSLCCLIParserUnitTests.cpp @@ -97,19 +97,11 @@ class WSLCCLIParserUnitTests { VERIFY_IS_TRUE(args.Contains(ArgType::ForwardArgs)); auto forwardArgs = args.Get(); - std::wstring forwardArgsConcat; - for (const auto& arg : forwardArgs) - { - if (!forwardArgsConcat.empty()) - { - forwardArgsConcat += L" "; - } - forwardArgsConcat += arg; - } - VERIFY_IS_TRUE(forwardArgsConcat.find(L"hello world") != std::wstring::npos); // Forward args should contain hello world - VERIFY_IS_TRUE(forwardArgsConcat.find(L"cont1") == std::wstring::npos); // Forward args should not contain the containerId - VERIFY_IS_TRUE(forwardArgsConcat.find(L"command") == std::wstring::npos); // Forward args should not contain the command - LogComment(L"Forwarded Args: " + forwardArgsConcat); + std::string forwardArgsConcat = wsl::shared::string::Join(forwardArgs, ' '); + VERIFY_IS_TRUE(forwardArgsConcat.find("hello world") != std::string::npos); // Forward args should contain hello world + VERIFY_IS_TRUE(forwardArgsConcat.find("cont1") == std::string::npos); // Forward args should not contain the containerId + VERIFY_IS_TRUE(forwardArgsConcat.find("command") == std::string::npos); // Forward args should not contain the command + LogComment(L"Forwarded Args: " + std::wstring(forwardArgsConcat.begin(), forwardArgsConcat.end())); } if (testCase.commandLine.find(L"443") != std::wstring::npos) From 2bfd4d1df557f1e1972d73effc1284c199edfbee Mon Sep 17 00:00:00 2001 From: David Bennett Date: Fri, 20 Feb 2026 05:22:38 -0800 Subject: [PATCH 02/14] Add command to the arg string --- src/windows/wslc/arguments/ArgumentParser.cpp | 6 +++--- src/windows/wslc/arguments/ArgumentTypes.h | 2 +- src/windows/wslc/tasks/ContainerTasks.cpp | 12 +++++++++++- test/windows/wslc/WSLCCLIArgumentUnitTests.cpp | 8 ++++---- test/windows/wslc/WSLCCLIParserUnitTests.cpp | 10 +++++----- 5 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/windows/wslc/arguments/ArgumentParser.cpp b/src/windows/wslc/arguments/ArgumentParser.cpp index 81ad7edea..ede33f563 100644 --- a/src/windows/wslc/arguments/ArgumentParser.cpp +++ b/src/windows/wslc/arguments/ArgumentParser.cpp @@ -220,11 +220,11 @@ ParseArgumentsStateMachine::State ParseArgumentsStateMachine::ProcessAnchoredPos // currArg is the first forwarded argument // All the rest of the args are forward args. - std::vector forwardedArgs; - forwardedArgs.emplace_back(string::WideToMultiByte(std::wstring{currArg})); + std::vector forwardedArgs; + forwardedArgs.emplace_back(std::wstring{currArg}); while (m_invocationItr != m_invocation.end()) { - forwardedArgs.emplace_back(string::WideToMultiByte(*m_invocationItr)); + forwardedArgs.emplace_back(std::wstring{*m_invocationItr}); ++m_invocationItr; } diff --git a/src/windows/wslc/arguments/ArgumentTypes.h b/src/windows/wslc/arguments/ArgumentTypes.h index 8ddfc083f..43638107a 100644 --- a/src/windows/wslc/arguments/ArgumentTypes.h +++ b/src/windows/wslc/arguments/ArgumentTypes.h @@ -74,7 +74,7 @@ namespace details { template <> struct KindToType { - using type = std::vector; + using type = std::vector; }; template diff --git a/src/windows/wslc/tasks/ContainerTasks.cpp b/src/windows/wslc/tasks/ContainerTasks.cpp index f1f672fd4..b031e1f2a 100644 --- a/src/windows/wslc/tasks/ContainerTasks.cpp +++ b/src/windows/wslc/tasks/ContainerTasks.cpp @@ -49,9 +49,19 @@ namespace { options.Interactive = true; } + if (context.Args.Contains(ArgType::Command)) + { + options.Arguments.push_back(string::WideToMultiByte(context.Args.Get())); + } + if (context.Args.Contains(ArgType::ForwardArgs)) { - options.Arguments = context.Args.Get(); + auto const& forwardArgs = context.Args.Get(); + options.Arguments.reserve(options.Arguments.size() + forwardArgs.size()); + for (const auto& arg : forwardArgs) + { + options.Arguments.push_back(string::WideToMultiByte(arg)); + } } } } // anonymous namespace diff --git a/test/windows/wslc/WSLCCLIArgumentUnitTests.cpp b/test/windows/wslc/WSLCCLIArgumentUnitTests.cpp index 77402378f..0c6315bc8 100644 --- a/test/windows/wslc/WSLCCLIArgumentUnitTests.cpp +++ b/test/windows/wslc/WSLCCLIArgumentUnitTests.cpp @@ -76,7 +76,7 @@ class WSLCCLIArgumentUnitTests args.Add(argType, std::wstring(L"test")); break; case Kind::Forward: - args.Add(argType, std::vector{"forward1", "forward2"}); + args.Add(argType, std::vector{L"forward1", L"forward2"}); break; case Kind::Flag: args.Add(argType, true); @@ -112,7 +112,7 @@ class WSLCCLIArgumentUnitTests VERIFY_IS_TRUE(argsContainer.Contains(ArgType::Help)); argsContainer.Add(std::wstring(L"test")); VERIFY_IS_TRUE(argsContainer.Contains(ArgType::ContainerId)); - argsContainer.Add(std::vector{"test1", "test2"}); + argsContainer.Add(std::vector{L"test1", L"test2"}); VERIFY_IS_TRUE(argsContainer.Contains(ArgType::ForwardArgs)); // Verify basic retrieval @@ -121,8 +121,8 @@ class WSLCCLIArgumentUnitTests auto retrievedString = argsContainer.Get(); VERIFY_ARE_EQUAL(retrievedString, std::wstring(L"test")); auto retrievedStringSet = argsContainer.Get(); - VERIFY_ARE_EQUAL(retrievedStringSet[0], std::string("test1")); - VERIFY_ARE_EQUAL(retrievedStringSet[1], std::string("test2")); + VERIFY_ARE_EQUAL(retrievedStringSet[0], std::wstring(L"test1")); + VERIFY_ARE_EQUAL(retrievedStringSet[1], std::wstring(L"test2")); // Verify multimap functionality and Runtime Add argsContainer.Add(ArgType::Publish, std::wstring(L"test1")); diff --git a/test/windows/wslc/WSLCCLIParserUnitTests.cpp b/test/windows/wslc/WSLCCLIParserUnitTests.cpp index 10b434ba6..55fed31ae 100644 --- a/test/windows/wslc/WSLCCLIParserUnitTests.cpp +++ b/test/windows/wslc/WSLCCLIParserUnitTests.cpp @@ -97,11 +97,11 @@ class WSLCCLIParserUnitTests { VERIFY_IS_TRUE(args.Contains(ArgType::ForwardArgs)); auto forwardArgs = args.Get(); - std::string forwardArgsConcat = wsl::shared::string::Join(forwardArgs, ' '); - VERIFY_IS_TRUE(forwardArgsConcat.find("hello world") != std::string::npos); // Forward args should contain hello world - VERIFY_IS_TRUE(forwardArgsConcat.find("cont1") == std::string::npos); // Forward args should not contain the containerId - VERIFY_IS_TRUE(forwardArgsConcat.find("command") == std::string::npos); // Forward args should not contain the command - LogComment(L"Forwarded Args: " + std::wstring(forwardArgsConcat.begin(), forwardArgsConcat.end())); + std::wstring forwardArgsConcat = wsl::shared::string::Join(forwardArgs, L' '); + VERIFY_IS_TRUE(forwardArgsConcat.find(L"hello world") != std::wstring::npos); // Forward args should contain hello world + VERIFY_IS_TRUE(forwardArgsConcat.find(L"cont1") == std::wstring::npos); // Forward args should not contain the containerId + VERIFY_IS_TRUE(forwardArgsConcat.find(L"command") == std::wstring::npos); // Forward args should not contain the command + LogComment(L"Forwarded Args: " + forwardArgsConcat); } if (testCase.commandLine.find(L"443") != std::wstring::npos) From 01ae2e53f402a40ddd0b07b382fe37c88de48300 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Fri, 20 Feb 2026 05:40:53 -0800 Subject: [PATCH 03/14] Update tests --- src/windows/wslc/tasks/ContainerTasks.cpp | 4 ++-- test/windows/wslc/ParserTestCases.h | 13 +++++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/windows/wslc/tasks/ContainerTasks.cpp b/src/windows/wslc/tasks/ContainerTasks.cpp index b031e1f2a..c1dfff9aa 100644 --- a/src/windows/wslc/tasks/ContainerTasks.cpp +++ b/src/windows/wslc/tasks/ContainerTasks.cpp @@ -51,7 +51,7 @@ namespace { if (context.Args.Contains(ArgType::Command)) { - options.Arguments.push_back(string::WideToMultiByte(context.Args.Get())); + options.Arguments.emplace_back(string::WideToMultiByte(context.Args.Get())); } if (context.Args.Contains(ArgType::ForwardArgs)) @@ -60,7 +60,7 @@ namespace { options.Arguments.reserve(options.Arguments.size() + forwardArgs.size()); for (const auto& arg : forwardArgs) { - options.Arguments.push_back(string::WideToMultiByte(arg)); + options.Arguments.emplace_back(string::WideToMultiByte(arg)); } } } diff --git a/test/windows/wslc/ParserTestCases.h b/test/windows/wslc/ParserTestCases.h index ffc0f9d6b..fe91f2451 100644 --- a/test/windows/wslc/ParserTestCases.h +++ b/test/windows/wslc/ParserTestCases.h @@ -51,12 +51,14 @@ inline std::vector GetArgumentsForSet(ArgumentSet Argument::Create(ArgType::Interactive), Argument::Create(ArgType::Verbose), Argument::Create(ArgType::Remove), - Argument::Create(ArgType::Publish, false, 3), // Not required, up to 3 values. + Argument::Create(ArgType::Signal), + Argument::Create(ArgType::Time), + Argument::Create(ArgType::Publish, false, -1), // Not required, up to 3 values. }; case ArgumentSet::List: return { - Argument::Create(ArgType::ContainerId, false, 10), // Optional positional + Argument::Create(ArgType::ContainerId, false, -1), // Optional positional Argument::Create(ArgType::Help), Argument::Create(ArgType::Verbose), }; @@ -84,6 +86,7 @@ WSLC_PARSER_TEST_CASE(Run, true, LR"(wslc -p=80:80 cont1)") \ WSLC_PARSER_TEST_CASE(Run, true, LR"(wslc -p 80:80 cont1)") \ WSLC_PARSER_TEST_CASE(Run, true, LR"(wslc -p 80:80 -p 443:443 cont1)") \ WSLC_PARSER_TEST_CASE(Run, true, LR"(wslc -p=80:80 -p=443:443 cont1)") \ +WSLC_PARSER_TEST_CASE(Run, false, LR"(wslc --verbose --verbose cont1)") \ \ /* Flag parse tests */ \ WSLC_PARSER_TEST_CASE(Run, true, LR"(wslc -v cont1)") \ @@ -100,6 +103,12 @@ WSLC_PARSER_TEST_CASE(Run, false, LR"(wslc -prmiv 80:80 cont1)") \ WSLC_PARSER_TEST_CASE(Run, true, LR"(wslc -rmivp 80:80 cont1)") \ WSLC_PARSER_TEST_CASE(Run, true, LR"(wslc -rmivp=80:80 cont1)") \ \ +/* Validation tests */ \ +WSLC_PARSER_TEST_CASE(Run, false, LR"(wslc --signal FOO cont1)") \ +WSLC_PARSER_TEST_CASE(Run, true, LR"(wslc --signal SIGTERM cont1)") \ +WSLC_PARSER_TEST_CASE(Run, false, LR"(wslc -t blah)") \ +WSLC_PARSER_TEST_CASE(Run, true, LR"(wslc -t 5)") \ +\ /* Multi-positional tests */ \ WSLC_PARSER_TEST_CASE(Run, true, LR"(wslc cont1 command)") \ WSLC_PARSER_TEST_CASE(Run, true, LR"(wslc cont1 command --f -z forward hello world)") \ From 6c52f79951afe6c1d81d17c1cab05509744a6447 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Fri, 20 Feb 2026 12:04:00 -0800 Subject: [PATCH 04/14] Fix signal to be an int --- src/windows/wslc/arguments/ArgumentValidation.cpp | 10 +--------- src/windows/wslc/arguments/ArgumentValidation.h | 1 - src/windows/wslc/commands/ContainerRunCommand.cpp | 9 +++++++-- .../wslc/commands/ContainerStartCommand.cpp | 9 ++++++--- .../wslc/commands/ContainerStopCommand.cpp | 2 +- src/windows/wslc/services/ContainerModel.h | 15 --------------- test/windows/wslc/CommandLineTestCases.h | 2 +- test/windows/wslc/ParserTestCases.h | 2 +- 8 files changed, 17 insertions(+), 33 deletions(-) diff --git a/src/windows/wslc/arguments/ArgumentValidation.cpp b/src/windows/wslc/arguments/ArgumentValidation.cpp index 93dfbd14d..47f73c3b3 100644 --- a/src/windows/wslc/arguments/ArgumentValidation.cpp +++ b/src/windows/wslc/arguments/ArgumentValidation.cpp @@ -25,7 +25,7 @@ void Argument::Validate(const ArgMap& execArgs) const switch (m_argType) { case ArgType::Signal: - validation::ValidateWSLASignal(execArgs.Get(), m_name); + validation::ValidateUInteger(execArgs.Get(), m_name); break; case ArgType::Time: @@ -40,14 +40,6 @@ void Argument::Validate(const ArgMap& execArgs) const namespace wsl::windows::wslc::validation { -void ValidateWSLASignal(const std::wstring& value, const std::wstring& argName) -{ - if (!models::SignalMap.contains(value)) - { - throw ArgumentException(L"Invalid " + argName + L" argument value: " + value); - } -} - void ValidateUInteger(const std::wstring& value, const std::wstring& argName) { try diff --git a/src/windows/wslc/arguments/ArgumentValidation.h b/src/windows/wslc/arguments/ArgumentValidation.h index 87f12b3fb..991e9b2aa 100644 --- a/src/windows/wslc/arguments/ArgumentValidation.h +++ b/src/windows/wslc/arguments/ArgumentValidation.h @@ -20,7 +20,6 @@ using namespace wsl::windows::wslc::argument; namespace wsl::windows::wslc::validation { -void ValidateWSLASignal(const std::wstring& value, const std::wstring& argName); void ValidateUInteger(const std::wstring& value, const std::wstring& argName); } // namespace wsl::windows::wslc::validation \ No newline at end of file diff --git a/src/windows/wslc/commands/ContainerRunCommand.cpp b/src/windows/wslc/commands/ContainerRunCommand.cpp index 5f9885183..66bb87008 100644 --- a/src/windows/wslc/commands/ContainerRunCommand.cpp +++ b/src/windows/wslc/commands/ContainerRunCommand.cpp @@ -77,8 +77,13 @@ std::wstring ContainerRunCommand::LongDescription() const return {L"Runs a container. By default, the container is started in the background; use --detach to run in the foreground."}; } +// clang-format off void ContainerRunCommand::ExecuteInternal(CLIExecutionContext& context) const { - context << CreateSession << SetRunContainerOptionsFromArgs << RunContainer; + context + << CreateSession + << SetRunContainerOptionsFromArgs + << RunContainer; } -} // namespace wsl::windows::wslc \ No newline at end of file +// clang-format on +} // namespace wsl::windows::wslc diff --git a/src/windows/wslc/commands/ContainerStartCommand.cpp b/src/windows/wslc/commands/ContainerStartCommand.cpp index b20e664e6..dca1fb03f 100644 --- a/src/windows/wslc/commands/ContainerStartCommand.cpp +++ b/src/windows/wslc/commands/ContainerStartCommand.cpp @@ -53,9 +53,12 @@ std::wstring ContainerStartCommand::LongDescription() const L"Starts a container. Provides options to attach to the container's stdout and stderr streams and could be interactive " L"to keep stdin open."}; } - +// clang-format off void ContainerStartCommand::ExecuteInternal(CLIExecutionContext& context) const { - context << CreateSession << StartContainer; + context + << CreateSession + << StartContainer; } -} // namespace wsl::windows::wslc \ No newline at end of file +// clang-format on +} // namespace wsl::windows::wslc diff --git a/src/windows/wslc/commands/ContainerStopCommand.cpp b/src/windows/wslc/commands/ContainerStopCommand.cpp index a64c9a467..55e937f92 100644 --- a/src/windows/wslc/commands/ContainerStopCommand.cpp +++ b/src/windows/wslc/commands/ContainerStopCommand.cpp @@ -82,7 +82,7 @@ void ContainerStopCommand::ExecuteInternal(CLIExecutionContext& context) const StopContainerOptions options; if (context.Args.Contains(ArgType::Signal)) { - options.Signal = SignalMap.at(context.Args.Get()); + options.Signal = std::stoul(context.Args.Get()); } if (context.Args.Contains(ArgType::Time)) diff --git a/src/windows/wslc/services/ContainerModel.h b/src/windows/wslc/services/ContainerModel.h index f9270ba2d..137f552de 100644 --- a/src/windows/wslc/services/ContainerModel.h +++ b/src/windows/wslc/services/ContainerModel.h @@ -22,21 +22,6 @@ Module Name: namespace wsl::windows::wslc::models { -// Map of signal names to WSLASignal enum values -inline static const std::unordered_map SignalMap = { - {L"SIGHUP", WSLASignalSIGHUP}, {L"SIGINT", WSLASignalSIGINT}, {L"SIGQUIT", WSLASignalSIGQUIT}, - {L"SIGILL", WSLASignalSIGILL}, {L"SIGTRAP", WSLASignalSIGTRAP}, {L"SIGABRT", WSLASignalSIGABRT}, - {L"SIGIOT", WSLASignalSIGIOT}, {L"SIGBUS", WSLASignalSIGBUS}, {L"SIGFPE", WSLASignalSIGFPE}, - {L"SIGKILL", WSLASignalSIGKILL}, {L"SIGUSR1", WSLASignalSIGUSR1}, {L"SIGSEGV", WSLASignalSIGSEGV}, - {L"SIGUSR2", WSLASignalSIGUSR2}, {L"SIGPIPE", WSLASignalSIGPIPE}, {L"SIGALRM", WSLASignalSIGALRM}, - {L"SIGTERM", WSLASignalSIGTERM}, {L"SIGTKFLT", WSLASignalSIGTKFLT}, {L"SIGCHLD", WSLASignalSIGCHLD}, - {L"SIGCONT", WSLASignalSIGCONT}, {L"SIGSTOP", WSLASignalSIGSTOP}, {L"SIGTSTP", WSLASignalSIGTSTP}, - {L"SIGTTIN", WSLASignalSIGTTIN}, {L"SIGTTOU", WSLASignalSIGTTOU}, {L"SIGURG", WSLASignalSIGURG}, - {L"SIGXCPU", WSLASignalSIGXCPU}, {L"SIGXFSZ", WSLASignalSIGXFSZ}, {L"SIGVTALRM", WSLASignalSIGVTALRM}, - {L"SIGPROF", WSLASignalSIGPROF}, {L"SIGWINCH", WSLASignalSIGWINCH}, {L"SIGIO", WSLASignalSIGIO}, - {L"SIGPOLL", WSLASignalSIGPOLL}, {L"SIGPWR", WSLASignalSIGPWR}, {L"SIGSYS", WSLASignalSIGSYS}, -}; - struct ContainerCreateOptions { bool TTY = false; diff --git a/test/windows/wslc/CommandLineTestCases.h b/test/windows/wslc/CommandLineTestCases.h index f2341f3bc..f3d01c03c 100644 --- a/test/windows/wslc/CommandLineTestCases.h +++ b/test/windows/wslc/CommandLineTestCases.h @@ -47,7 +47,7 @@ COMMAND_LINE_TEST_CASE(L"container run ubuntu bash -c 'echo Hello World'", L"run COMMAND_LINE_TEST_CASE(L"container run ubuntu", L"run", true) COMMAND_LINE_TEST_CASE(L"container run -it --name foo ubuntu", L"run", true) COMMAND_LINE_TEST_CASE(L"stop --all", L"stop", true) -COMMAND_LINE_TEST_CASE(L"container stop -a --signal SIGKILL", L"stop", true) +COMMAND_LINE_TEST_CASE(L"container stop -a --signal 9", L"stop", true) COMMAND_LINE_TEST_CASE(L"container stop -a --signal sigkill", L"stop", false) COMMAND_LINE_TEST_CASE(L"start cont", L"start", true) COMMAND_LINE_TEST_CASE(L"container start cont", L"start", true) diff --git a/test/windows/wslc/ParserTestCases.h b/test/windows/wslc/ParserTestCases.h index fe91f2451..12fd774f8 100644 --- a/test/windows/wslc/ParserTestCases.h +++ b/test/windows/wslc/ParserTestCases.h @@ -105,7 +105,7 @@ WSLC_PARSER_TEST_CASE(Run, true, LR"(wslc -rmivp=80:80 cont1)") \ \ /* Validation tests */ \ WSLC_PARSER_TEST_CASE(Run, false, LR"(wslc --signal FOO cont1)") \ -WSLC_PARSER_TEST_CASE(Run, true, LR"(wslc --signal SIGTERM cont1)") \ +WSLC_PARSER_TEST_CASE(Run, true, LR"(wslc --signal 9 cont1)") \ WSLC_PARSER_TEST_CASE(Run, false, LR"(wslc -t blah)") \ WSLC_PARSER_TEST_CASE(Run, true, LR"(wslc -t 5)") \ \ From 42b6f5daef100b1b462cc94b84ede5a2ab71a980 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Fri, 20 Feb 2026 13:58:18 -0800 Subject: [PATCH 05/14] adjust clang formatting for more readability on commands --- src/windows/wslc/commands/ContainerCreateCommand.cpp | 4 ++++ src/windows/wslc/commands/ContainerRunCommand.cpp | 2 ++ 2 files changed, 6 insertions(+) diff --git a/src/windows/wslc/commands/ContainerCreateCommand.cpp b/src/windows/wslc/commands/ContainerCreateCommand.cpp index caf32b156..a276b7d91 100644 --- a/src/windows/wslc/commands/ContainerCreateCommand.cpp +++ b/src/windows/wslc/commands/ContainerCreateCommand.cpp @@ -33,6 +33,7 @@ namespace wsl::windows::wslc { // Container Create Command std::vector ContainerCreateCommand::GetArguments() const { + //clang-format off return { Argument::Create(ArgType::ImageId, true), Argument::Create(ArgType::Command), Argument::Create(ArgType::ForwardArgs), Argument::Create(ArgType::CIDFile), Argument::Create(ArgType::DNS), Argument::Create(ArgType::DNSDomain), @@ -43,6 +44,7 @@ std::vector ContainerCreateCommand::GetArguments() const Argument::Create(ArgType::SessionId), Argument::Create(ArgType::TMPFS), Argument::Create(ArgType::TTY), Argument::Create(ArgType::User), Argument::Create(ArgType::Volume), Argument::Create(ArgType::Virtual), }; + // clang-format on } std::wstring ContainerCreateCommand::ShortDescription() const @@ -57,8 +59,10 @@ std::wstring ContainerCreateCommand::LongDescription() const L"foreground."}; } +//clang-format off void ContainerCreateCommand::ExecuteInternal(CLIExecutionContext& context) const { context << CreateSession << SetCreateContainerOptionsFromArgs << CreateContainer; } +// clang-format on } // namespace wsl::windows::wslc \ No newline at end of file diff --git a/src/windows/wslc/commands/ContainerRunCommand.cpp b/src/windows/wslc/commands/ContainerRunCommand.cpp index 66bb87008..8c898c7ae 100644 --- a/src/windows/wslc/commands/ContainerRunCommand.cpp +++ b/src/windows/wslc/commands/ContainerRunCommand.cpp @@ -37,6 +37,7 @@ namespace wsl::windows::wslc { // Container Run Command std::vector ContainerRunCommand::GetArguments() const { + // clang-format off return { Argument::Create(ArgType::ImageId, true), Argument::Create(ArgType::Command), @@ -65,6 +66,7 @@ std::vector ContainerRunCommand::GetArguments() const Argument::Create(ArgType::Volume), Argument::Create(ArgType::Virtual), }; + // clang-format on } std::wstring ContainerRunCommand::ShortDescription() const From 0650dee1b8f4541dd1b5a060fa7255c27adf2ddf Mon Sep 17 00:00:00 2001 From: David Bennett Date: Fri, 20 Feb 2026 14:02:37 -0800 Subject: [PATCH 06/14] Actually fix formatting in Create --- .../wslc/commands/ContainerCreateCommand.cpp | 41 ++++++++++++++----- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/src/windows/wslc/commands/ContainerCreateCommand.cpp b/src/windows/wslc/commands/ContainerCreateCommand.cpp index a276b7d91..ce38f0eef 100644 --- a/src/windows/wslc/commands/ContainerCreateCommand.cpp +++ b/src/windows/wslc/commands/ContainerCreateCommand.cpp @@ -33,16 +33,32 @@ namespace wsl::windows::wslc { // Container Create Command std::vector ContainerCreateCommand::GetArguments() const { - //clang-format off + // clang-format off return { - Argument::Create(ArgType::ImageId, true), Argument::Create(ArgType::Command), Argument::Create(ArgType::ForwardArgs), - Argument::Create(ArgType::CIDFile), Argument::Create(ArgType::DNS), Argument::Create(ArgType::DNSDomain), - Argument::Create(ArgType::DNSOption), Argument::Create(ArgType::DNSSearch), Argument::Create(ArgType::Entrypoint), - Argument::Create(ArgType::Env, false, -1), Argument::Create(ArgType::EnvFile), Argument::Create(ArgType::GroupId), - Argument::Create(ArgType::Interactive), Argument::Create(ArgType::Name), Argument::Create(ArgType::NoDNS), - Argument::Create(ArgType::Progress), Argument::Create(ArgType::Remove), Argument::Create(ArgType::Scheme), - Argument::Create(ArgType::SessionId), Argument::Create(ArgType::TMPFS), Argument::Create(ArgType::TTY), - Argument::Create(ArgType::User), Argument::Create(ArgType::Volume), Argument::Create(ArgType::Virtual), + Argument::Create(ArgType::ImageId, true), + Argument::Create(ArgType::Command), + Argument::Create(ArgType::ForwardArgs), + Argument::Create(ArgType::CIDFile), + Argument::Create(ArgType::DNS), + Argument::Create(ArgType::DNSDomain), + Argument::Create(ArgType::DNSOption), + Argument::Create(ArgType::DNSSearch), + Argument::Create(ArgType::Entrypoint), + Argument::Create(ArgType::Env, false, -1), + Argument::Create(ArgType::EnvFile), + Argument::Create(ArgType::GroupId), + Argument::Create(ArgType::Interactive), + Argument::Create(ArgType::Name), + Argument::Create(ArgType::NoDNS), + Argument::Create(ArgType::Progress), + Argument::Create(ArgType::Remove), + Argument::Create(ArgType::Scheme), + Argument::Create(ArgType::SessionId), + Argument::Create(ArgType::TMPFS), + Argument::Create(ArgType::TTY), + Argument::Create(ArgType::User), + Argument::Create(ArgType::Volume), + Argument::Create(ArgType::Virtual), }; // clang-format on } @@ -59,10 +75,13 @@ std::wstring ContainerCreateCommand::LongDescription() const L"foreground."}; } -//clang-format off +// clang-format off void ContainerCreateCommand::ExecuteInternal(CLIExecutionContext& context) const { - context << CreateSession << SetCreateContainerOptionsFromArgs << CreateContainer; + context + << CreateSession + << SetCreateContainerOptionsFromArgs + << CreateContainer; } // clang-format on } // namespace wsl::windows::wslc \ No newline at end of file From 572d6b311aaf1eaf2b1998626da780f79f6d2bfa Mon Sep 17 00:00:00 2001 From: David Bennett Date: Fri, 20 Feb 2026 14:32:50 -0800 Subject: [PATCH 07/14] Add NO_LIMIT macro to avoid hardcoding -1, update tests, change throws into asserts in parser --- src/windows/wslc/arguments/Argument.h | 1 + src/windows/wslc/arguments/ArgumentParser.cpp | 25 ++++++------------- .../wslc/commands/ContainerCreateCommand.cpp | 6 ++--- .../wslc/commands/ContainerRunCommand.cpp | 4 +-- .../wslc/commands/ContainerStopCommand.cpp | 2 +- test/windows/wslc/ParserTestCases.h | 4 +-- 6 files changed, 15 insertions(+), 27 deletions(-) diff --git a/src/windows/wslc/arguments/Argument.h b/src/windows/wslc/arguments/Argument.h index 9606a0414..5a7febb4f 100644 --- a/src/windows/wslc/arguments/Argument.h +++ b/src/windows/wslc/arguments/Argument.h @@ -22,6 +22,7 @@ Module Name: #define WSLC_CLI_HELP_ARG L"h" #define WSLC_CLI_HELP_ARG_STRING WSLC_CLI_ARG_ID_STRING WSLC_CLI_HELP_ARG #define NO_ALIAS L"" +#define NO_LIMIT -1 using namespace wsl::windows::wslc::argument; diff --git a/src/windows/wslc/arguments/ArgumentParser.cpp b/src/windows/wslc/arguments/ArgumentParser.cpp index ede33f563..909073c60 100644 --- a/src/windows/wslc/arguments/ArgumentParser.cpp +++ b/src/windows/wslc/arguments/ArgumentParser.cpp @@ -144,11 +144,7 @@ ParseArgumentsStateMachine::State ParseArgumentsStateMachine::StepInternal() // Assumes non-empty and does not begin with '-'. ParseArgumentsStateMachine::State ParseArgumentsStateMachine::ProcessPositionalArgument(const std::wstring_view& currArg) { - if (currArg.empty() || currArg[0] == WSLC_CLI_ARG_ID_CHAR) - { - // Assumption invalid, there is a bug in the logic. - THROW_HR(E_UNEXPECTED); - } + WI_ASSERT(!currArg.empty() && currArg[0] != WSLC_CLI_ARG_ID_CHAR); const Argument* nextPositional = NextPositional(); if (!nextPositional) @@ -170,16 +166,12 @@ ParseArgumentsStateMachine::State ParseArgumentsStateMachine::ProcessPositionalA // Only Kind::Positional or Kind::Forward arguments should remain. ParseArgumentsStateMachine::State ParseArgumentsStateMachine::ProcessAnchoredPositionals(const std::wstring_view& currArg) { - if (!m_anchorPositional.has_value()) - { - // Invalid state, this is a programmer error. - THROW_HR(E_UNEXPECTED); - } + WI_ASSERT(m_anchorPositional.has_value()); // If we haven't reached the limit for the anchor positional, treat this as another anchor positional. - // Anchors with a -1 limit will never be full and therefore will always treat subsequent positionals as anchors. + // Anchors with NO_LIMIT will never be full and therefore will always treat subsequent positionals as anchors. if ((m_executionArgs.Count(m_anchorPositional.value().Type()) < m_anchorPositional.value().Limit()) || - (m_anchorPositional.value().Limit() < 0)) + (m_anchorPositional.value().Limit() == NO_LIMIT)) { // validate that we dont have any invalid argument specifiers. if (!currArg.empty() && currArg[0] == WSLC_CLI_ARG_ID_CHAR) @@ -235,11 +227,7 @@ ParseArgumentsStateMachine::State ParseArgumentsStateMachine::ProcessAnchoredPos // Assumes argument begins with '-' and is at least 2 characters. ParseArgumentsStateMachine::State ParseArgumentsStateMachine::ProcessAliasArgument(const std::wstring_view& currArg) { - if (currArg.length() < 2 || currArg[0] != WSLC_CLI_ARG_ID_CHAR || currArg[1] == WSLC_CLI_ARG_ID_CHAR) - { - // Assumption invalid, this is a programmer error. - THROW_HR(E_UNEXPECTED); - } + WI_ASSERT(currArg.length() >= 2 && currArg[0] == WSLC_CLI_ARG_ID_CHAR && currArg[1] != WSLC_CLI_ARG_ID_CHAR); // This may be a collection of boolean alias flags. // Helper to find an argument by alias starting at a specific position. @@ -340,7 +328,8 @@ ParseArgumentsStateMachine::State ParseArgumentsStateMachine::ProcessAliasArgume // Assumes the arg value begins with -- and is at least 2 characters long. ParseArgumentsStateMachine::State ParseArgumentsStateMachine::ProcessNamedArgument(const std::wstring_view& currArg) { - THROW_HR_IF(E_UNEXPECTED, !currArg.starts_with(L"--")); + WI_ASSERT(currArg.starts_with(L"--")); + if (currArg.length() == 2) { // Missing argument name after double dash, this is an error. diff --git a/src/windows/wslc/commands/ContainerCreateCommand.cpp b/src/windows/wslc/commands/ContainerCreateCommand.cpp index ce38f0eef..236e940e3 100644 --- a/src/windows/wslc/commands/ContainerCreateCommand.cpp +++ b/src/windows/wslc/commands/ContainerCreateCommand.cpp @@ -44,7 +44,7 @@ std::vector ContainerCreateCommand::GetArguments() const Argument::Create(ArgType::DNSOption), Argument::Create(ArgType::DNSSearch), Argument::Create(ArgType::Entrypoint), - Argument::Create(ArgType::Env, false, -1), + Argument::Create(ArgType::Env, false, NO_LIMIT), Argument::Create(ArgType::EnvFile), Argument::Create(ArgType::GroupId), Argument::Create(ArgType::Interactive), @@ -70,9 +70,7 @@ std::wstring ContainerCreateCommand::ShortDescription() const std::wstring ContainerCreateCommand::LongDescription() const { - return { - L"Creates a container. By default, the container is created in the background; use --detach to create in the " - L"foreground."}; + return {L"Creates a container."}; } // clang-format off diff --git a/src/windows/wslc/commands/ContainerRunCommand.cpp b/src/windows/wslc/commands/ContainerRunCommand.cpp index 8c898c7ae..661b8f1d7 100644 --- a/src/windows/wslc/commands/ContainerRunCommand.cpp +++ b/src/windows/wslc/commands/ContainerRunCommand.cpp @@ -49,13 +49,13 @@ std::vector ContainerRunCommand::GetArguments() const Argument::Create(ArgType::DNSOption), Argument::Create(ArgType::DNSSearch), Argument::Create(ArgType::Entrypoint), - Argument::Create(ArgType::Env, std::nullopt, -1), + Argument::Create(ArgType::Env, std::nullopt, NO_LIMIT), Argument::Create(ArgType::EnvFile), Argument::Create(ArgType::Interactive), Argument::Create(ArgType::Name), Argument::Create(ArgType::NoDNS), Argument::Create(ArgType::Progress), - Argument::Create(ArgType::Publish, std::nullopt, -1), + Argument::Create(ArgType::Publish, std::nullopt, NO_LIMIT), Argument::Create(ArgType::Pull), Argument::Create(ArgType::Remove), Argument::Create(ArgType::Scheme), diff --git a/src/windows/wslc/commands/ContainerStopCommand.cpp b/src/windows/wslc/commands/ContainerStopCommand.cpp index 55e937f92..5f8c11b76 100644 --- a/src/windows/wslc/commands/ContainerStopCommand.cpp +++ b/src/windows/wslc/commands/ContainerStopCommand.cpp @@ -40,7 +40,7 @@ namespace wsl::windows::wslc { std::vector ContainerStopCommand::GetArguments() const { return { - Argument::Create(ArgType::ContainerId, std::nullopt, -1), + Argument::Create(ArgType::ContainerId, std::nullopt, NO_LIMIT), Argument::Create(ArgType::All), Argument::Create(ArgType::SessionId), Argument::Create(ArgType::Signal, std::nullopt, std::nullopt, L"Signal to send (default: SIGTERM)"), diff --git a/test/windows/wslc/ParserTestCases.h b/test/windows/wslc/ParserTestCases.h index 12fd774f8..dd04511b7 100644 --- a/test/windows/wslc/ParserTestCases.h +++ b/test/windows/wslc/ParserTestCases.h @@ -53,12 +53,12 @@ inline std::vector GetArgumentsForSet(ArgumentSet Argument::Create(ArgType::Remove), Argument::Create(ArgType::Signal), Argument::Create(ArgType::Time), - Argument::Create(ArgType::Publish, false, -1), // Not required, up to 3 values. + Argument::Create(ArgType::Publish, false, NO_LIMIT), // Not required, unlimited. }; case ArgumentSet::List: return { - Argument::Create(ArgType::ContainerId, false, -1), // Optional positional + Argument::Create(ArgType::ContainerId, false, NO_LIMIT), // Optional positional Argument::Create(ArgType::Help), Argument::Create(ArgType::Verbose), }; From dd57e14f7ddef0af0a6da2712f3e56dbdf647461 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Fri, 20 Feb 2026 15:04:39 -0800 Subject: [PATCH 08/14] Update validation to validate all arg values in case of multiple --- .../wslc/arguments/ArgumentValidation.cpp | 21 +++++++++++-------- .../wslc/arguments/ArgumentValidation.h | 2 +- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/windows/wslc/arguments/ArgumentValidation.cpp b/src/windows/wslc/arguments/ArgumentValidation.cpp index 47f73c3b3..e13062ecd 100644 --- a/src/windows/wslc/arguments/ArgumentValidation.cpp +++ b/src/windows/wslc/arguments/ArgumentValidation.cpp @@ -25,11 +25,11 @@ void Argument::Validate(const ArgMap& execArgs) const switch (m_argType) { case ArgType::Signal: - validation::ValidateUInteger(execArgs.Get(), m_name); + validation::ValidateUInteger(execArgs.GetAll(), m_name); break; case ArgType::Time: - validation::ValidateUInteger(execArgs.Get(), m_name); + validation::ValidateUInteger(execArgs.GetAll(), m_name); break; default: @@ -40,15 +40,18 @@ void Argument::Validate(const ArgMap& execArgs) const namespace wsl::windows::wslc::validation { -void ValidateUInteger(const std::wstring& value, const std::wstring& argName) +void ValidateUInteger(const std::vector& values, const std::wstring& argName) { - try + for (const auto& value : values) { - [[maybe_unused]] auto intValue = std::stoul(value); - } - catch (const std::exception&) - { - throw ArgumentException(L"Invalid " + argName + L" argument value: " + value); + try + { + [[maybe_unused]] auto intValue = std::stoul(value); + } + catch (const std::exception&) + { + throw ArgumentException(L"Invalid " + argName + L" argument value: " + value); + } } } } // namespace wsl::windows::wslc::validation \ No newline at end of file diff --git a/src/windows/wslc/arguments/ArgumentValidation.h b/src/windows/wslc/arguments/ArgumentValidation.h index 991e9b2aa..0aefb6a5c 100644 --- a/src/windows/wslc/arguments/ArgumentValidation.h +++ b/src/windows/wslc/arguments/ArgumentValidation.h @@ -20,6 +20,6 @@ using namespace wsl::windows::wslc::argument; namespace wsl::windows::wslc::validation { -void ValidateUInteger(const std::wstring& value, const std::wstring& argName); +void ValidateUInteger(const std::vector& values, const std::wstring& argName); } // namespace wsl::windows::wslc::validation \ No newline at end of file From da1118705aec09ea901e8872415eb9c81b604286 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Fri, 20 Feb 2026 15:17:16 -0800 Subject: [PATCH 09/14] Copilot adjustments --- src/windows/wslc/CMakeLists.txt | 3 +++ src/windows/wslc/arguments/ArgumentValidation.cpp | 2 +- src/windows/wslc/commands/ContainerStopCommand.cpp | 4 ++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/windows/wslc/CMakeLists.txt b/src/windows/wslc/CMakeLists.txt index 9f68867ff..3fa6bdeaa 100644 --- a/src/windows/wslc/CMakeLists.txt +++ b/src/windows/wslc/CMakeLists.txt @@ -63,6 +63,9 @@ set(SOURCES # Used to build the executable and also unit testing components. add_library(wslclib OBJECT ${SOURCES} ${HEADERS}) set_target_properties(wslclib PROPERTIES +# TODO: UNITY builds for wslclib are currently disabled because they have not been +# validated for build performance and diagnostics in this project. Re-enable +# the following properties once UNITY builds have been evaluated and approved: # UNITY_BUILD ON # UNITY_BUILD_BATCH_SIZE 0 # 0 means CMake decides automatically FOLDER windows diff --git a/src/windows/wslc/arguments/ArgumentValidation.cpp b/src/windows/wslc/arguments/ArgumentValidation.cpp index e13062ecd..eaf11f6e1 100644 --- a/src/windows/wslc/arguments/ArgumentValidation.cpp +++ b/src/windows/wslc/arguments/ArgumentValidation.cpp @@ -48,7 +48,7 @@ void ValidateUInteger(const std::vector& values, const std::wstrin { [[maybe_unused]] auto intValue = std::stoul(value); } - catch (const std::exception&) + catch (...) { throw ArgumentException(L"Invalid " + argName + L" argument value: " + value); } diff --git a/src/windows/wslc/commands/ContainerStopCommand.cpp b/src/windows/wslc/commands/ContainerStopCommand.cpp index 5f8c11b76..25d3126b8 100644 --- a/src/windows/wslc/commands/ContainerStopCommand.cpp +++ b/src/windows/wslc/commands/ContainerStopCommand.cpp @@ -50,13 +50,13 @@ std::vector ContainerStopCommand::GetArguments() const std::wstring ContainerStopCommand::ShortDescription() const { - return {L"Stop a container."}; + return {L"Stop containers"}; } std::wstring ContainerStopCommand::LongDescription() const { return { - L"Stops a container. By default, the container is stopped in the background; use --detach to stop in the foreground."}; + L"Stops containers. Use --all to stop all running containers."}; } void ContainerStopCommand::ExecuteInternal(CLIExecutionContext& context) const From 3b306a8b1ec70934bcf07b2b5a49d92511bd33b5 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Fri, 20 Feb 2026 15:24:23 -0800 Subject: [PATCH 10/14] Fix formatting --- src/windows/wslc/commands/ContainerStopCommand.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/windows/wslc/commands/ContainerStopCommand.cpp b/src/windows/wslc/commands/ContainerStopCommand.cpp index 25d3126b8..3ddcc1880 100644 --- a/src/windows/wslc/commands/ContainerStopCommand.cpp +++ b/src/windows/wslc/commands/ContainerStopCommand.cpp @@ -55,8 +55,7 @@ std::wstring ContainerStopCommand::ShortDescription() const std::wstring ContainerStopCommand::LongDescription() const { - return { - L"Stops containers. Use --all to stop all running containers."}; + return {L"Stops containers. Use --all to stop all running containers."}; } void ContainerStopCommand::ExecuteInternal(CLIExecutionContext& context) const From 4666ddae8507815749f7b30ae8ecf2c2c65de736 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Fri, 20 Feb 2026 17:21:03 -0800 Subject: [PATCH 11/14] PR feedback and remove --all --- .../wslc/commands/ContainerStopCommand.cpp | 18 +----------------- src/windows/wslc/services/PullImageCallback.h | 1 + test/windows/wslc/CommandLineTestCases.h | 6 +++--- 3 files changed, 5 insertions(+), 20 deletions(-) diff --git a/src/windows/wslc/commands/ContainerStopCommand.cpp b/src/windows/wslc/commands/ContainerStopCommand.cpp index 3ddcc1880..f5c939303 100644 --- a/src/windows/wslc/commands/ContainerStopCommand.cpp +++ b/src/windows/wslc/commands/ContainerStopCommand.cpp @@ -41,7 +41,6 @@ std::vector ContainerStopCommand::GetArguments() const { return { Argument::Create(ArgType::ContainerId, std::nullopt, NO_LIMIT), - Argument::Create(ArgType::All), Argument::Create(ArgType::SessionId), Argument::Create(ArgType::Signal, std::nullopt, std::nullopt, L"Signal to send (default: SIGTERM)"), Argument::Create(ArgType::Time), @@ -55,7 +54,7 @@ std::wstring ContainerStopCommand::ShortDescription() const std::wstring ContainerStopCommand::LongDescription() const { - return {L"Stops containers. Use --all to stop all running containers."}; + return {L"Stops containers."}; } void ContainerStopCommand::ExecuteInternal(CLIExecutionContext& context) const @@ -63,21 +62,6 @@ void ContainerStopCommand::ExecuteInternal(CLIExecutionContext& context) const context << CreateSession; auto containersToStop = context.Args.GetAll(); - if (context.Args.Contains(ArgType::All)) - { - // All overwrites any specified container IDs. - containersToStop.clear(); - context << GetContainers; - const auto& allContainers = context.Data.Get(); - for (const auto& container : allContainers) - { - if (container.State == WSLA_CONTAINER_STATE::WslaContainerStateRunning) - { - containersToStop.emplace_back(string::MultiByteToWide(container.Name)); - } - } - } - StopContainerOptions options; if (context.Args.Contains(ArgType::Signal)) { diff --git a/src/windows/wslc/services/PullImageCallback.h b/src/windows/wslc/services/PullImageCallback.h index d56c1cc8d..bf7f0e6c0 100644 --- a/src/windows/wslc/services/PullImageCallback.h +++ b/src/windows/wslc/services/PullImageCallback.h @@ -29,6 +29,7 @@ class ChangeTerminalMode CONSOLE_CURSOR_INFO m_originalCursorInfo{}; }; +// TODO: Handle terminal resizes. class DECLSPEC_UUID("7A1D3376-835A-471A-8DC9-23653D9962D0") PullImageCallback : public Microsoft::WRL::RuntimeClass, IProgressCallback, IFastRundown> { diff --git a/test/windows/wslc/CommandLineTestCases.h b/test/windows/wslc/CommandLineTestCases.h index f3d01c03c..54e8d245b 100644 --- a/test/windows/wslc/CommandLineTestCases.h +++ b/test/windows/wslc/CommandLineTestCases.h @@ -46,9 +46,9 @@ COMMAND_LINE_TEST_CASE(L"run ubuntu", L"run", true) COMMAND_LINE_TEST_CASE(L"container run ubuntu bash -c 'echo Hello World'", L"run", true) COMMAND_LINE_TEST_CASE(L"container run ubuntu", L"run", true) COMMAND_LINE_TEST_CASE(L"container run -it --name foo ubuntu", L"run", true) -COMMAND_LINE_TEST_CASE(L"stop --all", L"stop", true) -COMMAND_LINE_TEST_CASE(L"container stop -a --signal 9", L"stop", true) -COMMAND_LINE_TEST_CASE(L"container stop -a --signal sigkill", L"stop", false) +COMMAND_LINE_TEST_CASE(L"stop", L"stop", true) +COMMAND_LINE_TEST_CASE(L"container stop cont1 --signal 9", L"stop", true) +COMMAND_LINE_TEST_CASE(L"container stop cont1 --signal sigkill", L"stop", false) COMMAND_LINE_TEST_CASE(L"start cont", L"start", true) COMMAND_LINE_TEST_CASE(L"container start cont", L"start", true) COMMAND_LINE_TEST_CASE(L"create ubuntu:latest", L"create", true) From f9dc17a8dd98b412fe4a0c7577c22909d90e8992 Mon Sep 17 00:00:00 2001 From: David Bennett Date: Mon, 23 Feb 2026 10:12:01 -0800 Subject: [PATCH 12/14] feedback update --- src/windows/wslc/services/ContainerModel.h | 1 - src/windows/wslc/services/ContainerService.cpp | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/windows/wslc/services/ContainerModel.h b/src/windows/wslc/services/ContainerModel.h index 137f552de..d21beee2c 100644 --- a/src/windows/wslc/services/ContainerModel.h +++ b/src/windows/wslc/services/ContainerModel.h @@ -17,7 +17,6 @@ Module Name: #include #include #include -#include #include namespace wsl::windows::wslc::models { diff --git a/src/windows/wslc/services/ContainerService.cpp b/src/windows/wslc/services/ContainerService.cpp index 56f937caa..9ec937e6b 100644 --- a/src/windows/wslc/services/ContainerService.cpp +++ b/src/windows/wslc/services/ContainerService.cpp @@ -18,6 +18,7 @@ Module Name: #include #include #include +#include namespace wsl::windows::wslc::services { using wsl::windows::common::ClientRunningWSLAProcess; From 04d562b8642ec81e09fc34596b9a2deb64e0effb Mon Sep 17 00:00:00 2001 From: David Bennett Date: Tue, 24 Feb 2026 09:10:33 -0800 Subject: [PATCH 13/14] Copilot review adjustments --- src/windows/wslc/arguments/ArgumentValidation.h | 2 +- src/windows/wslc/services/ContainerModel.h | 2 +- src/windows/wslc/services/ContainerService.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/windows/wslc/arguments/ArgumentValidation.h b/src/windows/wslc/arguments/ArgumentValidation.h index 0aefb6a5c..90d4780d5 100644 --- a/src/windows/wslc/arguments/ArgumentValidation.h +++ b/src/windows/wslc/arguments/ArgumentValidation.h @@ -4,7 +4,7 @@ Copyright (c) Microsoft. All rights reserved. Module Name: - ArgumentValidation.cpp + ArgumentValidation.h Abstract: diff --git a/src/windows/wslc/services/ContainerModel.h b/src/windows/wslc/services/ContainerModel.h index d21beee2c..9e7eb95c1 100644 --- a/src/windows/wslc/services/ContainerModel.h +++ b/src/windows/wslc/services/ContainerModel.h @@ -43,7 +43,7 @@ struct StopContainerOptions { static constexpr ULONG DefaultTimeout = -1; - int Signal = WSLASignalSIGTERM; + ULONG Signal = WSLASignalSIGTERM; ULONG Timeout = DefaultTimeout; }; diff --git a/src/windows/wslc/services/ContainerService.cpp b/src/windows/wslc/services/ContainerService.cpp index 9ec937e6b..382a0cdf7 100644 --- a/src/windows/wslc/services/ContainerService.cpp +++ b/src/windows/wslc/services/ContainerService.cpp @@ -93,7 +93,7 @@ static wsl::windows::common::RunningWSLAContainer CreateInternal( return std::move(*runningContainer); } -static void StopInternal(IWSLAContainer& container, int signal = WSLASignalNone, ULONG timeout = -1) +static void StopInternal(IWSLAContainer& container, ULONG signal = WSLASignalNone, ULONG timeout = -1) { THROW_IF_FAILED(container.Stop(static_cast(signal), timeout)); // TODO: Error message } From a8dab2103e6423e4255b9d3ec191c0a7c33ad24a Mon Sep 17 00:00:00 2001 From: David Bennett Date: Tue, 24 Feb 2026 12:17:52 -0800 Subject: [PATCH 14/14] Fix missed error condition in argument conversion for out of range, add tests for argument validation --- .../wslc/arguments/ArgumentValidation.cpp | 2 +- .../windows/wslc/WSLCCLIArgumentUnitTests.cpp | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/windows/wslc/arguments/ArgumentValidation.cpp b/src/windows/wslc/arguments/ArgumentValidation.cpp index ebff2d5ef..5d6672b59 100644 --- a/src/windows/wslc/arguments/ArgumentValidation.cpp +++ b/src/windows/wslc/arguments/ArgumentValidation.cpp @@ -59,7 +59,7 @@ T GetIntegerFromString(const std::wstring& value, const std::wstring& argName) T convertedValue{}; auto result = std::from_chars(narrowValue.c_str(), narrowValue.c_str() + narrowValue.size(), convertedValue); - if (result.ec == std::errc::invalid_argument) + if (result.ec != std::errc()) { throw ArgumentException(L"Invalid " + argName + L" argument value: " + value); } diff --git a/test/windows/wslc/WSLCCLIArgumentUnitTests.cpp b/test/windows/wslc/WSLCCLIArgumentUnitTests.cpp index 0c6315bc8..cf475f87a 100644 --- a/test/windows/wslc/WSLCCLIArgumentUnitTests.cpp +++ b/test/windows/wslc/WSLCCLIArgumentUnitTests.cpp @@ -18,6 +18,8 @@ Module Name: #include "Argument.h" #include "ArgumentTypes.h" +#include "ArgumentValidation.h" +#include "Exceptions.h" using namespace wsl::windows::wslc; using namespace wsl::windows::wslc::argument; @@ -101,6 +103,32 @@ class WSLCCLIArgumentUnitTests } } + // Test: Verify Argument::Create() successfully creates arguments for all ArgType enum values + TEST_METHOD(ArgumentValidation_ValueValidation) + { + // Verify integer conversion for supported types. + auto ulong = validation::GetIntegerFromString(L"123"); + VERIFY_ARE_EQUAL(ulong, 123UL); + VERIFY_THROWS(validation::GetIntegerFromString(L"abc"), ArgumentException); // Not a number + VERIFY_THROWS(validation::GetIntegerFromString(L"-123"), ArgumentException); // Negative number + + auto longlong = validation::GetIntegerFromString(L"1234567890123"); + VERIFY_ARE_EQUAL(longlong, 1234567890123LL); + VERIFY_THROWS(validation::GetIntegerFromString(L"abc"), ArgumentException); // Not a number + VERIFY_THROWS(validation::GetIntegerFromString(L"-92233720369999854775808"), ArgumentException); // Out of range + + // Verify Array loop for validation works. + std::vector validUlongValues = {L"1234", L"12345"}; + VERIFY_NO_THROW(validation::ValidateIntegerFromString(validUlongValues, L"testArg")); + std::vector invalidUlongValues = {L"1234", L"abc"}; + VERIFY_THROWS(validation::ValidateIntegerFromString(invalidUlongValues, L"testArg"), ArgumentException); + + std::vector validLonglongValues = {L"1234", L"-1234567890123"}; + VERIFY_NO_THROW(validation::ValidateIntegerFromString(validLonglongValues, L"testArg")); + std::vector invalidLonglongValues = {L"1234", L"-92233720369999854775808"}; + VERIFY_THROWS(validation::ValidateIntegerFromString(invalidLonglongValues, L"testArg"), ArgumentException); + } + // Test: Verify EnumVariantMap behavior with ArgTypes. TEST_METHOD(EnumVariantMap_AllDataTypes) {