Skip to content

Add wslc container cp command for tar archive upload#40835

Open
ptrivedi wants to merge 24 commits into
microsoft:masterfrom
ptrivedi:feature/wslc-container-cp
Open

Add wslc container cp command for tar archive upload#40835
ptrivedi wants to merge 24 commits into
microsoft:masterfrom
ptrivedi:feature/wslc-container-cp

Conversation

@ptrivedi

@ptrivedi ptrivedi commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements wslc container cp for bidirectional file copy between containers and the local filesystem, plus stdin tar upload into containers, via Docker's archive APIs.

Usage

# Local file to container
wslc container cp ./myfile.txt my_container:/app/

# Container to local
wslc container cp my_container:/etc/ssl/cert.pem C:\temp\certs\

# Pipe a tar archive into a container (stdin)
tar.exe -cf - cert.pem | wslc container cp - my_container:/etc/ssl/

Implementation

Full stack implementation across all layers:

Layer Change
IDL Added UploadArchive and DownloadArchive to IWSLCContainer
DockerHTTPClient Added PutArchive and GetArchive methods
WSLCContainerImpl Upload: relay stdin to Docker socket with SD_SEND on EOF. Download: stream tar from Docker, extract with tar.exe
ContainerService Added CopyToContainer and CopyFromContainer static methods
ContainerTasks Added CopyToContainer task with bidirectional CONTAINER:PATH parsing
ContainerCpCommand New command registered under container cp
Localization Added all user-facing strings
Tests CLI parsing test cases + e2e tests for all directions + error cases

Key Design Decisions

  • Bidirectional copy: Supports local→container, container→local, and stdin→container
  • Direction detection: Parses CONTAINER:PATH patterns (single letter before colon treated as Windows drive path)
  • Local file upload: Creates tar via tar.exe, then uploads with known Content-Length
  • Container download: Downloads tar from Docker, extracts to local directory via tar.exe
  • Stdin upload: Uses GetFileSizeEx when stdin is a file redirect; omits Content-Length header for pipes (relies on SD_SEND shutdown as EOF signal)
  • Error handling: Non-200 responses read synchronously with SO_RCVTIMEO timeout to avoid HTTP/1.1 keep-alive hang; JSON parse failures fall back to raw body text
  • --archive/-a flag: Accepted for Docker CLI compatibility (no-op — Docker always preserves uid/gid from tar headers)
  • Trailing path separator: Stripped before embedding in tar commands to avoid CRT \" escape bug; root paths preserved with size() > 1 guard

Testing

  • All WSLCCLI unit tests pass
  • Added CLI parsing test cases for container cp
  • Added e2e tests: stdin upload, local file upload, container→local download, trailing backslash regression, nonexistent path/dir/container errors, stopped container copy, invalid direction

Implements 'wslc container cp - CONTAINER:PATH' to copy a tar archive
from stdin into a running container via Docker's PUT /containers/{id}/archive API.

Usage: tar.exe -cf - files | wslc container cp - my_container:/dest

Changes across all layers:
- IDL: Added UploadArchive to IWSLCContainer
- DockerHTTPClient: Added PutArchive method (omits Content-Length for pipes)
- WSLCContainerImpl: Relay stdin to Docker socket with SD_SEND on EOF
- ContainerService: Added CopyToContainer static method
- ContainerTasks: Added CopyToContainer task with CONTAINER:PATH parsing
- ContainerCpCommand: New command registered under 'container cp'
- Localization: Added all user-facing strings
- Tests: Added CLI parsing test cases and updated e2e command list

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ptrivedi ptrivedi requested a review from a team as a code owner June 17, 2026 18:08
Copilot AI review requested due to automatic review settings June 17, 2026 18:08
@ptrivedi ptrivedi marked this pull request as draft June 17, 2026 18:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a new wslc container cp subcommand that uploads a tar archive from stdin into a running container by relaying stdin to Docker’s PUT /containers/{id}/archive endpoint, with supporting wiring through the CLI task/service layers and new localized help/error strings.

Changes:

  • Adds a new container cp command and CLI task/service method to parse CONTAINER:PATH and stream stdin into the container.
  • Extends the Docker HTTP client with a PutArchive request helper used by the session/container implementation.
  • Updates tests (CLI parsing + e2e container command list) and adds localized user-facing strings for the new command.

