Implement SDK Import and shared file spec by JohnMcPMS · Pull Request #14451 · microsoft/WSL

JohnMcPMS · 2026-03-16T23:34:43Z

Summary of the Pull Request

Allow both Import and Load to take in file path or HANDLE and implement Import.

PR Checklist

Closes: Link to issue #xxx
Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
Tests: Added/updated if needed and all pass
Localization: All end user facing strings can be localized
Dev docs: Added/updated if needed
Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Move both Import and Load options to use a shared file specification structure that can accept a path or HANDLE+length. Add image name to import options. Implement Import and Load using a shared path to normalize to HANDLE+length for runtime usage.

Validation Steps Performed

New tests.

Copilot

Pull request overview

This PR adds a shared “image file specifier” to the WSLC SDK so Load and Import can accept either a file path or a (HANDLE, length) pair, and implements WslcImportSessionImage end-to-end.

Changes:

Introduces WslcImageFileSpecifier / WslcImageFileContent and updates WslcLoadImageOptions / WslcImportImageOptions to use it.
Implements WslcImportSessionImage and refactors WslcLoadSessionImage to normalize inputs via a shared resolver.
Updates/extends Windows SDK tests to cover load/import via both path and handle+length, plus negative cases.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
test/windows/WslcSdkTests.cpp	Updates Load tests to use `imageFile` specifier; adds new Import coverage (path + handle).
src/windows/WslcSDK/wslcsdk.h	Adds shared file specifier structs and updates public option structs (Import/Load).
src/windows/WslcSDK/wslcsdk.cpp	Implements Import and adds a shared resolver that converts path → handle+length for runtime calls.

src/windows/WslcSDK/wslcsdk.cpp

src/windows/WslcSDK/wslcsdk.h

test/windows/WslcSdkTests.cpp

src/windows/WslcSDK/wslcsdk.cpp

Copilot

Pull request overview

This PR extends the WSLC SDK image import/load APIs to accept a shared “file specifier” (path or HANDLE+length) and wires up the previously unimplemented WslcImportSessionImage to the underlying session runtime, with updated SDK tests to validate both input modes.

Changes:

Introduces WslcImageFileSpecifier / WslcImageFileContent and updates WslcLoadImageOptions and WslcImportImageOptions to use the shared file spec.
Implements WslcImportSessionImage and normalizes both Import/Load to HANDLE+length via a shared resolver.
Updates/expands Windows SDK tests for Load and new Import coverage (path and handle variants, plus negative cases).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
test/windows/WslcSdkTests.cpp	Updates Load tests to use the shared file specifier and adds new Import tests (positive + negative).
src/windows/WslcSDK/wslcsdk.h	Adds the shared image file spec structs and updates Import/Load option structs accordingly.
src/windows/WslcSDK/wslcsdk.cpp	Implements Import and adds `ImageFileResolver` to normalize path/handle inputs for Import and Load.

src/windows/WslcSDK/wslcsdk.cpp

src/windows/WslcSDK/wslcsdk.h

Copilot

Pull request overview

This PR updates the WSLC SDK image load/import APIs to support both file-path and HANDLE+length inputs, and implements image import by normalizing inputs to a shared HANDLE+length path used by the runtime.

Changes:

Implement WslcImportSessionImage and add ...FromFile entrypoints for both Import and Load.
Refactor file input handling via a shared resolver that normalizes file paths to HANDLE+length.
Update SDK tests to cover the new Import/Load entrypoints and negative cases.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/windows/WslcSdkTests.cpp	Updates tests to use new Load/Import APIs and adds new Import coverage.
src/windows/WslcSDK/wslcsdk.h	Changes public SDK API signatures for Load/Import and adds `...FromFile` declarations.
src/windows/WslcSDK/wslcsdk.def	Exports the new `...FromFile` SDK entrypoints.
src/windows/WslcSDK/wslcsdk.cpp	Implements Import and adds shared file normalization logic + new `...FromFile` functions.
src/windows/WslcSDK/ProgressCallback.h	Makes progress callback creation safe when `options == nullptr`.

src/windows/WslcSDK/wslcsdk.cpp

src/windows/WslcSDK/wslcsdk.h

test/windows/WslcSdkTests.cpp

OneBlue

LGTM. Minor comments

src/windows/WslcSDK/wslcsdk.cpp

OneBlue · 2026-03-18T23:34:09Z

test/windows/WslcSdkTests.cpp

+
+        // Negative: zero ContentLength must fail.
+        VERIFY_ARE_EQUAL(WslcImportSessionImage(m_defaultSession, "zero-length:test", GetCurrentThreadEffectiveToken(), 0, &opts, nullptr), E_INVALIDARG);
    }


