Skip to content

fix(install): use src-layout package-dir so editable install ships a .pth - #748

Merged
zackees merged 1 commit into
mainfrom
fix/editable-install-static-analysis
Jun 22, 2026
Merged

fix(install): use src-layout package-dir so editable install ships a .pth#748
zackees merged 1 commit into
mainfrom
fix/editable-install-static-analysis

Conversation

@zackees

@zackeeszackees commented Jun 22, 2026

Copy link
Copy Markdown
Member

Summary

The per-package package-dir = {fbuild = "python/fbuild", "fbuild.api" = "python/fbuild/api"} map made setuptools' editable install emit a PEP 660 meta-path finder whose MAPPING only registered fbuild. fbuild.api resolved at runtime via importlib.machinery.PathFinder — fine for import fbuild.api, but invisible to static analyzers (ty, pyright, mypy) that walk top_level.txt / .pth files.

Downstream consumers (FastLED hit this on from fbuild.api import SerialMonitor) saw Cannot resolve imported module 'fbuild.api' even though runtime imports worked. The Stop-hook lint in FastLED failed on every bash lint after switching to an editable fbuild install.

Replace the per-package map with the canonical src-layout pattern package-dir = {"" = "python"}. Setuptools now emits a plain .pth pointing at python/, which every static analyzer handles, and the wheel-build path is unaffected.

Verification

Wheel artifact (python -m build of both states, cp310-cp310-win_amd64):

Wheel sizeSdist sizeWheel contents
Baseline10,454,675 B2,316,616 Bfbuild/__init__.py, fbuild/_native.pyd, fbuild/api/__init__.py, fbuild-2.2.31.data/scripts/fbuild.exe, dist-info
This PR10,454,675 B2,319,427 Bbyte-identical to baseline
  • Wheel is byte-for-byte identical.
  • Sdist is +2,811 B / +0.12% — entirely from fbuild.egg-info/ relocating to python/fbuild.egg-info/. Cosmetic.

Release path (ci/publish.py) is independent of [tool.setuptools] — it hand-rolls wheels from the hard-coded PYTHON_SHIMS_DIR = ROOT / "python". Smoke-tested: still finds the same 2 shims (fbuild/__init__.py, fbuild/api/__init__.py). Same EXTENSION_NAMES. No change.

