Dockerfile.windows: harden the xwin step (serialize splat, add --http-retry) - #377
Conversation
…race
xwin processes payloads on a rayon pool. With --arch x86_64,aarch64 there
are two SdkLibs MSIs and each, on completion, creates the same
sdk/lib/<version> -> . compatibility symlink guarded only by an
.exists() check (src/splat.rs:782 in xwin 0.9.0). When both finish close
together the second one fails with EEXIST:
Error: failed to splat Win11SDK_10.0.26100_libs_x86_64.msi
Caused by:
0: unable to symlink from /winsdk/sdk/lib/10.0.26100 to .
1: File exists (os error 17)
Run `xwin unpack` first on all cores (download + extract, the expensive
part) and then `xwin splat` with RAYON_NUM_THREADS=1 so the symlink
creation is serial. splat reuses the warmed download and unpack caches,
so the serial pass adds only a few seconds.
--http-retry defaults to 0, so a single dropped read from
download.visualstudio.microsoft.com fails the whole layer build
(seen in run 29997282267 on main):
Error: failed to retrieve https://download.visualstudio.microsoft.com/.../78fa3c824c2c48bd4a49ab5969adaaf7.cab
after 1 tries due to I/O failures reading the response body
robobun
changed the title
WalkthroughChangesThe Windows Docker image now unpacks xwin payloads before running single-threaded Windows cross-compilation
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — targeted CI hardening for two documented intermittent failures in the Windows cross-build base image.
What was reviewed:
- Verified against xwin 0.9.0 source:
--http-retryis the correct flag name (clap#[arg(long)] http_retry: u8), andunpack/splatare both valid subcommands. RAYON_NUM_THREADS=1on the splat pass serializes the racing per-arch symlink creation; the shared--cache-dir /xwin-cachelets splat reuse the parallel unpack's output.- Existing post-splat sanity checks (
test -f /winsdk/...) andrm -rf /xwin-cachecleanup are preserved.
Extended reasoning...
Overview
This PR modifies a single RUN step in Dockerfile.windows — the layer that uses xwin to download and lay out the MSVC CRT + Windows SDK for cross-compilation. It splits one xwin splat invocation into a two-phase xwin unpack (full parallelism) followed by xwin splat with RAYON_NUM_THREADS=1, and adds --http-retry 5 to both. A comment block documents the rationale. No other files are touched.
Security risks
None. This is CI/build infrastructure for a Docker base image. The xwin binary is still fetched from the same pinned release with the same SHA256 checksum, the CRT/SDK versions are unchanged, and the post-splat test -f assertions still verify the expected output layout. No new network endpoints, no credential handling, no changes to what gets built or shipped.
Level of scrutiny
Low. This is a mechanical CI-reliability fix in a Dockerfile, not runtime code. The two failure modes it addresses are cited with links to the actual failing CI runs and to the exact lines in xwin's source (ctx.rs rayon pool, splat.rs unguarded symlink) that cause the race. I independently verified the flag/subcommand names against the xwin 0.9.0 source: --http-retry (not --http-retries as the error message's prose says) matches the #[arg(long)] http_retry definition, and Unpack is a real subcommand. If anything here were wrong the layer would fail deterministically on every build, not intermittently.
Other factors
The approach is sound: unpack does the network- and CPU-bound work in parallel and warms --cache-dir /xwin-cache; the serial splat then reuses that cache for cheap file moves + symlinks, avoiding the TOCTOU on the sdk/lib/<version> symlink when both arches finish together. The alternative approaches the description rules out (whole-command retry, pre-creating the symlink) are correctly rejected with reasons. The rm -rf /xwin-cache cleanup and all downstream sanity checks are unchanged. No prior reviews or comments on the PR.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Dockerfile.windows`:
- Around line 96-102: Add regression assertions after the xwin splat step and
before removing /xwin-cache, using test -L to verify
/winsdk/sdk/lib/${WIN_SDK_VERSION} and /winsdk/sdk/include/${WIN_SDK_VERSION}
are symbolic links. Keep the existing checks unchanged and make the Docker build
fail if either versioned compatibility link is missing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 282eab4f-8ef2-4dad-813d-55778be0e32a
📒 Files selected for processing (1)
Dockerfile.windows
| RUN xwin --accept-license --arch x86_64,aarch64 --cache-dir /xwin-cache --http-retry 5 \ | ||
| --crt-version ${MSVC_CRT_VERSION} --sdk-version ${WIN_SDK_VERSION} \ | ||
| unpack && \ | ||
| RAYON_NUM_THREADS=1 \ | ||
| xwin --accept-license --arch x86_64,aarch64 --cache-dir /xwin-cache --http-retry 5 \ | ||
| --crt-version ${MSVC_CRT_VERSION} --sdk-version ${WIN_SDK_VERSION} \ | ||
| splat --preserve-ms-arch-notation --include-debug-libs --output /winsdk && \ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Add regression checks for the compatibility symlinks.
The checks on Lines 104-106 do not verify the versioned SDK links that caused the race. These checks can pass while /winsdk/sdk/lib/${WIN_SDK_VERSION} or /winsdk/sdk/include/${WIN_SDK_VERSION} is missing. Add test -L assertions before removing /xwin-cache. The pinned xwin 0.9.0 implementation creates these links during per-payload splat work. (raw.githubusercontent.com)
Proposed checks
splat --preserve-ms-arch-notation --include-debug-libs --output /winsdk && \
+ test -L "/winsdk/sdk/lib/${WIN_SDK_VERSION}" && \
+ test -L "/winsdk/sdk/include/${WIN_SDK_VERSION}" && \
rm -rf /xwin-cache && \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RUN xwin --accept-license --arch x86_64,aarch64 --cache-dir /xwin-cache --http-retry 5 \ | |
| --crt-version ${MSVC_CRT_VERSION} --sdk-version ${WIN_SDK_VERSION} \ | |
| unpack && \ | |
| RAYON_NUM_THREADS=1 \ | |
| xwin --accept-license --arch x86_64,aarch64 --cache-dir /xwin-cache --http-retry 5 \ | |
| --crt-version ${MSVC_CRT_VERSION} --sdk-version ${WIN_SDK_VERSION} \ | |
| splat --preserve-ms-arch-notation --include-debug-libs --output /winsdk && \ | |
| RUN xwin --accept-license --arch x86_64,aarch64 --cache-dir /xwin-cache --http-retry 5 \ | |
| --crt-version ${MSVC_CRT_VERSION} --sdk-version ${WIN_SDK_VERSION} \ | |
| unpack && \ | |
| RAYON_NUM_THREADS=1 \ | |
| xwin --accept-license --arch x86_64,aarch64 --cache-dir /xwin-cache --http-retry 5 \ | |
| --crt-version ${MSVC_CRT_VERSION} --sdk-version ${WIN_SDK_VERSION} \ | |
| splat --preserve-ms-arch-notation --include-debug-libs --output /winsdk && \ | |
| test -L "/winsdk/sdk/lib/${WIN_SDK_VERSION}" && \ | |
| test -L "/winsdk/sdk/include/${WIN_SDK_VERSION}" && \ |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Dockerfile.windows` around lines 96 - 102, Add regression assertions after
the xwin splat step and before removing /xwin-cache, using test -L to verify
/winsdk/sdk/lib/${WIN_SDK_VERSION} and /winsdk/sdk/include/${WIN_SDK_VERSION}
are symbolic links. Keep the existing checks unchanged and make the Docker build
fail if either versioned compatibility link is missing.
There was a problem hiding this comment.
LGTM — targeted CI reliability fix for the Windows cross-compile base image.
What was reviewed:
- The two-phase
unpack→RAYON_NUM_THREADS=1 splatsplit preserves all existing flags (--arch,--cache-dir, versions,--preserve-ms-arch-notation,--include-debug-libs,--output) and the downstreamtest -f/ PE-link sanity checks. --http-retry 5is a global xwin option applied to both invocations; verified it matches the flag name in the observed error message.- Confirmed
Dockerfile.windowsis not covered by CODEOWNERS and the change touches no runtime code.
Extended reasoning...
Overview
This PR modifies a single RUN step in Dockerfile.windows (the Windows cross-compile base image). It splits the existing xwin ... splat invocation into a parallel xwin unpack pass followed by a serialized (RAYON_NUM_THREADS=1) xwin splat pass, and adds --http-retry 5 to both. A detailed comment block documents the upstream xwin race (two per-arch SdkLibs payloads racing on the same sdk/lib/<version> -> . symlink) and the transient CDN download failure that motivated the retry flag. All other flags, output paths, and the trailing test -f sanity checks are unchanged.
Security risks
None. This is CI/build tooling only — no runtime code, no auth, no network-facing surfaces beyond the existing Microsoft CDN download that was already there. The added --http-retry flag only affects retry count for that download.
Level of scrutiny
Low. This is a mechanical hardening of a Docker build step to address two observed intermittent CI failures (linked in the PR description with concrete run IDs and xwin source references). The output layout is validated by the existing test -f /winsdk/sdk/include/um/windows.h and libcmt.lib checks plus the subsequent "compile + link a PE executable for both arches" smoke test, so a regression in the splat output would fail the image build immediately rather than silently. Dockerfile.windows is not covered by any CODEOWNERS entry.
Other factors
The only outstanding review comment is a CodeRabbit nitpick suggesting extra test -L assertions on the versioned compatibility symlinks. Those symlinks are not consumed by this build (all downstream references use /winsdk/sdk/lib/um/<arch> etc. directly), so the suggestion is optional and does not block. The bug-hunting system found no issues. The fix is well-researched, minimal, and strictly improves reliability over the previous single-invocation form.
Preview Builds
|
2 participants
Two intermittent failures of the
windows-crossbase-image layer seen onmain, both inDockerfile.windowsstep[base 6/14].1. Multi-arch splat symlink race
bun-webkit-windows-amd64in run 30514534688 (currentmainHEAD):The other four
windows-crossmatrix entries ran the same layer without cache in the same run and passed.xwinprocesses payloads on a rayon pool (payloads.into_par_iter()inctx.rs). With--arch x86_64,aarch64there are twoSdkLibsMSIs, and each, on completion, creates the samesdk/lib/<version> -> .compatibility symlink guarded only by an.exists()check (splat.rs:782-784). When both arches finish close together both see!exists()and both callsymlink(); the second loses withEEXIST. The same pattern exists forsdk/include/<version>. Still present on xwinmain(0.9.0 is latest).Fix: split the step into
xwin unpackwith full parallelism (download + extract, the network- and CPU-bound part) followed byxwin splatwithRAYON_NUM_THREADS=1so the per-payload symlink creation is serial.splatreuses the warmed download (get_and_validatecheckscache_path.exists()) and unpack (prep_unpackreads the.unpackmarker) caches, so the serial pass is just file moves and symlinks and adds only a few seconds to a layer that took ~15-22s before. Retrying the whole command is not deterministic, and pre-creating the symlink beforexwinruns doesn't work becauseprep_splatwipessdk/on entry.2. Transient download I/O failure
bun-webkit-windows-amd64in run 29997282267 (2026-07-23):--http-retrydefaults to 0.Fix: pass
--http-retry 5on both invocations.