Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 37 additions & 2 deletions crates/fbuild-build/src/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,19 @@ pub trait Compiler: Send + Sync {
extra_flags: &[String],
) -> Result<CompileResult>;

/// Tokens to strip from every compile command line. Default: none.
///
/// Platforms that want PlatformIO `build_unflags` applied against
/// framework/toolchain-contributed flags — not just user flags —
/// override this to return their stored set, typically
/// `&self.base.build_unflags`. The default `compile_c` / `compile_cpp`
/// impls below filter both the platform flags AND `extra_flags`
/// through this set before invoking `compile_one`. See
/// FastLED/fbuild#37.
fn build_unflags(&self) -> &[String] {
&[]
}

/// Compile a C source file to an object file.
fn compile_c(
&self,
Expand All @@ -81,7 +94,8 @@ pub trait Compiler: Send + Sync {
extra_flags: &[String],
) -> Result<CompileResult> {
let flags = self.c_flags();
self.compile_one(self.gcc_path(), source, output, &flags, extra_flags)
let (flags, extra) = apply_compile_unflags(flags, extra_flags, self.build_unflags());
self.compile_one(self.gcc_path(), source, output, &flags, &extra)
}

/// Compile a C++ source file to an object file.
Expand All @@ -92,7 +106,8 @@ pub trait Compiler: Send + Sync {
extra_flags: &[String],
) -> Result<CompileResult> {
let flags = self.cpp_flags();
self.compile_one(self.gxx_path(), source, output, &flags, extra_flags)
let (flags, extra) = apply_compile_unflags(flags, extra_flags, self.build_unflags());
self.compile_one(self.gxx_path(), source, output, &flags, &extra)
}

/// Compile a source file (auto-detect C vs C++).
Expand Down Expand Up @@ -250,6 +265,26 @@ impl CompilerBase {
}
}

/// Filter both `flags` and `extra_flags` through `unflags` using the shared
/// PlatformIO-compatible removal semantics in `pipeline::remove_unflagged_tokens`.
/// Returns the filtered pair ready to pass to `compile_one`. Short-circuits
/// when `unflags` is empty so platforms that don't opt in pay no overhead.
/// See FastLED/fbuild#37.
fn apply_compile_unflags(
flags: Vec<String>,
extra_flags: &[String],
unflags: &[String],
) -> (Vec<String>, Vec<String>) {
if unflags.is_empty() {
return (flags, extra_flags.to_vec());
}
let mut flags = flags;
let mut extra = extra_flags.to_vec();
crate::pipeline::remove_unflagged_tokens(&mut flags, unflags);
crate::pipeline::remove_unflagged_tokens(&mut extra, unflags);
(flags, extra)
}

fn depfile_path(object: &Path) -> PathBuf {
object.with_extension("d")
}
Expand Down
68 changes: 68 additions & 0 deletions crates/fbuild-build/src/esp32/esp32_compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ pub struct Esp32Compiler {
temp_dir: PathBuf,
/// Optional zccache path for compiler caching.
compiler_cache: Option<PathBuf>,
/// PlatformIO `build_unflags` tokens to strip from the effective
/// compile line. Populated post-construction by the orchestrator
/// from `BuildContext::build_unflags`. Empty by default so existing
/// callers don't need to change. See FastLED/fbuild#37.
build_unflags: Vec<String>,
}

impl Esp32Compiler {
Expand Down Expand Up @@ -87,9 +92,20 @@ impl Esp32Compiler {
profile,
temp_dir,
compiler_cache: crate::zccache::find_zccache().map(PathBuf::from),
build_unflags: Vec::new(),
}
}

/// Attach PlatformIO `build_unflags` to be stripped from every compile
/// command issued by this compiler. Consumed by the default `compile_c` /
/// `compile_cpp` impls via `Compiler::build_unflags`, so the
/// framework-level flags (not just user flags) are filtered too.
/// See FastLED/fbuild#37.
pub fn with_build_unflags(mut self, build_unflags: Vec<String>) -> Self {
self.build_unflags = build_unflags;
self
}

/// Build common compiler flags from the MCU config.
fn common_flags(&self) -> Vec<String> {
let mut flags = self.mcu_config.compiler_flags.common.clone();
Expand Down Expand Up @@ -135,6 +151,10 @@ impl Compiler for Esp32Compiler {
)
}

fn build_unflags(&self) -> &[String] {
&self.build_unflags
}

fn gcc_path(&self) -> &Path {
&self.gcc_path
}
Expand Down Expand Up @@ -278,6 +298,54 @@ mod tests {
assert!(content.contains("-I/path/to/include/199"));
}

/// Regression guard for FastLED/fbuild#37: with `build_unflags`
/// populated, a framework-contributed flag (e.g. `-std=gnu++2b`)
/// must be removed from the effective compile line. The trait's
/// default `compile_c`/`compile_cpp` route through
/// `Compiler::build_unflags` → `apply_compile_unflags`, so we can
/// verify by checking that the ESP32 compiler reports the
/// configured unflags back and that removing the framework default
/// via `remove_unflagged_tokens` leaves the right residue.
#[test]
fn with_build_unflags_exposes_them_via_trait_method() {
let compiler = test_compiler("esp32c6")
.with_build_unflags(vec!["-std=gnu++2b".to_string(), "-Os".to_string()]);
assert_eq!(
Compiler::build_unflags(&compiler),
&["-std=gnu++2b".to_string(), "-Os".to_string()]
);
}

