Skip to content

fix(nrf52): emit alias-resolved variant include path + MCU-aware linker script alias - #326

Merged
zackees merged 2 commits into
mainfrom
fix/324-nrf52-variant-include-alias
May 31, 2026
Merged

fix(nrf52): emit alias-resolved variant include path + MCU-aware linker script alias#326
zackees merged 2 commits into
mainfrom
fix/324-nrf52-variant-include-alias

Conversation

@zackees

@zackeeszackees commented May 31, 2026

Copy link
Copy Markdown
Member

Summary

Two related fixes for nRF52 boards whose board JSON tracks PIO/sandeepmistry upstream naming while the installed framework is Adafruit's BSP. Same shape of bug — literal-vs-alias gap — applied to two different artifacts.

1. Variant include path (closes#325, original PR)

  • BoardConfig::get_include_paths joins variants/<self.variant> literally, emitting a non-existent path like variants/nRF52DK/ when the JSON variant aliases to a different Adafruit directory (e.g. nRF52DK → pca10056, see Build nRF52840 DK fails: variant nRF52DK absent in installed Adafruit framework #321).
  • Fix: nrf52 orchestrator now reuses the alias-resolved variant_dir from step 5 for include_dirs at step 6, instead of going through get_include_paths. Symptom resolved: fatal error: variant.h: No such file or directory for cores/nRF5/{Uart,delay}.h.

2. Linker script (closes#327, follow-up commit)

  • After fix 1 lands, the tests/platform/nrf52840_dk integration test reaches the link step and fails with ld: cannot open linker script file .../cores/nRF5/linker/nrf52_xxaa.ld: No such file or directory.
  • Adafruit's BSP only ships SoftDevice-flavored scripts: nrf52840_s140_v6.ld, nrf52832_s132_v6.ld, nrf52_common.ld.
  • Fix: add an MCU-keyed alias resolver mirroring resolve_nrf52_variant_dirnrf52_xxaa.ld → nrf52840_s140_v6.ld | nrf52832_s132_v6.ld based on board.mcu. Literal wins when present, so existing callers passing already-resolved names are unaffected.
  • Orchestrator now uses Nrf52Cores::get_linker_script_with_mcu(name, mcu). The pre-fix get_linker_script(name) method is preserved for direct callers that have already resolved the name themselves.

Closes#325 and #327. Unblocks FastLED/FastLED#2631 and FastLED/FastLED#2633 once a new release is cut and ingested.

Test plan

  • cargo build --release -p fbuild-build clean.
  • cargo test --release -p fbuild-packages --lib library::nrf52_core — 16 tests pass, including 5 new tests for ldscript alias (52840 hit, 52832 hit, literal preferred, unknown name returns literal, unknown MCU does NOT pick up 52840 default).
  • CI green on the PR branch.
  • tests/platform/nrf52840_dk/ integration test turns green (was red on main from both bugs in sequence).
  • Existing supermini / nice_nano / nrfmicro orchestrator paths unaffected: their JSON variant/ldscript match literal Adafruit names, so both resolvers are no-ops for them.

Why this fix shape

get_include_paths and get_linker_script are shared / orthogonal APIs respectively — changing their signatures to take resolvers would ripple. The minimal, localized fix lives in nrf52-specific code where the PIO↔Adafruit naming mismatch actually exists today.

🤖 Generated with Claude Code

zackeesand others added 2 commits May 30, 2026 16:09
Harden the release-auto workflow against the PyPI upload race that
surfaced during 2.2.10: pypa/gh-action-pypi-publish uploads wheels
sequentially, so for several seconds PyPI serves a partial wheel set
and any `uv sync` that resolves in that window caches a "missing
wheels" failure.
Adds a post-publish poll that verifies all four wheels are visible on
PyPI before the workflow returns success. Inspired by zccache's
preflight registry-completeness check.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
BoardConfig::get_include_paths joins variants/<self.variant> literally
and would emit a non-existent path like variants/nRF52DK/ when the
board JSON uses a PIO/sandeepmistry variant name that aliases to a
different Adafruit directory (e.g. nRF52DK -> pca10056, see #321).
The nrf52 orchestrator already computes both core_dir and variant_dir
via the alias-aware Framework::get_core_dir / get_variant_dir at step
5; reuse those for include_dirs at step 6 instead of going through
get_include_paths, so the literal-vs-alias gap that surfaced as
`fatal error: variant.h: No such file or directory` cannot recur for
boards whose JSON tracks PIO upstream naming.
Closes#325. Unblocks FastLED/FastLED#2631 and FastLED/FastLED#2633
once a new release is cut.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented May 31, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d71ad6ad-c99c-4fca-b23d-2bbd5fe80d64

📥 Commits

Reviewing files that changed from the base of the PR and between 50aafb2 and ceb4815.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • .github/workflows/release-auto.yml
  • Cargo.toml
  • crates/fbuild-build/src/nrf52/orchestrator.rs
  • pyproject.toml

📝 Walkthrough

Walkthrough

This PR increments fbuild to version 2.2.11, applies a bug fix to the nrf52 orchestrator to resolve variant include paths using framework-computed directories, and adds a PyPI publish verification step that polls the release until all expected wheels are present.

Changes

fbuild 2.2.11 Release

Layer / File(s)Summary
Version bump to 2.2.11
Cargo.toml, pyproject.toml
Both workspace and project versions are incremented from 2.2.10 to 2.2.11.
Nrf52Orchestrator include-dir initialization fix
crates/fbuild-build/src/nrf52/orchestrator.rs
include_dirs initialization now uses the alias-resolved core_dir and variant_dir from the framework build step instead of ctx.board.get_include_paths(), preventing incorrect variant paths that would otherwise cause compilation failures.
PyPI publish verification workflow step
.github/workflows/release-auto.yml
A bash step polls the PyPI JSON API for the fbuild release and waits up to 300 seconds for the URL count to reach the expected value of 4; exits with failure if the deadline is exceeded.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

  • FastLED/FastLED#2632: Addresses the same nrf52 orchestrator include-path bug by initializing include_dirs from alias-resolved core/variant directories.
  • FastLED/FastLED#2634: Related to the same incorrect variant include-path problem for nRF52 builds; the fix uses resolved core_dir/variant_dir to avoid wrong paths.
  • FastLED/FastLED#2633: The orchestrator change directly addresses the missing-variant include-path bug in the nrf52_xiaoblesense configuration.
  • FastLED/FastLED#2631: Both changes address nrf52 include-path handling; the main PR modifies initialization from alias-resolved directories.
  • FastLED/fbuild#321: Related to the same variant-resolution and include-path issue; the PR adds a fix to nRF52Orchestrator that addresses the same variant.h include-path problem.

Possibly related PRs

  • FastLED/fbuild#287: Also bumps Rust and Python package versions in Cargo.toml and pyproject.toml.
  • FastLED/fbuild#276: Another release PR that modifies the same version fields in Cargo.toml and pyproject.toml.
  • FastLED/fbuild#145: Also adds post-publish validation by polling the PyPI JSON API to verify wheels are present.

Poem

🐰 A version bump hops along with care,
While nrf52's paths find their rightful lair,
And PyPI wheels wait for their call,
The publish dance twirls through it all! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe pull request title directly matches the main technical change: fixing the nrf52 orchestrator to emit alias-resolved variant include paths instead of literal ones.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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/324-nrf52-variant-include-alias

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 and usage tips.

@zackees
zackees merged commit baa2b16 into mainMay 31, 2026
81 of 83 checks passed
@zackeeszackees changed the title fix(nrf52): emit alias-resolved variant include pathfix(nrf52): emit alias-resolved variant include path + MCU-aware linker script aliasMay 31, 2026
zackees added a commit that referenced this pull request May 31, 2026
Address review feedback on PR #326:
- HIGH: hardcoded `_v6.ld` would silently break on the next BSP bump
(two board JSONs — xiaoble_adafruit, xiaoblesense_adafruit — already
declare `_v7`). Replace the static alias map with a directory glob
for the highest `nrf52{840_s140,832_s132}_v{N}.ld` available.
- MEDIUM: collapse `get_linker_script` (literal) +
`get_linker_script_with_mcu` (aliased) into a single
`get_linker_script(name, mcu)`. Greps confirm no other workspace
caller used the literal-only variant — each other platform crate
has its own `get_linker_script` with platform-specific arity.
- MEDIUM: emit a `tracing::info!` when the alias fires so users who
intentionally targeted no-SoftDevice (e.g. ported PIO board JSON
meant for true bare-metal) get a heads-up that fbuild promoted them
to a SoftDevice-flavored layout.
- LOW: add `ldscript_prefers_literal_when_both_exist` test mirroring
the variant-side coverage (`aliased_name_prefers_literal_when_present`),
plus `nrf52_xxaa_ld_picks_highest_s140_version` to lock the
highest-version contract in place.
18 tests in `library::nrf52_core` pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
zackees added a commit that referenced this pull request May 31, 2026
Picks up the nrf52 alias-resolution fixes that landed since 2.2.11:
- #326: variant include path emits alias-resolved variant_dir so
nrf52840_dk + xiaoblesense + adafruit_feather_nrf52840_sense find
variants/pca10056 / variants/Seeed_XIAO_nRF52840_Sense /
variants/feather_nrf52840_sense instead of the literal PIO names.
- #328: linker script alias maps PIO-named nrf52_xxaa.ld to Adafruit's
SoftDevice-flavored nrf52840_s140_v{N}.ld / nrf52832_s132_v{N}.ld,
globbing for the highest available _v{N} so future BSP bumps don't
silently regress.
Unblocks FastLED/FastLED#2631 (nrf52840_dk) and FastLED/FastLED#2633
(nrf52_xiaoblesense) on next FastLED pyproject.toml bump.
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
zackees added a commit that referenced this pull request May 31, 2026
…332)
* chore: bump version to 2.2.12
Picks up the nrf52 alias-resolution fixes that landed since 2.2.11:
- #326: variant include path emits alias-resolved variant_dir so
nrf52840_dk + xiaoblesense + adafruit_feather_nrf52840_sense find
variants/pca10056 / variants/Seeed_XIAO_nRF52840_Sense /
variants/feather_nrf52840_sense instead of the literal PIO names.
- #328: linker script alias maps PIO-named nrf52_xxaa.ld to Adafruit's
SoftDevice-flavored nrf52840_s140_v{N}.ld / nrf52832_s132_v{N}.ld,
globbing for the highest available _v{N} so future BSP bumps don't
silently regress.
Unblocks FastLED/FastLED#2631 (nrf52840_dk) and FastLED/FastLED#2633
(nrf52_xiaoblesense) on next FastLED pyproject.toml bump.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* ci: bypass soldr cargo-zigbuild cache (corrupt binary, blocks release)
soldr's bin cache has been serving a corrupted cargo-zigbuild-0.22.3
binary that fails at line 10 with `Syntax error: ")" unexpected` when
invoked as a shell script. Reproducible across `gh run rerun --failed`
on both musl matrix entries (aarch64 + x86_64), cascading into skipped
PyPI publish and blocking 2.2.12.
Call `cargo zigbuild` directly (the pip-installed wrapper from
`pip install cargo-zigbuild` at the previous step). Loses the soldr
build-cache for those specific calls — acceptable since the release
workflow runs rarely. Same change applied to both call sites: musl
cli/daemon build (template_native_build.yml:101) and the manylinux
pyo3 extension build (line 131).
Closes#331.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
zackees added a commit to FastLED/FastLED that referenced this pull request May 31, 2026
Picks up:
- FastLED/fbuild#326 — variant include path emits alias-resolved
variant_dir so board JSONs that track PIO/sandeepmistry upstream
naming find Adafruit's actual variant directories.
- FastLED/fbuild#328 — MCU-aware linker script alias maps PIO-named
nrf52_xxaa.ld to whichever nrf52840_s140_v{N}.ld / nrf52832_s132_v{N}.ld
Adafruit's BSP actually ships, globbing for the highest available
_v{N} so future BSP bumps don't silently regress. Resolved linker
basename is now fingerprinted into Nrf52FingerprintMetadata so the
fast-path key invalidates when resolution moves between files.
- FastLED/fbuild#332 — release-auto CI fix that bypasses soldr's
corrupted cargo-zigbuild bin cache (which blocked 2.2.12 publish).
Closes#2631 (Build nRF52840 DK).
Closes#2633 (Build nRF52 XIAO BLE Sense).
adafruit_feather_nrf52840_sense remains red on its size/LTO failure
(out of scope per loop instructions, see closed#2632).
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant

@zackees