feat(windows): green & blocking e2e, cache-key path-independence fixes, and an MSVC release binary (#201, #202, #203) - #209
Merged
Merged
Conversation
On Windows the binary-inspection step in `inspect_binary` looked for the run artifact at the fixture's suffix-less verify path (`./target/release/foo`), but the real file is `foo.exe`. `multi-dep` and `rust-workspace` therefore failed verify with "binary inspection: artifact not found" even though the binary ran fine (exit 0, correct stdout). The `Path::join` also left mixed `\.`/`/` separators in the reported path. Add `artifact_candidates` (pure, testable) + `resolve_artifact`: tidy the path via `portable_path`, then try `<path>` and `<path><EXE_SUFFIX>` in order, picking the first that exists. No-op on Unix (EXE_SUFFIX is empty).
The compiler-probe memo missed on the warm build on Windows, so the probe re-ran (`probe_runs = 1` where `c-hello`/`c-depinfo` assert `max_probe_runs = 0`). Everything else on that phase passed — the compile was a `local_hit`, the diff byte-identical — because the *main* cache key hashes only a curated env subset (RUSTFLAGS, CARGO_CFG_*), which is stable. The probe key, by contrast, mixes in `env_fingerprint()`, which hashes the *entire* environment. On Windows `std::env::vars()` surfaces cmd.exe's hidden `=`-prefixed pseudo-variables — the per-drive working directory (`=C:`, `=D:`) and the previous child's `=ExitCode`. Those shift between the cold and warm `make` invocations, so the probe key differed across builds and the warm `load()` looked up a record cold never wrote → miss → re-probe. Unix variable names can't contain `=`, so the bug is Windows-only. Filter `=`-prefixed names out of the env fingerprint. Refactored into a pure `fingerprint_env` core so the filtering is unit-testable without mutating the process environment; added regression tests. No-op on Unix.
…arget (#197, #198) Consolidated from PR #200. - Cross-platform fallback wrapper for Windows (#197): replaces the shell `fb.sh`/`fb-sccache.sh` scripts with a portable `e2e-fallback` binary so the rust-fallback / rust-sccache fixtures run on Windows. - Build rust-c-ffi with the windows-gnu target on Windows (#198).
Consolidated from PR #208 (kept open separately). - Per-arm e2e results artifact name `e2e-results-${{ matrix.os }}` (#203): upload-artifact@v4+ makes an artifact name unique per run, so all three matrix arms uploading `e2e-results` collide and `gh run download` returns an arbitrary arm's results. Naming per-arm keeps each results.json separate (this collision masked the real Windows results during #196). - Job- and checkout-level timeouts so a hung step on the persistent self-hosted runners is killed in minutes, not the 6h GitHub default.
The #202 fix only covered `inspect_binary`. The `[diff]` artifact reads (cold/warm baseline at runner.rs and both relocate snapshots) still did a literal `cwd.join(artifact)` / `relocated.join(artifact)`, so on Windows `multi-dep` and `rust-workspace` failed `diff_artifact_present` ("cannot find the file") because the real file is `<name>.exe`. Route all three reads through a new `read_declared_artifact` helper that reuses `resolve_artifact` (try `<path>` then `<path><EXE_SUFFIX>`). No-op on Unix.
rust-c-ffi failed to build on Windows (exit 101): cc-rs's `check_exe`
mangles `CC="<abs>\kache.exe cc"` — `EXE_EXTENSION` is non-empty on
Windows, so `set_extension("exe")` rewrites the final `kache.exe cc`
component to `kache.exe`, which exists, and `env_tool` returns early with
no trailing args. The `cc` subcommand is dropped and kache is invoked as
`kache.exe -O0 …`, which its CLI rejects. On Unix `EXE_EXTENSION` is empty
so cc-rs splits on whitespace and keeps `cc`.
Add `env` support to `[windows]` fixture overrides and point rust-c-ffi's
CC/CXX at MinGW `cc`/`c++` directly there. kache caches nothing for C
(passthrough only), so this loses no caching coverage; the Rust half still
runs through RUSTC_WRAPPER.
The `=`-prefixed env filter did not fix the warm probe re-run on Windows (`c-hello`/`c-depinfo` still `probe_runs=1`). Add temporary `kache::probe` debug logging — every probe_key input (compiler, stat fingerprint, env fingerprint, key_args, resulting key), the memo hit/miss, and the store persist result — so a cold-vs-warm comparison in the CI log pinpoints whether the key is unstable or the store/persist fails. To be removed once root-caused and replaced with the real fix.
The actual #201 root cause (the `=`-env-var filter in 3c9692f was a wrong hypothesis; the diagnostics in 6a9bf12 confirmed it by their absence — no `probe_key` log fired, i.e. `probe_key` returned `None` before logging). `resolve_program` searched `$PATH` for a file literally named `cc`, but on Windows the executable is `cc.exe`, so `dir.join("cc").is_file()` was always false → `resolve_program` returned `None` → `probe_key` returned `None` → the probe was never memoized and re-ran on every build, so the warm phase reported `probe_runs=1` (c-hello / c-depinfo assert `<= 0`). Try each PATH candidate both as written and with `EXE_SUFFIX` appended (and the same for an explicit compiler path). Split the suffix logic into pure `with_exe_suffix` / `find_program_in_dirs` helpers with unit tests. No-op on Unix where `EXE_SUFFIX` is empty. Removes the temporary `kache::probe` diagnostics from 6a9bf12. The volatile-env filter from 3c9692f is retained as complementary hardening (a `=ExitCode` shift would otherwise force needless re-probes).
Surfaces *why* a diff_match/relocate_diff_match artifact diverges — a PE TimeDateStamp shows as a few changed header bytes, an embedded machine-local path shows as readable ASCII — so a Windows diff failure can be triaged from results.json without re-running. Diagnostic aid for the multi-dep/rust-workspace/rust-c-ffi diff_match differences on Windows.
With the artifact-not-found bug fixed, `diff_match` finally ran on the cargo-bin fixtures and exposed benign Windows link nondeterminism: the warm binary is re-linked, and the linker stamps a wall-clock PE `TimeDateStamp` (multi-dep @240, rust-c-ffi @136) and, for MSVC, a random CodeView/RSDS PDB GUID (rust-workspace @107640) — so cold vs warm differ by a few header bytes despite identical code. The byte-window diagnostics confirmed these are header fields, not embedded paths. Make the link reproducible so the differential test is meaningful: `/Brepro` on the MSVC bins (multi-dep, rust-workspace) derives the timestamp and PDB GUID from a content hash; `-Wl,--no-insert-timestamp` zeroes the timestamp on the windows-gnu bin (rust-c-ffi). Set via `[windows].env` RUSTFLAGS; no effect on Unix, where rustc links are already reproducible.
…ate) multi-dep's relocate phase missed on 3 serde crates (max_misses=3): the build-script OUT_DIR absolute path leaked into the cache key, so a relocated build keyed differently and missed. Trace diff confirmed the `env_dep:OUT_DIR` value was `(unchanged)` — the path-normalizer never matched it. Two Windows-specific causes: 1. `canonical_string` returned `\\?\C:\...` (the verbatim prefix `Path::canonicalize` adds on Windows), so every rule prefix was `\\?\`-prefixed and could not substring-match the plain `C:\...` paths cargo emits — no normalization fired at all on Windows (OUT_DIR was `(unchanged)` even in the cold build). Strip the `\\?\` (and `\\?\UNC\`) prefix. 2. The relocate tempdir resolved to an 8.3 short path on the self-hosted runner (`C:\Windows\SERVIC~1\NETWOR~1\...`), so OUT_DIR was short while the (now `\\?\`-stripped) rule was long → still no match. Build the relocate copy under the canonicalized long-form path so OUT_DIR is in the form the normalizer expects. With both, OUT_DIR normalizes to `<WORKSPACE>\target\...\out` in the cold and relocate builds alike, so the relocated build hits. No-op on Unix (canonicalize adds no verbatim prefix and there are no 8.3 names).
The real reason multi-dep's relocate kept missing on the serde crates: rustc escapes `# env-dep:` values (`\`→`\\`, newline→`\n`, CR→`\r`) but `parse_env_dep_info` stored them verbatim. On Windows OUT_DIR therefore arrived doubled (`C:\\proj\\...\\out`), which never matched the path-normalizer's single-backslash rules — so OUT_DIR's absolute path leaked into the key (trace showed `env_dep:OUT_DIR=... (unchanged)`), and a relocated build keyed differently and missed. Unescape env-dep values when parsing dep-info. Combined with the `\\?\`-prefix strip and the long-form relocate dir (prior commits), the OUT_DIR value now reduces to `<WORKSPACE>\target\...\out` identically in the cold and relocate builds, so the relocate hits. No-op on Unix.
The last multi-dep relocate miss: with OUT_DIR now normalized, serde_core and serde still keyed differently across a relocate because their build-script-generated `out/private.rs` (identical content) was hashed at a different POSITION. Source files are hashed in absolute-path order, and the generated file under OUT_DIR sorts among the registry sources differently once the tree moves (`C:\Windows\...\tmp\...` sorts before `C:\actions-runner\...`), flipping the update order — so the key changed (trace: same file hash at index 19 cold vs index 1 relocate). Only the content hash enters the key (never the path), so hash the source files in content-hash order. The order now depends solely on contents, making it path-independent. No-op semantically on Unix; reorders keys once (cache rebuild), never a wrong hit.
Restores the full-fixture e2e run and `KACHE_LOG: kache=debug` after the #201 relocate root-cause was diagnosed and fixed.
…, fix comment Addresses the multi-agent regression review of this PR: - cache_key: bump CACHE_KEY_VERSION 9 -> 10. The content-order source hashing (and env-dep unescaping) change key bytes on ALL platforms, not just Windows; without a version bump that's a silent partial cache invalidation on Linux/macOS. Per the in-file convention, bump to invalidate prior entries cleanly. (review finding: cross-platform) - runner: gate the relocate-dir canonicalize to Windows only. The canonicalize is needed solely to expand 8.3 short names on Windows; on Unix it needlessly resolved the macOS `/tmp` -> `/private/tmp` symlink at the relocate cwd, which can perturb rustup toolchain resolution. Restores exact pre-PR Unix relocate behavior. (review finding #4) - runner: document that the `strings` binary-leak inspection is a Unix/binutils-only lens and that `relocate_diff_match` is the real cross-platform guard. (A raw-byte scan was tried to make it fire on Windows but false-positived on the toolchain's embedded std-source path, so it was reverted.) (review finding: false-green, mitigated) - rust-c-ffi fixture: correct the stale comment — kache DOES cache single-source `-c` C compiles (src/compiler/cc.rs), so the Windows CC=cc bypass drops real (narrow) C-caching coverage; the dedicated c-hello/c-depinfo/cpp-hello fixtures still cover C-through-kache on Windows. (review finding: coverage) No wrong-hit (cache-correctness) issues were found by the review.
…e on Windows Closes the coverage gap the review flagged: rust-c-ffi previously bypassed kache for its build.rs C compile on Windows (CC=cc), because cc-rs's `check_exe` mangles a multi-token `"<path>\kache.exe cc"` value (drops the `cc` subcommand). kache DOES cache single-source `-c` C compiles, so the bypass lost real coverage of the mixed-language (cc-crate) path on Windows. Add single-token shim binaries `kache-cc` / `kache-cxx` (built alongside the harness) that cc-rs accepts unmangled and that forward to `kache cc` / `kache c++` — locating the real kache as a sibling binary, so no env/path plumbing and survives relocation. The harness exposes them as `$KACHE_CC` / `$KACHE_CXX` (derived from `$KACHE`'s dir; replaced before `$KACHE` to avoid the prefix-collision), and rust-c-ffi's [windows] override now uses them instead of bare cc/c++. Verified locally: `kache-cc -c x.c -o x.o` forwards to `kache cc`, compiles, and a second run is a `cc local cache hit`. Unit test pins the token-ordering. No effect on Unix (rust-c-ffi keeps `$KACHE cc` there).
…eral) The shim wiring failed on Windows with `failed to find tool "$KACHE_CC"`: `load` ran the `$KACHE`/`$KACHE_CC` expansion pass over the base env *before* `apply_windows_overrides` merged the `[windows].env` values, so the override's `CC=$KACHE_CC` was never expanded and reached cc-rs literally. (It worked while the override was `CC=cc`, which has no token.) Apply the Windows overrides before the expansion pass so the merged override values are token-expanded too. Update the now-stale comments that claimed `[windows].env` values are taken literally.
Every fixture now passes on the Windows e2e arm (#196/#197/#198 port + #201/#202 harness fixes), so it no longer needs `continue-on-error`. Drop it: the `e2e` matrix job — and thus the tag-push `release` job that `needs` it — now fails if the Windows arm fails, like Linux/macOS. Add "E2E smoke (Windows)" to the crates publish gate (`publish-crates.yaml` → `require-ci-green.sh`) so a manual `release: published` also waits for the Windows arm to be green before publishing to crates.io. (require-ci-green reads the job's real conclusion, which only became meaningful once continue-on-error was removed.) service-image.yml is intentionally left gating on `Check (Linux)` only — the image ships kache-service (the server), not the e2e client.
…support Now that the Windows e2e arm is green and blocking, add Windows to the release outputs: - ci.yml release: add `x86_64-pc-windows-msvc` to targets and pass `runner_windows: [self-hosted, Windows, X64, kunobi-windows]` so it builds NATIVELY on the same runner the e2e already uses (cross/MinGW can't build msvc, and kache's aws-lc TLS doesn't cross-compile to windows-gnu). Requires the `runner_windows` input from Zondax/_workflows#93 on @main — merge that first. - docs/getting-started/installation.mdx: drop the stale "Windows is not supported and fails compilation explicitly" line (kache builds & passes e2e on Windows now), note source builds work on Windows (MSVC), and add the x86_64-pc-windows-msvc row to the supported-platforms table.
Was `@main`, which would track every future (incl. breaking) change to the shared `_release-rust.yml`. The `v9` floating major tag was advanced to the commit carrying the `runner_windows` input (Zondax/_workflows#93), so `@v9` has everything kache's Windows release needs while shielding it from breaking changes on main.
emmanuelm41
changed the title
This was referenced Jun 2, 2026
This was referenced Jun 2, 2026
Closed
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings Windows from a non-blocking, mostly-red e2e arm to fully green, blocking, and shipping a release binary — and fixes the kache-core cache-key bugs that the Windows e2e surfaced along the way.
Consolidates the remaining Windows e2e work (supersedes #200, #204, #205; folds in #208) and closes the last fixture failures (#201, #202), the artifact-collision issue (#203), and the fixture ports (#197, #198). Part of the Windows umbrella (#82 / #45).
Highlights
continue-on-error).CACHE_KEY_VERSIONbumped 9 → 10.x86_64-pc-windows-msvcrelease binary, and the crates publish gates on the Windows arm.What changed
Harness & fixtures (Windows e2e green)
runner.rsresolves the run artifact with the platform exe suffix in bothinspect_binaryand the[diff]artifact reads (<path>then<path>.exe), viaportable_path. No-op on Unix.e2e-fallbackbinary replacing the.shwrappers; rust-c-ffi builds on Windows.kache-cc/kache-cxxshims (cc-rs'scheck_exemangles a multi-tokenCC), exposed as$KACHE_CC/$KACHE_CXX;[windows].envoverrides are now token-expanded./Breprofor MSVC,-Wl,--no-insert-timestampfor gnu) so the cold-vs-warm byte compare is meaningful; plus a hex/ascii diff-window for triage.kache-core cache key (the deep one — #201 relocate)
The probe never memoized on Windows, and a relocated build missed — both path-leak / path-form bugs:
probe/cache.rs—resolve_programsearches$PATHtrying the exe suffix (was looking forcc, notcc.exe), so the probe is memoized again; also drops volatile=-prefixed Windows pseudo-env from the probe key.path_normalizer.rs— strip the\\?\verbatim prefix so rule prefixes match the plain paths cargo/rustc emit (no normalization fired at all on Windows otherwise).cache_key.rs— unescape rustc's# env-dep:values (doubled backslashes on Windows keptOUT_DIRfrom normalizing) and hash source files in content order, not path order (a build-script-generated file underOUT_DIRsorted differently when the tree moved, leaking path-order into the key).CACHE_KEY_VERSION9 → 10.CI / release
ci.yml— Windows e2e is blocking (removedcontinue-on-error); per-arm artifact names + job/checkout timeouts (CI: e2e matrix arms collide on artifact namee2e-results#203/ci: e2e reliability — per-arm artifact names (#203) + job timeouts #208).publish-crates.yaml— the crates.io gate now awaitsE2E smoke (Windows).ci.ymlrelease — shipsx86_64-pc-windows-msvc, built natively on thekunobi-windowsrunner via the newrunner_windowsinput, pinned to_release-rust.yml@v9(see feat(release-rust): opt-in native Windows runner for real -msvc builds Zondax/_workflows#93). MSVC, because cross/MinGW can't build it and kache's aws-lc TLS doesn't cross-compile to windows-gnu.docs/getting-started/installation.mdx— dropped the stale "Windows is not supported" line; added the MSVC platform row.Validation
cache_key/probe trace, not guessed.CACHE_KEY_VERSIONbump, gating the relocate canonicalize to Windows, and reverting a risky byte-scan.Notes
CACHE_KEY_VERSION9 → 10 invalidates prior on-disk entries once, on every platform (the content-order hashing changes keys everywhere) — by design, never a wrong hit.v*tag; every building block (native MSVC build, runner, matrix logic) is otherwise validated. Requires feat(release-rust): opt-in native Windows runner for real -msvc builds Zondax/_workflows#93 — merged, andv9advanced to include it.