/// Default trait impl returns an empty slice when `with_build_unflags`
/// was never called — guarantees zero behavior change for callers
/// that haven't opted in.
#[test]
fn default_build_unflags_is_empty() {
let compiler = test_compiler("esp32c6");
assert!(Compiler::build_unflags(&compiler).is_empty());
}

/// End-to-end check that the unflags set is actually applied to the
/// platform-level `cpp_flags()` when routed through the trait's
/// default compile path. We can't invoke the real compile (no
/// toolchain in tests) but we can mirror the filter the default
/// impl uses and confirm it drops the flag.
#[test]
fn configured_unflags_strip_framework_cpp_flag() {
let compiler =
test_compiler("esp32c6").with_build_unflags(vec!["-std=gnu++2b".to_string()]);
let mut flags = compiler.cpp_flags();
assert!(
flags.contains(&"-std=gnu++2b".to_string()),
"precondition: framework provides -std=gnu++2b"
);
crate::pipeline::remove_unflagged_tokens(&mut flags, Compiler::build_unflags(&compiler));
assert!(
!flags.contains(&"-std=gnu++2b".to_string()),
"unflags must strip framework-contributed -std=gnu++2b"
);
}

#[test]
fn test_mbedtls_compat_defines_in_flags() {
let compiler = test_compiler("esp32c6");
Expand Down
12 changes: 8 additions & 4 deletions crates/fbuild-build/src/esp32/orchestrator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,8 @@ impl BuildOrchestrator for Esp32Orchestrator {
params.profile,
params.verbose,
build_dir.join("tmp"),
);
)
.with_build_unflags(ctx.build_unflags.clone());
// Apply user build_flags to library compilation (matching PlatformIO behavior).
// User flags like -std=gnu++2a replace the MCU config's -std=gnu++2b.
let c_flags = apply_overlay_flags(&temp_compiler.c_flags(), &user_overlay, "dummy.c");
Expand Down Expand Up @@ -615,7 +616,8 @@ impl BuildOrchestrator for Esp32Orchestrator {
params.profile,
params.verbose,
build_dir.join("tmp"),
);
)
.with_build_unflags(ctx.build_unflags.clone());
let p_c_flags = apply_overlay_flags(&p_compiler.c_flags(), &src_overlay, "dummy.c");
let p_cpp_flags =
apply_overlay_flags(&p_compiler.cpp_flags(), &src_overlay, "dummy.cpp");
Expand Down Expand Up @@ -699,7 +701,8 @@ impl BuildOrchestrator for Esp32Orchestrator {
params.profile,
params.verbose,
build_dir.join("tmp"),
);
)
.with_build_unflags(ctx.build_unflags.clone());
let fw_c_flags =
apply_overlay_flags(&fw_compiler.c_flags(), &user_overlay, "dummy.c");
let fw_cpp_flags =
Expand Down Expand Up @@ -856,7 +859,8 @@ impl BuildOrchestrator for Esp32Orchestrator {
params.profile,
params.verbose,
build_dir.join("tmp"),
);
)
.with_build_unflags(ctx.build_unflags.clone());
let jobs = crate::parallel::effective_jobs(params.jobs);
tracing::info!("parallel compilation: {} jobs", jobs);

Expand Down
17 changes: 16 additions & 1 deletion crates/fbuild-build/src/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ pub struct BuildContext {
pub project_compile_overlay: LanguageExtraFlags,
pub overlay_link_flags: Vec<String>,
pub overlay_link_libs: Vec<String>,
/// Tokens from PlatformIO `build_unflags` to strip from the effective
/// compile command. Already applied to `user_flags` / `src_flags` /
/// `overlay_link_flags` by `BuildContext::new`; orchestrators can pass
/// this to their platform compiler (via e.g. `with_build_unflags`) to
/// also filter framework/toolchain-contributed flags. See
/// FastLED/fbuild#37.
pub build_unflags: Vec<String>,
}

impl BuildContext {
Expand Down Expand Up @@ -153,6 +160,7 @@ impl BuildContext {
project_compile_overlay: overlay.project_compile,
overlay_link_flags,
overlay_link_libs: overlay.link.libs,
build_unflags,
})
}
}
Expand Down Expand Up @@ -199,7 +207,14 @@ fn apply_debug_build_type(
(user_flags, src_flags, link_flags)
}

fn remove_unflagged_tokens(flags: &mut Vec<String>, build_unflags: &[String]) {
/// Remove tokens listed in `build_unflags` from `flags` in place, using
/// PlatformIO-compatible semantics: exact token matches and flag-value
/// pair matches for options that take values (like `-include`, `-D`).
/// Public so platform compilers can apply it to the full effective flag
/// set — framework + toolchain + user — not just the user-facing scopes
/// already handled by `apply_build_unflags` in `BuildContext::new`.
/// See FastLED/fbuild#37.
pub fn remove_unflagged_tokens(flags: &mut Vec<String>, build_unflags: &[String]) {
let mut i = 0;
while i < build_unflags.len() {
let token = &build_unflags[i];
Expand Down
Loading