nit: We should validate that fail with E_FAIL or similar if the caller tries to important an invalid file

Added duplicate of the load version... and it successfully fails even though load still "succeeds" despite their very common code paths...

Hmm what do you mean by 'load still "succeeds" ' ? Are you saying that there are code paths where we don't correctly detect errors ?

The test LoadImageNonTar and new ImportImageNonTar pass the current executable/module to construct the image. Import fails as expected. Load has always not failed, but not had any effect on the image list either.

If I shell into the session and run docker load with bad files it does return error strings. But when we call the engine it is just returning a 200 while ignoring the input (when I last debugged it).

Ok this is a runtime bug. I'll look into it

Here's the fix for that broken behavior: https://github.com/microsoft/WSL/pull/14483/changes

Feel free to merge now and do this as a followup once the runtime fix is merged

OneBlue

LGTM, one nit question in the tests

OneBlue · 2026-03-19T18:21:14Z

test/windows/WslcSdkTests.cpp

+        VERIFY_ARE_EQUAL(WslcLoadSessionImage(m_defaultSession, INVALID_HANDLE_VALUE, 1, &opts, nullptr), E_INVALIDARG);
+
+        // Negative: zero ContentLength must fail.
+        VERIFY_ARE_EQUAL(WslcLoadSessionImage(m_defaultSession, GetCurrentThreadEffectiveToken(), 0, &opts, nullptr), E_INVALIDARG);


What's the reason for passing GetCurrentThreadEffectiveToken() ? Are we just trying to get any valid handle here ?

Yes, because GetCurrentProcess uses INVALID_HANDLE_VALUE as its pseudo-handle and I wanted something that I knew would be guaranteed to be a real HANDLE.

JohnMcPMS added 3 commits March 16, 2026 16:16

Add shared file inputs, implement import

8879515

Import tests

be203cb

Format

43db45b

JohnMcPMS requested a review from a team as a code owner March 16, 2026 23:34

Copilot AI review requested due to automatic review settings March 16, 2026 23:34

Copilot started reviewing on behalf of JohnMcPMS March 16, 2026 23:35 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

src/windows/WslcSDK/wslcsdk.cpp Outdated Show resolved Hide resolved

src/windows/WslcSDK/wslcsdk.h Outdated Show resolved Hide resolved

test/windows/WslcSdkTests.cpp Show resolved Hide resolved

src/windows/WslcSDK/wslcsdk.cpp Outdated Show resolved Hide resolved

JohnMcPMS added 2 commits March 16, 2026 16:39

Format missed

b2b8c57

SAL

d2c9045

Copilot AI review requested due to automatic review settings March 16, 2026 23:44

Copilot started reviewing on behalf of JohnMcPMS March 16, 2026 23:45 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

src/windows/WslcSDK/wslcsdk.cpp Outdated Show resolved Hide resolved

src/windows/WslcSDK/wslcsdk.h Outdated Show resolved Hide resolved

OneBlue reviewed Mar 17, 2026

View reviewed changes

src/windows/WslcSDK/wslcsdk.h Outdated Show resolved Hide resolved

JohnMcPMS added 3 commits March 17, 2026 15:45

PR feedback: split APIs for handle+length and path

9d9d91b

format

ae2f106

better

864b272

Copilot AI review requested due to automatic review settings March 17, 2026 22:59

Copilot started reviewing on behalf of JohnMcPMS March 17, 2026 23:00 View session

Copilot AI reviewed Mar 17, 2026

View reviewed changes

src/windows/WslcSDK/wslcsdk.cpp Show resolved Hide resolved

src/windows/WslcSDK/wslcsdk.h Show resolved Hide resolved

test/windows/WslcSdkTests.cpp Show resolved Hide resolved

OneBlue approved these changes Mar 18, 2026

View reviewed changes

JohnMcPMS added 3 commits March 18, 2026 17:20

PR feedback: handle, test

9b18a49

.\FormatSource.ps1

01ca838

Empty commit to force a new merge build

b02b3d2

OneBlue approved these changes Mar 19, 2026

View reviewed changes

JohnMcPMS merged commit c5f9de1 into microsoft:feature/wsl-for-apps Mar 19, 2026
6 checks passed

JohnMcPMS deleted the sdk-ee branch March 19, 2026 18:50

Conversation

JohnMcPMS commented Mar 16, 2026

Summary of the Pull Request

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

OneBlue left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OneBlue left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants