Skip to content

wslc: add --digests to image list for docker parity - #41457

Open
ggarzia-MSFT wants to merge 18 commits into
masterfrom
user/ggarzia/wslc-images-digests
Open

wslc: add --digests to image list for docker parity#41457
ggarzia-MSFT wants to merge 18 commits into
masterfrom
user/ggarzia/wslc-images-digests

Conversation

@ggarzia-MSFT

@ggarzia-MSFT ggarzia-MSFT commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary of the Pull Request

Adds --digests to wslc image list (and its image ls / images spellings), matching docker images --digests. The DIGEST column was previously hardcoded to <none> in json output and absent from the table.

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

WSLCListImagesFlagsDigests was already defined and already honored end to end — WSLCSession::ListImages forwards it to Docker().ListImages(all, digests, filters) and populates WSLCImageInformation::Digest. The CLI never set the flag and never read the field, so ToImageOutput hardcoded Digest to <none>.

  • New ArgType::Digests (--digests, no short alias, matching docker where only listing commands' --filter carries -f).
  • models::ImageInformation gains a Digest field, and ImageService::List takes a defaulted bool digests that sets the flag and populates it.
  • Table output adds a DIGEST column between TAG and IMAGE ID only when --digests is passed, matching docker's defaultImageTableFormatWithDigest.
  • The service reports a repo digest (repo@sha256:...); docker's DIGEST column shows only the digest. New DigestFromRepoDigest helper in ImageModel.h performs that reduction and is unit tested directly.
  • The digest is only populated when the flag is set. This matters for json: docker gates on needDigest(ctx), which is ctx.Digest || format.Contains("{{.Digest}}"), and --format json satisfies neither. So image list --format json still reports <none>, and only --digests --format json reports a real digest. An earlier revision of this change leaked the digest into plain json output; the tests below lock the gated behavior in.
  • Column configs were hoisted into named constexpr values so the 5- and 6-column tables cannot drift.

Validation Steps Performed

All new and existing tests in the touched areas were run against a locally deployed build. 25/25 pass.

Unit tests (WSLCCLIImageDigestUnitTests, new file, 3 tests):

  • DigestFromRepoDigest_StripsRepositoryPrefix — plain, fully qualified, and registry-with-port repositories all reduce to the bare sha256: digest. The port case guards the split from being confused by a colon.
  • DigestFromRepoDigest_PassesThroughBareValues — empty, already-bare, and <none> values are untouched.
  • DigestFromRepoDigest_SplitsOnFirstSeparator — splitting happens once, so a digest is never truncated further.

Unit test (WSLCCLICommandUnitTests):

  • ImageListCommand_HasDigestsArgument — both the image list and root-scoped images constructions register --digests as an optional flag with no short alias.

E2E tests (WSLCE2EImageListTests, 22 total, 5 new):

  • Digests_AddsColumnBetweenTagAndImageId — asserts DIGEST sits after TAG and before IMAGE ID, and that the default listing does not gain the column.
  • Digests_ReportsDigestOnlyWhenRequested — json reports <none> without the flag; anything reported with the flag must be a bare sha256: digest with no repo@ prefix.
  • Digests_TableMatchesJson — every digest in json output appears in the table's DIGEST column, so the two renderings cannot drift.
  • Digests_QuietStillOutputsIdsOnly--quiet wins over --digests, emitting bare ids with no header, as docker does.
  • Digests_ListedInHelp--digests and its localized description appear in image list --help.

Note on the test images: they are provisioned via image load from a tarball, so they carry no repo digest and correctly report <none> even with --digests — the same as docker for a tar-loaded image. The digest-reduction logic is therefore covered by the unit tests rather than by asserting a live registry digest in e2e, which would make the suite network dependent.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings August 26, 2026 22:34

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 pull request adds --digests support to wslc image list to match Docker parity, wiring the flag through CLI argument parsing into the image listing service and updating both table and JSON render paths to only surface digests when explicitly requested.

Changes:

  • Added a new --digests flag (no short alias) for image list / image ls / images, and threaded it through ImageService::List.
  • Implemented digest formatting by reducing service “repo@sha256:…” values to Docker’s “sha256:…” form via DigestFromRepoDigest, and conditionally added a DIGEST column in table output.
  • Added unit + E2E tests to lock the gating behavior (digests only appear when --digests is passed) and validate output parity between JSON and table formats.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/windows/wslc/WSLCCLIImageDigestUnitTests.cpp New unit tests for DigestFromRepoDigest behavior.
test/windows/wslc/WSLCCLICommandUnitTests.cpp Verifies image list/images register --digests with no short alias.
test/windows/wslc/e2e/WSLCE2EImageListTests.cpp Adds E2E coverage for DIGEST column placement, gating, help output, and quiet behavior.
src/windows/wslc/tasks/ImageTasks.cpp Threads --digests into listing and conditionally adds DIGEST column + JSON field population.
src/windows/wslc/services/ImageService.h Extends ImageService::List signature with a digests parameter (defaulted).
src/windows/wslc/services/ImageService.cpp Sets the WSLC digest flag and populates ImageInformation::Digest only when requested.
src/windows/wslc/services/ImageModel.h Adds Digest field to the image model and introduces DigestFromRepoDigest.
src/windows/wslc/commands/ImageListCommand.cpp Registers the new ArgType::Digests argument for the list command.
src/windows/wslc/arguments/ArgumentDefinitions.h Defines the Digests argument metadata and localization key mapping.
localization/strings/en-US/Resources.resw Adds localized description string for --digests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread localization/strings/en-US/Resources.resw
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 26, 2026 22:57

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 10 out of 10 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

localization/strings/en-US/Resources.resw:3023

  • Most CLI argument descriptions in Resources.resw do not end with a period; this new string does, which makes help output inconsistent.
    <value>Show image digests.</value>

Comment thread src/windows/wslc/tasks/ImageTasks.cpp

@dkbennett David Bennett (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.

--quiet and --digests are valid options together and Docker CLI allows both. --quiet means only image ids are output, but --digests means that an image with multiple digests can appear multiple times in the output (same is true for tags)

Should add tests here which verify the --quiet and --digests combination (and also images with multiple tags should appear multiple times with --quiet).

This may be a pre-existing issue with quiet that could be addressed here.

Comment thread src/windows/wslc/tasks/ImageTasks.cpp Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 27, 2026 20:05

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 20 out of 20 changed files in this pull request and generated no new comments.

ggarzia-MSFT and others added 2 commits August 27, 2026 13:38
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…olumn

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

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 12 out of 12 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

localization/strings/en-US/Resources.resw:3024

  • Most CLI argument descriptions in Resources.resw omit trailing punctuation (e.g., "Run container in detached mode"). For consistency, drop the trailing period from this new description.
  <data name="WSLCCLI_DigestsArgDescription" xml:space="preserve">
    <value>Show image digests.</value>
  </data>

Comment thread src/windows/wslc/tasks/ImageTasks.cpp
Comment thread test/windows/wslc/e2e/WSLCE2EImageListTests.cpp
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 27, 2026 21:13

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 13 out of 13 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

test/windows/wslc/e2e/WSLCE2EImageListTests.cpp:495

  • This assertion is logically correct, but it reads as a double-negative ("IS_FALSE(find != npos)"). Using an explicit npos comparison makes failures easier to interpret and avoids precedence/clarity pitfalls in future edits.
        VERIFY_IS_FALSE(defaultResult.GetStdoutLines()[0].find(L"DIGEST") != std::wstring::npos);

src/windows/wslcsession/WSLCSession.cpp:1958

  • When WSLCListImagesFlagsDigests is not set, this branch still emits a digest value (it->second.front()) if RepoDigests are present. Since the flag is meant to gate whether digest info is included at all, it would be safer to always emit an empty digest when digests==false (even if the backend unexpectedly provides RepoDigests).
            else if (!digests)
            {
                rows.push_back({&e, tag, it->second.front()});
            }

Comment thread src/windows/wslcsession/WSLCSession.cpp Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 27, 2026 21:38

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 13 out of 13 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

test/windows/wslc/WSLCCLITableOutputUnitTests.cpp:274

  • In this test, namePos/statusPos are used for arithmetic without first verifying the substrings were found. If either find() returns npos, the expected-position math can overflow and the assertion may become misleading.
        const auto namePos = dataLine.find(L"container-a");
        const auto statusPos = dataLine.find(L"running");
        const auto expected = namePos + wcslen(L"container-a") + TableOutput<3>::DefaultColumnPadding + wcslen(L"DIGEST") +
                              TableOutput<3>::DefaultColumnPadding;
        VERIFY_ARE_EQUAL(expected, statusPos);

test/windows/wslc/WSLCCLITableOutputUnitTests.cpp:306

  • This assertion uses namePos in arithmetic without checking that "container-a" (and "running") were actually found. If either find() returns npos, the computed offset can overflow and hide formatting regressions.
        const auto& dataLine = cap.lines()[1];
        const auto namePos = dataLine.find(L"container-a");
        VERIFY_ARE_EQUAL(namePos + wcslen(L"container-a") + TableOutput<3>::DefaultColumnPadding, dataLine.find(L"running"));

localization/strings/en-US/Resources.resw:3024

  • This new argument description includes a trailing period, which is inconsistent with nearby argument descriptions (e.g., "Run container in detached mode") and will show up in --help output.
  <data name="WSLCCLI_DigestsArgDescription" xml:space="preserve">
    <value>Show image digests.</value>
  </data>

Comment thread src/windows/wslcsession/WSLCSession.cpp
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 28, 2026 01: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 13 out of 13 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

test/windows/wslc/e2e/WSLCE2EImageListTests.cpp:549

  • This assertion only checks whether the digest text appears anywhere in the table output. That can pass even if the value appears in a different column (or in multiple places), so it doesn't actually verify the DIGEST column as the comment claims. Consider extracting the DIGEST column slice using the header offsets and comparing against that field.
        // Every digest reported by json output must appear in the table's DIGEST column, so the two
        // renderings cannot drift.
        for (const auto& image : ParseNdjsonOutputAs<ImageOutputInformation>(jsonResult))
        {
            const auto digest = wsl::shared::string::MultiByteToWide(image.Digest);
            VERIFY_IS_TRUE(
                tableResult.StdoutContainsSubstring(digest),
                WEX::Common::String().Format(L"'%ls' was missing from the table DIGEST column", digest.c_str()));
        }

Comment thread localization/strings/en-US/Resources.resw

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 16 out of 16 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

src/windows/wslcsession/WSLCSession.cpp:1981

  • This branch also adds a digest value when digests is false (repoDigests.front()). To keep WSLCListImagesFlagsDigests semantics consistent, avoid populating the digest field unless digests is true (you can still keep the row if you intend to preserve repository-only-by-digest grouping).
            if (!digests)
            {
                rows.push_back({&e, repoName, repoDigests.front()});
            }

Comment thread src/windows/wslcsession/WSLCSession.cpp Outdated
Comment thread localization/strings/en-US/Resources.resw
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 28, 2026 22:21

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 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/windows/wslc/services/ImageModel.h:57

  • DigestFromRepoDigest returns a std::string_view that aliases the input buffer. This is easy to misuse (e.g., passing a temporary std::string and storing the returned view past the full-expression), which can lead to dangling references/UB. Since this helper is in a header and can be reused elsewhere, it would be safer for it to return an owning std::string (or otherwise guarantee lifetime) rather than a view.
inline std::string_view DigestFromRepoDigest(std::string_view repoDigest)
{
    const auto separator = repoDigest.find('@');
    return separator == std::string_view::npos ? repoDigest : repoDigest.substr(separator + 1);
}

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 31, 2026 21:54

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 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

test/windows/wslc/WSLCCLICommandUnitTests.cpp:156

  • This array initializer is very long and likely exceeds the 130-column limit enforced by clang-format (.clang-format ColumnLimit: 130). Splitting the entries across lines will avoid formatting-check failures and improve readability.
        const std::pair<std::wstring, std::vector<Argument>> spellings[] = {
            {L"image list", ImageListCommand(L"image").GetArguments()}, {L"images", ImageListCommand(L"wslc", true).GetArguments()}};

src/windows/wslc/tasks/ImageTasks.cpp:241

  • This braced initializer for the table column definitions is likely over the repo’s 130-column limit (see .clang-format ColumnLimit: 130), which can cause format verification to fail and hurts readability. Please wrap each column definition onto its own line.
        auto table =
            trunc
                ? wsl::windows::wslc::TableOutput<6>(
                      context.Terminal,
                      {{{L"REPOSITORY", c_shrink}, {L"TAG", c_shrink}, {L"DIGEST", c_shrink}, {L"IMAGE ID", c_imageId}, {L"CREATED", c_shrink}, {L"SIZE", c_shrink}}},
                      images.size())

ggarzia-MSFT and others added 3 commits August 31, 2026 15:43
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 September 2, 2026 01:27

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.

🟢 Approval recommended

The functional changes are well-scoped, gated appropriately, and backed by targeted unit/e2e tests; only a small doc-comment clarification was identified.

Review details
  • Files reviewed: 16/16 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread src/windows/wslc/core/TableOutput.h Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 2, 2026 17:57

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.

🟢 Approval recommended

The end-to-end --digests plumbing and gating look correct and well-tested; remaining feedback is limited to minor test robustness/readability nits.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

test/windows/wslc/e2e/WSLCE2EImageListTests.cpp:495

  • This assertion uses a double-negative and indexes GetStdoutLines()[0] without checking the vector is non-empty; capturing the lines once and asserting non-empty makes failures clearer and avoids an out-of-range access if the command ever emits no output.
    test/windows/wslc/WSLCCLITableOutputUnitTests.cpp:277
  • namePos/statusPos are used in arithmetic without first asserting that the substrings were found; if either find() returns npos the failure will be confusing and the expected offset will overflow.
  • Files reviewed: 16/16 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@ggarzia-MSFT
ggarzia-MSFT marked this pull request as ready for review September 2, 2026 20:36
@ggarzia-MSFT
ggarzia-MSFT requested review from a team as code owners September 2, 2026 20:36
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 10, 2026 19:55

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.

🟢 Approval recommended

The change is end-to-end wired with clear gating semantics and is supported by targeted unit and E2E coverage for the new flag and output formatting.

Review details
  • Files reviewed: 16/16 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings September 10, 2026 20: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.

🟢 Approval recommended

The change is well-scoped, preserves default output semantics via explicit gating, and includes strong unit + E2E coverage (only a minor performance nit remains).

Review details
  • Files reviewed: 16/16 changed files
  • Comments generated: 1
  • Review effort level: Lite

};

size_t index = 0;
std::vector<ImageRow> rows;
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.

3 participants