Partial SDK implementation#14211
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request implements a partial SDK for the WSL Container functionality, providing enough implementation for a sample demo to operate. The implementation bridges the public SDK API (wslcsdk.h) with the internal WSLA runtime services through COM interfaces. The PR also includes a change from #14187 that adds a Clang requirement to the vswhere usage in the build system.
Changes:
- Implements core SDK functions for session management, container creation/management, and process handling
- Adds callback implementations for progress reporting and session termination notifications
- Updates build configuration to require Clang installation in Visual Studio
- Changes API signature of WslcContainerCreate to require a session parameter
- Reduces WSLC_SESSION_OPTIONS_SIZE from 80 to 72 bytes (due to memoryMb type change from uint64_t to uint32_t)
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| CMakeLists.txt | Adds Clang requirement to vswhere command and improves error message to show missing path |
| src/windows/WslcSDK/wslcsdk.h | Changes WslcContainerCreate signature to add session parameter and adjusts WSLC_SESSION_OPTIONS_SIZE from 80 to 72 bytes |
| src/windows/WslcSDK/wslcsdk.cpp | Implements SDK functions by wrapping WSLA COM runtime APIs, including session/container/process management and image pulling |
| src/windows/WslcSDK/WslcsdkPrivate.h | Updates internal structures (memoryMb type change), adds GetInternalType helper declarations, and defines macros for opaque type conversions |
| src/windows/WslcSDK/WslcsdkPrivate.cpp | Implements GetInternalType helper functions for casting between opaque SDK types and internal implementation types |
| src/windows/WslcSDK/TerminationCallback.h | Defines ITerminationCallback implementation for forwarding session termination events to SDK callback |
| src/windows/WslcSDK/TerminationCallback.cpp | Implements termination callback conversion between runtime and SDK termination reason types |
| src/windows/WslcSDK/ProgressCallback.h | Defines IProgressCallback implementation for forwarding image operation progress to SDK callback |
| src/windows/WslcSDK/ProgressCallback.cpp | Implements progress callback with string-to-enum conversion for progress status values |
| src/windows/WslcSDK/CMakeLists.txt | Updates source and header file lists to include new callback implementation files |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
OneBlue
left a comment
OneBlue
left a comment
There was a problem hiding this comment.
Change looks good, mostly minor comments
benhillis
left a comment
benhillis
left a comment
There was a problem hiding this comment.
Looks like your PR from your fork isn't triggering checks. I'm not sure why, in the meantime you might want to push a user branch to the main WSL repo
seems to be working now. |
OneBlue
left a comment
OneBlue
left a comment
There was a problem hiding this comment.
Change LGTM, couple minor comments
JohnMcPMS
dismissed
benhillis’s stale review
Stale, based on checks not running at the time but they are now working.
3 participants
Summary of the Pull Request
Implements enough of the SDK for the sample demo to operate.
PR Checklist
Detailed Description of the Pull Request / Additional comments
Implements the functions called by the sample demo (as well as a few more trivial functions). These are implemented as best as possible, but there are several TODO callouts for updates that are likely needed to the SDK API surface to account for disparities with the runtime API. I intentionally did not make those changes to the SDK in this PR to limit the friction so that it might get merged sooner. I plan to make a follow-up PR with those changes that can have more/longer discussions.
Additionally, tests are not added for the SDK with this PR at this time. Again, a fast follow-up PR is expected to start adding those tests. Recommendation for new or existing test project are welcome.
Finally, I left the changes that were merged in #14187 to discuss how mainline changes should make it to the feature branch (as I'm dependent on this change for my machine setup to build WSL). Happy to remove it late in the PR process so I can keep building without constant git juggling.
Validation Steps Performed
Sample demo runs successfully and can be run multiple times.