diff --git a/.github/workflows/dylint.yml b/.github/workflows/dylint.yml index d97fdb15..2ed63d8f 100644 --- a/.github/workflows/dylint.yml +++ b/.github/workflows/dylint.yml @@ -1,9 +1,9 @@ name: Dylint -# Runs the custom `ban_raw_subprocess` dylint over the workspace. -# Lives in its own job (and uses its own nightly toolchain pin) so the -# stable workspace check on `check-ubuntu.yml` isn't blocked by dylint -# infrastructure issues. See #264. +# Runs the custom dylints over the workspace. This is enforced as its own +# required GitHub status check rather than being folded into `./lint`, because +# Dylint builds the lint crates and rustc-dev driver and would force a heavy +# recompile on every local lint call. See #264 and #994. on: workflow_dispatch: {} @@ -69,8 +69,7 @@ jobs: # `@.` but cargo emits them as bare # `.` on first build. After each cargo-dylint # invocation, alias any just-built libraries that lack the - # `@` suffix and retry. Same loop zccache uses in - # `ci/lint.py::lint_dylint_only`. + # `@` suffix and retry. run: | export PATH="${CARGO_HOME}/bin:${PATH}" set +e diff --git a/crates/fbuild-build/src/ch32v/ch32v_compiler.rs b/crates/fbuild-build/src/ch32v/ch32v_compiler.rs index f7e42387..e8fce11c 100644 --- a/crates/fbuild-build/src/ch32v/ch32v_compiler.rs +++ b/crates/fbuild-build/src/ch32v/ch32v_compiler.rs @@ -224,6 +224,44 @@ impl Compiler for Ch32vCompiler { self.build_unflags(), ) } + + fn artifact_cache_signature( + &self, + project_dir: &Path, + source: &Path, + extra_flags: &[String], + ) -> String { + let ext = source + .extension() + .unwrap_or_default() + .to_string_lossy() + .to_lowercase(); + let (compiler_path, flags) = match ext.as_str() { + "c" | "s" => (self.gcc_path(), self.c_flags()), + _ => (self.gxx_path(), self.cpp_flags()), + }; + let extra_owned: Vec = if self.is_framework_source(source) { + extra_flags + .iter() + .cloned() + .chain( + framework_suppression_flags() + .iter() + .map(|s| (*s).to_string()), + ) + .collect() + } else { + extra_flags.to_vec() + }; + crate::compiler::build_rebuild_signature_for_project( + project_dir, + compiler_path, + &flags, + &[], + &extra_owned, + self.build_unflags(), + ) + } } #[cfg(test)] diff --git a/crates/fbuild-build/src/compiler.rs b/crates/fbuild-build/src/compiler.rs index 0a8b054e..143044e5 100644 --- a/crates/fbuild-build/src/compiler.rs +++ b/crates/fbuild-build/src/compiler.rs @@ -179,6 +179,39 @@ pub trait Compiler: Send + Sync { self.build_unflags(), ) } + + /// Project-independent fingerprint for reusable artifact caches. + /// + /// Unlike [`Self::rebuild_signature`], this normalizes absolute + /// project-local include/source paths relative to `project_dir`. The normal + /// rebuild signature remains project-local because it is written into a + /// project's own `.cmdhash`; global artifact caches need the same logical + /// project shape to hash identically even when copied to a different + /// absolute checkout path or basename. + fn artifact_cache_signature( + &self, + project_dir: &Path, + source: &Path, + extra_flags: &[String], + ) -> String { + let ext = source + .extension() + .unwrap_or_default() + .to_string_lossy() + .to_lowercase(); + let (compiler_path, flags) = match ext.as_str() { + "c" | "s" => (self.gcc_path(), self.c_flags()), + _ => (self.gxx_path(), self.cpp_flags()), + }; + build_rebuild_signature_for_project( + project_dir, + compiler_path, + &flags, + &[], + extra_flags, + self.build_unflags(), + ) + } } /// Shared compiler utilities used by all platform-specific compilers. @@ -392,6 +425,48 @@ pub fn build_rebuild_signature( 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() { @@ -408,21 +483,25 @@ pub fn build_rebuild_signature( 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); + hash_signature_group(&mut hasher, group, normalize_value); hasher.update([0xff]); } format!("{:x}", hasher.finalize()) } -fn hash_signature_group(hasher: &mut Sha256, group: &[String]) { +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_signature_value(flag) + normalize_value(flag) } else { expects_path_value = is_split_path_flag(flag); - normalize_signature_flag(flag) + normalize_signature_flag(flag, normalize_value) }; hasher.update(normalized.as_bytes()); hasher.update([0]); @@ -436,10 +515,10 @@ fn is_split_path_flag(flag: &str) -> bool { ) } -fn normalize_signature_flag(flag: &str) -> String { +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_signature_value(value)); + return format!("{prefix}{}", normalize_value(value)); } } flag.to_string() @@ -456,6 +535,26 @@ fn normalize_signature_value(value: &str) -> 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 diff --git a/crates/fbuild-build/src/compiler_tests.rs b/crates/fbuild-build/src/compiler_tests.rs index f1d0ac8c..6b5da2f6 100644 --- a/crates/fbuild-build/src/compiler_tests.rs +++ b/crates/fbuild-build/src/compiler_tests.rs @@ -442,6 +442,41 @@ fn test_build_rebuild_signature_ignores_attached_include_root() { assert_eq!(sig_a, sig_b); } +#[test] +fn test_artifact_cache_signature_ignores_project_directory_name() { + let tmp = tempfile::tempdir().unwrap(); + let project_a = tmp.path().join("nds"); + let project_b = tmp.path().join("nds-copy"); + let include_a = project_a.join("src"); + let include_b = project_b.join("src"); + std::fs::create_dir_all(&include_a).unwrap(); + std::fs::create_dir_all(&include_b).unwrap(); + let flags_a = vec![format!("-I{}", include_a.display())]; + let flags_b = vec![format!("-I{}", include_b.display())]; + + let sig_a = build_rebuild_signature_for_project( + &project_a, + Path::new("/tmp/tool/bin/xtensa-esp32-elf-g++"), + &flags_a, + &[], + &[], + &[], + ); + let sig_b = build_rebuild_signature_for_project( + &project_b, + Path::new("/tmp/tool/bin/xtensa-esp32-elf-g++"), + &flags_b, + &[], + &[], + &[], + ); + + assert_eq!( + sig_a, sig_b, + "artifact cache keys must depend on project layout, not checkout basename" + ); +} + #[test] fn test_build_rebuild_signature_ignores_split_path_flag_values() { let flags_a = vec![ diff --git a/crates/fbuild-build/src/esp32/esp32_compiler.rs b/crates/fbuild-build/src/esp32/esp32_compiler.rs index 49269a4e..a4cd059e 100644 --- a/crates/fbuild-build/src/esp32/esp32_compiler.rs +++ b/crates/fbuild-build/src/esp32/esp32_compiler.rs @@ -222,6 +222,36 @@ impl Compiler for Esp32Compiler { Compiler::build_unflags(self), ) } + + fn artifact_cache_signature( + &self, + project_dir: &Path, + source: &Path, + extra_flags: &[String], + ) -> String { + let ext = source + .extension() + .unwrap_or_default() + .to_string_lossy() + .to_lowercase(); + let base_flags = match ext.as_str() { + "c" | "s" => self.c_flags(), + _ => self.cpp_flags(), + }; + let include_flags = self.base.build_include_flags(); + let compiler_path = match ext.as_str() { + "c" | "s" => self.gcc_path(), + _ => self.gxx_path(), + }; + crate::compiler::build_rebuild_signature_for_project( + project_dir, + compiler_path, + &base_flags, + &include_flags, + extra_flags, + Compiler::build_unflags(self), + ) + } } // Response file utilities (write_response_file, replace_path_backslashes) diff --git a/crates/fbuild-build/src/esp32/orchestrator/build.rs b/crates/fbuild-build/src/esp32/orchestrator/build.rs index 6c6847ac..294cb51d 100644 --- a/crates/fbuild-build/src/esp32/orchestrator/build.rs +++ b/crates/fbuild-build/src/esp32/orchestrator/build.rs @@ -631,7 +631,7 @@ impl BuildOrchestrator for Esp32Orchestrator { }; if let Some(cache) = core_cache.as_ref() { let _g = perf.phase("core-cache-hydrate"); - match cache.hydrate(core_build_dir) { + match cache.hydrate(core_build_dir, &compiler, &all_core_sources, &user_overlay) { Ok(stats) if stats.copied > 0 || stats.skipped > 0 => tracing::info!( "framework core cache hydrate key={} copied={} skipped={} from {}", cache.key(), diff --git a/crates/fbuild-build/src/framework_core_cache.rs b/crates/fbuild-build/src/framework_core_cache.rs index e5f24e81..6ea72682 100644 --- a/crates/fbuild-build/src/framework_core_cache.rs +++ b/crates/fbuild-build/src/framework_core_cache.rs @@ -6,13 +6,14 @@ //! `~/.fbuild/{dev|prod}/cache/core//` and hydrates project `core/` dirs //! before normal incremental rebuild checks run. +use std::collections::HashSet; use std::path::{Path, PathBuf}; use fbuild_core::path::NormalizedPath; use fbuild_core::BuildProfile; use sha2::{Digest, Sha256}; -use crate::compiler::Compiler; +use crate::compiler::{Compiler, CompilerBase}; use crate::flag_overlay::LanguageExtraFlags; const CORE_CACHE_VERSION: &str = "fbuild-core-artifacts-v1"; @@ -40,6 +41,7 @@ impl FrameworkCoreCache { extra_flags: &LanguageExtraFlags, ) -> Self { let key = core_cache_key( + project_dir, platform_label, env_name, profile, @@ -61,11 +63,26 @@ impl FrameworkCoreCache { &self.path } - pub fn hydrate(&self, core_build_dir: &Path) -> std::io::Result { + pub fn hydrate( + &self, + core_build_dir: &Path, + compiler: &dyn Compiler, + core_sources: &[PathBuf], + extra_flags: &LanguageExtraFlags, + ) -> std::io::Result { if !self.path.is_dir() { return Ok(ArtifactCopyStats::default()); } - copy_artifacts(&self.path, core_build_dir, false) + let mut outcome = copy_artifacts(&self.path, core_build_dir, false, false)?; + let refreshed = refresh_command_hashes( + core_build_dir, + compiler, + core_sources, + extra_flags, + &outcome.copied_objects, + )?; + outcome.stats.copied += refreshed; + Ok(outcome.stats) } pub fn store(&self, core_build_dir: &Path) -> std::io::Result { @@ -73,11 +90,12 @@ impl FrameworkCoreCache { return Ok(ArtifactCopyStats::default()); } std::fs::create_dir_all(&self.path)?; - copy_artifacts(core_build_dir, &self.path, true) + Ok(copy_artifacts(core_build_dir, &self.path, true, true)?.stats) } } fn core_cache_key( + project_dir: &Path, platform_label: &str, env_name: &str, profile: BuildProfile, @@ -102,9 +120,13 @@ fn core_cache_key( .map(|source| { // FastLED/fbuild#911 — path-shape slash normalization goes // through `NormalizedPath::display_slash()`. - let source_path = NormalizedPath::from(source.as_path()).display_slash(); + let source_path = if source.is_absolute() { + fbuild_core::path::path_arg_for_compile_cwd(source, project_dir) + } else { + NormalizedPath::from(source.as_path()).display_slash() + }; let source_flags = extra_flags.for_source(source); - let signature = compiler.rebuild_signature(source, &source_flags); + let signature = compiler.artifact_cache_signature(project_dir, source, &source_flags); let content = file_content_digest(source); (source_path, signature, content) }) @@ -138,37 +160,79 @@ fn copy_artifacts( src_dir: &Path, dst_dir: &Path, overwrite: bool, -) -> std::io::Result { + include_cmdhash: bool, +) -> std::io::Result { std::fs::create_dir_all(dst_dir)?; - let mut stats = ArtifactCopyStats::default(); + let mut outcome = ArtifactCopyOutcome::default(); for entry in std::fs::read_dir(src_dir)? { let entry = entry?; if !entry.file_type()?.is_file() { continue; } let src = entry.path(); - if !is_core_artifact(&src) { + if !is_core_artifact(&src, include_cmdhash) { continue; } let dst = dst_dir.join(entry.file_name()); if dst.exists() { if !overwrite { - stats.skipped += 1; + outcome.stats.skipped += 1; continue; } std::fs::remove_file(&dst)?; } copy_preserving_mtime(&src, &dst)?; - stats.copied += 1; + if is_object_artifact(&src) { + outcome.copied_objects.insert(entry.file_name()); + } + outcome.stats.copied += 1; } - Ok(stats) + Ok(outcome) } -fn is_core_artifact(path: &Path) -> bool { +#[derive(Default)] +struct ArtifactCopyOutcome { + stats: ArtifactCopyStats, + copied_objects: HashSet, +} + +fn is_core_artifact(path: &Path, include_cmdhash: bool) -> bool { matches!( path.extension().and_then(|ext| ext.to_str()), - Some("o" | "d" | "cmdhash") - ) + Some("o" | "d") + ) || (include_cmdhash + && matches!( + path.extension().and_then(|ext| ext.to_str()), + Some("cmdhash") + )) +} + +fn is_object_artifact(path: &Path) -> bool { + matches!(path.extension().and_then(|ext| ext.to_str()), Some("o")) +} + +fn refresh_command_hashes( + core_build_dir: &Path, + compiler: &dyn Compiler, + core_sources: &[PathBuf], + extra_flags: &LanguageExtraFlags, + copied_objects: &HashSet, +) -> std::io::Result { + let mut refreshed = 0; + for source in core_sources { + let obj = CompilerBase::object_path(source, core_build_dir); + let Some(name) = obj.file_name() else { + continue; + }; + if !copied_objects.contains(name) { + continue; + } + let source_flags = extra_flags.for_source(source); + let signature = compiler.rebuild_signature(source, &source_flags); + std::fs::write(obj.with_extension("cmdhash"), signature)?; + refreshed += 1; + } + Ok(refreshed) } fn copy_preserving_mtime(src: &Path, dst: &Path) -> std::io::Result { @@ -266,6 +330,7 @@ mod tests { asm: Vec::new(), }; let a = core_cache_key( + Path::new("/project"), "avr", "uno", BuildProfile::Release, @@ -274,6 +339,7 @@ mod tests { &plain, ); let b = core_cache_key( + Path::new("/project"), "avr", "uno", BuildProfile::Release, @@ -300,6 +366,7 @@ mod tests { }; let a = core_cache_key( + tmp.path(), "avr", "uno", BuildProfile::Release, @@ -308,6 +375,7 @@ mod tests { &plain, ); let b = core_cache_key( + tmp.path(), "avr", "uno", BuildProfile::Release, @@ -332,6 +400,7 @@ mod tests { std::fs::write(&source, b"first").unwrap(); let first = core_cache_key( + tmp.path(), "avr", "uno", BuildProfile::Release, @@ -341,6 +410,7 @@ mod tests { ); std::fs::write(&source, b"second").unwrap(); let second = core_cache_key( + tmp.path(), "avr", "uno", BuildProfile::Release, @@ -351,6 +421,47 @@ mod tests { assert_ne!(first, second); } + #[test] + fn key_is_independent_of_project_directory_name() { + let compiler = FakeCompiler::new(); + let tmp = tempfile::tempdir().unwrap(); + let project_a = tmp.path().join("nds"); + let project_b = tmp.path().join("nds-copy"); + let source_a = project_a.join("src").join("main.cpp"); + let source_b = project_b.join("src").join("main.cpp"); + std::fs::create_dir_all(source_a.parent().unwrap()).unwrap(); + std::fs::create_dir_all(source_b.parent().unwrap()).unwrap(); + std::fs::write(&source_a, b"same core").unwrap(); + std::fs::write(&source_b, b"same core").unwrap(); + let plain = LanguageExtraFlags { + common: Vec::new(), + c: Vec::new(), + cxx: Vec::new(), + asm: Vec::new(), + }; + + let a = core_cache_key( + &project_a, + "esp32", + "esp32dev", + BuildProfile::Release, + &compiler, + &[source_a], + &plain, + ); + let b = core_cache_key( + &project_b, + "esp32", + "esp32dev", + BuildProfile::Release, + &compiler, + &[source_b], + &plain, + ); + + assert_eq!(a, b); + } + #[test] fn hydrate_and_store_only_core_artifact_files() { let tmp = tempfile::tempdir().unwrap(); @@ -373,20 +484,42 @@ mod tests { let core = tmp.path().join("core"); std::fs::create_dir_all(&core).unwrap(); - std::fs::write(core.join("main.cpp.o"), b"obj").unwrap(); - std::fs::write(core.join("main.cpp.d"), b"dep").unwrap(); - std::fs::write(core.join("main.cpp.cmdhash"), b"hash").unwrap(); + let source = project.join("src").join("main.cpp"); + std::fs::create_dir_all(source.parent().unwrap()).unwrap(); + std::fs::write(&source, b"int main() { return 0; }\n").unwrap(); + let object = CompilerBase::object_path(&source, &core); + let depfile = object.with_extension("d"); + let cmdhash = object.with_extension("cmdhash"); + std::fs::write(&object, b"obj").unwrap(); + std::fs::write(&depfile, b"dep").unwrap(); + std::fs::write(&cmdhash, b"hash").unwrap(); std::fs::write(core.join("note.txt"), b"ignore").unwrap(); let stored = cache.store(&core).unwrap(); assert_eq!(stored.copied, 3); - assert!(cache.path().join("main.cpp.o").exists()); + assert!(cache.path().join(object.file_name().unwrap()).exists()); assert!(!cache.path().join("note.txt").exists()); let hydrated = tmp.path().join("hydrated"); - let stats = cache.hydrate(&hydrated).unwrap(); + let flags = LanguageExtraFlags { + common: Vec::new(), + c: Vec::new(), + cxx: Vec::new(), + asm: Vec::new(), + }; + let compiler = FakeCompiler::new(); + let sources = vec![source.clone()]; + let stats = cache + .hydrate(&hydrated, &compiler, &sources, &flags) + .unwrap(); assert_eq!(stats.copied, 3); - assert_eq!(std::fs::read(hydrated.join("main.cpp.o")).unwrap(), b"obj"); + 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_to_string(hydrated_cmdhash).unwrap(), + compiler.rebuild_signature(&source, &[]) + ); } #[test] @@ -399,9 +532,9 @@ mod tests { std::fs::write(src.join("main.cpp.o"), b"cache").unwrap(); std::fs::write(dst.join("main.cpp.o"), b"project").unwrap(); - let stats = copy_artifacts(&src, &dst, false).unwrap(); - assert_eq!(stats.copied, 0); - assert_eq!(stats.skipped, 1); + let outcome = copy_artifacts(&src, &dst, false, true).unwrap(); + assert_eq!(outcome.stats.copied, 0); + assert_eq!(outcome.stats.skipped, 1); assert_eq!(std::fs::read(dst.join("main.cpp.o")).unwrap(), b"project"); } } diff --git a/crates/fbuild-build/src/pipeline/sequential.rs b/crates/fbuild-build/src/pipeline/sequential.rs index bc661e63..34e4657e 100644 --- a/crates/fbuild-build/src/pipeline/sequential.rs +++ b/crates/fbuild-build/src/pipeline/sequential.rs @@ -130,7 +130,12 @@ pub async fn run_sequential_build_with_libs( }; if let Some(cache) = core_cache.as_ref() { let _g = perf.phase("core-cache-hydrate"); - match cache.hydrate(&ctx.core_build_dir) { + match cache.hydrate( + &ctx.core_build_dir, + compiler, + &core_and_variant, + &user_overlay, + ) { Ok(stats) if stats.copied > 0 || stats.skipped > 0 => tracing::info!( "framework core cache hydrate key={} copied={} skipped={} from {}", cache.key(), diff --git a/crates/fbuild-build/src/renesas/renesas_compiler.rs b/crates/fbuild-build/src/renesas/renesas_compiler.rs index 24243825..6e8155ea 100644 --- a/crates/fbuild-build/src/renesas/renesas_compiler.rs +++ b/crates/fbuild-build/src/renesas/renesas_compiler.rs @@ -259,6 +259,44 @@ impl Compiler for RenesasCompiler { self.build_unflags(), ) } + + fn artifact_cache_signature( + &self, + project_dir: &Path, + source: &Path, + extra_flags: &[String], + ) -> String { + let ext = source + .extension() + .unwrap_or_default() + .to_string_lossy() + .to_lowercase(); + let (compiler_path, flags) = match ext.as_str() { + "c" | "s" => (self.gcc_path(), self.c_flags()), + _ => (self.gxx_path(), self.cpp_flags()), + }; + let extra_owned: Vec = if is_c_source(source) && self.is_framework_source(source) { + extra_flags + .iter() + .cloned() + .chain( + framework_c_suppression_flags() + .iter() + .map(|s| (*s).to_string()), + ) + .collect() + } else { + extra_flags.to_vec() + }; + crate::compiler::build_rebuild_signature_for_project( + project_dir, + compiler_path, + &flags, + &[], + &extra_owned, + self.build_unflags(), + ) + } } #[cfg(test)]