From cebd4da13855e9f1259f9cbf863ae4993b0d37a9 Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 14 Aug 2026 11:27:52 -0700 Subject: [PATCH 1/4] fix(build-cache): compare the compiler running now, not the recorded one The `perry_build_id` gate re-fingerprinted the path stored IN the manifest and compared it against the value stored beside it: if file_fingerprint_from_str(&manifest.perry_build_id.path).ok() != Some(manifest.perry_build_id.clone()) That asks "is the binary I recorded still unchanged?" It is trivially true whenever a DIFFERENT perry performs the second build -- the recorded binary is sitting right where it was left, untouched. So the gate passed, the cache reported `"hit": true, "reason": "manifest-match"`, and the whole build was skipped: no relink, the output file not rewritten (same hash, same mtime), nothing printed, exit 0. Compare against `current_perry_fingerprint()` instead -- the same helper the manifest is written with, so the read and the write now agree on what identity means. `perry_version` above it does not cover this: during pass development the version rarely moves between rebuilds, which is precisely why `perry_build_id` exists (#544). The gate was there; it just answered the wrong question. Verified end to end with two compilers over one source and output path. Before: the second build reported manifest-match and returned the first compiler's executable. After: `"miss_reason": "perry-build-id"` and a real rebuild, while a repeat with the SAME compiler still reports manifest-match, so the cache keeps working rather than being disabled. The regression test asserts its own precondition -- that the recorded file IS unchanged on disk, which is what made the old check pass -- so it cannot go green by the setup drifting. Sabotaging it back to the old comparison fails it. --- .../544-build-cache-compiler-identity.md | 24 ++++++++ .../perry/src/commands/compile/build_cache.rs | 60 +++++++++++++++++-- 2 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 changelog.d/544-build-cache-compiler-identity.md diff --git a/changelog.d/544-build-cache-compiler-identity.md b/changelog.d/544-build-cache-compiler-identity.md new file mode 100644 index 0000000000..3e84f90ce1 --- /dev/null +++ b/changelog.d/544-build-cache-compiler-identity.md @@ -0,0 +1,24 @@ +### Fixed + +- **The build cache no longer hands back a binary built by a different + compiler.** The `perry_build_id` check re-fingerprinted the path *recorded in + the manifest* and compared it to the recorded value — which asks "is the + binary I recorded still unchanged?", and is trivially true whenever a + different `perry` runs the second build. The recorded binary is sitting + exactly where it was, so the check passed, the cache reported + `"hit": true, "reason": "manifest-match"`, and the build was skipped + entirely: no relink, output file untouched, nothing printed, exit 0. + + It now compares against the compiler running now, via the same + `current_perry_fingerprint()` used when the manifest is written. + + `perry_version` did not cover this. During pass development the version + rarely moves between rebuilds, which is the reason `perry_build_id` exists + at all (#544) — this restores the guarantee that issue was closed on. + + How it surfaced: a `.ts` probe compiled by a pre-fix compiler kept its stale + executable when recompiled by a fixed one, so a genuine fix read as not + working. The phantom was then bisected onto an unrelated commit before the + cache was suspected. Touching the source does not help, because sources are + verified by sha256 rather than mtime; only a different output path or a + cleared cache does. diff --git a/crates/perry/src/commands/compile/build_cache.rs b/crates/perry/src/commands/compile/build_cache.rs index a8b0fdf2e8..5d0b69eebf 100644 --- a/crates/perry/src/commands/compile/build_cache.rs +++ b/crates/perry/src/commands/compile/build_cache.rs @@ -134,7 +134,45 @@ const BUILD_CACHE_ENV_EXCLUSIONS: &[&str] = &[ #[cfg(test)] mod tests { - use super::{BUILD_CACHE_ENV_EXCLUSIONS, BUILD_CACHE_ENV_VARS}; + use super::{ + current_perry_fingerprint, file_fingerprint, file_fingerprint_from_str, + BUILD_CACHE_ENV_EXCLUSIONS, BUILD_CACHE_ENV_VARS, + }; + + /// The build cache must compare against the compiler RUNNING NOW, not the + /// one that wrote the manifest. + /// + /// The bug this pins: the check used to re-fingerprint the path recorded in + /// the manifest and compare it to the recorded value. That asks "is the + /// binary I recorded still unchanged?", which is trivially true when a + /// DIFFERENT perry runs the second build — the recorded binary is sitting + /// right where it was. The cache then reported `manifest-match`, skipped + /// the build, and handed back the first compiler's executable while + /// printing nothing and exiting 0. + /// + /// `perry_version` does not cover this: during pass development the + /// version rarely moves between rebuilds, which is why `perry_build_id` + /// exists at all (#544). + #[test] + fn a_manifest_from_another_compiler_does_not_match_this_one() { + // Stand in for "the compiler that wrote the manifest": any other file + // that exists and is not this executable. Its own fingerprint is + // self-consistent, which is exactly what made the old check pass. + let other = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("Cargo.toml"); + let recorded = file_fingerprint(&other).expect("fingerprint the stand-in"); + assert_eq!( + file_fingerprint_from_str(&recorded.path).ok(), + Some(recorded.clone()), + "precondition: the recorded binary is unchanged on disk, so the OLD \ + check would have passed here — without this the test proves nothing" + ); + + let running = current_perry_fingerprint().expect("fingerprint the test binary"); + assert_ne!( + running, recorded, + "a manifest written by a different compiler must not match" + ); + } #[test] fn binding_policy_switches_are_build_cache_inputs() { @@ -337,9 +375,23 @@ impl BuildCacheProbe { if manifest.output_path != absolute_identity(&self.output_path) { return miss("output-path"); } - if file_fingerprint_from_str(&manifest.perry_build_id.path).ok() - != Some(manifest.perry_build_id.clone()) - { + // Compare against the compiler RUNNING NOW, not the one the manifest + // was written by. Re-fingerprinting `manifest.perry_build_id.path` + // asks "is the binary I recorded still unchanged?", which is trivially + // true whenever a DIFFERENT perry does the second build — its path is + // not the recorded one, so the recorded binary sits there untouched + // and the check passes. The cache then hands back the first compiler's + // executable and skips the build entirely, reporting + // `"hit": true, "reason": "manifest-match"` and printing nothing. + // + // That is not hypothetical: it cost a full false-regression hunt. A + // probe compiled by a pre-fix perry kept its stale output when + // recompiled by a fixed one, the fix read as not working, and the + // phantom bisected onto an unrelated commit. `perry_version` above + // does not cover it either — during pass development the version + // rarely moves between rebuilds, which is the whole reason + // `perry_build_id` exists (#544). + if current_perry_fingerprint().ok() != Some(manifest.perry_build_id.clone()) { return miss("perry-build-id"); } if verify_files(&manifest.sources).is_err() { From 350e2e173251a9e6ac9d8ba6626165c4996de3cc Mon Sep 17 00:00:00 2001 From: jdalton Date: Fri, 14 Aug 2026 11:37:57 -0700 Subject: [PATCH 2/4] chore(changelog): key the fragment on the PR number, not the issue --- ...compiler-identity.md => 8091-build-cache-compiler-identity.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{544-build-cache-compiler-identity.md => 8091-build-cache-compiler-identity.md} (100%) diff --git a/changelog.d/544-build-cache-compiler-identity.md b/changelog.d/8091-build-cache-compiler-identity.md similarity index 100% rename from changelog.d/544-build-cache-compiler-identity.md rename to changelog.d/8091-build-cache-compiler-identity.md From c81d2746d411a9b7c28b86a0fc72921abbff8e9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 14 Aug 2026 23:14:33 +0200 Subject: [PATCH 3/4] test(build-cache): guard the fix at the probe, not just the expression The existing test pins the two comparisons in isolation but never calls `probe()`. Reverting the production call site to the buggy `file_fingerprint_from_str(&manifest.perry_build_id.path)` form leaves it green, so it documents the bug without guarding against its return. Add a test that writes a manifest claiming a different compiler's build id and drives the real decision path, asserting the miss is `perry-build-id` specifically rather than an incidental later check. Sabotage-verified: with the call site reverted, the new test fails and the expression test still passes. Restored and re-run, all four pass. --- .../perry/src/commands/compile/build_cache.rs | 78 ++++++++++++++++++- 1 file changed, 76 insertions(+), 2 deletions(-) diff --git a/crates/perry/src/commands/compile/build_cache.rs b/crates/perry/src/commands/compile/build_cache.rs index 5d0b69eebf..057e38d4d1 100644 --- a/crates/perry/src/commands/compile/build_cache.rs +++ b/crates/perry/src/commands/compile/build_cache.rs @@ -135,8 +135,9 @@ const BUILD_CACHE_ENV_EXCLUSIONS: &[&str] = &[ #[cfg(test)] mod tests { use super::{ - current_perry_fingerprint, file_fingerprint, file_fingerprint_from_str, - BUILD_CACHE_ENV_EXCLUSIONS, BUILD_CACHE_ENV_VARS, + absolute_identity, current_env, current_perry_fingerprint, file_fingerprint, + file_fingerprint_from_str, BuildCacheManifest, BuildCacheProbe, BUILD_CACHE_ENV_EXCLUSIONS, + BUILD_CACHE_ENV_VARS, BUILD_CACHE_MANIFEST_VERSION, }; /// The build cache must compare against the compiler RUNNING NOW, not the @@ -174,6 +175,79 @@ mod tests { ); } + /// The expression test above pins the two comparisons in isolation, but it + /// never calls `probe()` — reverting the production call site to the buggy + /// form leaves it green. This one drives the real decision path, so it is + /// the one that actually guards the fix. + /// + /// Verified by sabotage: restoring + /// `file_fingerprint_from_str(&manifest.perry_build_id.path)` at the call + /// site turns this red while the expression test stays green. + #[test] + fn a_foreign_build_id_misses_at_the_probe() { + let dir = tempfile::tempdir().expect("tempdir"); + let input = dir.path().join("in.ts"); + let output = dir.path().join("out.bin"); + let manifest_path = dir.path().join("manifest.json"); + std::fs::write(&input, b"export {}\n").expect("write input"); + std::fs::write(&output, b"binary").expect("write output"); + + // A build id belonging to some other compiler: any real file that is + // not this executable. It is unchanged on disk, which is precisely the + // condition under which the old self-comparison passed. + let foreign = file_fingerprint( + &std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("Cargo.toml"), + ) + .expect("fingerprint the foreign build id"); + assert_ne!( + foreign, + current_perry_fingerprint().expect("fingerprint the running binary"), + "precondition: the manifest must claim a DIFFERENT compiler" + ); + + let manifest = BuildCacheManifest { + version: BUILD_CACHE_MANIFEST_VERSION, + perry_version: env!("CARGO_PKG_VERSION").to_string(), + perry_build_id: foreign, + args_key: "args".to_string(), + env: current_env(), + input_path: absolute_identity(&input), + output_path: absolute_identity(&output), + target: "native".to_string(), + compiled_features: Vec::new(), + sources: Vec::new(), + config_inputs: Vec::new(), + runtime_inputs: Vec::new(), + object_fingerprints: Vec::new(), + native_modules: 0, + js_modules: 0, + output: file_fingerprint(&output).expect("fingerprint output"), + }; + std::fs::write( + &manifest_path, + serde_json::to_string(&manifest).expect("serialize manifest"), + ) + .expect("write manifest"); + + let probe = BuildCacheProbe { + args_key: "args".to_string(), + manifest_path, + output_path: output, + target_name: "native".to_string(), + input_path: input, + project_root: dir.path().to_path_buf(), + cache_root: dir.path().to_path_buf(), + eligible: Ok(()), + }; + + let stats = probe.probe(); + assert!(!stats.hit, "a manifest from another compiler must not hit"); + assert_eq!( + stats.reason, "perry-build-id", + "must miss on the build id specifically, not incidentally on a later check" + ); + } + #[test] fn binding_policy_switches_are_build_cache_inputs() { for name in [ From d48d8136fbc56a4b5c94ec627c90858be2109e95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 14 Aug 2026 23:15:47 +0200 Subject: [PATCH 4/4] docs(changelog): add file paths and validation notes to the 8091 fragment --- .../8091-build-cache-compiler-identity.md | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/changelog.d/8091-build-cache-compiler-identity.md b/changelog.d/8091-build-cache-compiler-identity.md index 3e84f90ce1..54c79730fd 100644 --- a/changelog.d/8091-build-cache-compiler-identity.md +++ b/changelog.d/8091-build-cache-compiler-identity.md @@ -16,9 +16,20 @@ rarely moves between rebuilds, which is the reason `perry_build_id` exists at all (#544) — this restores the guarantee that issue was closed on. - How it surfaced: a `.ts` probe compiled by a pre-fix compiler kept its stale - executable when recompiled by a fixed one, so a genuine fix read as not - working. The phantom was then bisected onto an unrelated commit before the - cache was suspected. Touching the source does not help, because sources are - verified by sha256 rather than mtime; only a different output path or a - cleared cache does. + How to recognise it: a `.ts` probe compiled by a pre-fix compiler kept its + stale executable when recompiled by a fixed one, so a genuine fix read as not + working and the phantom was bisected onto an unrelated commit. Touching the + source does not help, because sources are verified by sha256 rather than + mtime; only a different output path or a cleared cache does. + + Changed: `crates/perry/src/commands/compile/build_cache.rs` — the + `perry-build-id` arm of `BuildCacheProbe::probe`. + + Validation: `cargo test -p perry --bins build_cache` (4 passed). Two tests + cover it — one pins the two comparison expressions in isolation, and + `a_foreign_build_id_misses_at_the_probe` writes a manifest claiming a + different compiler's build id and drives the real decision path, asserting + the miss is `perry-build-id` specifically rather than an incidental later + check. Sabotage-verified: restoring the old self-comparison at the call site + turns the probe test red while the expression test stays green, so the + guarding test is the one that actually holds the fix in place.