Skip to content

fix(cache): follow symlinks in the warm-build watch set - #1765

Merged
zackees merged 3 commits into
mainfrom
agent/fix-symlink-lib-staleness
Oct 27, 2026
Merged

zackees merged 3 commits into
mainfrom
agent/fix-symlink-lib-staleness

Conversation

@zackees

@zackees zackees commented Oct 27, 2026

Copy link
Copy Markdown
Member

Fixes #1753.

Root cause

The warm-build fast path hashes a watch set of project files and reuses the
previous firmware.bin when the hash is unchanged. Both walks
(hash_watch_set, hash_watch_set_stamps) used WalkDir::new(root) without
follow_links, and walkdir reports a symlink entry as neither a file nor a
directory. So for

<proj>/lib/FastLED -> /home/niteris/dev/fastled2

the entire library was invisible to the fingerprint. A git checkout that
rewrote src/platforms/esp/32/core/fastspi_esp32_idf.h left
build_fingerprint.json byte-identical, the fast path hit, and the deploy
shipped the previous build's artifacts — the two branches measured
byte-for-byte identical firmware until --clean forced a full build. That is
the exact symptom in #1753, including why a plain lib/ (non-symlinked)
library was unaffected: its headers were already in the watch set.

check_with_stamps is the production branch (cancel... inputs.compiler_cache
is None for every orchestrator), so this is the path that produced the stale
firmware.

Change

  • One shared watched_files(watch) helper for both hashes, with
    .follow_links(true).
  • Cycles are safe: walkdir yields an error for a loop and .flatten() drops
    it; covered by test_hash_watch_set_stamps_tolerates_a_symlink_loop.
  • Excludes still prune by directory name, so a .git inside a link target
    stays out of the hash.
  • Symlinked files (e.g. a symlinked TU) are now hashed too.

Tests (fail before the fix, pass after)

Added to crates/fbuild-build-engine/src/build_fingerprint/mod.rs:

  • test_hash_watch_set_stamps_sees_edits_under_a_symlinked_library
  • test_hash_watch_set_sees_edits_under_a_symlinked_library
  • test_hash_watch_set_stamps_hashes_a_symlinked_source_file
  • test_hash_watch_set_stamps_tolerates_a_symlink_loop

All three "sees edits" tests fail on origin/main with identical
before/after hashes and pass with the change.

Gates

  • soldr cargo test -p fbuild-build-engine --lib → 504 passed, 0 failed, 4 ignored
  • soldr cargo test -p fbuild-build-esp --lib → 171 passed, 0 failed, 1 ignored
  • soldr cargo clippy --workspace --all-targets -- -D warnings → exit 0
  • soldr cargo fmt --all --check → exit 0

Not in scope (follow-up)

source_scanner::walk_sources walks the project src/ with the same
no-follow default, so a symlinked translation unit inside src/ is never
compiled. Same class of bug, different enumeration; left for its own change.

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` (#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).
@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

`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}`.
Dylint `ban_std_pathbuf` fires on `PathBuf` in type position outside the
legacy allowlist; build the fixture paths with `Path::join` instead.
@zackees
zackees added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit 49ef2ea Oct 27, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Symlinked local library not rebuilt after its sources change (stale objects without --clean)

1 participant