Perf (measured against the work in #743 / #744):

Test plan

  • python -m build produces byte-identical wheel
  • python -m build produces structurally-identical sdist
  • ci/publish.py::read_project_meta() + PYTHON_SHIMS_DIR.rglob('*.py') resolves correctly
  • FastLED editable install: __editable__.fbuild-2.2.31.pth contains <fbuild>/python
  • uv run ty check resolves both import fbuild and from fbuild.api import SerialMonitor
  • uv run fbuild --versionfbuild 2.2.31
  • Runtime: import fbuild.api resolves to python/fbuild/api/__init__.py
  • CI green (this PR)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Updated packaging configuration to improve how the package is exposed during development installations.

….pth (not a PEP 660 finder)
The per-package `package-dir = {fbuild = "python/fbuild",
"fbuild.api" = "python/fbuild/api"}` map worked for the wheel build but
made setuptools' editable install emit a PEP 660 meta-path finder
(`__editable___fbuild_*_finder.py`) whose `MAPPING` only registered
`fbuild`. `fbuild.api` resolved at runtime via
`importlib.machinery.PathFinder` — fine for `import fbuild.api`, but
invisible to static analyzers (ty, pyright, mypy) that walk
`top_level.txt` and `.pth` files. Downstream consumers (FastLED hit this
on `from fbuild.api import SerialMonitor`) saw
`Cannot resolve imported module 'fbuild.api'` even though runtime imports
worked.
Replace the per-package map with the canonical src-layout pattern
`package-dir = {"" = "python"}`. Setuptools now emits a plain
`.pth` pointing at `python/`, which every static analyzer handles, and
the wheel-build path is unaffected.
Verified:
- Wheel (`python -m build`): byte-for-byte identical to baseline
(10,454,675 B). Same contents: `fbuild/__init__.py`,
`fbuild/_native.pyd`, `fbuild/api/__init__.py`,
`fbuild-2.2.31.data/scripts/fbuild.exe`, dist-info.
- Sdist: +2,811 B / +0.12%, entirely from `fbuild.egg-info/` relocating
to `python/fbuild.egg-info/`. Cosmetic.
- `ci/publish.py::build_wheel` is independent of setuptools — it
hand-rolls wheels from the hard-coded `PYTHON_SHIMS_DIR = ROOT /
"python"`. Smoke-tested: still finds the same 2 shims
(`fbuild/__init__.py`, `fbuild/api/__init__.py`). Release path
unaffected.
- Editable reinstall path: ~8s (FastLED → fbuild via uv sources),
within noise of baseline. No regression to the perf work in #743 /
#744.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Jun 22, 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: 11551a4c-45fe-487f-b870-b48fe7bfe573

📥 Commits

Reviewing files that changed from the base of the PR and between a426a0d and 513b4cd.

📒 Files selected for processing (1)
  • pyproject.toml

📝 Walkthrough

Walkthrough

pyproject.toml's [tool.setuptools.package-dir] section is changed from two explicit per-package entries (fbuildpython/fbuild, fbuild.apipython/fbuild/api) to a single root mapping (""python), switching to a src-layout-style discovery that covers all packages under python/.

Changes

Setuptools Package Directory Mapping

Layer / File(s)Summary
Setuptools package-dir root mapping
pyproject.toml
Replaces explicit per-package entries with "" = "python", so all packages under python/ are discovered via a single root mapping. Editable installs now emit a plain .pth pointing at python/ instead of a meta-path finder.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Possibly related PRs

  • FastLED/fbuild#743: Also modifies pyproject.toml's [tool.setuptools.package-dir] mapping to adjust how python/ sources are exposed during builds and editable installs.

Poem

A bunny hopped through pyproject one day,
And found two package paths blocking the way.
"One root to rule them all!" it declared,
Mapping "" to python/ — problems repaired.
Now static analyzers can follow the trail 🐇✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title directly and accurately describes the main change: switching to src-layout package-dir to make editable installs produce a .pth file instead of a meta-path finder.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/editable-install-static-analysis

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 4dfa1e7 into mainJun 22, 2026
84 of 91 checks passed
@zackees
zackees deleted the fix/editable-install-static-analysis branch June 22, 2026 06:48
zackees added a commit that referenced this pull request Jun 22, 2026
Minor bump. Picks up:
- #747 native fbuild as raw wheel script (drops Python launcher; fixes
Windows stdout-ordering bug on COM25 303A:1001 USB Serial Device (COM25) ser=80:F1:B2:D1:DF:B1
└─ Espressif Systems / ESP32-S3 USB-CDC
COM1 [Unknown]
└─ (no USB identifier — Unknown endpoint)
COM3 [Unknown]
└─ (no USB identifier — Unknown endpoint)
COM22 303A:1001 USB Serial Device (COM22) ser=D8:3B:DA:41:64:C0
└─ Espressif Systems / ESP32-S3 USB-CDC
COM4 [Unknown]
└─ (no USB identifier — Unknown endpoint)
COM9 303A:1001 USB Serial Device (COM9) ser=8C:BF:EA:CF:87:B4
└─ Espressif Systems / ESP32-S3 USB-CDC
COM20 16C0:0483 USB Serial Device (COM20) ser=15821020
└─ Van Ooijen Technische Informatica / Teensyduino Serial
COM10 1FC9:0132 USB Serial Device (COM10) ser=0B03400A
└─ NXP Semiconductors / LPC-Link2 CMSIS-DAP
5 USB ports, 3 non-USB ports)
- #748 src-layout package-dir (static analyzers resolve fbuild.api)
- #750 daemon WS handler split into reader/writer/inbound
- #751 gitignore .extern-repos/
- #752 cargo fmt + extract test modules so 4 files fall under the
1000-LOC gate
No public-API changes beyond what's already documented in the merged
PRs above.
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

Status: Triage

Development

Successfully merging this pull request may close these issues.

1 participant

@zackees