Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 22 additions & 4 deletions crates/fbuild-build/src/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,11 @@ pub trait Compiler: Send + Sync {
"c" | "s" => (self.gcc_path(), self.c_flags()),
_ => (self.gxx_path(), self.cpp_flags()),
};
build_rebuild_signature(compiler_path, &flags, &[], extra_flags)
// Mirror compile_c/compile_cpp: build_unflags are applied before
// the compile, so the checked signature must hash the same
// filtered flag set as the written one (FastLED/fbuild#951).
let (flags, extra_flags) = apply_compile_unflags(flags, extra_flags, self.build_unflags());
build_rebuild_signature(compiler_path, &flags, &[], &extra_flags)
}
}

Expand Down Expand Up @@ -242,7 +246,16 @@ impl CompilerBase {

let depfile = depfile_path(object);
if depfile.exists() {
if dependency_is_newer_than_object(&depfile, obj_time).unwrap_or(true) {
// Compiles run with cwd = the project workspace (see
// zccache::compile_cwd_from_output), so -MMD depfiles list
// workspace-relative prerequisites. Resolve them against that
// same workspace — NOT the process cwd, which in the daemon is
// unrelated and made every stat fail → every TU "stale"
// (FastLED/fbuild#951).
let dep_base = crate::zccache::compile_cwd_from_output(object);
if dependency_is_newer_than_object(&depfile, obj_time, dep_base.as_deref())
.unwrap_or(true)
{
return true;
}
return false;
Expand Down Expand Up @@ -285,7 +298,7 @@ impl CompilerBase {
/// Returns the filtered pair ready to pass to `compile_one`. Short-circuits
/// when `unflags` is empty so platforms that don't opt in pay no overhead.
/// See FastLED/fbuild#37.
fn apply_compile_unflags(
pub(crate) fn apply_compile_unflags(
flags: Vec<String>,
extra_flags: &[String],
unflags: &[String],
Expand Down Expand Up @@ -496,14 +509,19 @@ fn compiler_version(path: &Path) -> String {
fn dependency_is_newer_than_object(
depfile: &Path,
object_time: SystemTime,
base: Option<&Path>,
) -> std::io::Result<bool> {
let depfile_time = depfile.metadata()?.modified()?;
if depfile_time > object_time {
return Ok(true);
}

for dependency in parse_depfile_paths(depfile)? {
let dep_time = std::fs::metadata(&dependency)?.modified()?;
let resolved = match base {
Some(base) if dependency.is_relative() => base.join(&dependency),
_ => dependency,
};
let dep_time = std::fs::metadata(&resolved)?.modified()?;
if dep_time > object_time {
return Ok(true);
}
Expand Down
43 changes: 43 additions & 0 deletions crates/fbuild-build/src/compiler_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,49 @@ fn test_needs_rebuild_uses_depfile_when_dependencies_are_current() {
assert!(!CompilerBase::needs_rebuild(&src, &obj));
}

/// FastLED/fbuild#951: compiles run with cwd = the project workspace
/// (see `zccache::compile_cwd_from_output`), so gcc's `-MMD` depfiles
/// list *relative* prerequisites. The staleness walk runs inside the
/// long-lived daemon whose process cwd is unrelated — resolving those
/// prerequisites against the process cwd made `metadata()` fail and
/// `.unwrap_or(true)` marked every TU stale, recompiling the whole
/// sketch + core variant on no-change rebuilds (~103 s per build).
#[test]
fn test_needs_rebuild_resolves_relative_depfile_deps_against_workspace() {
let tmp = tempfile::TempDir::new().unwrap();
let ws = tmp.path();
let src_dir = ws.join("src");
let build_dir = ws.join(".fbuild/build/demo/release/src");
std::fs::create_dir_all(&src_dir).unwrap();
std::fs::create_dir_all(&build_dir).unwrap();

let src = src_dir.join("main.cpp");
let header = src_dir.join("config.h");
let obj = build_dir.join("main.cpp.o");
let dep = build_dir.join("main.cpp.d");

std::fs::write(&src, "#include \"config.h\"\n").unwrap();
std::fs::write(&header, "#define X 1\n").unwrap();
// Relative prerequisites, exactly as gcc -MMD emits them when the
// compile cwd is the workspace root.
std::fs::write(
&dep,
".fbuild/build/demo/release/src/main.cpp.o: src/main.cpp src/config.h\n",
)
.unwrap();
std::thread::sleep(std::time::Duration::from_millis(20));
std::fs::write(&obj, "obj").unwrap();

// Everything is current — must NOT rebuild even though the deps are
// relative and the test process cwd is nowhere near the workspace.
assert!(!CompilerBase::needs_rebuild(&src, &obj));

// And a genuinely newer relative dep must still trigger a rebuild.
std::thread::sleep(std::time::Duration::from_millis(20));
std::fs::write(&header, "#define X 2\n").unwrap();
assert!(CompilerBase::needs_rebuild(&src, &obj));
}

#[test]
fn test_needs_rebuild_when_command_hash_changes() {
let tmp = tempfile::TempDir::new().unwrap();
Expand Down
43 changes: 42 additions & 1 deletion crates/fbuild-build/src/esp32/esp32_compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,19 @@ impl Compiler for Esp32Compiler {
"c" | "s" => self.gcc_path(),
_ => self.gxx_path(),
};
// Mirror compile_c/compile_cpp: build_unflags are applied before
// the compile, so the checked signature must hash the same
// filtered flag set as the written one (FastLED/fbuild#951).
let (base_flags, extra_flags) = crate::compiler::apply_compile_unflags(
base_flags,
extra_flags,
Compiler::build_unflags(self),
);
crate::compiler::build_rebuild_signature(
compiler_path,
&base_flags,
&include_flags,
extra_flags,
&extra_flags,
)
}
}
Expand Down Expand Up @@ -373,6 +381,39 @@ mod tests {
);
}

/// FastLED/fbuild#951: the staleness check's signature must equal the
/// one written after a compile. `compile_c`/`compile_cpp` apply
/// `build_unflags` before hashing, but `rebuild_signature` hashed the
/// raw flags — so any project with `build_unflags` (NightDriverStrip
/// sets `-std=gnu++11`) mismatched on every file and recompiled the
/// entire sketch + core on no-change rebuilds.
#[test]
fn rebuild_signature_matches_write_path_with_unflags() {
let compiler =
test_compiler("esp32c6").with_build_unflags(vec!["-std=gnu++2b".to_string()]);
let source = Path::new("src/main.cpp");
let extra = vec!["-DX=1".to_string()];

let check = compiler.rebuild_signature(source, &extra);

// Mirror the write path: compile_cpp applies unflags, then
// compile_source hashes (flags, include_flags, extra).
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(
compiler.gxx_path(),
&applied_flags,
&include_flags,
&applied_extra,
);

assert_eq!(check, written);
}

/// FastLED/fbuild#243: by default the compiler preserves eh_frame; the
/// STRIP_FLAGS must not leak into the effective compile line.
#[test]
Expand Down
Loading