perf(build): compile ESP32 framework libs workspace-relative (cross-project cache stability, #952)

[zackees]

Summary

The #942 profiling showed framework built-in libs (fw-libs) as the dominant warm-scenario cost (~185 s), and the root cause (posted on #952) was that fw-lib compiles pass absolute source/-o/-I paths with cwd = None, so their zccache keys embed the project directory and don't stay stable across project dirs — unlike sketch/core compiles, which relativize to the workspace root.

This aligns fw-libs with the sketch/core path:

• Relocates the three workspace-relativization helpers (compile_cwd_from_output, path_arg_for_compile_cwd, normalize_flags_for_compile_cwd) from fbuild-build/src/zccache.rs to fbuild_core::path, so fbuild-packages' library compiler can use them. zccache.rs re-exports them, so every existing crate::zccache::… call site (and compiler.rs) is unchanged. Their tests moved with them.
• Adds an opt-in compile_cwd to compile_library_with_jobs: Some(workspace) relativizes source/-o/project--I and runs the compiler from the workspace; None (every other caller: local libs, pipeline libs, library manager) is byte-identical to before. Only the ESP32 fw-libs orchestrator opts in.

Results

#942 Docker harness, clean cold→warm, vs the post-#951 baseline:

phase (warm)	baseline	this PR
fw-libs	185 s	141 s
total	473 s	376 s

A real ~24% fw-libs improvement, and fw-libs is now workspace-relative like the rest of the compile pipeline (which also makes its keys portable for cross-runner CI caching). It is not the full 150 s→seconds win: the remaining cost is a deeper firmware-compile zccache cross-project key-stability gap — sketch/core don't fully hit cross-project in a clean run either — which I'll file as a follow-up. This PR is the necessary, non-regressing first step and a correctness alignment.

Validation

• fbuild-packages 715 + fbuild-build 452 + fbuild-core (incl. the 6 relocated helper tests) all pass on Linux.
• NightDriverStrip env:demo builds end-to-end (firmware produced) in the harness.
• Every non-ESP32-fw-libs caller passes None → unchanged; CI covers the cross-platform library compile paths.

Part of #952 / #942.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added workspace-relative compile path handling to make build outputs and cache keys more consistent across different project locations.
  • Compiler flags and file paths are now normalized to use forward slashes and relative paths where possible.

• Bug Fixes

  • Improved handling of include paths, system root paths, and source/output arguments during compilation.
  • Reduced path-related build inconsistencies by standardizing the working directory used for compilation.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds workspace-relative compile CWD path helpers in fbuild-core, threads an optional compile_cwd parameter through compile_library_with_jobs/compile_one_source to normalize include flags and source/object arguments, updates call sites to pass it, and replaces zccache.rs's local helper implementations with re-exports.

Changes

Compile CWD normalization for zccache keys

Layer / File(s)	Summary
Path helpers for workspace-relative compile args crates/fbuild-core/src/path.rs	Adds compile_cwd_from_output, path_arg_for_compile_cwd, normalize_flags_for_compile_cwd, and private lexical-canonicalization/flag-splitting helpers, plus unit tests for workspace detection and flag rewriting.
Thread compile_cwd through library_compiler crates/fbuild-packages/src/library/library_compiler.rs	compile_library_with_jobs and compile_one_source gain an optional compile_cwd parameter used to normalize flags, source/object args, and process CWD across sequential and parallel compile paths.
Wire compile_cwd into call sites and zccache re-exports crates/fbuild-build/src/esp32/orchestrator/framework_libs.rs, crates/fbuild-build/src/esp32/orchestrator/local_libs.rs, crates/fbuild-build/src/pipeline/library.rs, crates/fbuild-packages/src/library/library_manager.rs, crates/fbuild-build/src/zccache.rs	Call sites pass a computed compile_cwd or None into compile_library_with_jobs; zccache.rs drops its local helper implementations and tests, re-exporting equivalents from fbuild-core.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Orchestrator
  participant library_compiler
  participant path_helpers
  participant CompilerProcess

  Orchestrator->>library_compiler: compile_library_with_jobs(compile_cwd)
  library_compiler->>path_helpers: normalize_flags_for_compile_cwd(flags, compile_cwd)
  path_helpers-->>library_compiler: workspace-relative flags
  library_compiler->>path_helpers: path_arg_for_compile_cwd(source/obj, compile_cwd)
  path_helpers-->>library_compiler: relative source/obj args
  library_compiler->>CompilerProcess: run_command(args, cwd=compile_cwd)
  CompilerProcess-->>library_compiler: compile result

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: making ESP32 framework library builds workspace-relative for cache stability.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ 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 fix/952-fwlibs-relative

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

crates/fbuild-core/src/path.rs (1)

385-402: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

strip_prefix is case-sensitive — confirm cwd/path casing always matches.

On case-insensitive filesystems (Windows, macOS), stable_path.strip_prefix(&stable_cwd) at Line 392 will fail if cwd and path differ only in case, silently falling back to the absolute path and re-introducing a project-dependent compile arg (defeating cache stability). In practice both come through canonicalize_lexical/compile_cwd_from_output so casing should agree, but this relies on canonicalization returning byte-identical case for the shared prefix.

Worth verifying that both the cwd produced by compile_cwd_from_output and the source/include paths canonicalize to identical casing on Windows/macOS.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/fbuild-core/src/path.rs` around lines 385 - 402,
`path_arg_for_compile_cwd` relies on `stable_path.strip_prefix(&stable_cwd)`
being case-identical, which can fail on case-insensitive platforms and fall back
to an absolute arg. Verify the casing produced by `compile_cwd_from_output` and
the paths passed through `canonicalize_lexical`/`strip_unc_prefix` in
`path_arg_for_compile_cwd`, and adjust the normalization so `cwd` and `path`
share the same canonical casing before calling `strip_prefix`.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/fbuild-core/src/path.rs`:
- Around line 385-402: `path_arg_for_compile_cwd` relies on
`stable_path.strip_prefix(&stable_cwd)` being case-identical, which can fail on
case-insensitive platforms and fall back to an absolute arg. Verify the casing
produced by `compile_cwd_from_output` and the paths passed through
`canonicalize_lexical`/`strip_unc_prefix` in `path_arg_for_compile_cwd`, and
adjust the normalization so `cwd` and `path` share the same canonical casing
before calling `strip_prefix`.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0bcf1e48-0d91-460f-87a8-8bdef8a3f032

📥 Commits

Reviewing files that changed from the base of the PR and between 9769bc4 and e5dfd72.

📒 Files selected for processing (7)

• crates/fbuild-build/src/esp32/orchestrator/framework_libs.rs
• crates/fbuild-build/src/esp32/orchestrator/local_libs.rs
• crates/fbuild-build/src/pipeline/library.rs
• crates/fbuild-build/src/zccache.rs
• crates/fbuild-core/src/path.rs
• crates/fbuild-packages/src/library/library_compiler.rs
• crates/fbuild-packages/src/library/library_manager.rs