fix(install): ship native fbuild as a raw wheel script (closes #746)#747
Conversation
Previously `pip install .` installed `fbuild` on PATH as a Python console-script .exe that imported `ci.bin_launcher.main`, which `os.execv`'d the real cargo-built binary bundled at `ci/bin/fbuild.exe`. On POSIX execv replaces the process; on Windows it is emulated as `CreateProcess` + parent exit (no true exec), so the Python shim returned to cmd.exe before the spawned native binary finished flushing stdout — the next shell prompt then raced ahead of `fbuild port scan`'s output. Drop the shim. Ship the cargo-built binary directly via setuptools' `scripts=` argument in setup.py. Files in `scripts=` land in `<name>-<version>.data/scripts/` in the wheel, which pip extracts straight into the install's Scripts/ (Windows) or bin/ (POSIX) directory as-is — `.exe` files are not wrapped. Result: `cmd → fbuild.exe → stdout`, no Python in the chain. Same mechanism `ci/publish.py` already uses for release wheels on PyPI; only the local-install path was broken. Stock distutils `build_scripts.copy_scripts` tries to detect a Python coding cookie via `tokenize.open(script)`, which raises SyntaxError on a PE / ELF binary. Add a `BuildBinaryScripts` subclass that does a byte-level `shutil.copy2` instead. - pyproject.toml: drop `[project.scripts] fbuild = ci.bin_launcher:main`, drop `ci` from `[tool.setuptools] packages`, drop `ci = ["bin/fbuild*"]` package-data entry. - setup.py: add `scripts=[STAGED_BINARY_PATH]` to setup() and register `BuildBinaryScripts` as the `build_scripts` cmdclass. - Delete `ci/bin_launcher.py` (no longer reachable). Verified on Windows: `file $(where fbuild)` now reports PE32+ executable (was Zip-archive console-script stub), and `cmd /c 'echo === BEFORE === & fbuild port scan & echo === AFTER ==='` prints `=== AFTER ===` after the scan output instead of before it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRemoves ChangesNative Binary Distribution
Sequence Diagram(s)sequenceDiagram
participant pip
participant BuildWithCargo
participant BuildBinaryScripts
participant venv_bin as venv bin/ or Scripts/
pip->>BuildWithCargo: build_py — cargo build, stage fbuild into ci/bin/
BuildWithCargo-->>pip: STAGED_BINARY_PATH available
pip->>BuildBinaryScripts: build_scripts phase — copy_scripts()
BuildBinaryScripts->>BuildBinaryScripts: up_to_date check (dep_util / mtime)
BuildBinaryScripts->>venv_bin: byte-copy fbuild[.exe]
venv_bin-->>pip: native binary on PATH, no Python shim
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
…#2 items 1+10) (#7) Previously the Python entry point `[project.scripts] template-python-rust-cmd = "template_python_rust_cmd.cli:main"` created a pip console-script `.exe` that subprocess-launched the staged `_bin/template-cli`. On Windows that pattern races the next shell prompt ahead of the child's stdout, because the Python shim returns to cmd.exe before the spawned native binary finishes flushing. Same class of bug fbuild fixed in FastLED/fbuild#747. Drop the shim. ci/build_wheel.py now post-processes the maturin-built wheel: opens the zip, injects the cargo-built `template-cli[.exe]` at `<name>-<ver>.data/scripts/template-cli[.exe]`, recomputes the RECORD row (sha256 + size), and stamps the Unix executable bit on the entry. Files in `.data/scripts/` are pip's canonical raw-script install location — pip drops them straight into the venv's `Scripts/` (Win) / `bin/` (POSIX) directory verbatim, with no Python wrapper for `.exe` files. Same mechanism cargo-dist and maturin's "bin" mode use. Removed: - `[project.scripts]` table from pyproject.toml. - `src/template_python_rust_cmd/cli.py` (the racy subprocess shim). - `src/template_python_rust_cmd/_bin/.gitkeep` + the gitignore lines that staged binary into the package data directory. - `action/cleanup/action.yml` step that rm'd the (now-nonexistent) `_bin/` staging directory. Verified end-to-end on Windows: - Fresh maturin wheel produced via `uv run --no-project --script ci/build_wheel.py`. - `unzip -l dist/*.whl` shows `template_python_rust_cmd-0.1.0.data/ scripts/template-cli.exe` (1.2 MB) — no `_bin/` entries. - `uv pip install` of that wheel into a fresh venv lands a `Scripts/ template-cli.exe` that `file` reports as `PE32+ executable` (NOT a Zip-archive console-script stub). - `cmd /c 'echo === BEFORE === & template-cli.exe --version & echo === AFTER ==='` prints `=== AFTER ===` AFTER the version output — acceptance criterion from issue #2 met. The `no-build-isolation-package` setting added in PR #5 was reverted in this PR: with maturin (vs. setuptools) the trade-off doesn't favor us — uv reuses a stale build env that lacks maturin, and the preview `extra-build-dependencies` mechanism is too fragile to ship in a template. The CARGO_TARGET_DIR pin from that PR stays — it already gives the cargo-incremental win without touching isolation. Closes #2 (items 1 + 10). Refs FastLED/fbuild#747. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
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>
Summary
[project.scripts] fbuild = ci.bin_launcher:mainPython shim and ship the cargo-built native binary directly via setuptools'scripts=mechanism (fbuild-<ver>.data/scripts/fbuild.exein the wheel).build_scripts.copy_scriptsso it byte-copies the binary instead of trying to detect a Python coding cookie (SyntaxError).ci/bin_launcher.pyand dropcifrom[tool.setuptools] packages+[tool.setuptools.package-data].Closes #746.
Why
On Windows,
[project.scripts]generates a pip console-script.exethat importsci.bin_launcher.main, whichos.execvs the bundled native binary. POSIXexecvreplaces the process; Windowsos.execvis emulated asCreateProcess+ parent exit, so the Python shim returned to cmd.exe before the spawned Rust binary finished flushing stdout — the next shell prompt then raced ahead offbuild port scan's output. Same mechanismci/publish.pyalready uses for the release wheels on PyPI; only the local-install path was wrong.Test plan
pip install --force-reinstall .producesScripts/fbuild.exethatfilereports asPE32+ executable(notZip archiveconsole-script stub).cmd /c 'echo === BEFORE === & fbuild port scan & echo === AFTER ==='prints=== AFTER ===after the scan output, not before.fbuild --version/fbuild --helpstill work.from fbuild.api import SerialMonitorstill imports (PyO3 bindings unaffected).unzip -l dist/*.whl) showsfbuild-2.2.31.data/scripts/fbuild.exeplusfbuild/,fbuild/api/packages, and dist-info — noci/entries.🤖 Generated with Claude Code
Summary by CodeRabbit