Made wslc build output more detailed

[craigloewen-msft]

Summary of the Pull Request

Changed wslc build to give more detailed build output.

PR Checklist

• Closes:
• 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

Summary of changes:

• Change WSLCSession to send over the completed JSON bits
• BuildImageCallback then takes in these bits, and parses them with a 'BuildView' class
• This 'BuildView' object stores everything needed to do the rendering
• BuildImageCallback then has a thread that renders every few ms to the console
• Rendering is done by just appending the output, but once we hit the max viewport height we switch to a 'fixed view' mode which renders just to the screen size.
• On destruction of that BuildImageCallback we print out the last result and any errors.

Validation Steps Performed

Tested with various Containerfiles some simple and some complex. Here is a sufficiently complex one that you can use to test yourself:

View file

This containerfile does multi target builds that are copying from eachother and then finally gives an error right at the end.

FROM alpine:latest AS builder
RUN apk add --no-cache gcc musl-dev
COPY hello.c /src/hello.c
RUN gcc -O2 -static -o /src/hello /src/hello.c && strip /src/hello


FROM python:3.12 AS assets
RUN pip install --no-cache-dir jinja2 markdown numpy matplotlib
COPY generate_page.py /work/generate_page.py
COPY template.html /work/template.html
COPY content.md /work/content.md
RUN python /work/generate_page.py