Reviewed changes

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

Show a summary per file
File Description
test/windows/wslc/e2e/WSLCE2EContainerTests.cpp Adds cp to the container subcommand help list assertions.
test/windows/wslc/CommandLineTestCases.h Adds command-line parsing coverage for container cp.
src/windows/wslcsession/WSLCContainer.h Declares new UploadArchive APIs on the container implementation and COM wrapper.
src/windows/wslcsession/WSLCContainer.cpp Implements archive upload by relaying stdin to Docker and handling HTTP responses/errors.
src/windows/wslcsession/DockerHTTPClient.h Declares PutArchive for Docker archive upload.
src/windows/wslcsession/DockerHTTPClient.cpp Implements PutArchive request creation (optionally omitting Content-Length).
src/windows/wslc/tasks/ContainerTasks.h Declares CopyToContainer task entrypoint.
src/windows/wslc/tasks/ContainerTasks.cpp Implements CONTAINER:PATH parsing and stdin validation/size detection.
src/windows/wslc/services/ContainerService.h Adds CopyToContainer service API.
src/windows/wslc/services/ContainerService.cpp Wires CLI service call to COM IWSLCContainer::UploadArchive.
src/windows/wslc/commands/ContainerCpCommand.cpp Adds the new container cp command and argument definitions.
src/windows/wslc/commands/ContainerCommand.h Declares ContainerCpCommand.
src/windows/wslc/commands/ContainerCommand.cpp Registers container cp under the container root command.
src/windows/service/inc/wslc.idl Adds UploadArchive to IWSLCContainer.
localization/strings/en-US/Resources.resw Adds help text and user-facing error messages for container cp.

Comment thread src/windows/service/inc/wslc.idl
Comment thread localization/strings/en-US/Resources.resw
Comment thread src/windows/wslcsession/DockerHTTPClient.cpp

@dkbennett dkbennett left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation looks good, but I think this should really have at least 1 E2E test verifying the copy actually works as expected.

Can put it in with one of the other container tests so we dont create an entirely new file for it if necessary, but should have something validating we do not regress this command's functionality in the future and have confidence that it is working correctly.

Comment thread src/windows/wslc/commands/ContainerCommand.cpp
@dkbennett

Copy link
Copy Markdown
Member

Just adding a note that this PR will have a conflict with #40832 due to the change from CreateSession (which is poorly named since it does an open or a create default only) to ResolveSession, depending on which one goes in first. If this one goes in first I can fix it in 40832.

Pooja Trivedi and others added 2 commits June 17, 2026 16:33
- Add 5 new CLI parsing test cases for cp in CommandLineTestCases.h
- Add RunWslcWithStdinFile helper to pipe file contents to wslc stdin
- Add WSLCE2EContainerCpTests with 11 e2e test methods covering:
  - Help output, missing arguments, invalid target formats
  - Stdin terminal detection, source validation
  - Container not found error handling
  - Successful tar upload to running container with exec verification
  - Copy to stopped container (Docker PUT /archive filesystem operation)
- CreateTestTarFile builds minimal POSIX tar at runtime for tests

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix ContainerCpDesc to accurately describe stdin-only tar upload
  (was misleadingly implying bidirectional file copy)
- Change PutArchive ContentLength parameter to std::optional<uint64_t>
  to distinguish 'unknown size' (pipe) from 'known zero size' (empty file)
- WSLCContainerImpl::UploadArchive passes std::nullopt when ContentSize is 0

Note: Kept UploadArchive on IWSLCContainer (not a separate IWSLCContainer2)
because wslc interfaces are internal — client and server are always deployed
together from the same build, so there is no ABI compatibility concern.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 17, 2026 21:35
@ptrivedi

Copy link
Copy Markdown
Contributor Author

