Skip to content

chore(build-engine): route production .fbuild walks through fbuild-paths - #1392

Merged
zackees merged 1 commit into
mainfrom
chore/1349-ratchet-batch4
Aug 23, 2026
Merged

chore(build-engine): route production .fbuild walks through fbuild-paths#1392
zackees merged 1 commit into
mainfrom
chore/1349-ratchet-batch4

Conversation

@zackees

@zackees zackees commented Aug 23, 2026

Copy link
Copy Markdown
Member

Fourth ratchet batch for #1349. Allowlist 33 → 28.

Scope: production sites only

The five fbuild-build-engine files here are the skip-lists and path walks
that decide what a build looks at:

file what it was doing
build_fingerprint/fast_path.rs FAST_PATH_EXCLUDES const array
source_scanner.rs scanner skip-list const array
framework_libs.rs two matches! arms
compiler.rs ancestor walk comparing the directory name
symbol_analyzer/mod.rs ELF search over <.fbuild>/<build>

FBUILD_DIR_NAME is a const &str, so it drops straight into the const
arrays and works as a const pattern in matches! — no restructuring needed.

These are worth more than a spelling fix. A skip-list that disagrees with the
real directory name silently stops excluding the build tree, and a warm build
then starts fingerprinting its own output.

The crate's five test-fixture files are deliberately left for the next
batch. They are a different kind of change — hand-rolled layouts inside
assertions — and mixing ten files of two kinds would bury the production edits
in fixture churn.

Verification, including a near-miss worth recording

Main already carried lint version 0.1.3 (from #1387), so the habitual
0.1.2 → 0.1.3 bump was a no-op. The first clean dylint run could
therefore have been a cached .so still holding the old allowlist — which
would allowlist exactly the five files under test and report zero findings for
the wrong reason.

Bumped to 0.1.4, re-ran: still clean. Then confirmed the lint actually fires
on these files by reintroducing a canary ".fbuild/canary" literal into
source_scanner.rs — exit 1, one finding — and removing it again.

  • soldr cargo test -p fbuild-build-engine --lib: 405 passed, 0 failed
  • clippy -D warnings: clean

Remaining

28 entries: fbuild-build 12, fbuild-build-engine 5 (tests), fbuild-cli 9, plus
the 2 sanctioned definition sites. fbuild-cli continues to sit out while
#1384 has cli/args.rs open.

Summary by CodeRabbit

  • Refactor
    • Centralized internal build-directory path handling to keep directory detection and exclusions consistent across build scanning and analysis.
    • Updated native output discovery to use the shared build-directory naming configuration.
  • Chores
    • Removed obsolete path-related lint exceptions.
    • Updated the path validation tool version.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: df88b457-b488-4670-9af5-a61c78b75ad2

📥 Commits

Reviewing files that changed from the base of the PR and between 3445e4f and 3a05043.

📒 Files selected for processing (7)
  • crates/fbuild-build-engine/src/build_fingerprint/fast_path.rs
  • crates/fbuild-build-engine/src/compiler.rs
  • crates/fbuild-build-engine/src/framework_libs.rs
  • crates/fbuild-build-engine/src/source_scanner.rs
  • crates/fbuild-build-engine/src/symbol_analyzer/mod.rs
  • dylints/ban_raw_fbuild_path/Cargo.toml
  • dylints/ban_raw_fbuild_path/src/allowlist.txt

📝 Walkthrough

Walkthrough

The build engine replaces hard-coded .fbuild paths with shared constants across fingerprinting, scanning, framework discovery, and ELF discovery. The raw-path lint package version and allowlist are updated accordingly.

Changes

Fbuild path constant adoption

Layer / File(s) Summary
Replace hard-coded fbuild paths
crates/fbuild-build-engine/src/build_fingerprint/fast_path.rs, crates/fbuild-build-engine/src/compiler.rs, crates/fbuild-build-engine/src/framework_libs.rs, crates/fbuild-build-engine/src/source_scanner.rs, crates/fbuild-build-engine/src/symbol_analyzer/mod.rs
Build-engine path exclusions, workspace detection, and ELF search paths now use shared directory-name constants.
Update raw-path lint enforcement
dylints/ban_raw_fbuild_path/Cargo.toml, dylints/ban_raw_fbuild_path/src/allowlist.txt
The lint package version changes to 0.1.4, and five obsolete allowlist entries are removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/1349-ratchet-batch4

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.

…paths

Fourth ratchet batch for #1349. Allowlist 33 -> 28.

Scoped to `fbuild-build-engine`'s *production* sites — the skip-lists and
path walks that decide what a build looks at:

  - `FAST_PATH_EXCLUDES` and the source scanner's skip-list are `const`
    arrays, and `FBUILD_DIR_NAME` is a `const &str`, so it drops straight in.
  - `framework_libs`' two `matches!` arms take it as a const pattern.
  - `compiler`'s ancestor walk compares against it.
  - `symbol_analyzer`'s ELF search builds `<.fbuild>/<build>` from both
    segments rather than spelling the pair.

These matter more than a spelling fix: a skip-list that disagrees with the
real directory name silently stops excluding the build tree, so a warm build
starts fingerprinting its own output.

The five test-fixture files in this crate are deliberately left for the next
batch — they are a different kind of change (hand-rolled layouts in
assertions) and mixing them here would bury the production edits.

Verified with a genuinely rebuilt lint: main already carried version 0.1.3,
so the usual bump was a no-op and the first clean run could have been a stale
`.so` still holding the old allowlist. Bumped to 0.1.4 and re-ran, then
confirmed the lint still fires here by reintroducing a canary literal
(exit 1, one finding) and removing it again.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@zackees
zackees force-pushed the chore/1349-ratchet-batch4 branch from 8edaa32 to 3a05043 Compare August 23, 2026 23:30
@zackees
zackees merged commit 0767fb6 into main Aug 23, 2026
96 of 97 checks passed
@zackees
zackees deleted the chore/1349-ratchet-batch4 branch August 23, 2026 23:30
@fastled-project-sync fastled-project-sync Bot moved this to Triage in FastLED Tracker Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

1 participant