FROM python:3.13 AS systools
RUN apt-get update && apt-get install -y --no-install-recommends \
    curl ca-certificates jq file \
    && rm -rf /var/lib/apt/lists/*
RUN echo "System tools layer ready"

FROM alpine:latest AS final
RUN apk add --no-cache tini libstdc++

# Copy the compiled binary from builder
COPY --from=builder /src/hello /usr/local/bin/hello

# Copy generated HTML from assets stage
COPY --from=assets /work/output/index.html /var/www/index.html

# Copy a utility from systools
COPY --from=systools /usr/bin/jq /usr/local/bin/jq

RUN echho 123

# Add a health-check script
RUN echo '#!/bin/sh' > /healthcheck.sh \
    && echo '/usr/local/bin/hello && echo "OK"' >> /healthcheck.sh \
    && chmod +x /healthcheck.sh


EXPOSE 8080
ENTRYPOINT ["/sbin/tini", "--"]
CMD ["/usr/local/bin/hello"]

I notice that we don't get all the data coming in - so we don't get signals of every layer extraction completion. I've left the PR as is which just does best effort on showing it.
I also didn't program in support for changing the Terminal Window while it's running, which I think is OK.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[ptrivedi]

nice!

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (8)

src/windows/wslc/services/BuildView.cpp:231

• [internal] BuildKit vertexes (load build context, parse Dockerfile, etc.) are now always rendered as their own targets/steps. The previous implementation skipped these unless --verbose was set. With the rewrite, every build will show "[internal]" stages in the live view, which clutters the output and is inconsistent with the verbose flag's intent. Consider filtering targets whose name begins with [internal] unless verbose is enabled.

    for (const auto& vertex : msg.vertexes)
    {
        size_t targetIdx, stepIdx;

        auto it = m_digestIndex.find(vertex.digest);
        if (it != m_digestIndex.end())
        {
            targetIdx = it->second.first;
            stepIdx = it->second.second;
        }
        else
        {
            auto parsed = ParseTarget(vertex.name);

            auto targetIt = m_targetNameToIdx.find(parsed.targetName);
            if (targetIt != m_targetNameToIdx.end())
            {
                targetIdx = targetIt->second;
            }
            else
            {
                targetIdx = m_targets.size();
                m_targets.push_back(ViewTarget{parsed.targetName, {}});
                m_targetNameToIdx[parsed.targetName] = targetIdx;
            }

            ViewStep newStep{};
            newStep.digest = vertex.digest;
            newStep.stepLabel = parsed.stepLabel;
            newStep.stepNum = parsed.stepNum;
            newStep.stepTotal = parsed.stepTotal;
            newStep.inputs = vertex.inputs;

            stepIdx = m_targets[targetIdx].steps.size();
            m_targets[targetIdx].steps.push_back(std::move(newStep));
            m_digestIndex[vertex.digest] = {targetIdx, stepIdx};

            if (std::find(targetsToSort.begin(), targetsToSort.end(), targetIdx) == targetsToSort.end())
            {
                targetsToSort.push_back(targetIdx);
            }
        }

src/windows/wslc/services/BuildImageCallback.cpp:219

• Malformed BuildKit JSON is silently swallowed here, which makes it very hard to diagnose issues if BuildKit ever changes its wire format or sends an unexpected line: the user simply sees a blank/stale live view with no indication that messages are being dropped. At minimum log the parse failure via LOG_CAUGHT_EXCEPTION() so it shows up in telemetry/traces, rather than discarding the exception with no record.

        catch (...)
        {
            // Ignore malformed JSON
        }

src/windows/wslc/services/BuildImageCallback.cpp:203

• The protocol between server (WSLCSession) and client (BuildImageCallback) is now an implicit contract: the server sends raw BuildKit JSON with id "buildkit", and any other id (or status without the "buildkit" id) is silently dropped on the client. This is brittle — there is no central definition of the "buildkit" sentinel shared by both sides. Consider exposing a single shared constant (in a header included by both WSLCSession.cpp and BuildImageCallback.cpp) and comparing against it on both ends to avoid future drift.

    // BuildKit JSON messages are identified by the "buildkit" id
    if (idStr == "buildkit")

src/windows/wslc/services/BuildImageCallback.cpp:475

• m_viewportHeight is sampled on the first render and never updated. If the user resizes the terminal after the first frame (especially shrinking it), the renderer will keep using the original height, which can produce corrupted output: in "fixed" mode it will write more lines than fit in the current viewport (each terminated with \033[K\n) and the screen-clear logic in the destructor (\033[H\033[J vs. cursor-up N) will be based on the wrong height. The author notes terminal resize is not supported, but the live build can run for many minutes so resize is realistic; at minimum re-snapshot srWindow.Bottom - srWindow.Top + 1 each frame instead of only on the first one.

    // Snapshot viewport height on first render.
    if (m_viewportHeight == 0)
    {
        m_viewportHeight = info.srWindow.Bottom - info.srWindow.Top + 1;
    }

src/windows/wslc/services/BuildView.cpp:374

• Logs are trimmed of leading whitespace (line 352-353) before being split into lines. For build steps such as npm or pip install output, leading indentation is often meaningful (tree-style output, nested package listings) and stripping it makes the live log peek much harder to read. Consider trimming only trailing whitespace from the whole blob and per-line trailing whitespace, but preserving leading indentation on each line.

        // Trim trailing whitespace.
        auto endPos = text.find_last_not_of(" \n\r\t");
        text = (endPos != std::string::npos) ? text.substr(0, endPos + 1) : "";

        // Trim leading whitespace.
        auto startPos = text.find_first_not_of(" \n\r\t");
        text = (startPos != std::string::npos) ? text.substr(startPos) : "";

        if (text.empty())
        {
            continue;
        }

        // Split on newlines
        std::istringstream stream(text);
        std::string line;
        while (std::getline(stream, line))
        {
            // Trim each sub-line
            auto lineEnd = line.find_last_not_of(" \r\t");
            line = (lineEnd != std::string::npos) ? line.substr(0, lineEnd + 1) : "";
            auto lineStart = line.find_first_not_of(" \r\t");
            line = (lineStart != std::string::npos) ? line.substr(lineStart) : "";
            if (!line.empty())
            {
                step.logOutput.push_back(std::move(line));
            }
        }

src/windows/wslc/services/BuildView.cpp:374

• step.logOutput is appended to without bound for the entire duration of a build step. For long-running steps that print verbose output (apt-get, pip install, compilation), this can grow to many megabytes per step and balloons memory usage; the previous implementation had c_maxAllLinesBytes = 10 MiB cap for exactly this reason. Consider keeping only the last N lines (e.g. the most recent few hundred) per step, since only the tail is ever displayed (c_maxLogPeekLines) and the final replay also doesn't need unbounded history.

            if (!line.empty())
            {
                step.logOutput.push_back(std::move(line));
            }
        }

src/windows/wslc/services/BuildImageCallback.cpp:84

• The destructor calls THROW_IF_WIN32_BOOL_FALSE(GetConsoleScreenBufferInfo(...)) and then performs significant work (BuildFrameLines, formatting, terminal writes). If the destructor is invoked during stack unwinding from a build error, std::uncaught_exceptions() > m_uncaughtExceptions is true and the error-replay path executes. A throw from GetConsoleScreenBufferInfo would be caught by the function-try CATCH_LOG(), but it would also short-circuit the entire error-replay block, so the user never sees the final build state or error log. Consider using LOG_IF_WIN32_BOOL_FALSE and falling back to a default width instead of throwing, so error replay still runs even if the console handle has been redirected/closed.

        CONSOLE_SCREEN_BUFFER_INFO info{};
        THROW_IF_WIN32_BOOL_FALSE(GetConsoleScreenBufferInfo(m_console, &info));
        const SHORT consoleWidth = std::max<SHORT>(40, info.srWindow.Right - info.srWindow.Left);

src/windows/wslc/services/BuildImageCallback.cpp:519

• In fixed mode, tailStart = allLines.size() - tailCount is computed without verifying that tailCount <= allLines.size(). The enclosing branch guards totalLines >= m_viewportHeight and tailCount = m_viewportHeight - 1, which keeps the subtraction safe today. However, m_viewportHeight could in principle be 0 (it's a snapshot of Bottom - Top + 1, which is normally >= 1 but the field is declared SHORT with no explicit guard). The earlier check if (m_viewportHeight == 0) sets it only when zero, so a degenerate console where the snapshot returns 0 would result in tailCount underflowing to SIZE_MAX. Consider clamping m_viewportHeight to at least 2 after the snapshot.

    // Snapshot viewport height on first render.
    if (m_viewportHeight == 0)
    {
        m_viewportHeight = info.srWindow.Bottom - info.srWindow.Top + 1;
    }

    auto allLines = BuildFrameLines(consoleWidth);
    auto totalLines = static_cast<SHORT>(allLines.size());

    std::wstring buffer = L"\033[?25l"; // Hide cursor during redraw.

    if (totalLines < m_viewportHeight)
    {
        // Append mode: content fits in viewport.
        if (m_linesWrittenLastFrame > 0)
        {
            buffer += std::format(L"\033[{}A\r", m_linesWrittenLastFrame);
        }

        for (const auto& line : allLines)
        {
            buffer += line;
            buffer += L"\033[K\n";
        }
        buffer += L"\033[J";

        m_linesWrittenLastFrame = totalLines;
    }
    else
    {
        // Fixed mode: fill the entire viewport.
        if (m_linesWrittenLastFrame >= m_viewportHeight)
        {
            // Already in fixed mode: cursor home to viewport top.
            buffer += L"\033[H";
        }
        else if (m_linesWrittenLastFrame > 0)
        {
            // Transition from append mode: go back to start of old frame.
            buffer += std::format(L"\033[{}A\r", m_linesWrittenLastFrame);
        }

        // Header line
        buffer += allLines[0];
        buffer += L"\033[K\n";

        // Tail: fill the rest of the viewport with the most recent content.
        auto tailCount = static_cast<size_t>(m_viewportHeight - 1);
        auto tailStart = allLines.size() - tailCount;

[craigloewen-msft]

Do we want to keep the --verbose option? Not sure what it would do given this new implementation.

[dkbennett]

Output looks good, I'm not entirely sure what it should look like, but I'd just normally ask Craig if it is good, and since this is Craig's PR I'm assuming it is correct. :)

[benhillis]

Hi! This PR is approved but has merge conflicts with master. Could you rebase when you have a chance so we can land it? Thanks!

[kvega005]

Just a thought, have you considered StrFormatByteSizeEx ? It doesn't exactly match the docker style format, so I understand why you might not choose it. But it is more consistent with other windows components, and it has built-in localization for the decimal seperator.

[kvega005]

This is computed twice here and I think once in TableOutput::GetConsoleWidth(), we could probably create a hellper GetConsoleWidth that can be used in this file and in TableOutput.

[kvega005]

Minor: each std::format(...) here allocates a temporary std::wstring that's immediately copied into buffer. Consider std::format_to(std::back_inserter(buffer), L"...", ...) to write directly into the buffer and skip the intermediate allocation. Same pattern shows up in RenderFrame() and BuildFrameLines() if you want to apply it more broadly — not blocking, just a small perf/cleanliness nit.

[kvega005]

This pattern is repeated quite a bit. Would be nice to add a TruncateWithEllipsis helper like:

static void TruncateWithEllipsis(std::wstring& s, size_t maxWidth)
{
    if (s.size() > maxWidth && maxWidth > 3)
    {
        s.resize(maxWidth - 3);
        s += L"...";
    }
}

[kvega005]

std::format supports dynamic width and alignment: {:<{}} (left-pad) and {:>{}} (right-pad). Several spots build a std::wstring(pad, L' ') just to splice in — could be folded into the format string. E.g. the header line:
lines.push_back(std::format(L"\033[2m{:>{}}\033[0m", label, consoleWidth));

Same applies to FormatStepLine and AppendCachedFromLines.

[kvega005]

nit: std::ranges::any_of(target.steps, [](const auto& s) { return s.started || s.completed; })

[kvega005]

The string is searched twice. Once in the if condition, once to get pos. The second find is pure waste — its result is guaranteed to equal the first. Do one find, then test its result. Also the + 6 is a magic number. It's "skip past ] FROM" (6 characters). If someone changes the search string to "] FROM:" or "] FROM " (extra space), they have to remember to update the 6 — and the compiler won't catch the mistake.

auto* inputStep = m_buildView.StepByDigest(digest);
if (inputStep)
{
    auto pos = inputStep->stepLabel.find(c_fromMarker);
    if (pos != std::string::npos)
    {
        fromImage = inputStep->stepLabel.substr(pos + c_fromMarker.size());
        break;
    }
}

[kvega005]

This pattern is repeated quite a bit. There is a StripLeadingWhitespace in wsl::windows::common::string, I think adding a StripWhitespace or Trim() is fitting.

[kvega005]

Or just add a StripTrailingWhiteSpace and do both.

[kvega005]

Erasing from the front of std::vector is O(n) — and you do it on every overflow log line, so writing N lines into a step is O(N · c_maxLogLinesPerStep). I think you can switch logOutput to std::dequestd::stringstd::string — pop_front() is O(1).
Realistically, the perf hit is probably not very big, so it shouldn't be a huge issue, but its worth considering.

[kvega005]

Nit: magic number here. use constexpr std::string_view prefix = "sha256:"; and prefix.size() or strlen.

[kvega005]

Viewport height is captured once on the first render and never updated. If the user resizes their terminal mid-build (especially shrinks it), the fixed-mode layout will render past the visible area or leave stale content. Width is re-read every frame, so this is the only resize blind spot. Probably not worth fixing now — just worth a comment here noting the limitation, or re-querying m_viewportHeight each frame.

[kvega005]

Nit: Might make sense to add unit tests for BuildView::ProcessMessage. See test/wslc/*UnitTests.cpp

[kvega005]

I don't see any callers for this. Did I miss something? If its not being used, best to remove.