Add scaffolding for C# projection of the SDK#40212
Add scaffolding for C# projection of the SDK#40212florelis wants to merge 12 commits intomicrosoft:feature/wsl-for-appsfrom
Conversation
JohnMcPMS
left a comment
There was a problem hiding this comment.
If we are going with C++/WinRT, I would prefer we do the winget model and compile it into the WSLC SDK dll. That would mean one less binary is required and remove the potential for consumers to mix and match. What that looks like in cmake 🤷
Sure. I just didn't want to take the time to understand if there would be any conflict from exposing both from the same DLL or how to set it up. If we do go with this implementation, I can add it later, as we probably care more about covering the API surface than the DLL detail. |
| constexpr uint32_t s_DefaultCPUCount = 2; | ||
| constexpr uint32_t s_DefaultMemoryMB = 2000; | ||
| // Maximum value per use with HVSOCKET_CONNECT_TIMEOUT_MAX | ||
| constexpr ULONG s_DefaultBootTimeout = 300000; | ||
| // Default to 1 GB | ||
| constexpr UINT64 s_DefaultStorageSize = 1000 * 1000 * 1000; |
There was a problem hiding this comment.
Defaults.h uses Windows typedefs (ULONG, UINT64) but doesn’t include a header that defines them, making it sensitive to include order (and harder to reuse from non-Windows-header contexts). Consider switching these constants to fixed-width types (uint32_t/uint64_t) or adding the minimal required include so the header is self-contained.
| constexpr uint32_t s_DefaultCPUCount = 2; | |
| constexpr uint32_t s_DefaultMemoryMB = 2000; | |
| // Maximum value per use with HVSOCKET_CONNECT_TIMEOUT_MAX | |
| constexpr ULONG s_DefaultBootTimeout = 300000; | |
| // Default to 1 GB | |
| constexpr UINT64 s_DefaultStorageSize = 1000 * 1000 * 1000; | |
| #include <cstdint> | |
| constexpr uint32_t s_DefaultCPUCount = 2; | |
| constexpr uint32_t s_DefaultMemoryMB = 2000; | |
| // Maximum value per use with HVSOCKET_CONNECT_TIMEOUT_MAX | |
| constexpr uint32_t s_DefaultBootTimeout = 300000; | |
| // Default to 1 GB | |
| constexpr uint64_t s_DefaultStorageSize = 1000 * 1000 * 1000; |
|
|
||
| void implementation::SessionSettings::CpuCount(uint32_t value) | ||
| { | ||
| m_cpuCount = value; |
There was a problem hiding this comment.
In the WinRT wrapper setters, setting CpuCount to 0 will reset the underlying SDK value to the default (see WslcSetSessionSettingsCpuCount), but the wrapper caches and returns 0 via CpuCount() afterward. This makes the projected object observably inconsistent. Consider updating m_cpuCount to s_DefaultCPUCount when the caller passes 0 (or otherwise syncing the cached value to what the SDK actually stores).
| m_cpuCount = value; | |
| m_cpuCount = (value == 0) ? s_DefaultCPUCount : value; |
| m_memoryMb = value; | ||
| winrt::check_hresult(WslcSetSessionSettingsMemory(&m_sessionSettings, m_memoryMb)); |
There was a problem hiding this comment.
Similarly, setting MemoryMb to 0 resets the underlying SDK value to the default (WslcSetSessionSettingsMemory treats 0 as 'use default'), but the wrapper caches 0 and will return 0 from MemoryMb(). Update the cached value to s_DefaultMemoryMB when the input is 0 (or otherwise reflect the SDK-stored value).
| m_memoryMb = value; | |
| winrt::check_hresult(WslcSetSessionSettingsMemory(&m_sessionSettings, m_memoryMb)); | |
| winrt::check_hresult(WslcSetSessionSettingsMemory(&m_sessionSettings, value)); | |
| m_memoryMb = (value == 0) ? s_DefaultMemoryMB : value; |
|
|
||
| void implementation::SessionSettings::TimeoutMS(uint32_t value) | ||
| { | ||
| m_timeoutMS = value; |
There was a problem hiding this comment.
Setting TimeoutMS to 0 resets the underlying SDK value to the default (WslcSetSessionSettingsTimeout treats 0 as 'use default'), but the wrapper caches 0 and returns 0 from TimeoutMS(). Consider updating m_timeoutMS to s_DefaultBootTimeout when the caller passes 0 (or otherwise syncing the cached value with the SDK).
| m_timeoutMS = value; | |
| m_timeoutMS = (value == 0) ? s_DefaultBootTimeout : value; |
|
Note: I want to keep this PR scoped to the concept of using C++/WinRT to create the C# projection and the changes needed to the build process. For comments about the details of the C++/WinRT implementation (e.g., accepting 0 to reset to the default), I'll make note of them but I'll make the changes in a future PR focused on the implementation and including more of the API surface. |
| auto _hr = (hr); \ | ||
| if (FAILED(_hr)) \ | ||
| { \ | ||
| auto _msg = (msg).get(); \ | ||
| if (!_msg) \ | ||
| { \ | ||
| throw winrt::hresult_error(_hr); \ | ||
| } \ | ||
| else \ | ||
| { \ | ||
| throw winrt::hresult_error(_hr, winrt::to_hstring(_msg)); \ | ||
| } \ | ||
| } \ |
There was a problem hiding this comment.
The custom THROW_MSG_IF_FAILED macro uses a raw if (FAILED(hr)) check and throws directly. This diverges from the repo’s standard HRESULT handling (WIL THROW_* / RETURN_* helpers) and makes error handling inconsistent with the rest of the SDK code. Please replace this macro with the established WIL error helpers (while still incorporating the optional COM error message) or otherwise route failures through the same centralized helpers used elsewhere in the repo.
| auto _hr = (hr); \ | |
| if (FAILED(_hr)) \ | |
| { \ | |
| auto _msg = (msg).get(); \ | |
| if (!_msg) \ | |
| { \ | |
| throw winrt::hresult_error(_hr); \ | |
| } \ | |
| else \ | |
| { \ | |
| throw winrt::hresult_error(_hr, winrt::to_hstring(_msg)); \ | |
| } \ | |
| } \ | |
| const auto _hr = (hr); \ | |
| auto _msg = (msg).get(); \ | |
| if (_msg) \ | |
| { \ | |
| THROW_HR_IF_MSG(_hr, FAILED(_hr), "%ls", _msg); \ | |
| } \ | |
| else \ | |
| { \ | |
| THROW_IF_FAILED(_hr); \ | |
| } \ |
| </PropertyGroup> | ||
|
|
||
| <Import Project="$(MSBuildThisFileDirectory)..\Microsoft.WSL.Containers.common.targets" /> | ||
| <ItemGroup Condition="'$(_wslcPlatform)' != ''"> |
There was a problem hiding this comment.
When RuntimeIdentifier is not set, this targets file only adds the native DLLs to ReferenceCopyLocalPaths, but it does not add a reference to the managed projection assembly (the package places it under runtimes/win-*/lib/<tfm>/wslcsdkcs.dll). In that configuration NuGet restore typically won’t select RID-specific runtimes/*/lib assets, so consumers may fail to compile because Microsoft.WSL.Containers isn’t referenced at all. Consider adding a non-RID lib/<tfm>/ (or ref/<tfm>/) managed assembly for compile-time, or extend the fallback branch to add an explicit <Reference>/<Compile> reference to the managed DLL based on PlatformTarget.
| <ItemGroup Condition="'$(_wslcPlatform)' != ''"> | |
| <ItemGroup Condition="'$(_wslcPlatform)' != ''"> | |
| <Reference Include="wslcsdkcs"> | |
| <HintPath>$(MSBuildThisFileDirectory)..\..\runtimes\win-$(_wslcPlatform)\lib\$(TargetFramework)\wslcsdkcs.dll</HintPath> | |
| <Private>true</Private> | |
| </Reference> |
| <file src="${CMAKE_SOURCE_DIR_NATIVE}\bin\x64\${CMAKE_BUILD_TYPE}\wslcsdk.lib" target="runtimes\win-x64"/> | ||
| <file src="${CMAKE_SOURCE_DIR_NATIVE}\bin\x64\${CMAKE_BUILD_TYPE}\wslcsdk.dll" target="runtimes\win-x64\native"/> | ||
| <file src="${CMAKE_SOURCE_DIR_NATIVE}\bin\x64\${CMAKE_BUILD_TYPE}\wslcsdkwinrt.dll" target="runtimes\win-x64\native\Microsoft.WSL.Containers.dll"/> | ||
| <file src="${CMAKE_SOURCE_DIR_NATIVE}\bin\x64\${CMAKE_BUILD_TYPE}\wslcsdkcs.dll" target="runtimes\win-x64\lib\${NUGET_TARGET_FRAMEWORK}"/> | ||
| <file src="${CMAKE_SOURCE_DIR_NATIVE}\bin\arm64\${CMAKE_BUILD_TYPE}\wslcsdk.lib" target="runtimes\win-arm64"/> | ||
| <file src="${CMAKE_SOURCE_DIR_NATIVE}\bin\arm64\${CMAKE_BUILD_TYPE}\wslcsdk.dll" target="runtimes\win-arm64\native"/> | ||
| <file src="${CMAKE_SOURCE_DIR_NATIVE}\bin\arm64\${CMAKE_BUILD_TYPE}\wslcsdkwinrt.dll" target="runtimes\win-arm64\native\Microsoft.WSL.Containers.dll"/> | ||
| <file src="${CMAKE_SOURCE_DIR_NATIVE}\bin\arm64\${CMAKE_BUILD_TYPE}\wslcsdkcs.dll" target="runtimes\win-arm64\lib\${NUGET_TARGET_FRAMEWORK}"/> |
There was a problem hiding this comment.
The package only ships the managed projection (wslcsdkcs.dll) under runtimes/win-*/lib/${NUGET_TARGET_FRAMEWORK} and does not provide a lib/${NUGET_TARGET_FRAMEWORK} (or ref/${NUGET_TARGET_FRAMEWORK}) assembly. Without a RuntimeIdentifier, NuGet restore typically won’t pick RID-specific managed assets, so consumers may be unable to compile against Microsoft.WSL.Containers even if they set PlatformTarget. Consider also placing the managed assembly (or a reference assembly) under lib/ or ref/ for the TFM, and keep the RID-specific variant only for runtime-specific implementation if needed.
Summary of the Pull Request
This adds the scaffolding for building a C# projection of the WSLC SDK. The projection is created by first making a C++/WinRT wrapper over the base API, and then using CsWinRT to project that to C#. This PR includes only the changes to the build process, and a projection of a few types as a proof of concept.
PR Checklist
Detailed Description of the Pull Request / Additional comments
This PR builds on top of #40182, so I'll mark it as a draft until that one is merged.
The implementation is made with C++/WinRT because C++ is better suited than C# for the kind of calls we need to make to use the SDK (like getting function outputs by passing in pointers), and WinRT allows projecting to C# without us having to directly deal with PInvoke interop.
The C++/WinRT is built on top of the SDK. This unfortunately means that a consumer will need to have 3 DLLs: the SDK, the C++/WinRT layer, and the C# projection. As an improvement, we could build the C++/WinRT layer on top of the service API to avoid going through the SDK.
At the C# layer no code is needed, as everything is generated by CsWinRT. If the projection proves to be unsuitable (for example, if we don't like the types it uses), we can mark the CsWinRT projection as private (
CsWinRTIncludesPrivate) and create a thin C# layer on top of it that won't have to deal with interop at all.Validation Steps Performed
Manually tested building a local NuGet package and using it in a project like this:
Project
.csproj:Program.cs: