feat(build): apply build_unflags to framework compile flags, ESP32 wired (#37)

[zackees]

Summary

Extends PlatformIO build_unflags past user-facing scopes (already handled by BuildContext::new) into framework/toolchain-contributed flags. Before, build_unflags = -std=gnu++11 in platformio.ini stripped the token from build_flags entries but left the framework's own -std=gnu++11 in place, so the compile line still carried the flag the user meant to remove.

Mechanism

• New Compiler::build_unflags() trait method with default &[] — zero behavior change for platforms that don't opt in.
• Default compile_c / compile_cpp impls route platform flags AND extra_flags through a new apply_compile_unflags helper that filters via pipeline::remove_unflagged_tokens. Early return keeps the zero-unflags path allocation-free.
• pipeline::remove_unflagged_tokens is now pub so platform code can use the same PlatformIO-compatible semantics (exact-token + value-pair removal) already used by BuildContext::new.
• BuildContext gains a build_unflags field so orchestrators can pass the set to their compiler.

ESP32 reference impl

• Esp32Compiler gets a build_unflags field and a with_build_unflags(...) builder.
• Trait override returns &self.build_unflags.
• All four Esp32Compiler::with_temp_dir(...) construction sites in esp32::orchestrator chain .with_build_unflags(ctx.build_unflags.clone()).

Other platforms remain unchanged. Follow-up PRs can wire them using the same one-line builder pattern if desired.

Context

Addresses #37 — which asked for build_unflags to apply to effective native flag scopes (compile / link / assembler), not just the user-provided build_flags layer. This PR lands the compile scope; link is already handled by the existing remove_unflagged_tokens(&mut overlay_link_flags, ...) call in BuildContext::new.

Test plan

• uv run cargo clippy --workspace --all-targets -- -D warnings clean
• uv run cargo test -p fbuild-build --lib — 447 passed (3 new)
• CI green on Linux / macOS / Windows

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• New Features

  • Added support for excluding specific compiler flags from compilation commands, enabling fine-grained control over compiler behavior across all build phases.

• Tests

  • Added unit tests for flag removal functionality.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR introduces support for the build_unflags configuration mechanism throughout the fbuild compilation framework. The feature enables stripping specific compiler flags from compilation commands, with configuration sourced from PlatformIO settings and propagated from the build context through the orchestrator into compiler instances.

Changes

Cohort / File(s)	Summary
Compiler Trait & Core Logic crates/fbuild-build/src/compiler.rs	Added build_unflags(&self) -> &[String] trait method (defaults to empty slice) and introduced apply_compile_unflags() function to filter both platform-provided and extra compiler flags through the unflag list.
ESP32 Compiler Implementation crates/fbuild-build/src/esp32/esp32_compiler.rs	Extended Esp32Compiler with new build_unflags field, added with_build_unflags() builder method, implemented the trait hook, and added three unit tests validating unflag exposure and flag stripping behavior.
Orchestrator Integration crates/fbuild-build/src/esp32/orchestrator.rs	Threaded ctx.build_unflags into four compilation phase initializations (dependency/library, project-as-library, framework built-in, and sketch compilation) via the new with_build_unflags() builder.
Build Pipeline Configuration crates/fbuild-build/src/pipeline.rs	Added build_unflags: Vec<String> field to BuildContext, populated from PlatformIOConfig::get_build_unflags(), promoted remove_unflagged_tokens() from private to public with expanded documentation, and ensured unflags are stripped from overlay link flags.

Sequence Diagram

sequenceDiagram
    participant Config as PlatformIO Config
    participant BuildCtx as BuildContext
    participant Orch as Orchestrator
    participant Compiler as Esp32Compiler
    participant Pipeline as Compile Pipeline
    
    Config->>BuildCtx: get_build_unflags()
    BuildCtx->>BuildCtx: store build_unflags
    BuildCtx->>Orch: pass ctx with build_unflags
    Orch->>Compiler: with_build_unflags(ctx.build_unflags)
    Compiler->>Compiler: store build_unflags
    Pipeline->>Compiler: compile_c/compile_cpp
    Compiler->>Compiler: apply_compile_unflags(flags, unflags)
    Compiler->>Pipeline: compile_one(filtered_flags)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• Apply PlatformIO build_unflags to the effective native flag scopes #37: This PR directly implements the build_unflags hook propagation across compiler scopes, addressing the feature request to apply build_unflags against effective native flag sets at the trait, implementation, orchestration, and context levels.

Poem

🐰 Flags stripped like carrots from the garden bed,
Unwanted -std options now quietly shed,
Through compiler and context, a pathway so clean,
Where PlatformIO's unflagging is perfectly seen! 🎯

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main feature: applying build_unflags to framework compile flags with ESP32 implementation. It accurately captures the PR's primary objective.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/issue-37-build-unflags-framework-scope

Warning

Review ran into problems

🔥 Problems

Timed out fetching pipeline failures after 30000ms

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/fbuild-build/src/esp32/orchestrator.rs (1)

544-585: ⚠️ Potential issue | 🟠 Major

Filter temp compiler flags through remove_unflagged_tokens at extraction points; .with_build_unflags(...) on temp compilers is a no-op.

The library helpers ensure_libraries_sync and compile_library_with_jobs accept raw &[String] flags and do not re-apply unflags. Since Esp32Compiler::c_flags() and cpp_flags() don't consult self.build_unflags, the .with_build_unflags(...) calls on temp compilers strip nothing. Only compile_c/compile_cpp trait methods apply them—but those are bypassed when you extract and pass flags directly.

Result: library archives and the compile_commands.json file advertise unfiltered flags while the sketch is compiled with them stripped. This risks ABI/ODR mismatches (e.g., -std=gnu++2b in archives vs. -std=gnu++20 in sketch) and confuses clangd/IDEs.

Affected paths:

• Lines 544–585: dep libs via ensure_libraries_sync
• Lines 609–667: project-as-library via compile_project_as_library
• Lines 694–810: framework built-in libs (WiFi/SPI/…) via compile_library_with_jobs
• Lines 976–1014: local lib/* via compile_library_with_jobs
• Lines 882–886 and 1045–1060: compile_commands.json via raw c_flags()/cpp_flags()

Fix: Apply pipeline::remove_unflagged_tokens right after each c_flags()/cpp_flags() extraction, before passing to helpers:

Proposed fix pattern

             let temp_compiler = Esp32Compiler::with_temp_dir(
                 ...
                 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");
-            let cpp_flags =
-                apply_overlay_flags(&temp_compiler.cpp_flags(), &user_overlay, "dummy.cpp");
+            let mut base_c = temp_compiler.c_flags();
+            let mut base_cpp = temp_compiler.cpp_flags();
+            crate::pipeline::remove_unflagged_tokens(&mut base_c, &ctx.build_unflags);
+            crate::pipeline::remove_unflagged_tokens(&mut base_cpp, &ctx.build_unflags);
+            let c_flags = apply_overlay_flags(&base_c, &user_overlay, "dummy.c");
+            let cpp_flags = apply_overlay_flags(&base_cpp, &user_overlay, "dummy.cpp");

Repeat for p_compiler (609–623), fw_compiler (694–709), local-libs (976–1014), and both generate_compile_db call sites (882–886, 1045–1060).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/fbuild-build/src/esp32/orchestrator.rs` around lines 544 - 585, The
temp compilers' c_flags()/cpp_flags() are used directly so
.with_build_unflags(...) is a no-op; call pipeline::remove_unflagged_tokens on
each extracted flag vector before passing them to library helpers (e.g., after
temp_compiler.c_flags()/cpp_flags(), p_compiler, fw_compiler and local lib
compiler extractions) and before building compile_commands.json; update the uses
around Esp32Compiler::with_temp_dir (temp_compiler), the p_compiler block
(compile_project_as_library), fw_compiler block (compile_library_with_jobs),
local lib compilation sites, and the two generate_compile_db call sites to
replace raw c_flags()/cpp_flags() with the cleaned vectors from
pipeline::remove_unflagged_tokens.

🧹 Nitpick comments (1)

crates/fbuild-build/src/esp32/orchestrator.rs (1)

501-506: Minor: re-reads build_flags from config, bypassing ctx.user_flags.

Lines 504 re-parse ctx.config.get_build_flags(&params.env_name)? instead of using ctx.user_flags, which BuildContext::new already produced with user-scope build_unflags applied. So even after the library-path fix above, the user's own build_flags feeding user_overlay here haven't had unflags applied at the source — they'll only be stripped later via the framework-flag filter, which works only for exact token matches already present in both scopes.

Not a blocker (the overlay still gets filtered for framework tokens), but it's an inconsistency worth cleaning up so one source of truth (ctx.user_flags) flows through the whole orchestrator.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/fbuild-build/src/esp32/orchestrator.rs` around lines 501 - 506,
Replace the redundant call to ctx.config.get_build_flags(&params.env_name)? with
the already-processed ctx.user_flags so the orchestrator uses the single source
of truth produced by BuildContext::new; specifically, in the block that builds
user_flags/user_build_flags (variables user_flags, user_build_flags, and where
params.extra_build_flags is extended), use ctx.user_flags as the base (and then
extend with params.extra_build_flags and prepend sdk_defines as before) so
unflags applied in BuildContext::new are preserved when computing the
user_overlay.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@crates/fbuild-build/src/esp32/orchestrator.rs`:
- Around line 544-585: The temp compilers' c_flags()/cpp_flags() are used
directly so .with_build_unflags(...) is a no-op; call
pipeline::remove_unflagged_tokens on each extracted flag vector before passing
them to library helpers (e.g., after temp_compiler.c_flags()/cpp_flags(),
p_compiler, fw_compiler and local lib compiler extractions) and before building
compile_commands.json; update the uses around Esp32Compiler::with_temp_dir
(temp_compiler), the p_compiler block (compile_project_as_library), fw_compiler
block (compile_library_with_jobs), local lib compilation sites, and the two
generate_compile_db call sites to replace raw c_flags()/cpp_flags() with the
cleaned vectors from pipeline::remove_unflagged_tokens.

---

Nitpick comments:
In `@crates/fbuild-build/src/esp32/orchestrator.rs`:
- Around line 501-506: Replace the redundant call to
ctx.config.get_build_flags(&params.env_name)? with the already-processed
ctx.user_flags so the orchestrator uses the single source of truth produced by
BuildContext::new; specifically, in the block that builds
user_flags/user_build_flags (variables user_flags, user_build_flags, and where
params.extra_build_flags is extended), use ctx.user_flags as the base (and then
extend with params.extra_build_flags and prepend sdk_defines as before) so
unflags applied in BuildContext::new are preserved when computing the
user_overlay.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 331ccb8a-5c88-43f0-866c-6936e0adb56c

📥 Commits

Reviewing files that changed from the base of the PR and between 588879e and ef0f56e.

📒 Files selected for processing (4)

• crates/fbuild-build/src/compiler.rs
• crates/fbuild-build/src/esp32/esp32_compiler.rs
• crates/fbuild-build/src/esp32/orchestrator.rs
• crates/fbuild-build/src/pipeline.rs