diff --git a/crates/fbuild-build-arm/src/nxplpc/orchestrator.rs b/crates/fbuild-build-arm/src/nxplpc/orchestrator.rs index a52061f0..72e21c66 100644 --- a/crates/fbuild-build-arm/src/nxplpc/orchestrator.rs +++ b/crates/fbuild-build-arm/src/nxplpc/orchestrator.rs @@ -22,7 +22,7 @@ use std::time::Instant; use fbuild_core::{FbuildError, Platform, Result}; use crate::compile_database::TargetArchitecture; -use crate::flag_overlay::{apply_overlay_flags, LanguageExtraFlags}; +use crate::flag_overlay::apply_overlay_flags; use crate::generic_arm::{ArmCompiler, ArmLinker}; use crate::pipeline; use crate::{BuildOrchestrator, BuildParams, BuildResult, SourceScanner}; @@ -280,17 +280,10 @@ impl BuildOrchestrator for NxpLpcOrchestrator { let gcc_ar_path = toolchain.get_gcc_ar_path(); let raw_c_flags = crate::compiler::Compiler::c_flags(&compiler); let raw_cpp_flags = crate::compiler::Compiler::cpp_flags(&compiler); - let user_overlay = LanguageExtraFlags { - common: ctx - .user_flags - .iter() - .cloned() - .chain(ctx.global_compile_overlay.common.iter().cloned()) - .collect(), - c: ctx.global_compile_overlay.c.clone(), - cxx: ctx.global_compile_overlay.cxx.clone(), - asm: ctx.global_compile_overlay.asm.clone(), - }; + // FastLED/fbuild#574: shared overlay assembly (only the user overlay is + // needed here — the sketch/core/local-lib overlays are applied inside + // `run_sequential_build_with_libs`). + let (user_overlay, _src_overlay) = ctx.compile_overlays(); let c_flags = apply_overlay_flags(&raw_c_flags, &user_overlay, "dummy.c"); let cpp_flags = apply_overlay_flags(&raw_cpp_flags, &user_overlay, "dummy.cpp"); let lib_ar_path = pipeline::pick_archiver(&ar_path, &gcc_ar_path, &c_flags, &cpp_flags); diff --git a/crates/fbuild-build-engine/src/pipeline/context.rs b/crates/fbuild-build-engine/src/pipeline/context.rs index f11eb638..31f4dcc0 100644 --- a/crates/fbuild-build-engine/src/pipeline/context.rs +++ b/crates/fbuild-build-engine/src/pipeline/context.rs @@ -169,8 +169,16 @@ impl BuildContext { (user_flags, src_flags, overlay_link_flags) }; let build_unflags = config.get_build_unflags(env_name)?; - let (user_flags, src_flags, all_src_flags) = + let (mut user_flags, src_flags, all_src_flags) = apply_build_unflags(user_flags, src_flags, &build_unflags); + // FastLED/fbuild#574: fold caller-injected one-off flags (e.g. the + // QEMU-emulation `extra_build_flags`) into `user_flags` so they + // propagate to framework + library + sketch compiles on EVERY + // orchestrator. The shared sequential pipeline previously dropped them + // (only the ESP32 path applied them), so the same build behaved + // differently across platforms. Appended last so they intentionally + // override board/user defaults, and NOT subject to `build_unflags`. + user_flags.extend(params.extra_build_flags.iter().cloned()); remove_unflagged_tokens(&mut overlay_link_flags, &build_unflags); if let Some(p) = perf.as_mut() { p.record("flag-collect", t0.elapsed()); @@ -195,4 +203,124 @@ impl BuildContext { build_unflags, }) } + + /// The `(user_overlay, src_overlay)` pair applied to every compile — the + /// single source of truth so `[env:*] build_flags` (and caller-injected + /// `extra_build_flags`, folded into `user_flags` at construction) reach + /// framework/core, library, AND sketch TUs uniformly on every orchestrator. + /// + /// - `user_overlay` → framework/core + library compiles (`build_flags`). + /// - `src_overlay` → sketch + local-lib compiles (adds `build_src_flags` + /// and the project script overlay on top of `user_overlay`). + /// + /// FastLED/fbuild#574 — replaces three hand-copied overlay blocks + /// (`pipeline::sequential`, `esp32` orchestrator, `nxplpc` orchestrator). + pub fn compile_overlays(&self) -> (LanguageExtraFlags, LanguageExtraFlags) { + self.compile_overlays_with_base(&self.user_flags) + } + + /// As [`compile_overlays`](Self::compile_overlays) but with an explicit + /// base user-flag list — the ESP32 path prepends SDK defines to + /// `ctx.user_flags` and passes the combined list here. + pub fn compile_overlays_with_base( + &self, + user_flags: &[String], + ) -> (LanguageExtraFlags, LanguageExtraFlags) { + assemble_compile_overlays( + user_flags, + &self.global_compile_overlay, + &self.src_flags, + &self.project_compile_overlay, + ) + } + + /// Build the typed [`EnvNamespace`] routing key for `env_id` on `platform` + /// — the `(env_id, platform, board, framework)` triplet from + /// `platformio.ini [env:]`. FastLED/fbuild#574. Package fetchers, cache + /// keys, and output dirs can route off this instead of ad-hoc strings. + pub fn env_namespace( + &self, + env_id: &str, + platform: fbuild_core::Platform, + ) -> fbuild_core::EnvNamespace { + let (board, framework) = self + .config + .get_env_config(env_id) + .map(|m| { + ( + m.get("board").cloned().unwrap_or_default(), + m.get("framework").cloned().unwrap_or_default(), + ) + }) + .unwrap_or_default(); + fbuild_core::EnvNamespace::new(env_id, platform, board, framework) + } +} + +/// Pure overlay assembly (unit-tested). `user_flags` (which include +/// `[env:*] build_flags` plus caller-injected `extra_build_flags`) land in BOTH +/// the user overlay (core/framework and libraries) and — via the src overlay — +/// the sketch and local libraries; `src_flags` (`build_src_flags`) land ONLY in +/// the src overlay. FastLED/fbuild#574. +fn assemble_compile_overlays( + user_flags: &[String], + global: &LanguageExtraFlags, + src_flags: &[String], + project: &LanguageExtraFlags, +) -> (LanguageExtraFlags, LanguageExtraFlags) { + let user_overlay = LanguageExtraFlags { + common: user_flags + .iter() + .cloned() + .chain(global.common.iter().cloned()) + .collect(), + c: global.c.clone(), + cxx: global.cxx.clone(), + asm: global.asm.clone(), + }; + let src_overlay = LanguageExtraFlags::combined(&[ + &user_overlay, + &LanguageExtraFlags { + common: src_flags.to_vec(), + c: Vec::new(), + cxx: Vec::new(), + asm: Vec::new(), + }, + project, + ]); + (user_overlay, src_overlay) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn user_flags_reach_both_overlays_src_flags_only_src() { + // FastLED/fbuild#574: `[env:*] build_flags` (+ folded `extra_build_flags`) + // must reach framework/core, library AND sketch compiles; `build_src_flags` + // only the sketch/local-lib compiles. + let (user, src) = assemble_compile_overlays( + &["-DENV_FLAG".to_string()], + &LanguageExtraFlags::default(), + &["-DSRC_ONLY".to_string()], + &LanguageExtraFlags::default(), + ); + assert!( + user.common.contains(&"-DENV_FLAG".to_string()), + "build_flags must reach the framework/library overlay" + ); + assert!( + !user.common.contains(&"-DSRC_ONLY".to_string()), + "build_src_flags must NOT reach the framework overlay" + ); + assert!( + src.common.contains(&"-DENV_FLAG".to_string()), + "build_flags must also reach the sketch overlay" + ); + assert!( + src.common.contains(&"-DSRC_ONLY".to_string()), + "build_src_flags must reach the sketch overlay" + ); + } } diff --git a/crates/fbuild-build-engine/src/pipeline/sequential.rs b/crates/fbuild-build-engine/src/pipeline/sequential.rs index 34e4657e..de3c9d2c 100644 --- a/crates/fbuild-build-engine/src/pipeline/sequential.rs +++ b/crates/fbuild-build-engine/src/pipeline/sequential.rs @@ -8,7 +8,6 @@ use fbuild_core::Result; use crate::compile_database::TargetArchitecture; use crate::compiler::Compiler; -use crate::flag_overlay::LanguageExtraFlags; use crate::source_scanner::SourceCollection; use crate::{BuildParams, BuildResult}; @@ -54,27 +53,10 @@ pub async fn run_sequential_build_with_libs( .chain(sources.variant_sources.iter()) .cloned() .collect(); - let user_overlay = LanguageExtraFlags { - common: ctx - .user_flags - .iter() - .cloned() - .chain(ctx.global_compile_overlay.common.iter().cloned()) - .collect(), - c: ctx.global_compile_overlay.c.clone(), - cxx: ctx.global_compile_overlay.cxx.clone(), - asm: ctx.global_compile_overlay.asm.clone(), - }; - let src_overlay = LanguageExtraFlags::combined(&[ - &user_overlay, - &LanguageExtraFlags { - common: ctx.src_flags.clone(), - c: Vec::new(), - cxx: Vec::new(), - asm: Vec::new(), - }, - &ctx.project_compile_overlay, - ]); + // FastLED/fbuild#574: single source of truth for the compile overlays so + // `[env:*] build_flags` reach core/framework + libs (user_overlay) and + // sketch + local libs (src_overlay) uniformly. + let (user_overlay, src_overlay) = ctx.compile_overlays(); // compiledb_only: generate compile_commands.json without compiling if params.compiledb_only { diff --git a/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs b/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs index 294cb51d..c8322497 100644 --- a/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs +++ b/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs @@ -26,7 +26,7 @@ use crate::build_fingerprint::{ FastPathPersistInputs, BUILD_FINGERPRINT_VERSION, }; use crate::compiler::Compiler as _; -use crate::flag_overlay::{apply_overlay_flags, LanguageExtraFlags}; +use crate::flag_overlay::apply_overlay_flags; use crate::linker::LinkerScripts; use crate::{BuildOrchestrator, BuildParams, BuildResult, SourceScanner}; @@ -311,26 +311,9 @@ impl BuildOrchestrator for Esp32Orchestrator { let mut user_build_flags = ctx.config.get_build_flags(¶ms.env_name)?; user_build_flags.extend(params.extra_build_flags.clone()); user_flags.extend(user_build_flags.clone()); - let user_overlay = LanguageExtraFlags { - common: user_flags - .iter() - .cloned() - .chain(ctx.global_compile_overlay.common.iter().cloned()) - .collect(), - c: ctx.global_compile_overlay.c.clone(), - cxx: ctx.global_compile_overlay.cxx.clone(), - asm: ctx.global_compile_overlay.asm.clone(), - }; - let src_overlay = LanguageExtraFlags::combined(&[ - &user_overlay, - &LanguageExtraFlags { - common: ctx.src_flags.clone(), - c: Vec::new(), - cxx: Vec::new(), - asm: Vec::new(), - }, - &ctx.project_compile_overlay, - ]); + // FastLED/fbuild#574: shared overlay assembly. ESP32 prepends SDK + // defines to the user flags, so it passes the combined base explicitly. + let (user_overlay, src_overlay) = ctx.compile_overlays_with_base(&user_flags); // Emit a warning if CDC on boot is effectively enabled (may cause Serial to block // when no USB host is connected). diff --git a/crates/fbuild-core/src/env_namespace.rs b/crates/fbuild-core/src/env_namespace.rs new file mode 100644 index 00000000..058cd509 --- /dev/null +++ b/crates/fbuild-core/src/env_namespace.rs @@ -0,0 +1,125 @@ +//! The `platformio.ini [env:]` routing triplet. +//! +//! FastLED/fbuild#574. A PlatformIO env block names the three coordinates that +//! determine where every artifact fbuild fetches or generates lives: +//! +//! ```ini +//! [env:lpc845brk] +//! platform = nxplpc +//! board = lpc845brk +//! framework = arduino +//! ``` +//! +//! `EnvNamespace` captures `(env_id, platform, board, framework)` as one typed +//! value so package fetchers, compile invocations, cache keys, and output dirs +//! can be routed off a single namespace instead of ad-hoc string plumbing. + +use crate::Platform; + +/// Typed `[env:*]` routing key: the identity of one build environment. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct EnvNamespace { + /// The `[env:]` name, e.g. `lpc845brk`. + pub env_id: String, + /// Resolved platform (from the board's platform, e.g. `NxpLpc`). + pub platform: Platform, + /// Board id, e.g. `lpc845brk`. + pub board: String, + /// Framework string, e.g. `arduino` (empty when the env declares none). + pub framework: String, +} + +impl EnvNamespace { + pub fn new( + env_id: impl Into, + platform: Platform, + board: impl Into, + framework: impl Into, + ) -> Self { + Self { + env_id: env_id.into(), + platform, + board: board.into(), + framework: framework.into(), + } + } + + /// Filesystem-safe slug identifying this env's per-env artifacts + /// (`build//…`, env-scoped lib cache). Two envs sharing platform but + /// differing in board/env get distinct slugs; the same env is stable. + pub fn slug(&self) -> String { + sanitize(&format!("{}-{}", self.env_id, self.board)) + } + + /// Framework cache segment, e.g. `framework-arduino-nxplpc`. Empty + /// framework yields `framework-none-`. + pub fn framework_segment(&self) -> String { + let fw = if self.framework.is_empty() { + "none" + } else { + &self.framework + }; + sanitize(&format!( + "framework-{}-{}", + fw, + format!("{:?}", self.platform).to_ascii_lowercase() + )) + } +} + +/// Replace anything outside `[A-Za-z0-9._-]` with `_` so the value is safe as a +/// single path segment on every OS. +fn sanitize(s: &str) -> String { + s.chars() + .map(|c| { + if c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-') { + c + } else { + '_' + } + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn slug_is_stable_and_distinct_per_env() { + let a = EnvNamespace::new("lpc845brk", Platform::NxpLpc, "lpc845brk", "arduino"); + assert_eq!(a.slug(), "lpc845brk-lpc845brk"); + assert_eq!(a.slug(), a.clone().slug(), "slug must be stable"); + + // Same platform, different board+env → different slug. + let b = EnvNamespace::new( + "esp32s3-xiao", + Platform::Espressif32, + "seeed_xiao_esp32s3", + "arduino", + ); + let c = EnvNamespace::new( + "esp32s3-devkit", + Platform::Espressif32, + "esp32-s3-devkitc-1", + "arduino", + ); + assert_ne!(b.slug(), c.slug()); + } + + #[test] + fn framework_segment_covers_none_and_named() { + let named = EnvNamespace::new("e", Platform::NxpLpc, "b", "arduino"); + assert_eq!(named.framework_segment(), "framework-arduino-nxplpc"); + let none = EnvNamespace::new("e", Platform::NxpLpc, "b", ""); + assert!(none.framework_segment().starts_with("framework-none-")); + } + + #[test] + fn slug_sanitizes_unsafe_chars() { + let ns = EnvNamespace::new("weird/env:name", Platform::AtmelAvr, "b oard", "arduino"); + assert!(!ns.slug().contains('/')); + assert!(!ns.slug().contains(':')); + assert!(!ns.slug().contains(' ')); + } +} diff --git a/crates/fbuild-core/src/lib.rs b/crates/fbuild-core/src/lib.rs index 8268f032..9a617a7c 100644 --- a/crates/fbuild-core/src/lib.rs +++ b/crates/fbuild-core/src/lib.rs @@ -12,6 +12,7 @@ pub mod compiler_flags; pub mod containment; pub mod elapsed; pub mod emulator; +pub mod env_namespace; pub mod fs; pub mod http; pub mod install_status; @@ -24,6 +25,7 @@ pub mod time; pub mod usb; pub use build_log::BuildLog; +pub use env_namespace::EnvNamespace; use serde::{Deserialize, Serialize}; diff --git a/docs/namespacing.md b/docs/namespacing.md new file mode 100644 index 00000000..a33931e6 --- /dev/null +++ b/docs/namespacing.md @@ -0,0 +1,73 @@ +# Env-driven artifact namespacing (`EnvNamespace`) + +FastLED/fbuild#574. A PlatformIO `[env:*]` block names the three coordinates +that determine where every artifact fbuild fetches or generates for that +environment lives: + +```ini +[env:lpc845brk] +platform = nxplpc +board = lpc845brk +framework = arduino +build_flags = -DRELEASE +``` + +`fbuild_core::EnvNamespace { env_id, platform, board, framework }` captures that +triplet (plus the env id) as one typed value so routing is keyed on a single +namespace instead of ad-hoc string plumbing. + +## Where it comes from + +`BuildContext::env_namespace(env_id, platform)` +(`fbuild-build-engine/src/pipeline/context.rs`) builds it from the parsed +`platformio.ini` for the current build: `board` and `framework` from the env +section, `platform` from the orchestrator (which already knows it), `env_id` +from `BuildParams.env_name`. Every orchestrator's `build()` already resolves all +four during `BuildContext::new`. + +## Namespace layout + +| Artifact | Segment | Keyed on | +|---|---|---| +| framework source tree | `framework--/…` | `EnvNamespace::framework_segment()` | +| platform SDK / toolchain | `platform-/…` | `platform` | +| board definition + variant + linker | `board-/…` | `board` | +| per-env build output | `build///…` | `EnvNamespace::slug()` (`-`) | +| env-scoped library cache | `lib//…` under the env slug | `slug` + flags hash | + +`slug()` and `framework_segment()` sanitize to a single filesystem-safe path +segment on every OS. Two envs that share a `platform` but differ in +`board`/`env_id` get distinct slugs (so they isolate board variant + linker + +per-env build output) while still being able to share the platform/framework +cache. + +## `build_flags` propagation + +`[env:*] build_flags` are the canonical place to inject `-D` defines, and they +must reach **framework/core, library, AND sketch** translation units uniformly. +This is enforced structurally by a single overlay-assembly seam rather than +per-orchestrator copies: + +- `BuildContext::compile_overlays() -> (user_overlay, src_overlay)` is the one + source of truth. `user_overlay` (= `build_flags` + global script overlay) is + applied to core/framework + library compiles; `src_overlay` (= `user_overlay` + + `build_src_flags` + project script overlay) is applied to sketch + local-lib + compiles. The shared sequential pipeline, the ESP32 orchestrator, and the + nxplpc orchestrator all call it (previously each had a hand-copied version). +- Caller-injected one-off flags (`BuildParams.extra_build_flags`, e.g. QEMU + emulation defines) are folded into `user_flags` in `BuildContext::new`, so + they now propagate on **every** orchestrator — previously the sequential + pipeline dropped them and only ESP32 applied them. + +The pure `assemble_compile_overlays` function is unit-tested to guarantee +`build_flags` reach both overlays and `build_src_flags` reach only the sketch +overlay. + +## Status / scope + +This lands the typed `EnvNamespace` (success criterion 1), the uniform +`build_flags` propagation guarantee via a single de-duplicated overlay seam +(criterion 3), and this doc (criterion 4). Threading `EnvNamespace` into every +package fetcher and cache-path deriver (criterion 2's full breadth) and the +`fbuild cache gc --env ` reconciliation are follow-ups that build on this +foundation.