fix(e2e): Windows fixture ports — fallback wrapper + rust-c-ffi gnu target (#197, #198) - #200
Closed
emmanuelm41 wants to merge 2 commits into
Closed
fix(e2e): Windows fixture ports — fallback wrapper + rust-c-ffi gnu target (#197, #198)#200emmanuelm41 wants to merge 2 commits into
emmanuelm41 wants to merge 2 commits into
Conversation
emmanuelm41
force-pushed
the
fix/windows-e2e-fixtures
branch
from
June 1, 2026 22:43
15b4f87 to
73091be
Compare
emmanuelm41
force-pushed
the
fix/windows-e2e-fixtures
branch
2 times, most recently
from
June 1, 2026 23:20
44d77b7 to
dc495b8
Compare
emmanuelm41
marked this pull request as draft
June 1, 2026 23:25
rust-fallback/rust-sccache pointed KACHE_FALLBACK at #!/bin/sh scripts (fb.sh, fb-sccache.sh); kache execs the fallback directly, and a .sh can't run on Windows (CreateProcess → os error 193). .bat/.cmd aren't a safe substitute (CVE-2024-24576 territory). Replace both scripts with one cross-platform e2e-fallback binary in the kache-e2e crate, exposed to fixtures as a new $FALLBACK token (absolute path → survives cp -R relocate). Behaviour is env-driven (E2E_FALLBACK_DELEGATE / _CRATE / _CFG), mirroring the old scripts: optional sccache delegation + a --cfg marker on the fixture crate only. Verified on Unix: rust-fallback and rust-sccache both pass through the new wrapper.
On the windows-msvc target, the fixture's build.rs cc-rs compile of the
C half (via `$KACHE cc` = MinGW gcc) is rejected — GNU C objects can't
link into an MSVC Rust binary ("GNU compiler is not supported for this
target"). Build this fixture with the matching x86_64-pc-windows-gnu
target instead.
Adds a general per-fixture `[windows]` override (build command, verify
run path, diff artifact paths) applied only when the harness runs on
Windows; the windows-gnu output dir shifts those paths. CI runs
`rustup target add x86_64-pc-windows-gnu` on the Windows arm; the MinGW
linker is already provisioned. No-op on other platforms (verified:
rust-c-ffi unchanged on macOS/Linux).
emmanuelm41
force-pushed
the
fix/windows-e2e-fixtures
branch
from
June 2, 2026 11:40
dc495b8 to
a30c2df
Compare
Member
Author
jleni
pushed a commit
that referenced
this pull request
Jun 2, 2026
…s, and an MSVC release binary (#201, #202, #203) (#209) * fix(e2e): resolve inspected artifact with platform exe suffix (#202) 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). * fix(probe): exclude volatile env pseudo-vars from probe key (#201) 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. * fix(e2e): Windows fixture ports — fallback wrapper + rust-c-ffi gnu target (#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). * ci: e2e reliability — per-arm artifact names (#203) + job timeouts 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. * fix(e2e): apply exe-suffix resolution to diff-artifact reads (#202) 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. * fix(e2e): route rust-c-ffi C half straight to cc on Windows (#198) 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. * debug(probe): TEMP diagnostics for the Windows warm-probe miss (#201) 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. * fix(probe): resolve compiler via PATH with exe suffix on Windows (#201) 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). * test(e2e): dump hex+ascii window at first diff in diff_match failures 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. * fix(e2e): make Windows rustc links reproducible for diff_match (#202) 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. * TEMP diag: scope Windows e2e to multi-dep + cache_key=trace (#201/#202) Temporary: capture the cache_key component divergence for the serde crates that miss on multi-dep's relocate phase on Windows. Reverted after diagnosis. * fix(cache): normalize OUT_DIR in the cache key on Windows (#201 relocate) 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). * fix(cache): unescape rustc env-dep values so OUT_DIR normalizes (#201) 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. * fix(cache): hash source files in content order, not path order (#201) 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. * ci: revert temporary multi-dep/cache_key=trace diagnostic scoping Restores the full-fixture e2e run and `KACHE_LOG: kache=debug` after the #201 relocate root-cause was diagnosed and fixed. * review: bump cache-key version, gate relocate canonicalize to Windows, 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. * feat(e2e): kache-cc/kache-cxx shims — route rust-c-ffi C through kache 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). * fix(e2e): expand tokens in [windows].env overrides ($KACHE_CC was literal) 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. * ci: make Windows e2e blocking and gate the crates publish on it 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. * ci+docs: ship x86_64-pc-windows-msvc release binary; reflect Windows 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. * ci: pin the release reusable workflow to _workflows@v9 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.
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
1 participant
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.
Stacked on #199 (#196). Ports the two remaining Unix-only e2e fixtures to Windows.
#197 — fallback wrappers
rust-fallback/rust-sccachepointedKACHE_FALLBACKat#!/bin/shscripts, which can't exec on Windows (.bat/.cmdaren't a safe substitute). Replaced both with one cross-platforme2e-fallbackbinary in kache-e2e, exposed via a new$FALLBACKtoken (absolute path → survivescp -R). Env-driven (E2E_FALLBACK_DELEGATE/_CRATE/_CFG). Verified on Unix: both pass through the new wrapper.#198 — rust-c-ffi
GNU C (MinGW
$KACHE cc) can't link into an MSVC Rust binary. Build withx86_64-pc-windows-gnuon Windows via a new general per-fixture[windows]override (build cmd / verify run / diff artifacts). CI adds the gnu target on the Windows arm. No-op elsewhere (rust-c-ffi unchanged on macOS/Linux).Not in scope (filed separately)
The #196 Windows run also surfaced, beyond #197/#198:
c-hello/c-depinfowarm)..exesuffix (multi-dep/rust-workspace).e2e-results#203 — e2e arms collide on thee2e-resultsartifact name.Windows e2e stays non-blocking until those land.
Fixes #197, #198.