Skip to content
3 changes: 3 additions & 0 deletions bench/fastled-examples/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
//!
//! Refs: #205 Phase 7 (AC#5), #218.

use std::collections::HashMap;
use std::path::{Path, PathBuf};
use std::time::Instant;

Expand Down Expand Up @@ -211,10 +212,12 @@ fn measure_example(
let kv_dir = tempfile::tempdir_in(fbuild_paths::temp_subdir("fastled-examples-bench"))?;
let kv = FileKvStore::open(kv_dir.path().join("kv"))?;

let preprocessor_defines = HashMap::new();
let inputs = CacheKeyInputs {
toolchain_triple: "teensy-arm-none-eabi",
framework_install_path: framework_root,
framework_version: "bench-fastled-examples-v1",
preprocessor_defines: &preprocessor_defines,
};

let (cold, cold_ms) = timed(|| resolve_cached(&seeds, &search_paths, libraries, &inputs, &kv))?;
Expand Down
65 changes: 31 additions & 34 deletions crates/fbuild-build-arm/src/stm32/orchestrator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use fbuild_packages::{Framework, Toolchain};

use crate::compile_database::TargetArchitecture;
use crate::framework_libs::{
library_select_kv_store, resolve_framework_library_sources,
library_select_kv_store, resolve_framework_library_sources_active,
resolve_framework_library_sources_cached,
};
use crate::generic_arm::{ArmCompiler, ArmLinker};
Expand Down Expand Up @@ -146,6 +146,33 @@ impl BuildOrchestrator for Stm32Orchestrator {
// STM32duino only exposes bundled libraries via this framework-level
// discovery (PlatformIO's LDF does the same for `framework = arduino`).
let framework_libs = framework.get_framework_libraries();
let mut mcu_config =
super::mcu_config::get_stm32_config_for_mcu(&ctx.board.mcu.to_lowercase())?;
// Extract MCU family from variant path (e.g. "STM32F1xx" from
// "STM32F1xx/F103C8T_..."). LDF must see precisely the same defines
// as the compiler so it does not discard a conditional dependency.
let family = resolved_variant.split('/').next().unwrap_or("STM32F1xx");
let mut defines = ctx.board.get_defines();
defines.extend(mcu_config.defines_map());
if let Some(board_define) = framework_props
.as_ref()
.and_then(|props| props.get("board"))
{
defines.insert(format!("ARDUINO_{board_define}"), "1".to_string());
}
defines.insert(family.to_string(), "1".to_string());
let generic_board = stm32_generic_board_define(&ctx.board.mcu);
defines.insert(format!("ARDUINO_{generic_board}"), "1".to_string());
defines.insert("USE_HAL_DRIVER".to_string(), "1".to_string());
defines.insert("USE_FULL_LL_DRIVER".to_string(), "1".to_string());
defines.insert(
"VARIANT_H".to_string(),
format!("\\\"{}\\\"", selected_variant.header),
);
// UART HAL is disabled by default in stm32yyxx_hal_conf.h; enable it
// so WSerial can create Serial. USB PCD is needed by USB variants.
defines.insert("HAL_UART_MODULE_ENABLED".to_string(), "1".to_string());
defines.insert("HAL_PCD_MODULE_ENABLED".to_string(), "1".to_string());
// WHY: STM32duino targets every Cortex-M family from M0 (F0xx) up
// through M7 (H7xx) but the toolchain triple is constant
// (`arm-none-eabi`). The cache key already includes
Expand All @@ -159,6 +186,7 @@ impl BuildOrchestrator for Stm32Orchestrator {
toolchain_triple: "stm32-arm-none-eabi",
framework_install_path: &framework_info.install_path,
framework_version: &framework_info.version,
preprocessor_defines: &defines,
};
resolve_framework_library_sources_cached(
&framework_libs,
Expand All @@ -168,10 +196,11 @@ impl BuildOrchestrator for Stm32Orchestrator {
store,
)
}
None => resolve_framework_library_sources(
None => resolve_framework_library_sources_active(
&framework_libs,
&params.project_dir,
&ctx.src_dir,
&defines,
),
};
if !framework_library_sources.is_empty() {
Expand All @@ -190,11 +219,6 @@ impl BuildOrchestrator for Stm32Orchestrator {
);

// 6. Build include dirs + compiler
// Extract MCU family from variant path (e.g. "STM32F1xx" from "STM32F1xx/F103C8T_...")
let family = resolved_variant.split('/').next().unwrap_or("STM32F1xx");

let mut mcu_config =
super::mcu_config::get_stm32_config_for_mcu(&ctx.board.mcu.to_lowercase())?;
// STM32duino linker scripts reference LD_MAX_SIZE, LD_MAX_DATA_SIZE, and
// LD_FLASH_OFFSET as symbols. Provide them via --defsym from board config.
let max_flash = ctx.board.max_flash.unwrap_or(65536);
Expand All @@ -208,33 +232,6 @@ impl BuildOrchestrator for Stm32Orchestrator {
mcu_config
.linker_flags
.push("-Wl,--defsym=LD_FLASH_OFFSET=0".to_string());
let mut defines = ctx.board.get_defines();
defines.extend(mcu_config.defines_map());
if let Some(board_define) = framework_props
.as_ref()
.and_then(|props| props.get("board"))
{
defines.insert(format!("ARDUINO_{board_define}"), "1".to_string());
}
// STM32duino's stm32_def.h checks for STM32YYxx (e.g. STM32F1xx). The board
// JSON extra_flags may only have STM32F1, so ensure the full family define.
defines.insert(family.to_string(), "1".to_string());
// STM32duino variant sources are guarded by ARDUINO_GENERIC_<MCU> defines.
// Derive from the MCU name: stm32f103c8t6 → ARDUINO_GENERIC_F103C8TX
let generic_board = stm32_generic_board_define(&ctx.board.mcu);
defines.insert(format!("ARDUINO_{generic_board}"), "1".to_string());
// STM32duino requires these defines for HAL/LL and variant header resolution.
defines.insert("USE_HAL_DRIVER".to_string(), "1".to_string());
defines.insert("USE_FULL_LL_DRIVER".to_string(), "1".to_string());
defines.insert(
"VARIANT_H".to_string(),
format!("\\\"{}\\\"", selected_variant.header),
);
// UART HAL module is disabled by default in stm32yyxx_hal_conf.h — enable it
// so WSerial.h can create the Serial instance.
defines.insert("HAL_UART_MODULE_ENABLED".to_string(), "1".to_string());
defines.insert("HAL_PCD_MODULE_ENABLED".to_string(), "1".to_string());

// Build include dirs manually (can't use get_include_paths because
// STM32duino core dir is "arduino", not the board JSON's "stm32")
let mut include_dirs = vec![core_dir.clone(), variant_dir.clone()];
Expand Down
10 changes: 8 additions & 2 deletions crates/fbuild-build-arm/src/teensy/orchestrator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use crate::build_fingerprint::{
use crate::compile_database::TargetArchitecture;
use crate::compiler::Compiler as _;
use crate::framework_libs::{
library_select_kv_store, resolve_framework_library_sources,
library_select_kv_store, resolve_framework_library_sources_active,
resolve_framework_library_sources_cached,
};
use crate::pipeline;
Expand Down Expand Up @@ -185,6 +185,10 @@ impl BuildOrchestrator for TeensyOrchestrator {
.retain(|p| p.file_name().map(|f| f != "Blink.cc").unwrap_or(true));

let framework_libs = framework.get_framework_libraries();
let ldf_mcu_config =
super::mcu_config::get_teensy_config_for_mcu(&ctx.board.mcu.to_lowercase())?;
let mut ldf_defines = ctx.board.get_defines();
ldf_defines.extend(ldf_mcu_config.defines_map());
// WHY: Teensy 3.x/4.x and TeensyLC all share teensyduino's
// arm-none-eabi toolchain — a single stable triple covers every
// board this orchestrator handles. The triple feeds the cache key
Expand All @@ -197,6 +201,7 @@ impl BuildOrchestrator for TeensyOrchestrator {
toolchain_triple: "teensy-arm-none-eabi",
framework_install_path: &framework_info.install_path,
framework_version: &framework_info.version,
preprocessor_defines: &ldf_defines,
};
resolve_framework_library_sources_cached(
&framework_libs,
Expand All @@ -206,10 +211,11 @@ impl BuildOrchestrator for TeensyOrchestrator {
store,
)
}
None => resolve_framework_library_sources(
None => resolve_framework_library_sources_active(
&framework_libs,
&params.project_dir,
&ctx.src_dir,
&ldf_defines,
),
};
if !framework_library_sources.is_empty() {
Expand Down
130 changes: 115 additions & 15 deletions crates/fbuild-build-engine/src/framework_libs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@
//! framework libraries (FNET/Snooze/RadioHead/mbedtls on teensyLC, for
//! example) stay out of the compile set. See FastLED/fbuild#205.

use std::collections::HashSet;
use std::collections::{HashMap, HashSet};
use std::path::{Path, PathBuf};
use std::sync::OnceLock;

use fbuild_library_select::cache::{CacheKeyInputs, FileKvStore, resolve_cached};
use fbuild_library_select::resolve as resolve_library_selection;
use fbuild_library_select::{
resolve as resolve_library_selection, resolve_active as resolve_active_library_selection,
};
use fbuild_packages::library::FrameworkLibrary;
use walkdir::{DirEntry, WalkDir};

Expand All @@ -32,6 +34,20 @@ pub fn resolve_framework_library_sources(
resolve_framework_library_sources_from_libraries(&filtered, &roots)
}

/// Resolve framework libraries using active preprocessor branches only.
pub fn resolve_framework_library_sources_active(
libraries: &[FrameworkLibrary],
project_dir: &Path,
src_dir: &Path,
defines: &HashMap<String, String>,
) -> Vec<PathBuf> {
let roots = framework_include_scan_roots(project_dir, src_dir);
let filtered = filter_framework_libs_shadowed_by_project(libraries, &roots);
let seeds = collect_project_seeds(&roots);
let search_paths = project_search_paths(&roots);
resolve_active_library_selection(&seeds, &search_paths, &filtered, defines).source_files
}

/// Drop framework libraries whose primary header (`<lib_name>.h`) is
/// shadowed by a same-basename header anywhere under the supplied
/// `shadowing_roots`. See FastLED/fbuild#263.
Expand Down Expand Up @@ -174,7 +190,7 @@ pub fn resolve_framework_library_sources_from_libraries(
}

let seeds = collect_project_seeds(roots);
let search_paths: Vec<PathBuf> = roots.to_vec();
let search_paths = project_search_paths(roots);
let selection = resolve_library_selection(&seeds, &search_paths, libraries);

for name in &selection.required_libraries {
Expand Down Expand Up @@ -239,7 +255,7 @@ pub(crate) fn resolve_framework_library_sources_cached_with_hit(
}

let seeds = collect_project_seeds(&roots);
let search_paths: Vec<PathBuf> = roots.clone();
let search_paths = project_search_paths(&roots);

match resolve_cached(&seeds, &search_paths, &filtered, key_inputs, store) {
Ok(cached) => {
Expand All @@ -266,7 +282,12 @@ pub(crate) fn resolve_framework_library_sources_cached_with_hit(
"library-select cache backend error; falling back to uncached resolve"
);
(
resolve_framework_library_sources_from_libraries(&filtered, &roots),
resolve_framework_library_sources_active(
&filtered,
project_dir,
src_dir,
key_inputs.preprocessor_defines,
),
false,
)
}
Expand Down Expand Up @@ -333,13 +354,39 @@ fn push_existing_unique(roots: &mut Vec<PathBuf>, path: PathBuf) {
}
}

/// Collect every source file under each root as a walker seed. Headers are
/// intentionally included so libraries referenced only from a `.h` in the
/// project tree still get picked up.
/// Include search paths for the project and its local Arduino libraries.
///
/// Local libraries live under `lib/<name>/` (or `lib/<name>/src/`), but the
/// `lib/` directory itself cannot resolve `<FastLED.h>`. Add each library's
/// public root while retaining the project roots ahead of framework libraries.
fn project_search_paths(roots: &[PathBuf]) -> Vec<PathBuf> {
let mut paths = roots.to_vec();
for root in roots {
if !is_library_root(root) {
continue;
}
let Ok(entries) = std::fs::read_dir(root) else {
continue;
};
for entry in entries.flatten() {
let dir = entry.path();
if !dir.is_dir() {
continue;
}
push_existing_unique(&mut paths, dir.clone());
push_existing_unique(&mut paths, dir.join("src"));
}
}
paths
}

/// Collect translation units as walker seeds. Headers must be reached through
/// the sketch's transitive include graph; scanning every header under `lib/`
/// turns inactive library code into false framework-library dependencies.
fn collect_project_seeds(roots: &[PathBuf]) -> Vec<PathBuf> {
let mut seeds = Vec::new();
for root in roots {
if !root.exists() {
if !root.exists() || is_library_root(root) {
continue;
}
for entry in WalkDir::new(root)
Expand All @@ -350,14 +397,21 @@ fn collect_project_seeds(roots: &[PathBuf]) -> Vec<PathBuf> {
if !entry.file_type().is_file() {
continue;
}
if is_source_or_header_file(entry.path()) {
if is_translation_unit(entry.path()) {
seeds.push(entry.path().to_path_buf());
}
}
}
seeds
}

fn is_library_root(path: &Path) -> bool {
path.file_name()
.and_then(|name| name.to_str())
.map(|name| name.eq_ignore_ascii_case("lib"))
.unwrap_or(false)
}

Comment on lines +400 to +414

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy lift

Local library .cpp dependencies are silently ignored (and logic is duplicated).

Both files duplicate collect_project_seeds, which completely skips lib/ roots (is_library_root). By entirely excluding the lib/ directory, translation units (e.g., .cpp files) inside local libraries are dropped from the walker seeds. If an active local library includes a framework header like <SPI.h> in its .cpp file but not in its .h file, the framework library will never be evaluated or selected by the walker, resulting in linker errors (undefined references).

Additionally, the path and seed resolution logic is exactly duplicated between the build engine and the CLI.

  • crates/fbuild-build-engine/src/framework_libs.rs#L398-L412: Fix the LDF logic to ensure .cpp files of active local libraries are correctly scanned for dependencies (e.g., by seeding dynamically from discovered local libraries, or avoiding wholesale pruning of lib/ translation units), and extract these path/seed helpers into a shared, public module.
  • crates/fbuild-cli/src/lib_select.rs#L317-L337: Import the shared, corrected helpers from the build engine instead of duplicating the logic.
📍 Affects 2 files
  • crates/fbuild-build-engine/src/framework_libs.rs#L398-L412 (this comment)
  • crates/fbuild-cli/src/lib_select.rs#L317-L337
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/fbuild-build-engine/src/framework_libs.rs` around lines 398 - 412,
Update the shared seed/path-resolution logic around collect_project_seeds and
is_library_root so translation units in active local lib directories are scanned
for framework dependencies instead of being pruned wholesale; move these
corrected helpers into a public build-engine module. In
crates/fbuild-build-engine/src/framework_libs.rs#L398-L412, apply the shared
implementation; in crates/fbuild-cli/src/lib_select.rs#L317-L337, remove the
duplicate logic and import the public helpers.

fn should_scan_entry(entry: &DirEntry) -> bool {
let name = entry.file_name().to_string_lossy().to_lowercase();
!matches!(
Expand All @@ -376,16 +430,13 @@ fn should_scan_entry(entry: &DirEntry) -> bool {
)
}

fn is_source_or_header_file(path: &Path) -> bool {
fn is_translation_unit(path: &Path) -> bool {
let ext = path
.extension()
.and_then(|ext| ext.to_str())
.unwrap_or_default()
.to_lowercase();
matches!(
ext.as_str(),
"c" | "cpp" | "cc" | "cxx" | "s" | "ino" | "h" | "hh" | "hpp" | "hxx"
)
matches!(ext.as_str(), "c" | "cpp" | "cc" | "cxx" | "s" | "ino")
}

#[cfg(test)]
Expand Down Expand Up @@ -539,6 +590,53 @@ mod tests {
assert_eq!(sources, vec![spi_dir.join("SPI.cpp")]);
}

#[test]
fn inactive_local_library_header_cannot_select_framework_library() {
// FastLED/fbuild#1094: a header anywhere under project lib/ used to
// become an independent seed. Its inactive include then selected a
// framework library even though the sketch could not reach it.
let tmp = tempfile::TempDir::new().unwrap();
let project_src = tmp.path().join("project").join("src");
let project_lib = tmp.path().join("project").join("lib");
let fastled = project_lib.join("FastLED");
std::fs::create_dir_all(&project_src).unwrap();
std::fs::create_dir_all(&fastled).unwrap();
std::fs::write(project_src.join("main.cpp"), "#include <FastLED.h>\n").unwrap();
std::fs::write(fastled.join("FastLED.h"), "#include <SPI.h>\n").unwrap();
std::fs::write(fastled.join("inactive_audio.h"), "#include <Audio.h>\n").unwrap();

let spi_dir = tmp.path().join("framework").join("libraries").join("SPI");
std::fs::create_dir_all(&spi_dir).unwrap();
std::fs::write(spi_dir.join("SPI.h"), "").unwrap();
std::fs::write(spi_dir.join("SPI.cpp"), "").unwrap();

let audio_dir = tmp.path().join("framework").join("libraries").join("Audio");
std::fs::create_dir_all(&audio_dir).unwrap();
std::fs::write(audio_dir.join("Audio.h"), "").unwrap();
std::fs::write(audio_dir.join("Audio.cpp"), "").unwrap();

let libraries = vec![
FrameworkLibrary {
name: "Audio".to_string(),
dir: audio_dir.clone(),
include_dirs: vec![audio_dir.clone()],
source_files: vec![audio_dir.join("Audio.cpp")],
},
FrameworkLibrary {
name: "SPI".to_string(),
dir: spi_dir.clone(),
include_dirs: vec![spi_dir.clone()],
source_files: vec![spi_dir.join("SPI.cpp")],
},
];

let sources = resolve_framework_library_sources_from_libraries(
&libraries,
&[project_src, project_lib],
);
assert_eq!(sources, vec![spi_dir.join("SPI.cpp")]);
}

#[test]
fn prefers_local_library_over_framework() {
let tmp = tempfile::TempDir::new().unwrap();
Expand Down Expand Up @@ -817,10 +915,12 @@ mod tests {
}];

let framework_root = tmp.path().join("framework");
let defines = HashMap::new();
let key_inputs = CacheKeyInputs {
toolchain_triple: "test-arm-none-eabi",
framework_install_path: &framework_root,
framework_version: "0.0.0-test",
preprocessor_defines: &defines,
};

let kv = FileKvStore::open(tmp.path().join("kv")).unwrap();
Expand Down
Loading
Loading