Addressed code review feedback:

  1. Description mismatch (fixed): Updated \ContainerCpDesc\ to accurately say 'Copy a tar archive from stdin into a container' instead of implying bidirectional copy.

  2. Content-Length ambiguity (fixed): Changed \PutArchive\ to use \std::optional<uint64_t>\ for ContentLength, properly distinguishing 'unknown size' (pipe, passes \std::nullopt) from 'known zero size' (empty file, passes \

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread src/windows/service/inc/wslc.idl
Comment thread src/windows/wslcsession/WSLCContainer.cpp
Comment thread test/windows/wslc/e2e/WSLCExecutor.h
Comment thread src/windows/wslcsession/WSLCContainer.cpp
Comment thread localization/strings/en-US/Resources.resw
Copilot AI review requested due to automatic review settings June 17, 2026 22:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread src/windows/service/inc/wslc.idl
Comment thread src/windows/wslcsession/WSLCContainer.cpp
Comment on lines +76 to +80
// Running without piped stdin should fail with a terminal error.
// RunWslcAndRedirectToFile gives the child a real console stdout handle,
// and since RunWslc pipes NUL to stdin, we use RunWslcAndRedirectToFile
// with no output path to get a real console for the child.
const auto result = RunWslcAndRedirectToFile(L"container cp - fakecontainer:/path");
Comment thread localization/strings/en-US/Resources.resw
Pooja Trivedi and others added 2 commits June 17, 2026 19:43
- Add Archive argument type in ArgumentDefinitions.h
- Add --archive/-a flag to ContainerCpCommand arguments
- Add WSLCCLI_ArchiveArgDescription localization string
- Add 2 CLI parsing test cases for -a and --archive
- Add 2 e2e tests: ArchiveFlag (-a) and ArchiveFlagLongForm (--archive)

The flag is accepted for docker cp compatibility. Since the tar stream
is relayed directly to Docker's PUT /archive API, uid/gid information
from the source tar is always preserved (equivalent to -a behavior).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rue/false)

Add ParseBoolValue() helper to ArgumentParser that accepts true/false/1/0
(case-insensitive). Modify ProcessNamedArgument() and ProcessAliasArgument()
to parse adjoined boolean values for flag-type arguments.

- --flag=true and -f=true set the flag (equivalent to --flag / -f)
- --flag=false and -f=false leave the flag unset
- Invalid values produce FlagInvalidBooleanError
- Add 8 CLI parsing unit test cases for boolean flag values
- Add 5 e2e tests for -a/--archive boolean value variants
- All 143 unit tests and 18 e2e tests pass

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 18, 2026 01:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.

Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
contentSize = static_cast<ULONGLONG>(fileSize.QuadPart);
}

ContainerService::CopyToContainer(session, containerId, destPath, stdinHandle, contentSize);
Comment thread src/windows/service/inc/wslc.idl
Comment thread src/windows/wslcsession/DockerHTTPClient.cpp
Pooja Trivedi and others added 2 commits June 17, 2026 21:40
- Validate DestPath is non-empty in UploadArchive (E_INVALIDARG)
- Unify CONTAINER:DEST_PATH to CONTAINER:PATH in long description
- Guard FromJson with try-catch for non-JSON error responses
- Fix RunWslcWithStdinFile declaration line length (>130 col)
- Add comment explaining archive flag is intentional no-op for stdin tar

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 18, 2026 01:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread src/windows/service/inc/wslc.idl
Comment on lines +2452 to +2454
<value>Copy a tar archive from stdin into a container. Use '-' as the source to read from stdin.
Usage: wslc container cp - CONTAINER:PATH</value>
<comment>{Locked="wslc"}{Locked="container cp"}{Locked="CONTAINER:PATH"}</comment>
Comment on lines +74 to +85
WSLC_TEST_METHOD(WSLCE2E_Container_Cp_StdinIsTerminal)
{
// Running without piped stdin should fail with a terminal error.
// RunWslcAndRedirectToFile gives the child a real console stdout handle,
// and since RunWslc pipes NUL to stdin, we use RunWslcAndRedirectToFile
// with no output path to get a real console for the child.
const auto result = RunWslcAndRedirectToFile(L"container cp - fakecontainer:/path");
VERIFY_IS_TRUE(result.ExitCode.has_value());
VERIFY_ARE_EQUAL(1u, result.ExitCode.value());
VERIFY_IS_TRUE(result.Stderr.has_value());
VERIFY_ARE_NOT_EQUAL(0u, result.Stderr.value().size());
}
Comment thread src/windows/wslcsession/WSLCContainer.cpp
@ptrivedi ptrivedi marked this pull request as ready for review June 18, 2026 19:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread src/windows/wslc/tasks/ContainerTasks.cpp Outdated
Comment thread src/windows/wslcsession/WSLCContainer.cpp
Comment thread src/windows/wslcsession/WSLCContainer.cpp
Comment thread src/windows/wslcsession/WSLCContainer.cpp Outdated
Comment thread test/windows/wslc/e2e/WSLCExecutor.cpp
Pooja Trivedi and others added 2 commits June 24, 2026 17:54
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 24, 2026 22:02
Mirrors DownloadArchive's pattern: if the error body is empty or
truncated (e.g. timeout), fall back to surfacing the raw body text
instead of letting FromJson throw an unhelpful exception.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated 6 comments.

Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
Comment thread src/windows/wslc/tasks/ContainerTasks.cpp
Comment thread src/windows/wslc/tasks/ContainerTasks.h
Use size() > 1 guard to preserve root paths like C:\ (matching the
upload path logic), preventing tar.exe from receiving invalid 'C:'.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 24, 2026 22:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread src/windows/wslcsession/WSLCContainer.cpp
Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
Comment thread src/windows/wslc/tasks/ContainerTasks.h Outdated
};

