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. :)