fix(fbuild-python): prefer venv-adjacent fbuild-daemon over PATH (#275)#311
Conversation
When the PyO3 binding spawned the daemon, it relied on the OS PATH search to resolve `fbuild-daemon`. On Windows venvs whose `.venv/Scripts` is not at the front of PATH, that picked up a stale user-level binary at `~/.local/bin/fbuild-daemon.exe` instead of the venv-shipped daemon. The stale daemon could miss features the in-process `_native` extension depends on (notably `.S` source support in FastLED `library.json` srcFilter), producing wrong builds. Resolve `sys.executable` via PyO3, look for `fbuild-daemon[.exe]` in its parent directory, and prefer that absolute path when present. Fall back to the bare PATH-relative name when no venv-adjacent binary is found so `pip install --user` / system installs keep working unchanged. The async `AsyncDaemon::ensure_running` resolves the path before entering the future to avoid holding the GIL across `.await`.
|
Warning Review limit reached
More reviews will be available in 47 minutes and 19 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
1 participant
Summary
fbuild-daemonviasys.executable.parent() / DAEMON_BIN_NAMEbefore falling back to PATH, so venv installs always run the daemon that ships with the active Python.fbuild-daemon.exeon Windows,fbuild-daemonelsewhere; reused by both venv-adjacent lookup and PATH fallback.Daemon::ensure_runningresolves under the GIL it already holds. Async path resolves before entering the future so the GIL is never held across.await.Closes #275.
Test plan
daemon::tests::*(file present, file absent, dir-with-matching-name rejected, missing parent dir, platform filename guard)soldr cargo check --workspace --all-targetscleansoldr cargo clippy --workspace --all-targets -- -D warningscleanuv run ci/test.pyfull workspace — no regressions🤖 Generated with Claude Code