Initial wslc settings support#14548
Conversation
…yaosun/wslcsettings
There was a problem hiding this comment.
Pull request overview
Adds initial user-settings support to wslc, backed by a YAML file, and wires those settings into session default options and a new wslc settings command.
Changes:
- Introduces
UserSettingssingleton +SettingsMapfor loading/validating settings fromUserSettings.yamlwith.bakfallback. - Updates session default options to read CPU/memory/storage defaults from settings.
- Adds
wslc settings/wslc settings resetcommands, plus unit tests and a newyaml-cppdependency.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test/windows/wslc/WSLCCLISettingsUnitTests.cpp | Adds unit tests for settings parsing, fallback, and validation. |
| test/windows/wslc/CMakeLists.txt | Adds settings include directory for tests. |
| src/windows/wslc/settings/UserSettings.h | Declares settings enums/mappings, SettingsMap, warnings, and UserSettings API. |
| src/windows/wslc/settings/UserSettings.cpp | Implements YAML loading/validation, defaults template, and reset/prepare behaviors. |
| src/windows/wslc/services/SessionModel.cpp | Switches session defaults (CPU/memory/storage) to come from UserSettings. |
| src/windows/wslc/core/Main.cpp | Emits settings load warnings at startup. |
| src/windows/wslc/commands/SettingsCommand.h | Declares settings and settings reset CLI commands. |
| src/windows/wslc/commands/SettingsCommand.cpp | Implements opening the settings file and resetting it. |
| src/windows/wslc/commands/RootCommand.cpp | Registers settings under the root command. |
| src/windows/wslc/CMakeLists.txt | Adds settings subdir and links yaml-cpp into wslclib. |
| CMakeLists.txt | Fetches/builds yaml-cpp via FetchContent. |
…ft/WSL into user/yaosun/wslcsettings
OneBlue
left a comment
There was a problem hiding this comment.
LGTM.
I think in the longer term we might want to add more advanced defaults for CPU & memory like we do in WSL (by default we use all the CPU cores, and half of the available memory), but I think this is something that we should spec and definitely not required for selfhost.
Thank you for doing this !
| std::optional<uint32_t> ParseSettingsMemoryValue(const std::string& value) | ||
| { | ||
| auto parsed = wsl::shared::string::ParseMemorySize(value.c_str()); | ||
| auto converted = parsed.has_value() ? *parsed / 1048576 : 0; // To Mb, and anything less than 1Mb is considered invalid. |
There was a problem hiding this comment.
nit: We can use the _1MB macro for this (in helpers.hpp)
There was a problem hiding this comment.
I'll update in the followup pr for networkMode setting
Summary of the Pull Request
Initial settings support. Still need localization as current wslc does not have localization framework hooked up yet.
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed
Added tests. Also manually verified settings commands work and settings values do get populated to session objects defaults.