nxplpc: retire #576's board-JSON build_flags workaround (concrete sub-issue from #574) #587

Activity

1. fastled-project-sync

   added this to FastLED Trackeron Jun 14, 2026
2. zackees commented on Jun 14, 2026

   

   zackees

   on Jun 14, 2026

   MemberAuthor

   More actions

   Correction — my prior audit was wrong

   The body of this issue (and the entry at #586 (comment) announcing it) claimed the build_flags propagation gap was already fixed in nxplpc. It is not. I re-checked the code paths directly and PR #576's explicit note — "build_flags are not propagated by the nxplpc orchestrator today" — is still accurate.

   What I got right vs. wrong

   	Reality
   BuildContext::new populates user_flags from config.get_build_flags()	✅ correct (pipeline/context.rs:155)
   user_flags reach the sketch compile via pipeline/sequential.rs:57-67 (chained into user_overlay.common)	✅ correct
   user_flags reach the library compile via nxplpc/orchestrator.rs:247 constructing LibraryBuildEnv	❌ wrong — that struct gets c_flags: &compiler.c_flags() raw, with no user_flags overlay applied

   The library compile path is exactly where -DRELEASE=1 -DFASTLED_DISABLE_DBG=1 need to land for FastLED's fl/log/log.h (and the rest of the library) to actually disable FASTLED_FORCE_DBG. Until the LPC library compile mirrors the ESP32 fix at esp32/orchestrator/build.rs:423-425, those defines only fire in the sketch translation unit — which is why PR #576 had to bake them into lpc845brk.json's extra_flags instead.

   The real fix template — what ESP32 does

   crates/fbuild-build/src/esp32/orchestrator/build.rs:423-425:

   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");
   // ...
   let lib_env = crate::pipeline::LibraryBuildEnv {
       c_flags: &p_c_flags,
       cpp_flags: &p_cpp_flags,
       // ...
   };

   src_overlay here is the combined overlay (incl. ctx.user_flags); apply_overlay_flags folds those into the per-source flag set that ends up on every library compile invocation.

   crates/fbuild-build/src/nxplpc/orchestrator.rs:247-257 (current — broken):

   let lib_env = pipeline::LibraryBuildEnv {
       c_flags: &c_flags,        // ← raw compiler flags, no user_flags
       cpp_flags: &cpp_flags,    // ← same
       // ...
   };

   c_flags / cpp_flags here come from compiler.c_flags() / compiler.cpp_flags() directly — ctx.user_flags are never folded in. Same bug exists in avr/orchestrator.rs:262.

   Revised scope for this issue

   1. Add user_flags to LPC library compile — port the ESP32 pattern: build a src_overlay over the lib_env's c_flags / cpp_flags, fold ctx.user_flags in via apply_overlay_flags, store the result in fresh Vec<String> locals, and point LibraryBuildEnv::c_flags / cpp_flags at those.
   2. Verify with a test fixture: a tiny tests/platform/lpc845_build_flags/ project where [env:lpc845brk] build_flags = -DFROM_PLATFORMIO_INI=1 and a library source #errors if the define isn't set. Rust integration test asserts the build succeeds — this is the regression-proof for the bug.
   3. Only THEN drop the stopgap from lpc845brk.json — remove -DRELEASE=1 -DFASTLED_DISABLE_DBG=1 from the extra_flags field. Note: it's only lpc845brk.json that has these defines today; lpc845.json, lpcxpresso804.json, lpcxpresso845max.json carry just chip-identification defines.
   4. Move the LPC845-BRK bring-up defines into the consuming sketch's platformio.ini so they continue to apply where they're actually wanted.
   5. Optional same-fix for AVR — avr/orchestrator.rs:262 has the identical pattern; track separately if not in scope.

   Updated acceptance

   • LPC library compile picks up [env:*] build_flags defines — proven by the lpc845_build_flags fixture's #error not firing
   • grep -E '\-DRELEASE|FASTLED_DISABLE_DBG' crates/fbuild-config/assets/boards/json/lpc845brk.json returns no matches
   • LPC845-BRK JSON-RPC sketch still links + boots end-to-end with the defines sourced from its own platformio.ini
   • PR feat(nxplpc): find_core_root + LPC845 RELEASE defines + pin bump to ACLPC#29 #576 has a follow-up comment noting the workaround was retired here

   What stays the same

   The link to #574 (broader env-namespaced routing design) and #586 (LPC burn-down meta) are unchanged. The cleanup phase (steps 3-4 above) is the same intent the prior body described — the difference is that step 1 is also required, not optional. The empirical sign-off (LPC845-BRK JSON-RPC bringup still working with the defines moved out of board JSON) is the same end-state.

   Sorry for the noise — the previous body misled. The fix is a 5-10 line change in the orchestrator plus the test fixture, modeled directly on the ESP32 pattern.
3. zackees

   added a commit that references this issue on Jun 14, 2026

   fix(nxplpc): propagate platformio.ini build_flags to library compile (#…

   ...

   958cb72
4. zackees

   closed this as completedin #588on Jun 14, 2026
5. fastled-project-sync

   moved this to Triage in FastLED Trackeron Jun 15, 2026
6. zackees

   added a commit that references this issue on Jul 10, 2026

   feat(build): EnvNamespace + unified build_flags overlay propagation (#…

   ...

   f41a34f