From d96b2e0a1213a07145e29ad02eedba4ed8ef237f Mon Sep 17 00:00:00 2001 From: zackees Date: Sun, 23 Aug 2026 06:30:33 -0700 Subject: [PATCH] fix(ldf): seed from every compiled TU, and treat __has_include as undecidable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes FastLED/fbuild#1337. Teensyduino bundled libraries landed on the include path but their sources never reached the link line, so any TU that included them compiled and then failed with `undefined reference`. All eight Teensy workflows red on FastLED master; the same class as #1214, which the symptom outlived. ## Local library sources are dependencies too `collect_project_seeds` skipped the `lib/` root wholesale, so a local library's own translation units were never scanned. FastLED expresses its Adafruit_NeoPixel and Audio dependencies exactly there — in `.cpp` files it compiles and links — and the resolver never saw them. The rule was "seeds are project translation units only", introduced by #1094 to stop an *inactive header* under `lib/` from self-selecting a framework library. That protection is intact: headers are still never seeds. What changes is the invariant it rested on, from "sketch only" to **what compiles is what seeds** — if the compiler builds a TU, its includes are real dependencies, and the scanner disagreeing with the compiler is the bug. Layout matters, and getting it wrong breaks the invariant the other way. An Arduino 1.5 library keeps sources in `src/`, a 1.0 library at the root, and neither compiles `examples/`, `extras/` or test trees. Seeding an example sketch would have the scanner claim dependencies the build never links — #1337 mirrored — so those directories are excluded and a test pins it. ## `__has_include` is undecidable, not false Nothing `#define`s a compiler builtin, so no macro set can settle it, and answering "false" is how an include the compiler *does* take stayed invisible. It is now Unknown: the arm is scanned, and the walker's own header resolution decides — which is the same question `__has_include` asks, so scanner and compiler agree by construction. Its argument is consumed too, along with any function-like macro's, so the `<` in `FL_HAS_INCLUDE()` is not parsed as a less-than. ## Unknown-ness combines per operand Found by the counterweight test rather than by reasoning. FastLED's current guard is `defined(FASTLED_USE_ADAFRUIT_NEOPIXEL) && FL_HAS_INCLUDE()` — deliberately two signals. With a single global unknown flag the undecidable half poisoned the whole expression, so the library was selected even with the opt-in absent, putting sources on the link line that nothing references. `&&` now stays decidably false when either operand is decidably false, and `||` decidably true when either is decidably true, regardless of unknowns elsewhere. Both directions of the gate are tested. ## Also - `LDF_MODE_VERSION` 5→6: seeding semantics changed, so warm caches must not answer with the old rule. - `framework_libs.rs` was 998 LOC and would have crossed the 1000 gate; tests moved to `framework_libs_tests.rs` behind `#[path]`, the established pattern. No test content changed. - Doc comments updated where they still promised sketch-only seeding. ## Verified RED first: the reproduction fails on the old seeding, exactly as reported. Counterweights cover the mirrored risks — opt-in absent selects nothing, `examples/` never seeds, and #1094's inactive-header canary still passes. fbuild-build-engine 405, fbuild-header-scan 63, fbuild-library-select 31, workspace clippy `-D warnings`, full `dylint --all` sweep: all clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../fbuild-build-engine/src/framework_libs.rs | 586 ++------------- .../src/framework_libs_tests.rs | 688 ++++++++++++++++++ crates/fbuild-header-scan/src/scanner.rs | 85 ++- .../fbuild-header-scan/src/scanner_tests.rs | 70 ++ crates/fbuild-library-select/src/cache.rs | 2 +- 5 files changed, 914 insertions(+), 517 deletions(-) create mode 100644 crates/fbuild-build-engine/src/framework_libs_tests.rs diff --git a/crates/fbuild-build-engine/src/framework_libs.rs b/crates/fbuild-build-engine/src/framework_libs.rs index 4ce42629..b24bb4c3 100644 --- a/crates/fbuild-build-engine/src/framework_libs.rs +++ b/crates/fbuild-build-engine/src/framework_libs.rs @@ -50,9 +50,10 @@ pub fn resolve_framework_library_sources_active( /// cannot infer, which previously had no lever at all on the Teensy/STM32 /// path (FastLED/fbuild#1214). /// -/// The scan itself is unchanged: seeds are still project translation units -/// only, so #1094's "an inactive local library header must not select a -/// framework library" invariant still holds. +/// Seeds are every translation unit the build compiles — project sources and +/// local-library sources alike (FastLED/fbuild#1337). Headers are still never +/// seeds, which is what preserves #1094's "an inactive local library header +/// must not select a framework library". pub fn resolve_framework_library_sources_active_declared( libraries: &[FrameworkLibrary], project_dir: &Path, @@ -429,13 +430,31 @@ fn project_search_paths(roots: &[PathBuf]) -> Vec { 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. +/// Collect translation units as walker seeds. +/// +/// Headers are never seeds: they must be reached through some TU's include +/// graph, or an inactive header anywhere under `lib/` turns into a false +/// framework-library dependency (FastLED/fbuild#1094). +/// +/// Translation units under `lib/` *are* seeds, though, and that is a change +/// from the original sketch-only rule. A local library's `.cpp` files are +/// compiled and linked, so an include one of them makes is a real dependency +/// — FastLED expresses its Adafruit_NeoPixel and Audio dependencies exactly +/// there, and seeding only the sketch meant those libraries were on the +/// include path but never on the link line, failing all eight Teensy boards +/// with `undefined reference` (FastLED/fbuild#1337, the #1214 class). +/// +/// The invariant that replaces "sketch only" is *"what compiles is what +/// seeds"*: the scanner's view of the build has to match the compiler's, or +/// the two disagree about a dependency and the link breaks. fn collect_project_seeds(roots: &[PathBuf]) -> Vec { let mut seeds = Vec::new(); for root in roots { - if !root.exists() || is_library_root(root) { + if !root.exists() { + continue; + } + if is_library_root(root) { + collect_local_library_seeds(root, &mut seeds); continue; } for entry in WalkDir::new(root) @@ -461,6 +480,50 @@ fn is_library_root(path: &Path) -> bool { .unwrap_or(false) } +/// Seed from the translation units a local library actually compiles. +/// +/// Layout matters here, and getting it wrong breaks the invariant in the +/// other direction. An Arduino 1.5 library keeps its sources in `src/`; a 1.0 +/// library keeps them at the library root. Either way `examples/`, `extras/` +/// and test trees are **not** compiled — seeding an example sketch would make +/// the scanner claim dependencies the build never links, which is the same +/// disagreement #1337 is about, mirrored. +fn collect_local_library_seeds(lib_root: &Path, seeds: &mut Vec) { + let Ok(entries) = std::fs::read_dir(lib_root) else { + return; + }; + for entry in entries.flatten() { + let library_dir = entry.path(); + if !library_dir.is_dir() { + continue; + } + let src = library_dir.join("src"); + let scan_root = if src.is_dir() { src } else { library_dir }; + for found in WalkDir::new(&scan_root) + .into_iter() + .filter_entry(should_scan_library_entry) + .flatten() + { + if found.file_type().is_file() && is_translation_unit(found.path()) { + seeds.push(found.path().to_path_buf()); + } + } + } +} + +/// [`should_scan_entry`] plus the directories a library ships but never +/// compiles. +fn should_scan_library_entry(entry: &DirEntry) -> bool { + if !should_scan_entry(entry) { + return false; + } + let name = entry.file_name().to_string_lossy().to_lowercase(); + !matches!( + name.as_str(), + "examples" | "example" | "extras" | "test" | "tests" | "docs" + ) +} + fn should_scan_entry(entry: &DirEntry) -> bool { let name = entry.file_name().to_string_lossy().to_lowercase(); !matches!( @@ -489,510 +552,5 @@ fn is_translation_unit(path: &Path) -> bool { } #[cfg(test)] -mod tests { - use super::*; - - #[test] - fn resolves_libraries_from_project_includes() { - let tmp = tempfile::TempDir::new().unwrap(); - let project_src = tmp.path().join("project").join("src"); - std::fs::create_dir_all(&project_src).unwrap(); - std::fs::write( - project_src.join("main.cpp"), - "#include \n#include \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 octo_dir = tmp - .path() - .join("framework") - .join("libraries") - .join("OctoWS2811"); - std::fs::create_dir_all(&octo_dir).unwrap(); - std::fs::write(octo_dir.join("OctoWS2811.h"), "").unwrap(); - std::fs::write(octo_dir.join("OctoWS2811.cpp"), "").unwrap(); - std::fs::write(octo_dir.join("OctoWS2811_imxrt.cpp"), "").unwrap(); - - let libraries = vec![ - FrameworkLibrary { - name: "OctoWS2811".to_string(), - dir: octo_dir.clone(), - include_dirs: vec![octo_dir.clone()], - source_files: vec![ - octo_dir.join("OctoWS2811.cpp"), - octo_dir.join("OctoWS2811_imxrt.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 mut sources = resolve_framework_library_sources_from_libraries( - &libraries, - std::slice::from_ref(&project_src), - ); - sources.sort(); - - let mut expected = vec![ - octo_dir.join("OctoWS2811.cpp"), - octo_dir.join("OctoWS2811_imxrt.cpp"), - spi_dir.join("SPI.cpp"), - ]; - expected.sort(); - assert_eq!(sources, expected); - } - - #[test] - fn follows_transitive_includes() { - let tmp = tempfile::TempDir::new().unwrap(); - let project_src = tmp.path().join("project").join("src"); - std::fs::create_dir_all(&project_src).unwrap(); - std::fs::write(project_src.join("main.cpp"), "#include \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 wrapper_dir = tmp - .path() - .join("framework") - .join("libraries") - .join("NeedsSpi"); - std::fs::create_dir_all(&wrapper_dir).unwrap(); - std::fs::write(wrapper_dir.join("NeedsSpi.h"), "#include \n").unwrap(); - std::fs::write(wrapper_dir.join("NeedsSpi.cpp"), "").unwrap(); - - let libraries = vec![ - FrameworkLibrary { - name: "NeedsSpi".to_string(), - dir: wrapper_dir.clone(), - include_dirs: vec![wrapper_dir.clone()], - source_files: vec![wrapper_dir.join("NeedsSpi.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 mut sources = resolve_framework_library_sources_from_libraries( - &libraries, - std::slice::from_ref(&project_src), - ); - sources.sort(); - - let mut expected = vec![wrapper_dir.join("NeedsSpi.cpp"), spi_dir.join("SPI.cpp")]; - expected.sort(); - assert_eq!(sources, expected); - } - - #[test] - fn unrelated_library_not_selected() { - // Regression guard for #204: libraries whose headers are never - // referenced must not appear in the compile set. - let tmp = tempfile::TempDir::new().unwrap(); - let project_src = tmp.path().join("project").join("src"); - std::fs::create_dir_all(&project_src).unwrap(); - std::fs::write(project_src.join("main.cpp"), "#include \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 fnet_dir = tmp.path().join("framework").join("libraries").join("FNET"); - std::fs::create_dir_all(&fnet_dir).unwrap(); - std::fs::write(fnet_dir.join("fnet.h"), "").unwrap(); - std::fs::write(fnet_dir.join("fnet.cpp"), "").unwrap(); - - let libraries = vec![ - FrameworkLibrary { - name: "FNET".to_string(), - dir: fnet_dir.clone(), - include_dirs: vec![fnet_dir.clone()], - source_files: vec![fnet_dir.join("fnet.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, - std::slice::from_ref(&project_src), - ); - 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 \n").unwrap(); - std::fs::write(fastled.join("FastLED.h"), "#include \n").unwrap(); - std::fs::write(fastled.join("inactive_audio.h"), "#include \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(); - let project_src = tmp.path().join("project").join("src"); - let project_lib = tmp - .path() - .join("project") - .join("lib") - .join("FastLED") - .join("src"); - std::fs::create_dir_all(&project_src).unwrap(); - std::fs::create_dir_all(&project_lib).unwrap(); - std::fs::write(project_src.join("main.cpp"), "#include \n").unwrap(); - std::fs::write(project_lib.join("FastLED.h"), "#include \n").unwrap(); - std::fs::write(project_lib.join("FastLED.cpp"), "").unwrap(); - - let framework_fastled_dir = tmp - .path() - .join("framework") - .join("libraries") - .join("FastLED"); - std::fs::create_dir_all(&framework_fastled_dir).unwrap(); - std::fs::write(framework_fastled_dir.join("FastLED.h"), "").unwrap(); - std::fs::write(framework_fastled_dir.join("FastLED.cpp"), "").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 libraries = vec![ - FrameworkLibrary { - name: "FastLED".to_string(), - dir: framework_fastled_dir.clone(), - include_dirs: vec![framework_fastled_dir.clone()], - source_files: vec![framework_fastled_dir.join("FastLED.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 roots = vec![project_src, project_lib]; - let sources = resolve_framework_library_sources_from_libraries(&libraries, &roots); - - assert_eq!(sources, vec![spi_dir.join("SPI.cpp")]); - } - - /// Regression for FastLED/fbuild#263 — case A: when the user's project - /// IS the library (FastLED's own source tree has `src/FastLED.h` - /// directly under one of the walker's roots), the framework's bundled - /// FastLED at `cores/teensy4/libraries/FastLED/` must not get selected. - /// This case works in the LDF resolver today because path-prefix - /// attribution finds `project/src/FastLED.h` first. - #[test] - fn project_is_the_library_does_not_pull_in_bundled_copy() { - let tmp = tempfile::TempDir::new().unwrap(); - - let project_src = tmp.path().join("project").join("src"); - std::fs::create_dir_all(&project_src).unwrap(); - std::fs::write(project_src.join("FastLED.h"), "// the real FastLED\n").unwrap(); - std::fs::write(project_src.join("FastLED.cpp"), "// user impl\n").unwrap(); - std::fs::write( - project_src.join("example_main.cpp"), - "#include \n", - ) - .unwrap(); - - let bundled_fastled_dir = tmp - .path() - .join("framework") - .join("libraries") - .join("FastLED"); - std::fs::create_dir_all(&bundled_fastled_dir).unwrap(); - std::fs::write( - bundled_fastled_dir.join("FastLED.h"), - "// bundled (stale) FastLED\n", - ) - .unwrap(); - std::fs::write(bundled_fastled_dir.join("FastLED.cpp"), "// bundled impl\n").unwrap(); - - let libraries = vec![FrameworkLibrary { - name: "FastLED".to_string(), - dir: bundled_fastled_dir.clone(), - include_dirs: vec![bundled_fastled_dir.clone()], - source_files: vec![bundled_fastled_dir.join("FastLED.cpp")], - }]; - - let sources = resolve_framework_library_sources_from_libraries( - &libraries, - std::slice::from_ref(&project_src), - ); - - assert!( - sources.is_empty(), - "bundled FastLED must NOT be selected when the project owns FastLED.h \ - directly under src/ — see #263. Got: {sources:?}" - ); - } - - /// Regression for FastLED/fbuild#263 — case B: the user's project owns - /// FastLED.h at a path that is NOT one of the walker roots passed to - /// the resolver (e.g. `/src/FastLED.h` while the resolver only - /// sees `/tests/platform/teensy41/src/`). The walker then can - /// only find FastLED.h via the framework's bundled - /// `cores/teensy4/libraries/FastLED/` include dir, mis-attributes the - /// include to the bundled library, and pulls its sources into the - /// build set — duplicate-symbol time. The fix in `framework_libs.rs` - /// drops framework libraries whose primary header is shadowed by a - /// project header even when the project header isn't first in the - /// search order. - #[test] - fn example_only_root_does_not_pull_in_bundled_fastled_when_user_owns_fastled() { - let tmp = tempfile::TempDir::new().unwrap(); - - // The repo: user's local FastLED lives at /src/, which is - // NOT among the resolver's roots for the per-example build. - let repo_src = tmp.path().join("repo").join("src"); - std::fs::create_dir_all(&repo_src).unwrap(); - std::fs::write(repo_src.join("FastLED.h"), "// the real FastLED\n").unwrap(); - std::fs::write(repo_src.join("FastLED.cpp"), "// user impl\n").unwrap(); - - // The per-example project root the resolver actually sees. - let example_src = tmp - .path() - .join("repo") - .join("tests") - .join("platform") - .join("teensy41") - .join("src"); - std::fs::create_dir_all(&example_src).unwrap(); - std::fs::write( - example_src.join("example_main.cpp"), - "#include \n", - ) - .unwrap(); - - // Framework bundles its own FastLED. - let bundled_fastled_dir = tmp - .path() - .join("framework") - .join("libraries") - .join("FastLED"); - std::fs::create_dir_all(&bundled_fastled_dir).unwrap(); - std::fs::write(bundled_fastled_dir.join("FastLED.h"), "// bundled\n").unwrap(); - std::fs::write(bundled_fastled_dir.join("FastLED.cpp"), "// bundled impl\n").unwrap(); - - let libraries = vec![FrameworkLibrary { - name: "FastLED".to_string(), - dir: bundled_fastled_dir.clone(), - include_dirs: vec![bundled_fastled_dir.clone()], - source_files: vec![bundled_fastled_dir.join("FastLED.cpp")], - }]; - - // The fbuild build pipeline calls `local_overridden_framework_libs` - // with both the example root AND the repo's actual src/ as - // shadowing roots. The repo src/FastLED.h shadows the framework's - // FastLED → framework library is filtered out before the resolver - // ever sees it. - let shadowing_roots = vec![example_src.clone(), repo_src.clone()]; - let filtered = filter_framework_libs_shadowed_by_project(&libraries, &shadowing_roots); - - // Resolver runs on the FILTERED library set. - let sources = resolve_framework_library_sources_from_libraries( - &filtered, - std::slice::from_ref(&example_src), - ); - - assert!( - sources.is_empty(), - "bundled FastLED must be filtered out because the user's repo owns \ - FastLED.h even when it's not in the per-example walker roots — see #263. \ - Got: {sources:?}" - ); - } - - /// Regression for FastLED/fbuild#284 — a nested project header whose - /// basename happens to collide with a framework library's primary - /// header must NOT trigger the shadowing filter. FastLED ships - /// `lib/FastLED/fl/channels/spi.h`, which is includeable only as - /// ``, never as ``. The framework's `SPI` - /// library must therefore stay in the build set, otherwise every - /// Teensy 4.x example fails at link with `undefined reference to - /// SPIClass::*`. - /// - /// At the same time, the existing `#263` behaviour for headers - /// reachable as bare `` (e.g. `lib/FastLED/noise.h` or - /// `project/src/FastLED.h`) must still drop the matching framework - /// library. - #[test] - fn nested_basename_does_not_shadow_framework_library() { - let tmp = tempfile::TempDir::new().unwrap(); - - // PIO project layout: lib/FastLED/ contains FastLED's source - // tree directly (1.0 flat layout — no src/ subdir). spi.h is - // nested deep, noise.h sits at FastLED's include root. - let project_dir = tmp.path().join("project"); - let lib_dir = project_dir.join("lib"); - let fastled_dir = lib_dir.join("FastLED"); - let nested_spi_dir = fastled_dir.join("fl").join("channels"); - std::fs::create_dir_all(&nested_spi_dir).unwrap(); - std::fs::write(nested_spi_dir.join("spi.h"), "// FastLED internal\n").unwrap(); - std::fs::write(fastled_dir.join("FastLED.h"), "").unwrap(); - std::fs::write(fastled_dir.join("noise.h"), "// shadows framework Noise\n").unwrap(); - - let src_dir = project_dir.join("src"); - std::fs::create_dir_all(&src_dir).unwrap(); - - // Framework libs: SPI (must SURVIVE the filter) and Noise (must - // be dropped because the project owns noise.h at the FastLED - // library include root). - 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 noise_dir = tmp.path().join("framework").join("libraries").join("Noise"); - std::fs::create_dir_all(&noise_dir).unwrap(); - std::fs::write(noise_dir.join("noise.h"), "").unwrap(); - std::fs::write(noise_dir.join("noise.cpp"), "").unwrap(); - - let libraries = vec![ - FrameworkLibrary { - name: "Noise".to_string(), - dir: noise_dir.clone(), - include_dirs: vec![noise_dir.clone()], - source_files: vec![noise_dir.join("noise.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 shadowing_roots = framework_include_scan_roots(&project_dir, &src_dir); - let filtered = filter_framework_libs_shadowed_by_project(&libraries, &shadowing_roots); - - let surviving: Vec<&str> = filtered.iter().map(|l| l.name.as_str()).collect(); - assert!( - surviving.contains(&"SPI"), - "framework SPI must SURVIVE — nested fl/channels/spi.h is not reachable \ - as and must not trigger the shadowing filter — see #284. \ - Surviving libraries: {surviving:?}" - ); - assert!( - !surviving.contains(&"Noise"), - "framework Noise must be dropped — lib/FastLED/noise.h sits at the \ - FastLED library include root and is reachable as — see #263. \ - Surviving libraries: {surviving:?}" - ); - } - - #[test] - fn cached_resolution_round_trips_through_file_store() { - let tmp = tempfile::TempDir::new().unwrap(); - let project_dir = tmp.path().join("project"); - let src_dir = project_dir.join("src"); - std::fs::create_dir_all(&src_dir).unwrap(); - std::fs::write(src_dir.join("main.cpp"), "#include \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 libraries = vec![FrameworkLibrary { - name: "SPI".to_string(), - dir: spi_dir.clone(), - include_dirs: vec![spi_dir.clone()], - source_files: vec![spi_dir.join("SPI.cpp")], - }]; - - 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, - declared_deps: &[], - }; - - let kv = FileKvStore::open(tmp.path().join("kv")).unwrap(); - - let (first, hit_first) = resolve_framework_library_sources_cached_with_hit( - &libraries, - &project_dir, - &src_dir, - &key_inputs, - &kv, - ); - assert!(!hit_first, "first call must miss the cache"); - assert_eq!(first, vec![spi_dir.join("SPI.cpp")]); - - let (second, hit_second) = resolve_framework_library_sources_cached_with_hit( - &libraries, - &project_dir, - &src_dir, - &key_inputs, - &kv, - ); - assert!(hit_second, "second call must hit the cache"); - assert_eq!(first, second, "cache hit must yield identical sources"); - } -} +#[path = "framework_libs_tests.rs"] +mod tests; diff --git a/crates/fbuild-build-engine/src/framework_libs_tests.rs b/crates/fbuild-build-engine/src/framework_libs_tests.rs new file mode 100644 index 00000000..f04c7c7c --- /dev/null +++ b/crates/fbuild-build-engine/src/framework_libs_tests.rs @@ -0,0 +1,688 @@ +//! Tests for [`super`] — framework-library selection. +//! +//! Split out to keep `framework_libs.rs` under the workspace 1000-LOC +//! limit; `compiler_tests.rs` is the same pattern. + +use super::*; + +#[test] +fn resolves_libraries_from_project_includes() { + let tmp = tempfile::TempDir::new().unwrap(); + let project_src = tmp.path().join("project").join("src"); + std::fs::create_dir_all(&project_src).unwrap(); + std::fs::write( + project_src.join("main.cpp"), + "#include \n#include \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 octo_dir = tmp + .path() + .join("framework") + .join("libraries") + .join("OctoWS2811"); + std::fs::create_dir_all(&octo_dir).unwrap(); + std::fs::write(octo_dir.join("OctoWS2811.h"), "").unwrap(); + std::fs::write(octo_dir.join("OctoWS2811.cpp"), "").unwrap(); + std::fs::write(octo_dir.join("OctoWS2811_imxrt.cpp"), "").unwrap(); + + let libraries = vec![ + FrameworkLibrary { + name: "OctoWS2811".to_string(), + dir: octo_dir.clone(), + include_dirs: vec![octo_dir.clone()], + source_files: vec![ + octo_dir.join("OctoWS2811.cpp"), + octo_dir.join("OctoWS2811_imxrt.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 mut sources = resolve_framework_library_sources_from_libraries( + &libraries, + std::slice::from_ref(&project_src), + ); + sources.sort(); + + let mut expected = vec![ + octo_dir.join("OctoWS2811.cpp"), + octo_dir.join("OctoWS2811_imxrt.cpp"), + spi_dir.join("SPI.cpp"), + ]; + expected.sort(); + assert_eq!(sources, expected); +} + +#[test] +fn follows_transitive_includes() { + let tmp = tempfile::TempDir::new().unwrap(); + let project_src = tmp.path().join("project").join("src"); + std::fs::create_dir_all(&project_src).unwrap(); + std::fs::write(project_src.join("main.cpp"), "#include \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 wrapper_dir = tmp + .path() + .join("framework") + .join("libraries") + .join("NeedsSpi"); + std::fs::create_dir_all(&wrapper_dir).unwrap(); + std::fs::write(wrapper_dir.join("NeedsSpi.h"), "#include \n").unwrap(); + std::fs::write(wrapper_dir.join("NeedsSpi.cpp"), "").unwrap(); + + let libraries = vec![ + FrameworkLibrary { + name: "NeedsSpi".to_string(), + dir: wrapper_dir.clone(), + include_dirs: vec![wrapper_dir.clone()], + source_files: vec![wrapper_dir.join("NeedsSpi.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 mut sources = resolve_framework_library_sources_from_libraries( + &libraries, + std::slice::from_ref(&project_src), + ); + sources.sort(); + + let mut expected = vec![wrapper_dir.join("NeedsSpi.cpp"), spi_dir.join("SPI.cpp")]; + expected.sort(); + assert_eq!(sources, expected); +} + +#[test] +fn unrelated_library_not_selected() { + // Regression guard for #204: libraries whose headers are never + // referenced must not appear in the compile set. + let tmp = tempfile::TempDir::new().unwrap(); + let project_src = tmp.path().join("project").join("src"); + std::fs::create_dir_all(&project_src).unwrap(); + std::fs::write(project_src.join("main.cpp"), "#include \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 fnet_dir = tmp.path().join("framework").join("libraries").join("FNET"); + std::fs::create_dir_all(&fnet_dir).unwrap(); + std::fs::write(fnet_dir.join("fnet.h"), "").unwrap(); + std::fs::write(fnet_dir.join("fnet.cpp"), "").unwrap(); + + let libraries = vec![ + FrameworkLibrary { + name: "FNET".to_string(), + dir: fnet_dir.clone(), + include_dirs: vec![fnet_dir.clone()], + source_files: vec![fnet_dir.join("fnet.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, + std::slice::from_ref(&project_src), + ); + assert_eq!(sources, vec![spi_dir.join("SPI.cpp")]); +} + +/// FastLED/fbuild#1337: a local library's own *source* must be able to +/// select a framework library. +/// +/// The Teensy shape. FastLED lives under `lib/FastLED/`, and the include +/// that needs `Adafruit_NeoPixel` sits in one of its `.cpp` translation +/// units — not in any header the sketch reaches. Seeding only the sketch +/// meant the include was never scanned, so the library compiled against a +/// header on the include path whose sources were never on the link line: +/// `undefined reference to Adafruit_NeoPixel::*`, on all eight Teensy +/// boards. +/// +/// The guard is FastLED's current one — an explicit opt-in `-D` *and* a +/// header probe — so this also pins the two-signal behavior. +#[test] +fn local_library_source_selects_a_framework_library() { + let tmp = tempfile::TempDir::new().unwrap(); + let project = tmp.path().join("project"); + let project_src = project.join("src"); + let fastled_src = project.join("lib").join("FastLED").join("src"); + std::fs::create_dir_all(&project_src).unwrap(); + std::fs::create_dir_all(&fastled_src).unwrap(); + + std::fs::write(project_src.join("main.cpp"), "#include \n").unwrap(); + std::fs::write(fastled_src.join("FastLED.h"), "// no adafruit here\n").unwrap(); + // The compiled TU of the local library, and the only place the + // dependency is expressed. + std::fs::write( + fastled_src.join("adafruit_driver.cpp"), + "#include \"has_include.h\"\n\ + #if defined(FASTLED_USE_ADAFRUIT_NEOPIXEL) && FL_HAS_INCLUDE()\n\ + #include \n\ + #endif\n", + ) + .unwrap(); + std::fs::write( + fastled_src.join("has_include.h"), + "#ifndef FL_HAS_INCLUDE_H\n#define FL_HAS_INCLUDE_H\n\ + #define FL_HAS_INCLUDE(x) __has_include(x)\n#endif\n", + ) + .unwrap(); + + let neopixel = tmp + .path() + .join("framework") + .join("libraries") + .join("Adafruit_NeoPixel"); + std::fs::create_dir_all(&neopixel).unwrap(); + std::fs::write(neopixel.join("Adafruit_NeoPixel.h"), "").unwrap(); + std::fs::write(neopixel.join("Adafruit_NeoPixel.cpp"), "").unwrap(); + + let libraries = vec![FrameworkLibrary { + name: "Adafruit_NeoPixel".to_string(), + dir: neopixel.clone(), + include_dirs: vec![neopixel.clone()], + source_files: vec![neopixel.join("Adafruit_NeoPixel.cpp")], + }]; + + let mut defines = HashMap::new(); + defines.insert("FASTLED_USE_ADAFRUIT_NEOPIXEL".to_string(), "1".to_string()); + + let sources = + resolve_framework_library_sources_active(&libraries, &project, &project_src, &defines); + assert!( + sources.iter().any(|p| p.ends_with("Adafruit_NeoPixel.cpp")), + "a compiled library source's include must reach the link line: {sources:?}" + ); +} + +/// The other direction of the same agreement: no opt-in, no link. +/// +/// FastLED's guard is `defined(FASTLED_USE_ADAFRUIT_NEOPIXEL) && +/// FL_HAS_INCLUDE(...)`, deliberately two signals. Nobody `#define`s the +/// opt-in, so it is honestly false and the real driver is not compiled — +/// selecting the library here would put sources on the link line that +/// nothing references. Scanner and compiler have to agree both ways. +#[test] +fn local_library_source_without_the_opt_in_selects_nothing() { + let tmp = tempfile::TempDir::new().unwrap(); + let project = tmp.path().join("project"); + let project_src = project.join("src"); + let fastled_src = project.join("lib").join("FastLED").join("src"); + std::fs::create_dir_all(&project_src).unwrap(); + std::fs::create_dir_all(&fastled_src).unwrap(); + + std::fs::write(project_src.join("main.cpp"), "#include \n").unwrap(); + std::fs::write(fastled_src.join("FastLED.h"), "// no adafruit here\n").unwrap(); + std::fs::write( + fastled_src.join("adafruit_driver.cpp"), + "#include \"has_include.h\"\n\ + #if defined(FASTLED_USE_ADAFRUIT_NEOPIXEL) && FL_HAS_INCLUDE()\n\ + #include \n\ + #endif\n", + ) + .unwrap(); + std::fs::write( + fastled_src.join("has_include.h"), + "#ifndef FL_HAS_INCLUDE_H\n#define FL_HAS_INCLUDE_H\n\ + #define FL_HAS_INCLUDE(x) __has_include(x)\n#endif\n", + ) + .unwrap(); + + let neopixel = tmp + .path() + .join("framework") + .join("libraries") + .join("Adafruit_NeoPixel"); + std::fs::create_dir_all(&neopixel).unwrap(); + std::fs::write(neopixel.join("Adafruit_NeoPixel.h"), "").unwrap(); + std::fs::write(neopixel.join("Adafruit_NeoPixel.cpp"), "").unwrap(); + + let libraries = vec![FrameworkLibrary { + name: "Adafruit_NeoPixel".to_string(), + dir: neopixel.clone(), + include_dirs: vec![neopixel.clone()], + source_files: vec![neopixel.join("Adafruit_NeoPixel.cpp")], + }]; + + // No `FASTLED_USE_ADAFRUIT_NEOPIXEL` in the defines. + let sources = resolve_framework_library_sources_active( + &libraries, + &project, + &project_src, + &HashMap::new(), + ); + assert!( + sources.is_empty(), + "an un-opted-in driver must not put a library on the link line: {sources:?}" + ); +} + +/// A library's `examples/` are shipped but never compiled, so they must +/// never seed. +/// +/// This is the mirror-image failure of #1337: seeding an example sketch +/// would make the scanner claim a dependency the build does not link, +/// which is the same scanner/compiler disagreement pointed the other way. +#[test] +fn local_library_examples_do_not_seed() { + let tmp = tempfile::TempDir::new().unwrap(); + let project = tmp.path().join("project"); + let project_src = project.join("src"); + let fastled = project.join("lib").join("FastLED"); + std::fs::create_dir_all(&project_src).unwrap(); + std::fs::create_dir_all(fastled.join("src")).unwrap(); + std::fs::create_dir_all(fastled.join("examples").join("Demo")).unwrap(); + + std::fs::write(project_src.join("main.cpp"), "#include \n").unwrap(); + std::fs::write(fastled.join("src").join("FastLED.h"), "").unwrap(); + std::fs::write(fastled.join("src").join("FastLED.cpp"), "").unwrap(); + // Compiled by nobody, and it names a library the build must not link. + std::fs::write( + fastled.join("examples").join("Demo").join("Demo.ino"), + "#include \n", + ) + .unwrap(); + + let audio = tmp.path().join("framework").join("libraries").join("Audio"); + std::fs::create_dir_all(&audio).unwrap(); + std::fs::write(audio.join("Audio.h"), "").unwrap(); + std::fs::write(audio.join("Audio.cpp"), "").unwrap(); + + let libraries = vec![FrameworkLibrary { + name: "Audio".to_string(), + dir: audio.clone(), + include_dirs: vec![audio.clone()], + source_files: vec![audio.join("Audio.cpp")], + }]; + + let sources = resolve_framework_library_sources_active( + &libraries, + &project, + &project_src, + &HashMap::new(), + ); + assert!( + sources.is_empty(), + "an example sketch is not compiled, so it must not select: {sources:?}" + ); +} + +#[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 \n").unwrap(); + std::fs::write(fastled.join("FastLED.h"), "#include \n").unwrap(); + std::fs::write(fastled.join("inactive_audio.h"), "#include \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(); + let project_src = tmp.path().join("project").join("src"); + let project_lib = tmp + .path() + .join("project") + .join("lib") + .join("FastLED") + .join("src"); + std::fs::create_dir_all(&project_src).unwrap(); + std::fs::create_dir_all(&project_lib).unwrap(); + std::fs::write(project_src.join("main.cpp"), "#include \n").unwrap(); + std::fs::write(project_lib.join("FastLED.h"), "#include \n").unwrap(); + std::fs::write(project_lib.join("FastLED.cpp"), "").unwrap(); + + let framework_fastled_dir = tmp + .path() + .join("framework") + .join("libraries") + .join("FastLED"); + std::fs::create_dir_all(&framework_fastled_dir).unwrap(); + std::fs::write(framework_fastled_dir.join("FastLED.h"), "").unwrap(); + std::fs::write(framework_fastled_dir.join("FastLED.cpp"), "").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 libraries = vec![ + FrameworkLibrary { + name: "FastLED".to_string(), + dir: framework_fastled_dir.clone(), + include_dirs: vec![framework_fastled_dir.clone()], + source_files: vec![framework_fastled_dir.join("FastLED.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 roots = vec![project_src, project_lib]; + let sources = resolve_framework_library_sources_from_libraries(&libraries, &roots); + + assert_eq!(sources, vec![spi_dir.join("SPI.cpp")]); +} + +/// Regression for FastLED/fbuild#263 — case A: when the user's project +/// IS the library (FastLED's own source tree has `src/FastLED.h` +/// directly under one of the walker's roots), the framework's bundled +/// FastLED at `cores/teensy4/libraries/FastLED/` must not get selected. +/// This case works in the LDF resolver today because path-prefix +/// attribution finds `project/src/FastLED.h` first. +#[test] +fn project_is_the_library_does_not_pull_in_bundled_copy() { + let tmp = tempfile::TempDir::new().unwrap(); + + let project_src = tmp.path().join("project").join("src"); + std::fs::create_dir_all(&project_src).unwrap(); + std::fs::write(project_src.join("FastLED.h"), "// the real FastLED\n").unwrap(); + std::fs::write(project_src.join("FastLED.cpp"), "// user impl\n").unwrap(); + std::fs::write( + project_src.join("example_main.cpp"), + "#include \n", + ) + .unwrap(); + + let bundled_fastled_dir = tmp + .path() + .join("framework") + .join("libraries") + .join("FastLED"); + std::fs::create_dir_all(&bundled_fastled_dir).unwrap(); + std::fs::write( + bundled_fastled_dir.join("FastLED.h"), + "// bundled (stale) FastLED\n", + ) + .unwrap(); + std::fs::write(bundled_fastled_dir.join("FastLED.cpp"), "// bundled impl\n").unwrap(); + + let libraries = vec![FrameworkLibrary { + name: "FastLED".to_string(), + dir: bundled_fastled_dir.clone(), + include_dirs: vec![bundled_fastled_dir.clone()], + source_files: vec![bundled_fastled_dir.join("FastLED.cpp")], + }]; + + let sources = resolve_framework_library_sources_from_libraries( + &libraries, + std::slice::from_ref(&project_src), + ); + + assert!( + sources.is_empty(), + "bundled FastLED must NOT be selected when the project owns FastLED.h \ + directly under src/ — see #263. Got: {sources:?}" + ); +} + +/// Regression for FastLED/fbuild#263 — case B: the user's project owns +/// FastLED.h at a path that is NOT one of the walker roots passed to +/// the resolver (e.g. `/src/FastLED.h` while the resolver only +/// sees `/tests/platform/teensy41/src/`). The walker then can +/// only find FastLED.h via the framework's bundled +/// `cores/teensy4/libraries/FastLED/` include dir, mis-attributes the +/// include to the bundled library, and pulls its sources into the +/// build set — duplicate-symbol time. The fix in `framework_libs.rs` +/// drops framework libraries whose primary header is shadowed by a +/// project header even when the project header isn't first in the +/// search order. +#[test] +fn example_only_root_does_not_pull_in_bundled_fastled_when_user_owns_fastled() { + let tmp = tempfile::TempDir::new().unwrap(); + + // The repo: user's local FastLED lives at /src/, which is + // NOT among the resolver's roots for the per-example build. + let repo_src = tmp.path().join("repo").join("src"); + std::fs::create_dir_all(&repo_src).unwrap(); + std::fs::write(repo_src.join("FastLED.h"), "// the real FastLED\n").unwrap(); + std::fs::write(repo_src.join("FastLED.cpp"), "// user impl\n").unwrap(); + + // The per-example project root the resolver actually sees. + let example_src = tmp + .path() + .join("repo") + .join("tests") + .join("platform") + .join("teensy41") + .join("src"); + std::fs::create_dir_all(&example_src).unwrap(); + std::fs::write( + example_src.join("example_main.cpp"), + "#include \n", + ) + .unwrap(); + + // Framework bundles its own FastLED. + let bundled_fastled_dir = tmp + .path() + .join("framework") + .join("libraries") + .join("FastLED"); + std::fs::create_dir_all(&bundled_fastled_dir).unwrap(); + std::fs::write(bundled_fastled_dir.join("FastLED.h"), "// bundled\n").unwrap(); + std::fs::write(bundled_fastled_dir.join("FastLED.cpp"), "// bundled impl\n").unwrap(); + + let libraries = vec![FrameworkLibrary { + name: "FastLED".to_string(), + dir: bundled_fastled_dir.clone(), + include_dirs: vec![bundled_fastled_dir.clone()], + source_files: vec![bundled_fastled_dir.join("FastLED.cpp")], + }]; + + // The fbuild build pipeline calls `local_overridden_framework_libs` + // with both the example root AND the repo's actual src/ as + // shadowing roots. The repo src/FastLED.h shadows the framework's + // FastLED → framework library is filtered out before the resolver + // ever sees it. + let shadowing_roots = vec![example_src.clone(), repo_src.clone()]; + let filtered = filter_framework_libs_shadowed_by_project(&libraries, &shadowing_roots); + + // Resolver runs on the FILTERED library set. + let sources = resolve_framework_library_sources_from_libraries( + &filtered, + std::slice::from_ref(&example_src), + ); + + assert!( + sources.is_empty(), + "bundled FastLED must be filtered out because the user's repo owns \ + FastLED.h even when it's not in the per-example walker roots — see #263. \ + Got: {sources:?}" + ); +} + +/// Regression for FastLED/fbuild#284 — a nested project header whose +/// basename happens to collide with a framework library's primary +/// header must NOT trigger the shadowing filter. FastLED ships +/// `lib/FastLED/fl/channels/spi.h`, which is includeable only as +/// ``, never as ``. The framework's `SPI` +/// library must therefore stay in the build set, otherwise every +/// Teensy 4.x example fails at link with `undefined reference to +/// SPIClass::*`. +/// +/// At the same time, the existing `#263` behaviour for headers +/// reachable as bare `` (e.g. `lib/FastLED/noise.h` or +/// `project/src/FastLED.h`) must still drop the matching framework +/// library. +#[test] +fn nested_basename_does_not_shadow_framework_library() { + let tmp = tempfile::TempDir::new().unwrap(); + + // PIO project layout: lib/FastLED/ contains FastLED's source + // tree directly (1.0 flat layout — no src/ subdir). spi.h is + // nested deep, noise.h sits at FastLED's include root. + let project_dir = tmp.path().join("project"); + let lib_dir = project_dir.join("lib"); + let fastled_dir = lib_dir.join("FastLED"); + let nested_spi_dir = fastled_dir.join("fl").join("channels"); + std::fs::create_dir_all(&nested_spi_dir).unwrap(); + std::fs::write(nested_spi_dir.join("spi.h"), "// FastLED internal\n").unwrap(); + std::fs::write(fastled_dir.join("FastLED.h"), "").unwrap(); + std::fs::write(fastled_dir.join("noise.h"), "// shadows framework Noise\n").unwrap(); + + let src_dir = project_dir.join("src"); + std::fs::create_dir_all(&src_dir).unwrap(); + + // Framework libs: SPI (must SURVIVE the filter) and Noise (must + // be dropped because the project owns noise.h at the FastLED + // library include root). + 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 noise_dir = tmp.path().join("framework").join("libraries").join("Noise"); + std::fs::create_dir_all(&noise_dir).unwrap(); + std::fs::write(noise_dir.join("noise.h"), "").unwrap(); + std::fs::write(noise_dir.join("noise.cpp"), "").unwrap(); + + let libraries = vec![ + FrameworkLibrary { + name: "Noise".to_string(), + dir: noise_dir.clone(), + include_dirs: vec![noise_dir.clone()], + source_files: vec![noise_dir.join("noise.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 shadowing_roots = framework_include_scan_roots(&project_dir, &src_dir); + let filtered = filter_framework_libs_shadowed_by_project(&libraries, &shadowing_roots); + + let surviving: Vec<&str> = filtered.iter().map(|l| l.name.as_str()).collect(); + assert!( + surviving.contains(&"SPI"), + "framework SPI must SURVIVE — nested fl/channels/spi.h is not reachable \ + as and must not trigger the shadowing filter — see #284. \ + Surviving libraries: {surviving:?}" + ); + assert!( + !surviving.contains(&"Noise"), + "framework Noise must be dropped — lib/FastLED/noise.h sits at the \ + FastLED library include root and is reachable as — see #263. \ + Surviving libraries: {surviving:?}" + ); +} + +#[test] +fn cached_resolution_round_trips_through_file_store() { + let tmp = tempfile::TempDir::new().unwrap(); + let project_dir = tmp.path().join("project"); + let src_dir = project_dir.join("src"); + std::fs::create_dir_all(&src_dir).unwrap(); + std::fs::write(src_dir.join("main.cpp"), "#include \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 libraries = vec![FrameworkLibrary { + name: "SPI".to_string(), + dir: spi_dir.clone(), + include_dirs: vec![spi_dir.clone()], + source_files: vec![spi_dir.join("SPI.cpp")], + }]; + + 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, + declared_deps: &[], + }; + + let kv = FileKvStore::open(tmp.path().join("kv")).unwrap(); + + let (first, hit_first) = resolve_framework_library_sources_cached_with_hit( + &libraries, + &project_dir, + &src_dir, + &key_inputs, + &kv, + ); + assert!(!hit_first, "first call must miss the cache"); + assert_eq!(first, vec![spi_dir.join("SPI.cpp")]); + + let (second, hit_second) = resolve_framework_library_sources_cached_with_hit( + &libraries, + &project_dir, + &src_dir, + &key_inputs, + &kv, + ); + assert!(hit_second, "second call must hit the cache"); + assert_eq!(first, second, "cache hit must yield identical sources"); +} diff --git a/crates/fbuild-header-scan/src/scanner.rs b/crates/fbuild-header-scan/src/scanner.rs index 00da80cc..2c8234e7 100644 --- a/crates/fbuild-header-scan/src/scanner.rs +++ b/crates/fbuild-header-scan/src/scanner.rs @@ -552,21 +552,58 @@ struct ConditionParser<'a> { } impl<'a> ConditionParser<'a> { + /// `||`, where a decidably-true operand settles the result. + /// + /// Unknown-ness has to combine per operand rather than globally, or one + /// unseeable macro poisons an expression another term already decided. + /// `defined(OPT_IN) && FL_HAS_INCLUDE()` is the case that matters: + /// when nothing defines `OPT_IN` the guard is *false*, whatever the + /// undecidable half says, and treating the whole thing as unknown would + /// select a library the build never compiles (FastLED/fbuild#1337). fn parse_or(&mut self) -> i64 { + let outer = self.saw_unknown_macro; + self.saw_unknown_macro = false; let mut value = self.parse_and(); + let mut unknown = self.saw_unknown_macro; while self.consume(b"||") { + self.saw_unknown_macro = false; let rhs = self.parse_and(); - value = i64::from(value != 0 || rhs != 0); + let rhs_unknown = self.saw_unknown_macro; + let lhs_true = !unknown && value != 0; + let rhs_true = !rhs_unknown && rhs != 0; + if lhs_true || rhs_true { + value = 1; + unknown = false; + } else { + unknown = unknown || rhs_unknown; + value = i64::from(value != 0 || rhs != 0); + } } + self.saw_unknown_macro = outer || unknown; value } + /// `&&`, where a decidably-false operand settles the result. fn parse_and(&mut self) -> i64 { + let outer = self.saw_unknown_macro; + self.saw_unknown_macro = false; let mut value = self.parse_equality(); + let mut unknown = self.saw_unknown_macro; while self.consume(b"&&") { + self.saw_unknown_macro = false; let rhs = self.parse_equality(); - value = i64::from(value != 0 && rhs != 0); + let rhs_unknown = self.saw_unknown_macro; + let lhs_false = !unknown && value == 0; + let rhs_false = !rhs_unknown && rhs == 0; + if lhs_false || rhs_false { + value = 0; + unknown = false; + } else { + unknown = unknown || rhs_unknown; + value = i64::from(value != 0 && rhs != 0); + } } + self.saw_unknown_macro = outer || unknown; value } @@ -626,6 +663,21 @@ impl<'a> ConditionParser<'a> { if token.is_empty() { return 0; } + // `__has_include()` is a compiler builtin: nothing `#define`s it, + // so the corpus set can never settle it, and answering "false" is how + // an include the compiler *does* take became invisible + // (FastLED/fbuild#1337). Undecidable is the honest answer — the arm is + // scanned, and the walker's own header resolution then decides, which + // is the same question `__has_include` asks. + if token == "__has_include" { + self.skip_balanced_parens(); + self.saw_any_macro = true; + self.saw_unknown_macro = true; + return 0; + } + // A function-like macro invocation: consume its argument list so a + // `<` inside it is not mistaken for a comparison operator. + self.skip_balanced_parens(); self.saw_any_macro = true; match self.macros.get(token).and_then(|value| parse_number(value)) { Some(value) => value, @@ -638,6 +690,35 @@ impl<'a> ConditionParser<'a> { } } + /// Consume a balanced `( ... )` group if one starts here. + /// + /// Used after an identifier so a function-like macro's arguments do not + /// leak into the expression grammar — `FL_HAS_INCLUDE()` would + /// otherwise leave `` behind and the `<` would parse as a + /// less-than. + fn skip_balanced_parens(&mut self) { + self.skip_ws(); + if self.index >= self.input.len() || self.input[self.index] != b'(' { + return; + } + let mut depth = 0usize; + while self.index < self.input.len() { + match self.input[self.index] { + b'(' => depth += 1, + b')' => { + depth -= 1; + self.index += 1; + if depth == 0 { + return; + } + continue; + } + _ => {} + } + self.index += 1; + } + } + fn consume(&mut self, expected: &[u8]) -> bool { self.skip_ws(); if self.input[self.index..].starts_with(expected) { diff --git a/crates/fbuild-header-scan/src/scanner_tests.rs b/crates/fbuild-header-scan/src/scanner_tests.rs index a582a6bd..65de48e9 100644 --- a/crates/fbuild-header-scan/src/scanner_tests.rs +++ b/crates/fbuild-header-scan/src/scanner_tests.rs @@ -483,3 +483,73 @@ fn a_non_guard_ifndef_is_still_undecidable() { assert!(paths.contains(&"fallback.h"), "{paths:?}"); assert!(paths.contains(&"feature.h"), "{paths:?}"); } + +/// `__has_include` is a compiler builtin, so nothing can settle it from a +/// macro set — but answering "false" is how an include the compiler *does* +/// take became invisible (FastLED/fbuild#1337). Undecidable is the honest +/// answer, and the walker's own header resolution then decides. +#[test] +fn has_include_is_undecidable_and_its_argument_is_consumed() { + let refs = scan_active_with_known( + "#if __has_include()\n#include \n#endif\n", + &HashMap::new(), + &HashSet::new(), + ); + let paths: Vec<&str> = refs.iter().map(|r| r.path.as_str()).collect(); + assert_eq!(paths, vec!["SPI.h"], "{paths:?}"); +} + +/// A decidably-false operand settles `&&`, even when the other half cannot be +/// decided. +/// +/// This is FastLED's opt-in gate: `defined(FASTLED_USE_ADAFRUIT_NEOPIXEL) && +/// FL_HAS_INCLUDE()`. Nothing defines the opt-in, so the +/// guard is false — and letting the undecidable half win would select a +/// library the build never compiles. +#[test] +fn a_false_operand_settles_and_despite_an_undecidable_one() { + let known: HashSet = ["FL_HAS_INCLUDE".to_string()].into(); + let refs = scan_active_with_known( + "#if defined(OPT_IN) && FL_HAS_INCLUDE()\n#include \n#else\n#include \n#endif\n", + &HashMap::new(), + &known, + ); + let paths: Vec<&str> = refs.iter().map(|r| r.path.as_str()).collect(); + assert_eq!( + paths, + vec!["fake.h"], + "an unsatisfiable opt-in must settle the guard: {paths:?}" + ); +} + +/// With the opt-in supplied, the undecidable half governs and both arms are +/// scanned — the walker decides which header actually resolves. +#[test] +fn an_undecidable_operand_governs_once_the_others_are_satisfied() { + let known: HashSet = ["FL_HAS_INCLUDE".to_string()].into(); + let mut defines = HashMap::new(); + defines.insert("OPT_IN".to_string(), "1".to_string()); + let refs = scan_active_with_known( + "#if defined(OPT_IN) && FL_HAS_INCLUDE()\n#include \n#else\n#include \n#endif\n", + &defines, + &known, + ); + let paths: Vec<&str> = refs.iter().map(|r| r.path.as_str()).collect(); + assert!(paths.contains(&"SPI.h"), "{paths:?}"); + assert!(paths.contains(&"fake.h"), "{paths:?}"); +} + +/// A decidably-true operand settles `||` the same way. +#[test] +fn a_true_operand_settles_or_despite_an_undecidable_one() { + let known: HashSet = ["MAYBE".to_string()].into(); + let mut defines = HashMap::new(); + defines.insert("SURE".to_string(), "1".to_string()); + let refs = scan_active_with_known( + "#if defined(SURE) || defined(MAYBE)\n#include \n#else\n#include \n#endif\n", + &defines, + &known, + ); + let paths: Vec<&str> = refs.iter().map(|r| r.path.as_str()).collect(); + assert_eq!(paths, vec!["taken.h"], "{paths:?}"); +} diff --git a/crates/fbuild-library-select/src/cache.rs b/crates/fbuild-library-select/src/cache.rs index 92d451b2..8bb50b99 100644 --- a/crates/fbuild-library-select/src/cache.rs +++ b/crates/fbuild-library-select/src/cache.rs @@ -39,7 +39,7 @@ pub const SCANNER_VERSION: u32 = 3; /// Bump when the resolver's 2-pass LDF semantics change (seed expansion, /// attribution, convergence rule, etc.). -pub const LDF_MODE_VERSION: u32 = 5; +pub const LDF_MODE_VERSION: u32 = 6; /// Namespace for the library-selection file cache. pub const NAMESPACE: &str = "library-selection";