Skip to content

fix(library): resolve named local dependencies - #1257

Merged
zackees merged 1 commit into
mainfrom
fix/local-symlink-library-deps
Aug 6, 2026
Merged

fix(library): resolve named local dependencies#1257
zackees merged 1 commit into
mainfrom
fix/local-symlink-library-deps

Conversation

@zackees

@zackees zackees commented Aug 6, 2026

Copy link
Copy Markdown
Member

Closes #1256\n\nResolves named Name=symlink://... and Name=file://... dependencies relative to the project directory, adds their include directories, and keeps generated library objects/archives under fbuild's build directory rather than the source checkout. Includes Windows ile:///C:/... normalization and focused resolver/archive-path tests.\n\nValidation:\n- soldr cargo test -p fbuild-library named_local_symlink\n- soldr cargo test -p fbuild-library external_build_dir\n- soldr cargo test -p fbuild-library windows_file_uri\n- soldr cargo test -p fbuild-build-esp --no-run\n- Local FastLED est-emu compiled the ESP32 core and sketch with FastLED=symlink://../..; the local CLI request did not return after compilation, so the QEMU completion marker remains unverified.

Summary by CodeRabbit

  • New Features

    • Added support for named local library dependencies using symlink:// and file:// paths.
    • Local libraries can now be resolved relative to the project and built in separate directories.
    • Improved handling of local library includes and Windows file paths.
  • Bug Fixes

    • Corrected archive placement when libraries use an external build directory.
  • Chores

    • Updated the project version to 2.5.12.

@zackees
zackees merged commit ad8aac0 into main Aug 6, 2026
88 of 92 checks passed
@zackees
zackees deleted the fix/local-symlink-library-deps branch August 6, 2026 01:58
@coderabbitai

coderabbitai Bot commented Aug 6, 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: 717e1bf2-242c-4447-97a2-231d74d049f2

📥 Commits

Reviewing files that changed from the base of the PR and between 86d8741 and b10e6d9.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • Cargo.toml
  • crates/fbuild-build-esp/src/esp32/orchestrator/build.rs
  • crates/fbuild-library/src/library/README.md
  • crates/fbuild-library/src/library/library_info.rs
  • crates/fbuild-library/src/library/library_manager.rs
  • crates/fbuild-library/src/library/library_spec.rs
  • pyproject.toml

📝 Walkthrough

Walkthrough

The change adds named symlink:// and file:// local dependencies. Relative paths resolve from the project directory. Local libraries build under libs_dir, separate from their source directories. Package versions update to 2.5.12.

Changes

Local library resolution

Layer / File(s) Summary
Local dependency parsing
crates/fbuild-library/src/library/library_spec.rs, crates/fbuild-library/src/library/README.md
LibrarySpec stores named local paths and parses symlink:// and file:// dependencies. Tests cover invalid names, Windows paths, and unnamed local paths.
Separate build artifacts
crates/fbuild-library/src/library/library_info.rs
InstalledLibrary supports a separate build_dir. Archive paths and compilation outputs use that directory.
Resolution and compilation integration
crates/fbuild-library/src/library/library_manager.rs, crates/fbuild-build-esp/src/esp32/orchestrator/build.rs, Cargo.toml, pyproject.toml
Library resolution receives project_dir, stages local builds under libs_dir, and updates callers and tests. Package versions update to 2.5.12.

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

Sequence Diagram(s)

sequenceDiagram
  participant BuildOrchestrator
  participant LibraryManager
  participant LibrarySpec
  participant LibraryCompiler
  BuildOrchestrator->>LibraryManager: pass project_dir and libs_dir
  LibraryManager->>LibrarySpec: inspect local_path
  LibrarySpec-->>LibraryManager: return named local path
  LibraryManager->>LibraryCompiler: compile using separate build_dir
  LibraryCompiler-->>LibraryManager: produce archive and include paths
Loading

Possibly related issues

Possibly related PRs

  • FastLED/fbuild#965 — Both modify ESP32/library compilation orchestration and related compilation call sites.
  • FastLED/fbuild#1249 — Both modify fbuild library dependency resolution, although they target different platforms and resolution sources.
✨ 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/local-symlink-library-deps

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.

@fastled-project-sync fastled-project-sync Bot moved this to Triage in FastLED Tracker Aug 6, 2026
zackees added a commit that referenced this pull request Aug 7, 2026
…ctation

Two pre-existing failures from #1256/#1257/#1258 that turned main's
Dylint and Check macOS/Windows jobs red:

- library_spec.rs used std::path::PathBuf for local_path, which the
  ban_std_pathbuf dylint denies (the file is not allowlisted). Migrate
  to fbuild_core::path::NormalizedPath.
- test_named_relative_local_symlink_adds_include_dir compared raw
  tempfile paths against production's canonicalized output, failing on
  Windows (8.3 short names, RUNNER~1) and macOS (/var -> /private/var).
  Canonicalize the expectation the same way production does.

Co-Authored-By: Claude <noreply@anthropic.com>
zackees added a commit that referenced this pull request Aug 7, 2026
* ci: fail board builds that leave stray files in the working tree

Linker helpers (collect2/lto-wrapper) running in the daemon's inherited
cwd twice dropped scratch files (a stray 0-byte `-r`) into the repo
root (#1267, #1269, FastLED/FastLED#3867). Every board build now asserts
`git status --porcelain` is empty after the quick + release builds so
the regression class is caught in CI instead of by users.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(library): make main green — PathBuf ban + canonicalized test expectation

Two pre-existing failures from #1256/#1257/#1258 that turned main's
Dylint and Check macOS/Windows jobs red:

- library_spec.rs used std::path::PathBuf for local_path, which the
  ban_std_pathbuf dylint denies (the file is not allowlisted). Migrate
  to fbuild_core::path::NormalizedPath.
- test_named_relative_local_symlink_adds_include_dir compared raw
  tempfile paths against production's canonicalized output, failing on
  Windows (8.3 short names, RUNNER~1) and macOS (/var -> /private/var).
  Canonicalize the expectation the same way production does.

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

test-emu does not resolve symlink library dependencies on Windows

1 participant