diff --git a/crates/fbuild-build-arm/src/renesas/renesas_compiler.rs b/crates/fbuild-build-arm/src/renesas/renesas_compiler.rs index 6e8155ea..e2f58af1 100644 --- a/crates/fbuild-build-arm/src/renesas/renesas_compiler.rs +++ b/crates/fbuild-build-arm/src/renesas/renesas_compiler.rs @@ -226,7 +226,7 @@ impl Compiler for RenesasCompiler { /// or expands. Without this, an object compiled with the old flags would /// be considered up-to-date after the suppressions change. See /// FastLED/fbuild#404. - fn rebuild_signature(&self, source: &Path, extra_flags: &[String]) -> String { + fn rebuild_signature(&self, source: &Path, extra_flags: &[String], output: &Path) -> String { let ext = source .extension() .unwrap_or_default() @@ -249,9 +249,12 @@ impl Compiler for RenesasCompiler { } else { extra_flags.to_vec() }; - // build_unflags stripped inside build_rebuild_signature (shared core), - // matching compile_c/compile_cpp on the write side (FastLED/fbuild#970). - crate::compiler::build_rebuild_signature( + // build_unflags stripped inside build_rebuild_signature_for_workspace + // (shared core), matching compile_c/compile_cpp on the write side + // (FastLED/fbuild#970). The workspace anchor keeps sibling workspaces + // with identical effective commands hash-equal (FastLED/fbuild#1346). + crate::compiler::build_rebuild_signature_for_workspace( + crate::zccache::compile_cwd_from_output(output).as_deref(), compiler_path, &flags, &[], @@ -454,8 +457,18 @@ mod tests { std::fs::write(&user_src, "// stub").unwrap(); let compiler = test_compiler().with_framework_root(framework_root); - let fsp_sig = compiler.rebuild_signature(&fsp_src, &[]); - let user_sig = compiler.rebuild_signature(&user_src, &[]); + // Object paths inside the resolved build layout so both sides + // relativize against the same workspace; the assertion targets the + // suppression-flag difference. + let build_dir = fbuild_paths::BuildLayout::new( + tmp.path().to_path_buf(), + "uno_r4".to_string(), + BuildProfile::Release, + ) + .resolve(); + let fsp_sig = + compiler.rebuild_signature(&fsp_src, &[], &build_dir.join("fsp/r_ioport.c.o")); + let user_sig = compiler.rebuild_signature(&user_src, &[], &build_dir.join("main.c.o")); assert_ne!( fsp_sig, user_sig, "FSP C signature must include the four -Wno-error= flags; \ @@ -479,8 +492,16 @@ mod tests { std::fs::write(&baseline_cpp, "// stub").unwrap(); let compiler = test_compiler().with_framework_root(framework_root); - let framework_cpp_sig = compiler.rebuild_signature(&cpp_src, &[]); - let baseline_cpp_sig = compiler.rebuild_signature(&baseline_cpp, &[]); + let build_dir = fbuild_paths::BuildLayout::new( + tmp.path().to_path_buf(), + "uno_r4".to_string(), + BuildProfile::Release, + ) + .resolve(); + let framework_cpp_sig = + compiler.rebuild_signature(&cpp_src, &[], &build_dir.join("core/Serial.cpp.o")); + let baseline_cpp_sig = + compiler.rebuild_signature(&baseline_cpp, &[], &build_dir.join("Serial.cpp.o")); assert_eq!( framework_cpp_sig, baseline_cpp_sig, "framework C++ sources must not pick up the C-only FSP \ diff --git a/crates/fbuild-build-engine/src/compiler.rs b/crates/fbuild-build-engine/src/compiler.rs index 507fe24d..e73b4e96 100644 --- a/crates/fbuild-build-engine/src/compiler.rs +++ b/crates/fbuild-build-engine/src/compiler.rs @@ -6,14 +6,20 @@ use fbuild_core::Result; use fbuild_core::path::NormalizedPath; use serde::Deserialize; -use sha2::{Digest, Sha256}; use std::collections::HashMap; use std::ffi::OsString; -use std::path::Component; use std::path::{Path, PathBuf}; -use std::sync::{Mutex, OnceLock}; use std::time::SystemTime; +// Rebuild-signature fingerprints live in `rebuild_signature.rs` (extracted +// to stay under the 1000-LOC limit); re-exported here so the public +// `crate::compiler::*` paths — used by the platform crates and this +// module's own write side — are unchanged. +pub use crate::rebuild_signature::{ + build_rebuild_signature, build_rebuild_signature_for_project, + build_rebuild_signature_for_workspace, +}; + // ── Shared config types (used by all platform MCU configs) ────────────── /// Compiler flags split by language. @@ -61,8 +67,6 @@ pub struct CompileResult { pub exit_code: i32, } -static COMPILER_IDENTITY_CACHE: OnceLock>> = OnceLock::new(); - /// Trait for platform-specific compilers. /// /// FastLED/fbuild#820 (Phase B of #813): `compile_one` is `async` so @@ -158,7 +162,16 @@ pub trait Compiler: Send + Sync { /// Stable fingerprint of the effective compile configuration for one source file. /// /// Used for incremental rebuild invalidation when flags or compiler paths change. - fn rebuild_signature(&self, source: &Path, extra_flags: &[String]) -> String { + /// + /// `output` is the object file path this source would produce. It anchors + /// the signature to the compile workspace (`compile_cwd_from_output`), so + /// workspace-relative include flags normalize identically on the write + /// side and the check side no matter which project directory hosts them — + /// this is what lets a stage-1 `.cmdhash` seeded into a sibling stage-2 + /// workspace match its fresh check-side signature (FastLED/fbuild#1346). + /// Pass any path when no object exists yet; only the workspace ancestry of + /// the path matters, never the object itself. + fn rebuild_signature(&self, source: &Path, extra_flags: &[String], output: &Path) -> String { let ext = source .extension() .unwrap_or_default() @@ -168,10 +181,12 @@ pub trait Compiler: Send + Sync { "c" | "s" => (self.gcc_path(), self.c_flags()), _ => (self.gxx_path(), self.cpp_flags()), }; - // build_unflags are stripped inside build_rebuild_signature (the shared - // core), matching compile_c/compile_cpp on the write side + let compile_cwd = crate::zccache::compile_cwd_from_output(output); + // build_unflags are stripped inside build_rebuild_signature_for_workspace + // (the shared core), matching compile_c/compile_cpp on the write side // (FastLED/fbuild#951, #970). - build_rebuild_signature( + build_rebuild_signature_for_workspace( + compile_cwd.as_deref(), compiler_path, &flags, &[], @@ -406,266 +421,6 @@ pub fn absolute_from_cwd(path: &Path) -> PathBuf { } } -/// Stable fingerprint of a compile invocation, used for incremental rebuild -/// invalidation. -/// -/// `unflags` are applied to `flags` and `extra_flags` **inside** this function -/// (matching the write side, where `compile_c`/`compile_cpp` run -/// `apply_compile_unflags` over exactly those two groups before compiling). -/// Centralizing the stripping here — rather than at each caller — is load -/// bearing: every `Compiler::rebuild_signature` override funnels through this -/// one function, so none of them can silently forget to strip `build_unflags` -/// and drift from the written signature (FastLED/fbuild#970). `pre_flags` -/// (e.g. ESP32 include flags) are **not** unflag-filtered, mirroring the write -/// side. Platforms with no `build_unflags` pass an empty slice → the hash is -/// byte-identical to before, so no signature churn for them. -pub fn build_rebuild_signature( - compiler_path: &Path, - flags: &[String], - pre_flags: &[String], - extra_flags: &[String], - unflags: &[String], -) -> String { - build_rebuild_signature_with_normalizer( - compiler_path, - flags, - pre_flags, - extra_flags, - unflags, - &normalize_signature_value, - ) -} - -/// Variant of [`build_rebuild_signature`] for global artifact cache keys. -/// -/// Any absolute path under `project_dir` is reduced to `.project/` -/// before hashing, so two fresh checkouts with the same project layout produce -/// the same cache key even when their absolute roots or basenames differ. -pub fn build_rebuild_signature_for_project( - project_dir: &Path, - compiler_path: &Path, - flags: &[String], - pre_flags: &[String], - extra_flags: &[String], - unflags: &[String], -) -> String { - let normalize = |value: &str| normalize_signature_value_for_project(value, project_dir); - build_rebuild_signature_with_normalizer( - compiler_path, - flags, - pre_flags, - extra_flags, - unflags, - &normalize, - ) -} - -fn build_rebuild_signature_with_normalizer( - compiler_path: &Path, - flags: &[String], - pre_flags: &[String], - extra_flags: &[String], - unflags: &[String], - normalize_value: &dyn Fn(&str) -> String, -) -> String { - let strip = |group: &[String]| -> Vec { - if unflags.is_empty() { - return group.to_vec(); - } - let mut filtered = group.to_vec(); - crate::pipeline::remove_unflagged_tokens(&mut filtered, unflags); - filtered - }; - let flags = strip(flags); - let extra_flags = strip(extra_flags); - - let mut hasher = Sha256::new(); - hasher.update(compiler_identity(compiler_path).as_bytes()); - hasher.update([0]); - for group in [flags.as_slice(), pre_flags, extra_flags.as_slice()] { - hash_signature_group(&mut hasher, group, normalize_value); - hasher.update([0xff]); - } - format!("{:x}", hasher.finalize()) -} - -fn hash_signature_group( - hasher: &mut Sha256, - group: &[String], - normalize_value: &dyn Fn(&str) -> String, -) { - let mut expects_path_value = false; - for flag in group { - let normalized = if expects_path_value { - expects_path_value = false; - normalize_value(flag) - } else { - expects_path_value = is_split_path_flag(flag); - normalize_signature_flag(flag, normalize_value) - }; - hasher.update(normalized.as_bytes()); - hasher.update([0]); - } -} - -fn is_split_path_flag(flag: &str) -> bool { - matches!( - flag, - "-I" | "-isystem" | "-iquote" | "-include" | "--sysroot" - ) -} - -fn normalize_signature_flag(flag: &str, normalize_value: &dyn Fn(&str) -> String) -> String { - for prefix in ["-I", "-isystem=", "-iquote=", "-include=", "--sysroot="] { - if let Some(value) = flag.strip_prefix(prefix) { - return format!("{prefix}{}", normalize_value(value)); - } - } - flag.to_string() -} - -fn normalize_signature_value(value: &str) -> String { - if value.is_empty() { - return String::new(); - } - let path = Path::new(value); - if !looks_like_absolute_path(path, value) { - return value.to_string(); - } - normalize_signature_path(path) -} - -fn normalize_signature_value_for_project(value: &str, project_dir: &Path) -> String { - if value.is_empty() { - return String::new(); - } - let path = Path::new(value); - if !looks_like_absolute_path(path, value) { - return value.to_string(); - } - let arg = fbuild_core::path::path_arg_for_compile_cwd(path, project_dir); - if !looks_like_absolute_path(Path::new(&arg), &arg) { - if arg == "." { - ".project".to_string() - } else { - format!(".project/{arg}") - } - } else { - normalize_signature_path(path) - } -} - -fn normalize_signature_path(path: &Path) -> String { - let normalized = normalize_signature_components(path); - if let Some(index) = normalized - .iter() - .position(|component| component.eq_ignore_ascii_case(".fbuild")) - { - return normalized[index..].join("/"); - } - if let Some(index) = normalized - .iter() - .position(|component| component.eq_ignore_ascii_case(".build")) - { - return normalized[index..].join("/"); - } - const TAIL_COMPONENTS: usize = 2; - let start = normalized.len().saturating_sub(TAIL_COMPONENTS); - normalized[start..].join("/") -} - -fn normalize_signature_components(path: &Path) -> Vec { - // FastLED/fbuild#911 — every per-component slash rewrite delegates - // to `NormalizedPath::display_slash()`, which owns the Windows - // `\` → `/` transform (and the UNC prefix strip) for the workspace. - // Same hand-rolled anti-pattern the compile pipeline used to have. - path.components() - .filter_map(|component| match component { - Component::Prefix(prefix) => { - Some(NormalizedPath::new(prefix.as_os_str()).display_slash()) - } - Component::RootDir => None, - Component::CurDir => None, - Component::ParentDir => Some("..".to_string()), - Component::Normal(value) => Some(NormalizedPath::new(value).display_slash()), - }) - .collect() -} - -fn looks_like_absolute_path(path: &Path, raw: &str) -> bool { - path.is_absolute() - || path.has_root() - || raw.starts_with('/') - || raw.starts_with('\\') - || raw.as_bytes().get(1) == Some(&b':') -} - -fn compiler_identity(path: &Path) -> String { - let cache = COMPILER_IDENTITY_CACHE.get_or_init(|| Mutex::new(HashMap::new())); - if let Some(identity) = cache - .lock() - .unwrap_or_else(|e| e.into_inner()) - .get(path) - .cloned() - { - return identity; - } - - let stem = path - .file_stem() - .and_then(|value| value.to_str()) - .unwrap_or_default() - .to_string(); - let version = compiler_version(path); - let identity = format!("{stem}\0{version}"); - cache - .lock() - .unwrap_or_else(|e| e.into_inner()) - .insert(path.to_path_buf(), identity.clone()); - identity -} - -fn compiler_version(path: &Path) -> String { - // FastLED/fbuild#820 (Phase B of #813): `fbuild_core::subprocess:: - // run_command` is now `async`. `compiler_version` is called from - // the sync `rebuild_signature` trait method (which is in turn - // called from sync rebuild-check code paths), so we bridge to the - // ambient tokio runtime via `block_in_place` + `block_on`. This is - // safe because the daemon runs on a multi-thread tokio runtime and - // `block_in_place` permits this exact pattern. - let program = path.to_string_lossy().to_string(); - let result = match tokio::runtime::Handle::try_current() { - Ok(handle) => tokio::task::block_in_place(|| { - handle.block_on(async { - let args = [program.as_str(), "-dumpversion"]; - // FastLED/fbuild#809: `gcc -dumpversion` is trivial; a - // hung toolchain binary (corrupt EXE, missing-DLL hang - // on Windows) should not block the whole pipeline. - fbuild_core::subprocess::run_command( - &args, - None, - None, - Some(std::time::Duration::from_secs(5)), - ) - .await - }) - }), - Err(_) => { - // No ambient runtime — happens in unit-test contexts that - // don't spin up a tokio runtime. Returning an empty version - // is a graceful degradation: rebuild-signature loses the - // compiler-version contribution but still encodes path + - // flags, which is enough for the tests that don't touch a - // real toolchain. - return String::new(); - } - }; - match result { - Ok(output) if output.success() => output.stdout.trim().to_string(), - _ => String::new(), - } -} - fn dependency_is_newer_than_object( depfile: &Path, object_time: SystemTime, @@ -878,8 +633,19 @@ pub async fn compile_source( // compile_c/compile_cpp before reaching compile_one, so pass empty unflags // (re-stripping would be a no-op). The read side reconstructs the same // filtered set inside build_rebuild_signature (FastLED/fbuild#970). - let rebuild_signature = - build_rebuild_signature(compiler, flags, extra_pre_flags, extra_flags, &[]); + // + // The signature is anchored to `compile_cwd` — the same workspace the + // argv above was relativized against — so the written `.cmdhash` matches + // the check-side signature of any workspace that would run an identical + // compiler command (FastLED/fbuild#1346). + let rebuild_signature = build_rebuild_signature_for_workspace( + compile_cwd.as_deref(), + compiler, + flags, + extra_pre_flags, + extra_flags, + &[], + ); all_flags.extend(["-c".to_string(), source_arg, "-o".to_string(), output_arg]); let global = crate::compile_backend::get_global().ok_or_else(|| { diff --git a/crates/fbuild-build-engine/src/compiler_tests.rs b/crates/fbuild-build-engine/src/compiler_tests.rs index 1c5c1be4..05fa8d5d 100644 --- a/crates/fbuild-build-engine/src/compiler_tests.rs +++ b/crates/fbuild-build-engine/src/compiler_tests.rs @@ -372,6 +372,124 @@ fn test_needs_rebuild_when_command_hash_changes() { )); } +#[test] +fn test_build_rebuild_signature_for_workspace_relativizes_workspace_includes() { + // FastLED/fbuild#1346: sibling workspaces whose effective compile + // commands are byte-identical after cwd relativization must hash + // identically, or a stage-1 `.cmdhash` seeded into stage 2 never matches. + let tmp = tempfile::tempdir().unwrap(); + let build_dir_for = |root: &std::path::Path| { + fbuild_paths::BuildLayout::new( + root.to_path_buf(), + "uno".to_string(), + fbuild_core::BuildProfile::Release, + ) + .resolve() + }; + let (s0, s1) = (tmp.path().join("s0"), tmp.path().join("s1")); + for dir in [&s0, &s1] { + std::fs::create_dir_all(dir.join("src")).unwrap(); + } + let flags_s0 = vec![format!("-I{}", s0.join("src").display()), "-Os".to_string()]; + let flags_s1 = vec![format!("-I{}", s1.join("src").display()), "-Os".to_string()]; + let compiler = Path::new("/tool/bin/avr-g++"); + let obj_for = |root: &std::path::Path| build_dir_for(root).join("core/CDC.cpp.o"); + + let sig_s0 = build_rebuild_signature_for_workspace( + crate::zccache::compile_cwd_from_output(&obj_for(&s0)).as_deref(), + compiler, + &flags_s0, + &[], + &[], + &[], + ); + let sig_s1 = build_rebuild_signature_for_workspace( + crate::zccache::compile_cwd_from_output(&obj_for(&s1)).as_deref(), + compiler, + &flags_s1, + &[], + &[], + &[], + ); + + assert_eq!(sig_s0, sig_s1); +} + +#[test] +fn test_build_rebuild_signature_for_workspace_none_matches_legacy() { + // compile_cwd = None must stay byte-for-byte the legacy builder so + // no-.fbuild-ancestor layouts keep their historical signatures. + let flags = vec!["-I/tmp/ws/project/include".to_string(), "-Os".to_string()]; + let legacy = build_rebuild_signature(Path::new("/tool/bin/gcc"), &flags, &[], &[], &[]); + let workspace = build_rebuild_signature_for_workspace( + None, + Path::new("/tool/bin/gcc"), + &flags, + &[], + &[], + &[], + ); + assert_eq!(legacy, workspace); +} + +#[test] +fn test_build_rebuild_signature_for_workspace_outside_paths_keep_legacy_normalization() { + // Include dirs outside the workspace (global framework cache) fall back + // to the project-independent normalization — unchanged behavior. + let tmp_a = tempfile::tempdir().unwrap(); + let tmp_b = tempfile::tempdir().unwrap(); + let global_a = tmp_a.path().join(".fbuild/cache/framework/cores/arduino"); + let global_b = tmp_b.path().join(".fbuild/cache/framework/cores/arduino"); + let ws_a = tmp_a.path().join("proj-a"); + let ws_b = tmp_b.path().join("other-name-proj-b"); + std::fs::create_dir_all(&ws_a).unwrap(); + std::fs::create_dir_all(&ws_b).unwrap(); + let flags_a = vec![format!("-I{}", global_a.display())]; + let flags_b = vec![format!("-I{}", global_b.display())]; + let compiler = Path::new("/tool/bin/gcc"); + + let sig_a = + build_rebuild_signature_for_workspace(Some(&ws_a), compiler, &flags_a, &[], &[], &[]); + let sig_b = + build_rebuild_signature_for_workspace(Some(&ws_b), compiler, &flags_b, &[], &[], &[]); + + assert_eq!(sig_a, sig_b); +} + +#[test] +fn test_build_rebuild_signature_distinguishes_different_effective_includes() { + // Two workspaces with genuinely different include targets inside them + // must NOT collide — only identical effective commands hash equal. + let tmp = tempfile::tempdir().unwrap(); + let (s0, s1) = (tmp.path().join("s0"), tmp.path().join("s1")); + for dir in [&s0, &s1] { + std::fs::create_dir_all(dir.join("src")).unwrap(); + } + let compiler = Path::new("/tool/bin/gcc"); + let shared = vec!["-Os".to_string()]; + let flags_s0 = vec![format!("-I{}", s0.join("src").display())]; + let flags_s1 = vec![format!("-I{}", s1.join("include").display())]; + + let sig_s0 = build_rebuild_signature_for_workspace( + Some(&s0), + compiler, + &[flags_s0, shared.clone()].concat(), + &[], + &[], + &[], + ); + let sig_s1 = build_rebuild_signature_for_workspace( + Some(&s1), + compiler, + &[flags_s1, shared].concat(), + &[], + &[], + &[], + ); + + assert_ne!(sig_s0, sig_s1); +} + #[test] fn test_build_rebuild_signature_ignores_absolute_compiler_path() { let flags = vec!["-Os".to_string(), "-mmcu=atmega328p".to_string()]; diff --git a/crates/fbuild-build-engine/src/framework_core_cache.rs b/crates/fbuild-build-engine/src/framework_core_cache.rs index 1a2e4054..88119ade 100644 --- a/crates/fbuild-build-engine/src/framework_core_cache.rs +++ b/crates/fbuild-build-engine/src/framework_core_cache.rs @@ -274,7 +274,7 @@ fn refresh_command_hashes( continue; } let source_flags = extra_flags.for_source(source); - let signature = compiler.rebuild_signature(source, &source_flags); + let signature = compiler.rebuild_signature(source, &source_flags, &obj); std::fs::write(obj.with_extension("cmdhash"), signature)?; refreshed += 1; } @@ -348,7 +348,12 @@ mod tests { vec!["-Os".to_string(), "-std=gnu++17".to_string()] } - fn rebuild_signature(&self, source: &Path, extra_flags: &[String]) -> String { + fn rebuild_signature( + &self, + source: &Path, + extra_flags: &[String], + _output: &Path, + ) -> String { let mut hasher = Sha256::new(); hasher.update(source.to_string_lossy().as_bytes()); for flag in extra_flags { @@ -567,10 +572,10 @@ mod tests { assert_eq!(stats.copied, 3); let hydrated_object = hydrated.join(object.file_name().unwrap()); let hydrated_cmdhash = hydrated_object.with_extension("cmdhash"); - assert_eq!(std::fs::read(hydrated_object).unwrap(), b"obj"); + assert_eq!(std::fs::read(&hydrated_object).unwrap(), b"obj"); assert_eq!( std::fs::read_to_string(hydrated_cmdhash).unwrap(), - compiler.rebuild_signature(&source, &[]) + compiler.rebuild_signature(&source, &[], &hydrated_object) ); } diff --git a/crates/fbuild-build-engine/src/lib.rs b/crates/fbuild-build-engine/src/lib.rs index c16fd093..65ace706 100644 --- a/crates/fbuild-build-engine/src/lib.rs +++ b/crates/fbuild-build-engine/src/lib.rs @@ -28,6 +28,7 @@ pub mod package_override; pub mod parallel; pub mod perf_log; pub mod pipeline; +pub mod rebuild_signature; pub mod resolution; pub mod script_runtime; pub mod shrink; diff --git a/crates/fbuild-build-engine/src/parallel.rs b/crates/fbuild-build-engine/src/parallel.rs index 19f78d63..5b5162c2 100644 --- a/crates/fbuild-build-engine/src/parallel.rs +++ b/crates/fbuild-build-engine/src/parallel.rs @@ -65,7 +65,10 @@ pub async fn compile_sources_parallel( for source in sources { let obj = CompilerBase::object_path(source, build_dir); let source_flags = extra_flags.for_source(source); - let signature = compiler.rebuild_signature(source, &source_flags); + // The object path anchors the workspace: a `.cmdhash` written by a + // sibling workspace with an identical effective compile command must + // match this check (stage-2 seeding, FastLED/fbuild#1346). + let signature = compiler.rebuild_signature(source, &source_flags, &obj); if CompilerBase::needs_rebuild_with_signature(source, &obj, Some(&signature)) { work.push((source.clone(), obj.clone())); } diff --git a/crates/fbuild-build-engine/src/rebuild_signature.rs b/crates/fbuild-build-engine/src/rebuild_signature.rs new file mode 100644 index 00000000..f0331d0a --- /dev/null +++ b/crates/fbuild-build-engine/src/rebuild_signature.rs @@ -0,0 +1,338 @@ +//! Rebuild-signature fingerprints for incremental compile invalidation. +//! +//! Extracted from `compiler.rs` to keep both files under the workspace's +//! 1000-LOC limit; all public items are re-exported from +//! [`crate::compiler`] so external paths are unchanged. + +use std::collections::HashMap; +use std::path::{Component, Path}; +use std::sync::{Mutex, OnceLock}; + +use fbuild_core::path::NormalizedPath; +use sha2::{Digest, Sha256}; + +/// Memoized `compiler_identity` results, keyed by +/// `fbuild_core::path::normalize_for_key` rather than the raw path. Two +/// spellings of the same compiler (case differences on Windows, a +/// verbatim prefix, a trailing slash) are the same toolchain and must +/// not each pay for a `--version` subprocess — FastLED/fbuild#952. +static COMPILER_IDENTITY_CACHE: OnceLock>> = OnceLock::new(); + +/// Stable fingerprint of a compile invocation, used for incremental rebuild +/// invalidation. +/// +/// `unflags` are applied to `flags` and `extra_flags` **inside** this function +/// (matching the write side, where `compile_c`/`compile_cpp` run +/// `apply_compile_unflags` over exactly those two groups before compiling). +/// Centralizing the stripping here — rather than at each caller — is load +/// bearing: every `Compiler::rebuild_signature` override funnels through this +/// one function, so none of them can silently forget to strip `build_unflags` +/// and drift from the written signature (FastLED/fbuild#970). `pre_flags` +/// (e.g. ESP32 include flags) are **not** unflag-filtered, mirroring the write +/// side. Platforms with no `build_unflags` pass an empty slice → the hash is +/// byte-identical to before, so no signature churn for them. +pub fn build_rebuild_signature( + compiler_path: &Path, + flags: &[String], + pre_flags: &[String], + extra_flags: &[String], + unflags: &[String], +) -> String { + build_rebuild_signature_with_normalizer( + compiler_path, + flags, + pre_flags, + extra_flags, + unflags, + &normalize_signature_value, + ) +} + +/// Variant of [`build_rebuild_signature`] for global artifact cache keys. +/// +/// Any absolute path under `project_dir` is reduced to `.project/` +/// before hashing, so two fresh checkouts with the same project layout produce +/// the same cache key even when their absolute roots or basenames differ. +pub fn build_rebuild_signature_for_project( + project_dir: &Path, + compiler_path: &Path, + flags: &[String], + pre_flags: &[String], + extra_flags: &[String], + unflags: &[String], +) -> String { + let normalize = |value: &str| normalize_signature_value_for_project(value, project_dir); + build_rebuild_signature_with_normalizer( + compiler_path, + flags, + pre_flags, + extra_flags, + unflags, + &normalize, + ) +} + +/// Variant of [`build_rebuild_signature`] anchored to a compile workspace. +/// +/// Absolute path-bearing flag values that live *inside* `compile_cwd` are +/// relativized against it before hashing — exactly the transform the executed +/// argv undergoes in [`crate::compiler::compile_source`] — so two workspaces +/// that would run byte-identical compiler commands hash identically even when +/// their absolute roots differ. Values outside the workspace fall back to the +/// legacy project-independent normalization. With `compile_cwd = None` this is +/// byte-for-byte [`build_rebuild_signature`]. +/// +/// FastLED/fbuild#1346: without the workspace anchor, sibling stage-2 +/// workspaces (`.tmpX/s0/src` vs `.tmpX/s1/src`) hit the last-two-components +/// fallback and produced different signatures from identical effective +/// commands, so every seeded framework object failed its `.cmdhash` check and +/// stage 2 recompiled the whole framework. +pub fn build_rebuild_signature_for_workspace( + compile_cwd: Option<&Path>, + compiler_path: &Path, + flags: &[String], + pre_flags: &[String], + extra_flags: &[String], + unflags: &[String], +) -> String { + let normalize = |value: &str| match compile_cwd { + Some(cwd) => normalize_signature_value_for_workspace(value, cwd), + None => normalize_signature_value(value), + }; + build_rebuild_signature_with_normalizer( + compiler_path, + flags, + pre_flags, + extra_flags, + unflags, + &normalize, + ) +} + +fn build_rebuild_signature_with_normalizer( + compiler_path: &Path, + flags: &[String], + pre_flags: &[String], + extra_flags: &[String], + unflags: &[String], + normalize_value: &dyn Fn(&str) -> String, +) -> String { + let strip = |group: &[String]| -> Vec { + if unflags.is_empty() { + return group.to_vec(); + } + let mut filtered = group.to_vec(); + crate::pipeline::remove_unflagged_tokens(&mut filtered, unflags); + filtered + }; + let flags = strip(flags); + let extra_flags = strip(extra_flags); + + let mut hasher = Sha256::new(); + hasher.update(compiler_identity(compiler_path).as_bytes()); + hasher.update([0]); + for group in [flags.as_slice(), pre_flags, extra_flags.as_slice()] { + hash_signature_group(&mut hasher, group, normalize_value); + hasher.update([0xff]); + } + format!("{:x}", hasher.finalize()) +} + +fn hash_signature_group( + hasher: &mut Sha256, + group: &[String], + normalize_value: &dyn Fn(&str) -> String, +) { + let mut expects_path_value = false; + for flag in group { + let normalized = if expects_path_value { + expects_path_value = false; + normalize_value(flag) + } else { + expects_path_value = is_split_path_flag(flag); + normalize_signature_flag(flag, normalize_value) + }; + hasher.update(normalized.as_bytes()); + hasher.update([0]); + } +} + +fn is_split_path_flag(flag: &str) -> bool { + matches!( + flag, + "-I" | "-isystem" | "-iquote" | "-include" | "--sysroot" + ) +} + +fn normalize_signature_flag(flag: &str, normalize_value: &dyn Fn(&str) -> String) -> String { + for prefix in ["-I", "-isystem=", "-iquote=", "-include=", "--sysroot="] { + if let Some(value) = flag.strip_prefix(prefix) { + return format!("{prefix}{}", normalize_value(value)); + } + } + flag.to_string() +} + +fn normalize_signature_value(value: &str) -> String { + if value.is_empty() { + return String::new(); + } + let path = Path::new(value); + if !looks_like_absolute_path(path, value) { + return value.to_string(); + } + normalize_signature_path(path) +} + +/// Workspace-anchored counterpart of [`normalize_signature_value`]. +/// +/// Absolute values inside the compile workspace relativize to their +/// workspace-relative form (the same string the compiler actually sees on its +/// command line); everything else keeps the legacy project-independent +/// normalization. +fn normalize_signature_value_for_workspace(value: &str, compile_cwd: &Path) -> String { + if value.is_empty() { + return String::new(); + } + let path = Path::new(value); + if !looks_like_absolute_path(path, value) { + return value.to_string(); + } + let arg = fbuild_core::path::path_arg_for_compile_cwd(path, compile_cwd); + if !looks_like_absolute_path(Path::new(&arg), &arg) { + return arg; + } + normalize_signature_path(path) +} + +fn normalize_signature_value_for_project(value: &str, project_dir: &Path) -> String { + if value.is_empty() { + return String::new(); + } + let path = Path::new(value); + if !looks_like_absolute_path(path, value) { + return value.to_string(); + } + let arg = fbuild_core::path::path_arg_for_compile_cwd(path, project_dir); + if !looks_like_absolute_path(Path::new(&arg), &arg) { + if arg == "." { + ".project".to_string() + } else { + format!(".project/{arg}") + } + } else { + normalize_signature_path(path) + } +} + +fn normalize_signature_path(path: &Path) -> String { + let normalized = normalize_signature_components(path); + if let Some(index) = normalized + .iter() + .position(|component| component.eq_ignore_ascii_case(fbuild_paths::FBUILD_DIR_NAME)) + { + return normalized[index..].join("/"); + } + if let Some(index) = normalized + .iter() + .position(|component| component.eq_ignore_ascii_case(".build")) + { + return normalized[index..].join("/"); + } + const TAIL_COMPONENTS: usize = 2; + let start = normalized.len().saturating_sub(TAIL_COMPONENTS); + normalized[start..].join("/") +} + +fn normalize_signature_components(path: &Path) -> Vec { + // FastLED/fbuild#911 — every per-component slash rewrite delegates + // to `NormalizedPath::display_slash()`, which owns the Windows + // `\` → `/` transform (and the UNC prefix strip) for the workspace. + // Same hand-rolled anti-pattern the compile pipeline used to have. + path.components() + .filter_map(|component| match component { + Component::Prefix(prefix) => { + Some(NormalizedPath::new(prefix.as_os_str()).display_slash()) + } + Component::RootDir => None, + Component::CurDir => None, + Component::ParentDir => Some("..".to_string()), + Component::Normal(value) => Some(NormalizedPath::new(value).display_slash()), + }) + .collect() +} + +fn looks_like_absolute_path(path: &Path, raw: &str) -> bool { + path.is_absolute() + || path.has_root() + || raw.starts_with('/') + || raw.starts_with('\\') + || raw.as_bytes().get(1) == Some(&b':') +} + +fn compiler_identity(path: &Path) -> String { + let cache = COMPILER_IDENTITY_CACHE.get_or_init(|| Mutex::new(HashMap::new())); + let key = fbuild_core::path::normalize_for_key(path); + if let Some(identity) = cache + .lock() + .unwrap_or_else(|e| e.into_inner()) + .get(&key) + .cloned() + { + return identity; + } + + let stem = path + .file_stem() + .and_then(|value| value.to_str()) + .unwrap_or_default() + .to_string(); + let version = compiler_version(path); + let identity = format!("{stem}\0{version}"); + cache + .lock() + .unwrap_or_else(|e| e.into_inner()) + .insert(key, identity.clone()); + identity +} + +fn compiler_version(path: &Path) -> String { + // FastLED/fbuild#820 (Phase B of #813): `fbuild_core::subprocess:: + // run_command` is now `async`. `compiler_version` is called from + // the sync `rebuild_signature` trait method (which is in turn + // called from sync rebuild-check code paths), so we bridge to the + // ambient tokio runtime via `block_in_place` + `block_on`. This is + // safe because the daemon runs on a multi-thread tokio runtime and + // `block_in_place` permits this exact pattern. + let program = path.to_string_lossy().to_string(); + let result = match tokio::runtime::Handle::try_current() { + Ok(handle) => tokio::task::block_in_place(|| { + handle.block_on(async { + let args = [program.as_str(), "-dumpversion"]; + // FastLED/fbuild#809: `gcc -dumpversion` is trivial; a + // hung toolchain binary (corrupt EXE, missing-DLL hang + // on Windows) should not block the whole pipeline. + fbuild_core::subprocess::run_command( + &args, + None, + None, + Some(std::time::Duration::from_secs(5)), + ) + .await + }) + }), + Err(_) => { + // No ambient runtime — happens in unit-test contexts that + // don't spin up a tokio runtime. Returning an empty version + // is a graceful degradation: rebuild-signature loses the + // compiler-version contribution but still encodes path + + // flags, which is enough for the tests that don't touch a + // real toolchain. + return String::new(); + } + }; + match result { + Ok(output) if output.success() => output.stdout.trim().to_string(), + _ => String::new(), + } +} diff --git a/crates/fbuild-build-engine/src/zccache_embedded.rs b/crates/fbuild-build-engine/src/zccache_embedded.rs index 1e43cd8c..4e25e102 100644 --- a/crates/fbuild-build-engine/src/zccache_embedded.rs +++ b/crates/fbuild-build-engine/src/zccache_embedded.rs @@ -91,8 +91,16 @@ pub struct EmbeddedCompileOutcome { } impl FbuildZccacheService { - /// Start the embedded service on the caller's tokio runtime, - /// rooted at `~/.fbuild//zccache/`. + /// Start the embedded service on the caller's tokio runtime. + /// + /// Root resolution, in precedence order: + /// + /// 1. `FBUILD_ZCCACHE_ROOT`, used verbatim when set. This mirrors + /// `fbuild_paths`' own `FBUILD_CACHE_DIR` escape hatch and lets a + /// test harness isolate the embedded cache instead of contending + /// with the shared prod writer slot — FastLED/fbuild#1346, #1347. + /// 2. `fbuild_paths::get_fbuild_root().join("zccache")`, i.e. + /// `~/.fbuild//zccache/`, the production default. /// /// Idempotent only at the file-system level — `create_dir_all` /// on the cache root is safe under concurrent callers. Two @@ -101,15 +109,24 @@ impl FbuildZccacheService { /// daemon's startup path is single-threaded so this is not /// exercised today. pub async fn start() -> Result { + // Escape hatch mirroring fbuild_paths' `FBUILD_CACHE_DIR`: point + // the embedded service at an alternate root. Test harnesses use + // this to avoid contending with (or polluting) the shared prod + // writer slot — FastLED/fbuild#1346, #1347. + if let Some(root) = std::env::var_os("FBUILD_ZCCACHE_ROOT") { + return Self::start_in(PathBuf::from(root)).await; + } Self::start_in(fbuild_paths::get_fbuild_root().join("zccache")).await } /// Start with an explicit cache root. /// - /// Production callers should use [`Self::start`], which derives - /// the root from `fbuild_paths`. This entry point exists so the - /// smoke test (`tests/zccache_embedded_smoke.rs`) can point at a - /// per-test tempdir and not contaminate the user's real + /// The root is used exactly as given — neither + /// `FBUILD_ZCCACHE_ROOT` nor `fbuild_paths` is consulted here. + /// Production callers should use [`Self::start`], which applies + /// that precedence chain. This entry point exists so the smoke test + /// (`tests/zccache_embedded_smoke.rs`) can point at a per-test + /// tempdir and not contaminate the user's real /// `~/.fbuild//zccache/`. Phase 2 (#791) may also use this /// to host multiple service instances per integration test. pub async fn start_in(cache_root: PathBuf) -> Result { diff --git a/crates/fbuild-build-esp/src/esp32/esp32_compiler.rs b/crates/fbuild-build-esp/src/esp32/esp32_compiler.rs index dff90267..6bce5702 100644 --- a/crates/fbuild-build-esp/src/esp32/esp32_compiler.rs +++ b/crates/fbuild-build-esp/src/esp32/esp32_compiler.rs @@ -196,7 +196,7 @@ impl Compiler for Esp32Compiler { crate::compiler::build_cpp_flags(self.common_flags(), &self.mcu_config) } - fn rebuild_signature(&self, source: &Path, extra_flags: &[String]) -> String { + fn rebuild_signature(&self, source: &Path, extra_flags: &[String], output: &Path) -> String { let ext = source .extension() .unwrap_or_default() @@ -211,10 +211,13 @@ impl Compiler for Esp32Compiler { "c" | "s" => self.gcc_path(), _ => self.gxx_path(), }; - // build_unflags stripped inside build_rebuild_signature (shared core), - // matching compile_c/compile_cpp on the write side - // (FastLED/fbuild#951, #970). - crate::compiler::build_rebuild_signature( + // build_unflags stripped inside build_rebuild_signature_for_workspace + // (shared core), matching compile_c/compile_cpp on the write side + // (FastLED/fbuild#951, #970). The workspace anchor keeps sibling + // workspaces with identical effective commands hash-equal + // (FastLED/fbuild#1346). + crate::compiler::build_rebuild_signature_for_workspace( + crate::zccache::compile_cwd_from_output(output).as_deref(), compiler_path, &base_flags, &include_flags, @@ -417,20 +420,30 @@ mod tests { fn rebuild_signature_matches_write_path_with_unflags() { let compiler = test_compiler("esp32c6").with_build_unflags(vec!["-std=gnu++2b".to_string()]); + let workspace = tempfile::TempDir::new().unwrap(); + let build_dir = fbuild_paths::BuildLayout::new( + workspace.path().to_path_buf(), + "esp32c6".to_string(), + BuildProfile::Release, + ) + .resolve(); + let object = build_dir.join("src/main.cpp.o"); let source = Path::new("src/main.cpp"); let extra = vec!["-DX=1".to_string()]; - let check = compiler.rebuild_signature(source, &extra); + let check = compiler.rebuild_signature(source, &extra, &object); // Mirror the write path: compile_cpp applies unflags, then - // compile_source hashes (flags, include_flags, extra). + // compile_source hashes (flags, include_flags, extra) anchored to the + // same compile workspace. let (applied_flags, applied_extra) = crate::compiler::apply_compile_unflags( compiler.cpp_flags(), &extra, Compiler::build_unflags(&compiler), ); let include_flags = compiler.base.build_include_flags(); - let written = crate::compiler::build_rebuild_signature( + let written = crate::compiler::build_rebuild_signature_for_workspace( + crate::zccache::compile_cwd_from_output(&object).as_deref(), compiler.gxx_path(), &applied_flags, &include_flags, @@ -441,6 +454,48 @@ mod tests { assert_eq!(check, written); } + /// FastLED/fbuild#1346: two sibling workspaces running byte-identical + /// effective compile commands must produce identical rebuild signatures, + /// or a `.cmdhash` seeded from stage 1 never matches stage 2's fresh + /// check and the whole framework recompiles per sketch. + #[test] + fn rebuild_signature_matches_across_sibling_workspaces() { + let compiler = test_compiler("esp32c6"); + let tmp = tempfile::TempDir::new().unwrap(); + let build_dir_for = |root: &std::path::Path| { + fbuild_paths::BuildLayout::new( + root.to_path_buf(), + "esp32c6".to_string(), + BuildProfile::Release, + ) + .resolve() + }; + let s0 = tmp.path().join("s0"); + let s1 = tmp.path().join("s1"); + for dir in [&s0, &s1] { + std::fs::create_dir_all(build_dir_for(dir).join("core")).unwrap(); + std::fs::create_dir_all(dir.join("src")).unwrap(); + } + let source = Path::new("cores/arduino/CDC.cpp"); + // Each workspace's own compile carried its own sketch-src include; + // after argv relativization both run as plain `-Isrc`. + let extra_s0 = vec![format!("-I{}", s0.join("src").display())]; + let extra_s1 = vec![format!("-I{}", s1.join("src").display())]; + + let sig_s0 = compiler.rebuild_signature( + source, + &extra_s0, + &build_dir_for(&s0).join("core/CDC.cpp.o"), + ); + let sig_s1 = compiler.rebuild_signature( + source, + &extra_s1, + &build_dir_for(&s1).join("core/CDC.cpp.o"), + ); + + assert_eq!(sig_s0, sig_s1); + } + /// FastLED/fbuild#243: by default the compiler preserves eh_frame; the /// STRIP_FLAGS must not leak into the effective compile line. #[test] diff --git a/crates/fbuild-build-mcu/src/ch32v/ch32v_compiler.rs b/crates/fbuild-build-mcu/src/ch32v/ch32v_compiler.rs index 6286fbd7..b18cd8f2 100644 --- a/crates/fbuild-build-mcu/src/ch32v/ch32v_compiler.rs +++ b/crates/fbuild-build-mcu/src/ch32v/ch32v_compiler.rs @@ -191,7 +191,7 @@ impl Compiler for Ch32vCompiler { /// rebuild fingerprint changes whenever the third-party flag set changes. /// Otherwise an existing object compiled with the old flags would be /// considered up-to-date after the suppressions move or expand. - fn rebuild_signature(&self, source: &Path, extra_flags: &[String]) -> String { + fn rebuild_signature(&self, source: &Path, extra_flags: &[String], output: &Path) -> String { let ext = source .extension() .unwrap_or_default() @@ -214,9 +214,12 @@ impl Compiler for Ch32vCompiler { } else { extra_flags.to_vec() }; - // build_unflags stripped inside build_rebuild_signature (shared core), - // matching compile_c/compile_cpp on the write side (FastLED/fbuild#970). - crate::compiler::build_rebuild_signature( + // build_unflags stripped inside build_rebuild_signature_for_workspace + // (shared core), matching compile_c/compile_cpp on the write side + // (FastLED/fbuild#970). The workspace anchor keeps sibling workspaces + // with identical effective commands hash-equal (FastLED/fbuild#1346). + crate::compiler::build_rebuild_signature_for_workspace( + crate::zccache::compile_cwd_from_output(output).as_deref(), compiler_path, &flags, &[], @@ -386,8 +389,19 @@ mod tests { std::fs::write(&user_src, "// stub").unwrap(); let compiler = test_compiler().with_framework_root(framework_root); - let core_sig = compiler.rebuild_signature(&core_src, &[]); - let user_sig = compiler.rebuild_signature(&user_src, &[]); + // Object paths inside the resolved build layout so both sides + // relativize against the same workspace; the assertion targets the + // suppression-flag difference. + let build_dir = fbuild_paths::BuildLayout::new( + tmp.path().to_path_buf(), + "ch32v".to_string(), + BuildProfile::Release, + ) + .resolve(); + let core_obj = build_dir.join("core/analog.cpp.o"); + let user_obj = build_dir.join("main.cpp.o"); + let core_sig = compiler.rebuild_signature(&core_src, &[], &core_obj); + let user_sig = compiler.rebuild_signature(&user_src, &[], &user_obj); assert_ne!( core_sig, user_sig, "framework signature must include the suppression flags" diff --git a/crates/fbuild-build/tests/compile_many_stage2_perf.rs b/crates/fbuild-build/tests/compile_many_stage2_perf.rs index 3cdcaf5b..fd87f7e8 100644 --- a/crates/fbuild-build/tests/compile_many_stage2_perf.rs +++ b/crates/fbuild-build/tests/compile_many_stage2_perf.rs @@ -1,14 +1,20 @@ //! Real-toolchain regression gate for the stage-2 framework-archive -//! sharing fix (FastLED/fbuild#335 / PR #337). Empirically, with the seed -//! in place, each stage-2 sketch's `build_time_secs` should be a small -//! fraction of stage-1's — because stage 2 reuses stage 1's compiled -//! framework `core/` via the per-worker seed step in `run_stage2`. +//! sharing fix (FastLED/fbuild#335 / PR #337). Stage 2 reuses stage 1's +//! compiled framework `core/` via the per-worker seed step in `run_stage2`; +//! the orchestrator's freshness check must then skip every framework TU. //! -//! This test scaffolds 4 identical blink sketches, runs `compile_many` -//! cold (no shared cache other than the seed), and asserts that every -//! stage-2 sketch landed under a tight multiple of stage-1's wall time. -//! A regression that re-introduces per-stage-2 framework rebuilds would -//! push stage-2 times up to roughly stage-1 wall and fail the assertion. +//! This test scaffolds 4 identical blink sketches and runs `compile_many`. +//! The regression signal is **work done**, read from each stage-2 sketch's +//! `compile_many.log` (`Compiled N/M files` lines): with the seed working, +//! a stage-2 worker compiles only its own sketch translation unit (M == 1 +//! for this fixture). A regression that re-introduces per-stage-2 framework +//! rebuilds — e.g. FastLED/fbuild#1346, where seeded `.cmdhash` files never +//! matched because sibling workspaces hashed their workspace-relative +//! include flags differently — shows up as M == 26 here. +//! +//! Wall times are printed for diagnostics but not asserted: since the +//! global core-artifact cache can pre-hydrate stage 1, both stages' fixed +//! link/size costs dominate and wall ratios are machine-load noise. //! //! Gated `#[ignore]` because it downloads avr-gcc + Arduino-AVR core on //! the first run (cached afterward). Run with: @@ -22,7 +28,7 @@ use std::fs; use std::path::{Path, PathBuf}; use fbuild_build::compile_backend; -use fbuild_build::compile_many::{CompileManyRequest, Stage, compile_many}; +use fbuild_build::compile_many::{CompileManyRequest, SketchResult, Stage, compile_many}; use fbuild_core::BuildProfile; /// 15-min wall-clock cap for `--ignored` real-toolchain tests (FastLED/fbuild#806). @@ -128,47 +134,175 @@ async fn stage2_per_sketch_wall_is_a_fraction_of_stage1() { .collect(); assert_eq!(stage2.len(), 3, "three stage-2 results expected"); - let stage1_secs = stage1.build_time_secs; eprintln!( "stage 1 wall: {:.2}s ({})", - stage1_secs, + stage1.build_time_secs, stage1.sketch.display() ); - // Threshold: stage-2 must come in well under stage-1. If the seed - // is doing its job, every framework `compiler.compile` call is a - // zccache hit (or an mtime-fresh skip via the seed) and stage 2's - // remaining work is sketch.cpp + link + size — which empirically - // lands around 200ms vs stage-1's ~600ms-1.5s on the same hardware. - // - // The bound is intentionally loose (50%) so this passes on slow CI - // runners and tightens enough to catch a regression that puts - // stage-2 back at "rebuild the framework from scratch" (which - // would be ≥80% of stage-1). - let max_allowed = stage1_secs * 0.5; + // Primary oracle — work done, not wall time. Every stage-2 worker must + // compile only its own sketch TU against the seeded framework core. The + // fixture's blink sketch is a single translation unit, so every + // `Compiled N/M files` line in the build log must have M <= 1. A value + // of M == 26 means the whole framework recompiled despite a successful + // seed — the FastLED/fbuild#1346 failure signature. for r in &stage2 { eprintln!( - "stage 2 wall: {:.2}s ({}) [must be < {:.2}s]", + "stage 2 wall: {:.2}s ({}) seed_applied={} seed_time={:.3}s worker={:?}", r.build_time_secs, r.sketch.display(), - max_allowed + r.seed_applied, + r.seed_time_secs, + r.worker_index ); assert!( - r.build_time_secs < max_allowed, - "stage-2 sketch {} wall {:.2}s exceeded 50% of stage-1 \ - ({:.2}s); the framework-archive seed (FastLED/fbuild#337) \ - is likely not actually skipping the recompile. Inspect \ - {}/.fbuild/build/uno/release/compile_many.log — if it \ - prints `Compiled 25/25 files` followed by `Linking firmware.elf` \ - but the .o mtimes match stage-1's, the per-file zccache hit \ - is succeeding and the wall regression is elsewhere; otherwise \ - check that `seed_stage2_core_from_stage1` ran (look for the \ - `compile-many stage 2 seed: linked N + copied N` tracing \ - info line at info level).", - r.sketch.display(), - r.build_time_secs, - stage1_secs, + r.seed_applied, + "stage-2 sketch {} should have had a core seed applied +{}", r.sketch.display(), + stage2_failure_detail(r) + ); + assert!( + max_compiled_batch_size(r) <= 1, + "{}", + stage2_failure_detail(r) ); } } + +/// Largest work-list size M from `Compiled N/M files` lines in this +/// sketch's `compile_many.log`. M is the number of TUs the freshness +/// check queued — with the seed working it equals the sketch's own TU +/// count. +/// +/// Fails closed: a missing, unreadable, empty, or malformed log yields +/// `usize::MAX`, never 0. A log with no parsable work record proves +/// nothing about the stage-2 work limit, and returning 0 would let the +/// `<= 1` assertion pass vacuously — turning the #1346 regression oracle +/// into a no-op the moment the log format drifts. The caller surfaces +/// the log itself through [`stage2_failure_detail`]. +fn max_compiled_batch_size(r: &SketchResult) -> usize { + let Some(log_path) = r.log_path.as_deref() else { + return usize::MAX; + }; + let Ok(log) = fs::read_to_string(log_path) else { + return usize::MAX; + }; + max_compiled_batch_size_in(&log) +} + +/// The parse half of [`max_compiled_batch_size`], split out so the +/// fail-closed contract is covered by a cheap unit test instead of only by +/// the `#[ignore]`d real-toolchain oracle. +fn max_compiled_batch_size_in(log: &str) -> usize { + log.lines() + .filter_map(|line| { + let rest = line.strip_prefix("Compiled ")?; + let (_n, m) = rest.split_once('/')?; + let m = m.split_whitespace().next()?; + m.parse::().ok() + }) + .max() + .unwrap_or(usize::MAX) +} + +/// The oracle is only worth having if it cannot pass by accident. These +/// lock the fail-closed contract: every shape of log that fails to prove +/// how much work stage 2 did must read as `usize::MAX`, so the `<= 1` +/// assertion rejects it — FastLED/fbuild#1346. +#[test] +fn unparsable_logs_fail_closed() { + assert_eq!(max_compiled_batch_size_in(""), usize::MAX, "empty log"); + assert_eq!( + max_compiled_batch_size_in( + "Building sketch +Linking firmware.elf +" + ), + usize::MAX, + "log with no work record" + ); + assert_eq!( + max_compiled_batch_size_in( + "Compiled some/of files +" + ), + usize::MAX, + "work record with a non-numeric denominator" + ); + assert_eq!( + max_compiled_batch_size_in( + "Compiled 1 file +" + ), + usize::MAX, + "work record without the N/M separator" + ); +} + +/// The pass state (`M == 1`) and the #1346 failure signature +/// (`M == 26`) must both survive the parse, and multiple records take +/// the maximum rather than the last. +#[test] +fn work_records_parse_to_their_denominator() { + assert_eq!( + max_compiled_batch_size_in( + "Compiled 1/1 files +" + ), + 1 + ); + assert_eq!( + max_compiled_batch_size_in( + "Compiled 26/26 files +" + ), + 26 + ); + assert_eq!( + max_compiled_batch_size_in( + "Compiled 1/1 files +Compiled 5/26 files +" + ), + 26, + "a later small batch must not mask an earlier full rebuild" + ); +} + +/// Build the panic message for a failed stage-2 work assertion. Reads the +/// sketch's `compile_many.log` (still alive inside the TempDir at this +/// point) and embeds its head and tail so the failure is diagnosable after +/// the TempDir is dropped — FastLED/fbuild#1346. +fn stage2_failure_detail(r: &SketchResult) -> String { + let log_path = r + .log_path + .clone() + .unwrap_or_else(|| r.sketch.join(".fbuild/build/uno/release/compile_many.log")); + let log = fs::read_to_string(&log_path).unwrap_or_else(|e| { + format!( + "", + log_path.display() + ) + }); + let total_lines = log.lines().count(); + let head: Vec<&str> = log.lines().take(15).collect(); + let mut tail: Vec<&str> = log.lines().rev().take(40).collect::>(); + tail.reverse(); + format!( + "stage-2 sketch {} compiled more than its own sketch TU against the \ + seeded framework — the framework-archive seed (FastLED/fbuild#337) \ + is likely not actually skipping the recompile — \ + FastLED/fbuild#1346.\n\ + seed_applied={} seed_time={:.3}s worker={:?}\n\ + compile_many.log at {} ({} lines) head:\n{}\n…tail:\n{}", + r.sketch.display(), + r.seed_applied, + r.seed_time_secs, + r.worker_index, + log_path.display(), + total_lines, + head.join("\n"), + tail.join("\n"), + ) +}