From 56fd6d636f6d5a7606a3e83c5ef04d97465d058e Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Tue, 27 Oct 2026 15:18:30 -0700 Subject: [PATCH 1/3] fix(cache): follow symlinks in the warm-build watch set A PlatformIO project reaches a local library through a symlink (`lib/FastLED -> ~/dev/fastled2`). `WalkDir` skipped symlinked entries, so every file under the link target was missing from the fast-path fingerprint: editing a header there left `build_fingerprint.json` unchanged, the warm path reused the previous `firmware.bin`, and `fbuild deploy` flashed byte-identical firmware until `--clean` (FastLED/fbuild#1753). The two watch walks (content-hash and stamp-cache) now share one `watched_files` helper that follows links; traversal cycles are dropped (walkdir reports them as errors) so a self-referential link cannot hang the build, and the exclude list still prunes by directory name (`.git` under a link target stays out). --- .../src/build_fingerprint/README.md | 3 + .../src/build_fingerprint/mod.rs | 161 ++++++++++++++---- 2 files changed, 134 insertions(+), 30 deletions(-) diff --git a/crates/fbuild-build-engine/src/build_fingerprint/README.md b/crates/fbuild-build-engine/src/build_fingerprint/README.md index 14c49419c..e20ac657d 100644 --- a/crates/fbuild-build-engine/src/build_fingerprint/README.md +++ b/crates/fbuild-build-engine/src/build_fingerprint/README.md @@ -11,6 +11,9 @@ nothing relevant has changed. primitives (`hash_watch_set`, `hash_watch_set_stamps`, `hash_watch_set_stamps_cached`), and the `WatchSetStampCache` trait the daemon implements for cross-invocation memoisation. + The watched file set follows symlinks, so a local library reached through + `lib/ -> /elsewhere` still invalidates the warm path when a file under + the link target changes (FastLED/fbuild#1753); traversal cycles are dropped. - **`fast_path.rs`** -- Shared warm-build cache contract. Orchestrators declare required outputs through `FastPathContract`, then use `fast_path_check` for reuse decisions and diff --git a/crates/fbuild-build-engine/src/build_fingerprint/mod.rs b/crates/fbuild-build-engine/src/build_fingerprint/mod.rs index d9cd0cebc..484a7ebdd 100644 --- a/crates/fbuild-build-engine/src/build_fingerprint/mod.rs +++ b/crates/fbuild-build-engine/src/build_fingerprint/mod.rs @@ -160,6 +160,35 @@ pub fn hash_files(paths: &[PathBuf]) -> Result { Ok(format!("{:x}", hasher.finalize())) } +/// Every file under `watch.root` that shapes the fingerprint, sorted. +/// +/// Symlinks are followed. A PlatformIO project reaches a local library +/// through `lib/ -> /elsewhere` (FastLED/fbuild#1753), and a walk that +/// skips symlinked entries leaves every file under the link target out of +/// the fingerprint: an edit there never invalidated the warm path, which +/// then reused the previous `firmware.bin` and flashed byte-identical code. +/// Cycles are dropped (walkdir reports them as errors) so a self-referential +/// link cannot hang the build. +fn watched_files(watch: &FingerprintWatch) -> Vec { + let mut files = Vec::new(); + for entry in WalkDir::new(&watch.root) + .follow_links(true) + .into_iter() + .filter_entry(|entry| should_descend(entry.path(), &watch.root, &watch.excludes)) + .flatten() + { + if !entry.file_type().is_file() { + continue; + } + if !matches_extension(entry.path(), &watch.extensions) { + continue; + } + files.push(entry.into_path()); + } + files.sort(); + files +} + pub fn hash_watch_set(watches: &[FingerprintWatch]) -> Result { let mut ordered = watches.to_vec(); ordered.sort_by_key(watch_identity); @@ -174,21 +203,7 @@ pub fn hash_watch_set(watches: &[FingerprintWatch]) -> Result { continue; } - let mut files = Vec::new(); - for entry in WalkDir::new(&watch.root) - .into_iter() - .filter_entry(|entry| should_descend(entry.path(), &watch.root, &watch.excludes)) - .flatten() - { - if !entry.file_type().is_file() { - continue; - } - if !matches_extension(entry.path(), &watch.extensions) { - continue; - } - files.push(entry.into_path()); - } - files.sort(); + let files = watched_files(watch); for file in files { hasher.update(b"file\0"); @@ -276,21 +291,7 @@ where .collect(); let mut next_files = Vec::new(); - let mut files = Vec::new(); - for entry in WalkDir::new(&watch.root) - .into_iter() - .filter_entry(|entry| should_descend(entry.path(), &watch.root, &watch.excludes)) - .flatten() - { - if !entry.file_type().is_file() { - continue; - } - if !matches_extension(entry.path(), &watch.extensions) { - continue; - } - files.push(entry.into_path()); - } - files.sort(); + let files = watched_files(watch); for file in files { let relative_path = relative_path_for_hash(&watch.root, &file); @@ -926,4 +927,104 @@ mod tests { assert_eq!(a_hash, b_hash); } + // ---- FastLED/fbuild#1753: symlinked local libraries ---- + // + // A PlatformIO project reaches a working copy through a symlink + // (`lib/FastLED -> ~/dev/fastled2`). The fast-path walk used to skip + // symlinked entries entirely (`WalkDir` without `follow_links`), so an + // edit under the link target left the fingerprint untouched → the warm + // path reused the previous firmware.bin and flashed byte-identical code. + + fn make_symlinked_project(tmp: &Path) -> (PathBuf, PathBuf, PathBuf) { + let project = tmp.join("project"); + let lib = project.join("lib"); + fs::create_dir_all(&lib).unwrap(); + fs::write(project.join("main.cpp"), "int main() { return 0; }\n").unwrap(); + + let target = tmp.join("fastled2"); + fs::create_dir_all(target.join("src/platforms/esp/32")).unwrap(); + fs::write(target.join("library.json"), "{}\n").unwrap(); + let header = target.join("src/platforms/esp/32/fastspi_esp32_idf.h"); + fs::write(&header, "#pragma once\n#define SPI_PATH 1\n").unwrap(); + + std::os::unix::fs::symlink(&target, lib.join("FastLED")).unwrap(); + (project, target, header) + } + + #[test] + fn test_hash_watch_set_stamps_sees_edits_under_a_symlinked_library() { + if fbuild_core::platform::host::is_windows() { + return; + } + let tmp = tempfile::TempDir::new().unwrap(); + let (project, _, header) = make_symlinked_project(tmp.path()); + let watch = make_watch(&project, &tmp.path().join("project.json")); + + let first = hash_watch_set_stamps(std::slice::from_ref(&watch)).unwrap(); + std::thread::sleep(std::time::Duration::from_millis(20)); + fs::write(&header, "#pragma once\n#define SPI_PATH 2\n").unwrap(); + let second = hash_watch_set_stamps(std::slice::from_ref(&watch)).unwrap(); + + assert_ne!( + first, second, + "a header edited under a symlinked library must invalidate the watch set" + ); + } + + #[test] + fn test_hash_watch_set_sees_edits_under_a_symlinked_library() { + if fbuild_core::platform::host::is_windows() { + return; + } + let tmp = tempfile::TempDir::new().unwrap(); + let (project, _, header) = make_symlinked_project(tmp.path()); + let watch = make_watch(&project, &tmp.path().join("project.json")); + + let first = hash_watch_set(std::slice::from_ref(&watch)).unwrap(); + fs::write(&header, "#pragma once\n#define SPI_PATH 2\n").unwrap(); + let second = hash_watch_set(std::slice::from_ref(&watch)).unwrap(); + + assert_ne!(first, second); + } + + #[test] + fn test_hash_watch_set_stamps_hashes_a_symlinked_source_file() { + if fbuild_core::platform::host::is_windows() { + return; + } + let tmp = tempfile::TempDir::new().unwrap(); + let src = tmp.path().join("src"); + fs::create_dir_all(&src).unwrap(); + let real = tmp.path().join("shared").join("main.cpp"); + fs::create_dir_all(real.parent().unwrap()).unwrap(); + fs::write(&real, "int main() { return 1; }\n").unwrap(); + std::os::unix::fs::symlink(&real, src.join("main.cpp")).unwrap(); + + let watch = make_watch(&src, &tmp.path().join("project.json")); + let first = hash_watch_set_stamps(std::slice::from_ref(&watch)).unwrap(); + std::thread::sleep(std::time::Duration::from_millis(20)); + fs::write(&real, "int main() { return 2; }\n").unwrap(); + let second = hash_watch_set_stamps(std::slice::from_ref(&watch)).unwrap(); + + assert_ne!(first, second); + } + + #[test] + fn test_hash_watch_set_stamps_tolerates_a_symlink_loop() { + if fbuild_core::platform::host::is_windows() { + return; + } + let tmp = tempfile::TempDir::new().unwrap(); + let src = tmp.path().join("src"); + let nested = src.join("nested"); + fs::create_dir_all(&nested).unwrap(); + fs::write(src.join("main.cpp"), "int main() { return 1; }\n").unwrap(); + // `src/nested/loop -> src` would recurse forever under a naive walk. + std::os::unix::fs::symlink(&src, nested.join("loop")).unwrap(); + + let watch = make_watch(&src, &tmp.path().join("project.json")); + let hash = hash_watch_set_stamps(std::slice::from_ref(&watch)).unwrap(); + + assert!(!hash.is_empty()); + } } From 18a3469099b158d2ae3b4445d416c999c5ffaed7 Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Tue, 27 Oct 2026 15:28:07 -0700 Subject: [PATCH 2/3] refactor(cache): move symlink watch-set tests to their own module `mod.rs` crossed the 1000-LOC policy limit and the new tests used `std::os::unix::fs::symlink`, which the platform-boundary scanner counts as a host mechanic in engine code. Move the tests to `symlink_tests.rs` (933 LOC in `mod.rs`) and create links through `fbuild_core::platform::fs::{symlink_dir, symlink_file}`. --- .../src/build_fingerprint/mod.rs | 103 +--------------- .../src/build_fingerprint/symlink_tests.rs | 113 ++++++++++++++++++ 2 files changed, 116 insertions(+), 100 deletions(-) create mode 100644 crates/fbuild-build-engine/src/build_fingerprint/symlink_tests.rs diff --git a/crates/fbuild-build-engine/src/build_fingerprint/mod.rs b/crates/fbuild-build-engine/src/build_fingerprint/mod.rs index 484a7ebdd..beaceee57 100644 --- a/crates/fbuild-build-engine/src/build_fingerprint/mod.rs +++ b/crates/fbuild-build-engine/src/build_fingerprint/mod.rs @@ -460,6 +460,9 @@ fn should_descend(path: &Path, root: &Path, excludes: &[String]) -> bool { !excludes.iter().any(|exclude| exclude == name) } +#[cfg(test)] +mod symlink_tests; + #[cfg(test)] mod tests { use super::*; @@ -927,104 +930,4 @@ mod tests { assert_eq!(a_hash, b_hash); } - // ---- FastLED/fbuild#1753: symlinked local libraries ---- - // - // A PlatformIO project reaches a working copy through a symlink - // (`lib/FastLED -> ~/dev/fastled2`). The fast-path walk used to skip - // symlinked entries entirely (`WalkDir` without `follow_links`), so an - // edit under the link target left the fingerprint untouched → the warm - // path reused the previous firmware.bin and flashed byte-identical code. - - fn make_symlinked_project(tmp: &Path) -> (PathBuf, PathBuf, PathBuf) { - let project = tmp.join("project"); - let lib = project.join("lib"); - fs::create_dir_all(&lib).unwrap(); - fs::write(project.join("main.cpp"), "int main() { return 0; }\n").unwrap(); - - let target = tmp.join("fastled2"); - fs::create_dir_all(target.join("src/platforms/esp/32")).unwrap(); - fs::write(target.join("library.json"), "{}\n").unwrap(); - let header = target.join("src/platforms/esp/32/fastspi_esp32_idf.h"); - fs::write(&header, "#pragma once\n#define SPI_PATH 1\n").unwrap(); - - std::os::unix::fs::symlink(&target, lib.join("FastLED")).unwrap(); - (project, target, header) - } - - #[test] - fn test_hash_watch_set_stamps_sees_edits_under_a_symlinked_library() { - if fbuild_core::platform::host::is_windows() { - return; - } - let tmp = tempfile::TempDir::new().unwrap(); - let (project, _, header) = make_symlinked_project(tmp.path()); - let watch = make_watch(&project, &tmp.path().join("project.json")); - - let first = hash_watch_set_stamps(std::slice::from_ref(&watch)).unwrap(); - std::thread::sleep(std::time::Duration::from_millis(20)); - fs::write(&header, "#pragma once\n#define SPI_PATH 2\n").unwrap(); - let second = hash_watch_set_stamps(std::slice::from_ref(&watch)).unwrap(); - - assert_ne!( - first, second, - "a header edited under a symlinked library must invalidate the watch set" - ); - } - - #[test] - fn test_hash_watch_set_sees_edits_under_a_symlinked_library() { - if fbuild_core::platform::host::is_windows() { - return; - } - let tmp = tempfile::TempDir::new().unwrap(); - let (project, _, header) = make_symlinked_project(tmp.path()); - let watch = make_watch(&project, &tmp.path().join("project.json")); - - let first = hash_watch_set(std::slice::from_ref(&watch)).unwrap(); - fs::write(&header, "#pragma once\n#define SPI_PATH 2\n").unwrap(); - let second = hash_watch_set(std::slice::from_ref(&watch)).unwrap(); - - assert_ne!(first, second); - } - - #[test] - fn test_hash_watch_set_stamps_hashes_a_symlinked_source_file() { - if fbuild_core::platform::host::is_windows() { - return; - } - let tmp = tempfile::TempDir::new().unwrap(); - let src = tmp.path().join("src"); - fs::create_dir_all(&src).unwrap(); - let real = tmp.path().join("shared").join("main.cpp"); - fs::create_dir_all(real.parent().unwrap()).unwrap(); - fs::write(&real, "int main() { return 1; }\n").unwrap(); - std::os::unix::fs::symlink(&real, src.join("main.cpp")).unwrap(); - - let watch = make_watch(&src, &tmp.path().join("project.json")); - let first = hash_watch_set_stamps(std::slice::from_ref(&watch)).unwrap(); - std::thread::sleep(std::time::Duration::from_millis(20)); - fs::write(&real, "int main() { return 2; }\n").unwrap(); - let second = hash_watch_set_stamps(std::slice::from_ref(&watch)).unwrap(); - - assert_ne!(first, second); - } - - #[test] - fn test_hash_watch_set_stamps_tolerates_a_symlink_loop() { - if fbuild_core::platform::host::is_windows() { - return; - } - let tmp = tempfile::TempDir::new().unwrap(); - let src = tmp.path().join("src"); - let nested = src.join("nested"); - fs::create_dir_all(&nested).unwrap(); - fs::write(src.join("main.cpp"), "int main() { return 1; }\n").unwrap(); - // `src/nested/loop -> src` would recurse forever under a naive walk. - std::os::unix::fs::symlink(&src, nested.join("loop")).unwrap(); - - let watch = make_watch(&src, &tmp.path().join("project.json")); - let hash = hash_watch_set_stamps(std::slice::from_ref(&watch)).unwrap(); - - assert!(!hash.is_empty()); - } } diff --git a/crates/fbuild-build-engine/src/build_fingerprint/symlink_tests.rs b/crates/fbuild-build-engine/src/build_fingerprint/symlink_tests.rs new file mode 100644 index 000000000..96ed7012f --- /dev/null +++ b/crates/fbuild-build-engine/src/build_fingerprint/symlink_tests.rs @@ -0,0 +1,113 @@ +//! Watch-set coverage for symlinked local libraries (FastLED/fbuild#1753). +//! +//! A PlatformIO project reaches a working copy through `lib/ -> /elsewhere`. +//! The fast-path walk must see every file under the link target, or an edit +//! there never invalidates the warm path and the previous `firmware.bin` is +//! reused (and flashed) unchanged. + +use std::fs; +use std::path::{Path, PathBuf}; + +use super::{FingerprintWatch, hash_watch_set, hash_watch_set_stamps}; + +fn make_watch(root: &Path, cache_file: &Path) -> FingerprintWatch { + FingerprintWatch { + cache_file: cache_file.to_path_buf(), + root: root.to_path_buf(), + extensions: vec!["cpp".to_string(), "h".to_string()], + excludes: vec!["build".to_string()], + } +} + +fn make_symlinked_project(tmp: &Path) -> (PathBuf, PathBuf, PathBuf) { + let project = tmp.join("project"); + let lib = project.join("lib"); + fs::create_dir_all(&lib).unwrap(); + fs::write(project.join("main.cpp"), "int main() { return 0; }\n").unwrap(); + + let target = tmp.join("fastled2"); + fs::create_dir_all(target.join("src/platforms/esp/32")).unwrap(); + fs::write(target.join("library.json"), "{}\n").unwrap(); + let header = target.join("src/platforms/esp/32/fastspi_esp32_idf.h"); + fs::write(&header, "#pragma once\n#define SPI_PATH 1\n").unwrap(); + + fbuild_core::platform::fs::symlink_dir(&target, &lib.join("FastLED")).unwrap(); + (project, target, header) +} + +#[test] +fn test_hash_watch_set_stamps_sees_edits_under_a_symlinked_library() { + if fbuild_core::platform::host::is_windows() { + return; + } + let tmp = tempfile::TempDir::new().unwrap(); + let (project, _, header) = make_symlinked_project(tmp.path()); + let watch = make_watch(&project, &tmp.path().join("project.json")); + + let first = hash_watch_set_stamps(std::slice::from_ref(&watch)).unwrap(); + std::thread::sleep(std::time::Duration::from_millis(20)); + fs::write(&header, "#pragma once\n#define SPI_PATH 2\n").unwrap(); + let second = hash_watch_set_stamps(std::slice::from_ref(&watch)).unwrap(); + + assert_ne!( + first, second, + "a header edited under a symlinked library must invalidate the watch set" + ); +} + +#[test] +fn test_hash_watch_set_sees_edits_under_a_symlinked_library() { + if fbuild_core::platform::host::is_windows() { + return; + } + let tmp = tempfile::TempDir::new().unwrap(); + let (project, _, header) = make_symlinked_project(tmp.path()); + let watch = make_watch(&project, &tmp.path().join("project.json")); + + let first = hash_watch_set(std::slice::from_ref(&watch)).unwrap(); + fs::write(&header, "#pragma once\n#define SPI_PATH 2\n").unwrap(); + let second = hash_watch_set(std::slice::from_ref(&watch)).unwrap(); + + assert_ne!(first, second); +} + +#[test] +fn test_hash_watch_set_stamps_hashes_a_symlinked_source_file() { + if fbuild_core::platform::host::is_windows() { + return; + } + let tmp = tempfile::TempDir::new().unwrap(); + let src = tmp.path().join("src"); + fs::create_dir_all(&src).unwrap(); + let real = tmp.path().join("shared").join("main.cpp"); + fs::create_dir_all(real.parent().unwrap()).unwrap(); + fs::write(&real, "int main() { return 1; }\n").unwrap(); + fbuild_core::platform::fs::symlink_file(&real, &src.join("main.cpp")).unwrap(); + + let watch = make_watch(&src, &tmp.path().join("project.json")); + let first = hash_watch_set_stamps(std::slice::from_ref(&watch)).unwrap(); + std::thread::sleep(std::time::Duration::from_millis(20)); + fs::write(&real, "int main() { return 2; }\n").unwrap(); + let second = hash_watch_set_stamps(std::slice::from_ref(&watch)).unwrap(); + + assert_ne!(first, second); +} + +#[test] +fn test_hash_watch_set_stamps_tolerates_a_symlink_loop() { + if fbuild_core::platform::host::is_windows() { + return; + } + let tmp = tempfile::TempDir::new().unwrap(); + let src = tmp.path().join("src"); + let nested = src.join("nested"); + fs::create_dir_all(&nested).unwrap(); + fs::write(src.join("main.cpp"), "int main() { return 1; }\n").unwrap(); + // `src/nested/loop -> src` would recurse forever under a naive walk. + fbuild_core::platform::fs::symlink_dir(&src, &nested.join("loop")).unwrap(); + + let watch = make_watch(&src, &tmp.path().join("project.json")); + let hash = hash_watch_set_stamps(std::slice::from_ref(&watch)).unwrap(); + + assert!(!hash.is_empty()); +} From c8ef5bedb4f18c30f3a26f2e130f7daa8f18c193 Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Tue, 27 Oct 2026 15:43:11 -0700 Subject: [PATCH 3/3] test(cache): avoid naming PathBuf in symlink watch-set tests Dylint `ban_std_pathbuf` fires on `PathBuf` in type position outside the legacy allowlist; build the fixture paths with `Path::join` instead. --- .../src/build_fingerprint/symlink_tests.rs | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/crates/fbuild-build-engine/src/build_fingerprint/symlink_tests.rs b/crates/fbuild-build-engine/src/build_fingerprint/symlink_tests.rs index 96ed7012f..128acf674 100644 --- a/crates/fbuild-build-engine/src/build_fingerprint/symlink_tests.rs +++ b/crates/fbuild-build-engine/src/build_fingerprint/symlink_tests.rs @@ -6,7 +6,7 @@ //! reused (and flashed) unchanged. use std::fs; -use std::path::{Path, PathBuf}; +use std::path::Path; use super::{FingerprintWatch, hash_watch_set, hash_watch_set_stamps}; @@ -19,7 +19,10 @@ fn make_watch(root: &Path, cache_file: &Path) -> FingerprintWatch { } } -fn make_symlinked_project(tmp: &Path) -> (PathBuf, PathBuf, PathBuf) { +/// Lay out `/project/lib/FastLED -> /fastled2`, the shape #1753 +/// reported: the library (and every header in it) is reachable only through +/// the link. +fn make_symlinked_project(tmp: &Path) { let project = tmp.join("project"); let lib = project.join("lib"); fs::create_dir_all(&lib).unwrap(); @@ -32,7 +35,6 @@ fn make_symlinked_project(tmp: &Path) -> (PathBuf, PathBuf, PathBuf) { fs::write(&header, "#pragma once\n#define SPI_PATH 1\n").unwrap(); fbuild_core::platform::fs::symlink_dir(&target, &lib.join("FastLED")).unwrap(); - (project, target, header) } #[test] @@ -41,7 +43,12 @@ fn test_hash_watch_set_stamps_sees_edits_under_a_symlinked_library() { return; } let tmp = tempfile::TempDir::new().unwrap(); - let (project, _, header) = make_symlinked_project(tmp.path()); + make_symlinked_project(tmp.path()); + let project = tmp.path().join("project"); + let header = tmp + .path() + .join("fastled2") + .join("src/platforms/esp/32/fastspi_esp32_idf.h"); let watch = make_watch(&project, &tmp.path().join("project.json")); let first = hash_watch_set_stamps(std::slice::from_ref(&watch)).unwrap(); @@ -61,7 +68,12 @@ fn test_hash_watch_set_sees_edits_under_a_symlinked_library() { return; } let tmp = tempfile::TempDir::new().unwrap(); - let (project, _, header) = make_symlinked_project(tmp.path()); + make_symlinked_project(tmp.path()); + let project = tmp.path().join("project"); + let header = tmp + .path() + .join("fastled2") + .join("src/platforms/esp/32/fastspi_esp32_idf.h"); let watch = make_watch(&project, &tmp.path().join("project.json")); let first = hash_watch_set(std::slice::from_ref(&watch)).unwrap();