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..beaceee57 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); @@ -459,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::*; 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..128acf674 --- /dev/null +++ b/crates/fbuild-build-engine/src/build_fingerprint/symlink_tests.rs @@ -0,0 +1,125 @@ +//! 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; + +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()], + } +} + +/// 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(); + 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(); +} + +#[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(); + 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(); + 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(); + 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(); + 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()); +}