void CreateContainer(CLIExecutionContext& context);
void CopyToContainer(CLIExecutionContext& context);
Pooja Trivedi and others added 2 commits June 24, 2026 18:42
The task handles both directions (local→container and container→local),
so the old name was misleading. ContainerService::CopyToContainer
(upload-only) keeps its name since it genuinely only copies to.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 24, 2026 23:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated 6 comments.

Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
Comment thread src/windows/service/inc/wslc.idl
Comment on lines +412 to +419
auto wideTarget = MultiByteToWide(target);
auto absTarget = std::filesystem::absolute(wideTarget);

// Ensure target directory exists
std::error_code dirError;
std::filesystem::create_directories(absTarget, dirError);
THROW_HR_IF_MSG(HRESULT_FROM_WIN32(dirError.value()), !!dirError, "Failed to create directory: %ls", absTarget.c_str());

Comment on lines +163 to +175
wil::unique_hfile nul;
HANDLE effectiveStdin = nullptr;
if (stdinHandle.has_value())
{
effectiveStdin = stdinHandle.value();
THROW_IF_WIN32_BOOL_FALSE(SetHandleInformation(effectiveStdin, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT));
}
else
{
nul = wsl::windows::common::filesystem::OpenNulDevice(GENERIC_READ);
THROW_IF_WIN32_BOOL_FALSE(SetHandleInformation(nul.get(), HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT));
effectiveStdin = nul.get();
}
Copilot AI review requested due to automatic review settings June 24, 2026 23:14
@ptrivedi ptrivedi marked this pull request as ready for review June 24, 2026 23:14
@ptrivedi ptrivedi requested review from OneBlue and dkbennett June 24, 2026 23:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated 6 comments.

Comment thread src/windows/service/inc/wslc.idl
Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
Comment on lines +293 to +307
auto isContainerPath = [](const std::string& path) -> bool {
auto colonPos = path.find(':');
if (colonPos == std::string::npos || colonPos == 0)
{
return false;
}

// Single letter before colon is a Windows drive path
if (colonPos == 1 && std::isalpha(static_cast<unsigned char>(path[0])))
{
return false;
}

return true;
};
Comment on lines +412 to +418
auto wideTarget = MultiByteToWide(target);
auto absTarget = std::filesystem::absolute(wideTarget);

// Ensure target directory exists
std::error_code dirError;
std::filesystem::create_directories(absTarget, dirError);
THROW_HR_IF_MSG(HRESULT_FROM_WIN32(dirError.value()), !!dirError, "Failed to create directory: %ls", absTarget.c_str());
When the target path does not end with a separator and is not an existing
directory, treat it as a file destination instead of a directory. This
matches docker cp behavior where 'cp CONTAINER:/file.txt C:\local\out.txt'
creates out.txt as a file rather than extracting into a directory named
out.txt.

Implementation: extract to temp dir, then rename/copy the extracted file
to the target path. Falls back to copy+delete if rename fails across
volumes.

Also adds an e2e test verifying this behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

