fix(build): read CARGO_MANIFEST_DIR at build-script run time (shared target dirs) - #11330
Conversation
crates/perry/build/android_template.rs used env!("CARGO_MANIFEST_DIR"),
which bakes the path in when the build script is compiled. Cargo reuses a
compiled build script across checkouts that share a target dir, so the
baked path points at whichever checkout compiled it; once that worktree
is deleted every later build panics at build.rs:5 (read_dir on a missing
directory). Read the variable at run time, where cargo sets it for the
package being built.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe Android build script now reads ChangesAndroid template path resolution
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~5 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The build script now looks up the template using the runtime manifest directory. No actionable merge risk remains; the change is ready for normal checks. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 2 systems. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the problem, the fix, and one verification command. However, it omits the required Summary, Changes, Related issue, Test plan, and Checklist sections, and it does not state whether the standalone issue or checklist items apply. Resolution Rewrite the description using the repository template. Add the required section headings, summarize the concrete changes, specify a related issue or write "n/a", list the complete test plan and results, and complete the checklist items. Keep the existing failure scenario and fix rationale. Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
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 |
1 participant
crates/perry/build/android_template.rs(added with the bundled Android Gradle template, #11276) resolves the template viaenv!("CARGO_MANIFEST_DIR"), which bakes the path in when the build script is compiled. Cargo reuses a compiled build script across checkouts that share a target directory, so the baked path points at whichever checkout compiled it. Once that worktree is deleted, every later build panics:This bit the merge queue three times on a shared target dir. Fix: read
CARGO_MANIFEST_DIRfrom the environment at run time, where cargo sets it for the package being built. Behaviour on a fresh checkout is unchanged.cargo check -p perryis clean.Summary by CodeRabbit