feat(build): EnvNamespace + unified build_flags overlay propagation (#574)#1022
Conversation
…574) Two concrete pieces of the env-driven namespacing design: 1. **Typed `EnvNamespace { env_id, platform, board, framework }`** in fbuild-core — the `platformio.ini [env:*]` routing triplet, with `slug()` (per-env build output / lib cache key) and `framework_segment()` helpers. Constructed via `BuildContext::env_namespace(env_id, platform)`. 2. **Single-source-of-truth compile overlays.** The (user_overlay, src_overlay) assembly that decides which flags reach each translation unit was copy-pasted in three places (`pipeline::sequential`, esp32 orchestrator, nxplpc orchestrator) — a drift risk. Extracted `BuildContext::compile_overlays()` (+ `_with_base` for esp32's SDK-defines prepend) over a pure, unit-tested `assemble_compile_overlays`. All three now call it, so `[env:*] build_flags` reach framework/core + library + sketch compiles uniformly by construction. Fixes a real inconsistency along the way: `params.extra_build_flags` (caller-injected one-off flags, e.g. QEMU emulation defines) were applied on the ESP32 path but DROPPED by the shared sequential pipeline — the same build behaved differently across platforms. They're now folded into `user_flags` in `BuildContext::new`, so every orchestrator applies them. Plus `docs/namespacing.md` documenting the namespace layout + propagation rule. Note: the nxplpc "drops build_flags" symptom that motivated #574 was already retired on main (#576/#587); the residual work is the typing + the structural de-duplication that prevents that class of bug from recurring. Threading EnvNamespace into every fetcher/cache-path (criterion 2's full breadth) and `fbuild cache gc --env` are follow-ups on this foundation. Verified: EnvNamespace (3) + overlay-assembly (1) unit tests; workspace compiles; clippy -D warnings + fmt clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 4 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 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 |
1 participant
Fixes #574. Two concrete pieces of the env-driven namespacing design:
1. Typed
EnvNamespacefbuild_core::EnvNamespace { env_id, platform, board, framework }— theplatformio.ini [env:*]routing triplet — withslug()(per-env build output / lib cache key) andframework_segment(). Built viaBuildContext::env_namespace(env_id, platform).2. Single-source-of-truth compile overlays
The
(user_overlay, src_overlay)assembly that decides which flags reach each TU was copy-pasted in three places (pipeline::sequential, esp32, nxplpc) — a drift risk. ExtractedBuildContext::compile_overlays()(+_with_basefor esp32's SDK-defines prepend) over a pure, unit-testedassemble_compile_overlays. All three now call it, so[env:*] build_flagsreach framework/core + library + sketch compiles uniformly by construction.Fixes a real inconsistency:
params.extra_build_flags(caller one-off flags, e.g. QEMU defines) were applied on the ESP32 path but dropped by the shared sequential pipeline. Now folded intouser_flagsinBuildContext::new— every orchestrator applies them.Plus
docs/namespacing.md.Scope note
The nxplpc "drops build_flags" symptom that motivated #574 was already retired on main (#576/#587). The residual work is the typing (criterion 1) + the structural de-duplication that guarantees uniform propagation (criterion 3) + docs (criterion 4). Threading EnvNamespace into every fetcher/cache-path (criterion 2's full breadth) and
fbuild cache gc --envare follow-ups on this foundation.Verified — local + Linux Docker
cargo check --workspacegreen (2m04s); tests: core 237 + engine 366 + arm 195 + esp 92 pass on Linux (incl. EnvNamespace + overlay-assembly). clippy-D warnings+ fmt clean.🤖 Generated with Claude Code