// Parses a boolean string value (true/false/1/0, case-insensitive).
// Returns the parsed value, or std::nullopt if the string is not a valid boolean.
static std::optional<bool> ParseBoolValue(std::wstring_view value)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily a blocker, but in the future I think we should separate parser changes from new commands being introduced. This will make changes smaller and easier to review

// Returns the parsed value, or std::nullopt if the string is not a valid boolean.
static std::optional<bool> ParseBoolValue(std::wstring_view value)
{
if (string::IsEqual(value, L"true") || value == L"1")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend using wsl::shared::string::ParseBool() instead of reimplementing it here


LARGE_INTEGER fileSize{};
ULONGLONG contentSize = 0;
if (GetFileSizeEx(inputHandle, &fileSize))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long term, we'll probably need to read the entire handle into a temporary file to resolve this

else
{
// Local path → container: create tar from local path using tar.exe
auto widePath = MultiByteToWide(source);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to explicitly convert this path. std::filesystem and our localization methods can handle narrow and wide strings

parentDirStr.pop_back();
}

auto tarCmd = std::format(L"tar.exe -cf \"{}\" -C \"{}\" \"{}\"", tempPath, parentDirStr, fileName);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of passing the file path to tar, I recommend opening the file handle ourselves with FILE_FLAG_DELETE_ON_CLOSE and passing it as tar's stdout. Then we can reset the handle's file pointer and pass the same handle to the service.

Otherwise there's a race between tar closing the file and us opening it

}
else
{
if (isChunked)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend using DockerHttpResponseHandle, which already handles this

{
auto error = wsl::shared::FromJson<ErrorResponse>(errorJson.c_str());

THROW_HR_WITH_USER_ERROR_IF(WSLC_E_CONTAINER_NOT_FOUND, error.message, statusCode == 404);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments as above: 404 shouldn't be possible here, and the below catch block can be removed

VERIFY_IS_TRUE(result.ExitCode.has_value());
VERIFY_ARE_EQUAL(1u, result.ExitCode.value());
VERIFY_IS_TRUE(result.Stderr.has_value());
VERIFY_IS_TRUE(result.Stderr->find(L"tar") != std::wstring::npos);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend validating the exact error message (same for below)

auto tarCmd = std::format(L"tar.exe -cf \"{}\" -C \"{}\" testfile.txt", TarPath.wstring(), tarSrcDir.wstring());
STARTUPINFOW si{sizeof(si)};
PROCESS_INFORMATION pi{};
THROW_LAST_ERROR_IF(!CreateProcessW(nullptr, tarCmd.data(), nullptr, nullptr, FALSE, 0, nullptr, nullptr, &si, &pi));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend using the subprocess helper here as well

}

WSLCExecutionResult RunWslc(const std::wstring& commandLine, ElevationType elevationType)
WSLCExecutionResult RunWslc(const std::wstring& commandLine, ElevationType elevationType, std::optional<HANDLE> stdinHandle)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend using a regular HANDLE here with NULL as default (that way we can reassign easily without needing an additional variable)

Copilot AI review requested due to automatic review settings June 25, 2026 00:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 22 changed files in this pull request and generated 7 comments.

Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
Comment thread src/windows/wslc/arguments/ArgumentParser.cpp
Comment on lines +424 to +427
if (boolVal.value())
{
AddFlag(nextArg->Type());
}
Comment on lines +473 to +475
auto extractDir = std::filesystem::path(tempDir) / L"wslc-cp-extract";
std::filesystem::create_directories(extractDir);
auto cleanupExtract = wil::scope_exit([&] { std::filesystem::remove_all(extractDir); });
Comment on lines +515 to +517
std::error_code dirError;
std::filesystem::create_directories(absTarget.parent_path(), dirError);

Comment on lines +503 to +512
// Find the extracted file and move it to the target path.
// Docker's archive API returns a tar with the file at its basename.
std::filesystem::path extractedFile;
for (const auto& entry : std::filesystem::directory_iterator(extractDir))
{
extractedFile = entry.path();
break;
}

THROW_HR_WITH_USER_ERROR_IF(E_FAIL, "No file extracted from container archive", extractedFile.empty());
Comment thread src/windows/service/inc/wslc.idl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants