fix(build-cache): key PERRY_U8_INLINE_READ — it changes emitted code (#9342) - #9475
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe build cache now treats ChangesBuild cache configuration
Estimated code review effort: 1 (Trivial) | ~3 minutes Merge Risk: ⚪ Minimal · up to The change partitions build-cache entries by the code-generation setting so incompatible compiled outputs are not reused across configurations. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description explains the failure, the required code change, the cache-safety reason, the related issues, and the verification performed. It does not use all template headings or checklist items, but it contains the critical information for this focused change. Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Verified both sides before merging: the #6394 guard fails on Two notes worth attaching. This is fallout from #9436 (my merge of the #9342 content), so thanks for catching it — the guard test is a model of the kill-policy discipline: a codegen-affecting env var cannot exist without either keying the cache or carrying a written reason it can't. And it retroactively adds a second, independent reason my early |
cargo-testfails onmain:#9342 added the var but did not register it, and the #6394 guard correctly refuses to let a codegen-affecting env var go unaccounted for.
It belongs in
BUILD_CACHE_ENV_VARS, not the exclusion list. It is a codegen kill switch —perry-codegen/src/expr/u8_buffer_read.rsgates the inline u8-read lane on it, andissue_9342_u8_inline_read.rsasserts thatPERRY_U8_INLINE_READ=0removes the lane. Since it changes emitted code, an object built with the lane must not be served from cache to a build that turned it off, or vice versa. Excluding it would let exactly that happen — silently, and only under a cache hit.Verified:
codegen_env_vars_are_build_cache_inputspasses with the entry added.Blocking:
cargo-testis infull-suite-gate's needs, so this stops any release cut.Summary by CodeRabbit