diff --git a/ci/bin_launcher.py b/ci/bin_launcher.py deleted file mode 100644 index 4e824506..00000000 --- a/ci/bin_launcher.py +++ /dev/null @@ -1,40 +0,0 @@ -"""Console-script entry point for the bundled `fbuild` binary. - -Wheels produced by `pip install .` (see `setup.py`) ship a native -`fbuild[.exe]` binary inside `ci/bin/`. The wheel also declares this module -as the `fbuild` console script via `[project.scripts]` in `pyproject.toml`, -so `which fbuild` resolves to a tiny Python shim that lives next to this -launcher. The launcher then `execv`s the real binary, replacing the Python -process — so process semantics (PID, signal handling, stdio inheritance, -exit code) match what bare-cargo `target/release/fbuild` would give. - -The release wheels published to PyPI by the Autonomous Release GitHub -Action (`.github/workflows/release-auto.yml`, which calls into -`ci/publish.py`) follow the same layout: pre-built native binary at -`ci/bin/fbuild[.exe]`, this launcher as the entry point. -""" - -from __future__ import annotations - -import os -import sys -from pathlib import Path - -# IMPORTANT: keep this filename matching `TARGET_BINARY_NAME` in setup.py. -_BINARY_NAME = "fbuild.exe" if sys.platform == "win32" else "fbuild" - - -def main() -> None: - binary = Path(__file__).resolve().parent / "bin" / _BINARY_NAME - if not binary.exists(): - sys.stderr.write( - f"ERROR: bundled fbuild binary not found at {binary}.\n" - f"This wheel may be incomplete. Reinstall with `pip install --force-reinstall .` " - f"from the fbuild source tree, or install a release wheel from PyPI.\n" - ) - sys.exit(1) - - # execv replaces the Python process. argv[0] convention: pass the - # binary path so help text and error messages reference the real - # command, not the Python shim. - os.execv(str(binary), [str(binary)] + sys.argv[1:]) diff --git a/pyproject.toml b/pyproject.toml index 088dd904..a5515be7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,11 +10,12 @@ requires-python = ">=3.10" # agree on one version; embedding the binary removes that conflict. dependencies = [] -# Console script: `fbuild` on PATH after `pip install .` execs the native -# binary that setup.py stages into ci/bin/fbuild[.exe]. See setup.py + -# ci/bin_launcher.py for the full wiring (#423). -[project.scripts] -fbuild = "ci.bin_launcher:main" +# `fbuild` on PATH is the native cargo-built binary itself, shipped as a +# raw wheel script (setuptools' `scripts=` mechanism in setup.py). No +# Python wrapper sits in front of it — `cmd → fbuild → stdout`. See +# setup.py and #746 for why we don't use `[project.scripts]` (PEP 621 +# only supports Python entry points, which on Windows go through an +# `os.execv` emulation that races the next shell prompt ahead of stdout). [dependency-groups] # setuptools is in dev (not project deps) because fbuild uses @@ -56,23 +57,23 @@ requires = ["setuptools>=64"] build-backend = "setuptools.build_meta" [tool.setuptools] -packages = ["ci", "fbuild", "fbuild.api"] +packages = ["fbuild", "fbuild.api"] include-package-data = true # Map the `fbuild` and `fbuild.api` Python packages to their on-disk # location under python/. Without this, `pip install .` would skip the -# python/ tree entirely (only `ci` is at the repo root), and downstream -# consumers like FastLED would hit `ModuleNotFoundError: No module named -# 'fbuild'` on `from fbuild.api import SerialMonitor`. +# python/ tree entirely, and downstream consumers like FastLED would hit +# `ModuleNotFoundError: No module named 'fbuild'` on +# `from fbuild.api import SerialMonitor`. [tool.setuptools.package-dir] fbuild = "python/fbuild" "fbuild.api" = "python/fbuild/api" -# Ship the native binary that setup.py's BuildWithCargo cmdclass stages -# into ci/bin/ during the build_py phase. Glob is broad enough to match -# fbuild + fbuild.exe across platforms. Also ship the _native.pyd that -# python/fbuild/__init__.py imports from. +# Ship `_native.pyd`/.so that `python/fbuild/__init__.py` imports from. +# The cargo-built `fbuild[.exe]` CLI binary is NOT package data — it is +# declared as a raw wheel script via setup.py's `scripts=` argument so +# pip drops it straight into Scripts/bin as a native exe (no Python +# wrapper). See #746. [tool.setuptools.package-data] -ci = ["bin/fbuild*"] fbuild = ["_native.pyd", "_native.so"] diff --git a/setup.py b/setup.py index f6ad6168..f585cc14 100644 --- a/setup.py +++ b/setup.py @@ -2,15 +2,19 @@ `pip install ~/dev/fbuild` (or any `pip install .` from the repo root) goes through this file because `pyproject.toml` declares the setuptools build -backend. The plain backend would ship only `ci/` Python helpers — no working -`fbuild` command — because the actual CLI is a Rust crate (`fbuild-cli`) -that lives in the cargo workspace under `crates/`. +backend. The plain backend would ship only the `python/fbuild` Python +package — no working `fbuild` command — because the actual CLI is a Rust +crate (`fbuild-cli`) that lives in the cargo workspace under `crates/`. This file wires the install path through `soldr cargo build --release -p -fbuild-cli`, copies the resulting binary to `ci/bin/fbuild[.exe]`, and lets -setuptools pack it into the wheel. The `ci.bin_launcher:main` entry point -(declared in pyproject.toml) execs that binary, so `fbuild` on PATH after -install Just Works. +fbuild-cli`, copies the resulting binary to `ci/bin/fbuild[.exe]`, and +hands that path to setuptools as a raw wheel script (the `scripts=` +argument to `setup()` below). Pip drops raw scripts straight into the +venv's `Scripts/` (Windows) or `bin/` (POSIX) directory as-is — `.exe` +files are NOT wrapped, so `fbuild` on PATH is the literal cargo-built +binary with no Python shim in front of it (see #746 for why the previous +`[project.scripts] fbuild = "ci.bin_launcher:main"` approach broke stdout +ordering on Windows). This is the LOCAL DEV install path. The RELEASE path lives entirely in the Autonomous Release GitHub Action (`.github/workflows/release-auto.yml`): @@ -67,9 +71,17 @@ from pathlib import Path from typing import Optional +# Import setuptools FIRST so its distutils shim is installed before we +# pull `distutils.command.build_scripts` off the shim. Importing the +# distutils module without setuptools loaded first either misses the +# shim or (depending on Python/setuptools version) yields the stdlib +# distutils, which is deprecated and gone in Python 3.12+. from setuptools import setup from setuptools.command.build_py import build_py from setuptools.dist import Distribution +from distutils.command.build_scripts import ( # type: ignore[import-untyped] + build_scripts, +) REPO_ROOT = Path(__file__).resolve().parent @@ -291,7 +303,71 @@ def has_ext_modules(self) -> bool: # noqa: D401 — setuptools API name return True +class BuildBinaryScripts(build_scripts): + """Byte-copy variant of `build_scripts` for raw native binaries. + + Stock `build_scripts.copy_scripts` calls `tokenize.open(script)` on + each entry to detect a coding cookie and patch a shebang for source + scripts. That's right for `.py` files but wrong for a Rust-built + `.exe` / ELF binary — `tokenize.open` raises `SyntaxError: invalid or + missing encoding declaration` on the very first read. We override + `copy_scripts` to do a plain byte-level `shutil.copy2`, preserving + the executable bit on POSIX (cargo already sets it). The file lands + in `-.data/scripts/` in the wheel; pip then copies it + straight into the install's `Scripts/` (Windows) or `bin/` (POSIX) + directory verbatim — no shebang, no Python wrapper. See #746. + """ + + def copy_scripts(self): # noqa: D401 — distutils API name + self.mkpath(self.build_dir) + outfiles: list[str] = [] + updated_files: list[str] = [] + for script in self.scripts: + outfile = os.path.join(self.build_dir, os.path.basename(script)) + # `dep_util.newer` returns True if `script` is newer than + # `outfile`, mirroring stock build_scripts' "update or skip" + # behavior — avoids spurious rebuilds breaking caching. + try: + from distutils import dep_util # type: ignore[import-untyped] + + up_to_date = ( + os.path.exists(outfile) and not dep_util.newer(script, outfile) + ) + except ImportError: + # Python 3.12+ removed distutils.dep_util; fall back to + # an mtime compare. + up_to_date = os.path.exists(outfile) and ( + os.path.getmtime(script) <= os.path.getmtime(outfile) + ) + if up_to_date and not self.force: + outfiles.append(outfile) + continue + shutil.copy2(script, outfile) + outfiles.append(outfile) + updated_files.append(outfile) + return outfiles, updated_files + + +# `scripts=` is the legacy setuptools mechanism for shipping raw files +# (no shebang/no entry-point wrapping) into the install's Scripts/bin +# directory. Files land in `-.data/scripts/` inside the +# wheel; `pip install` then copies them straight into the venv as-is — +# on Windows pip does NOT generate a Python wrapper for `.exe` files +# (only for shebang-style script text). This is the same mechanism +# maturin's "bin" mode and cargo-dist use to ship native binaries via +# PyPI without a Python shim. See #746. +# +# Stock `build_scripts` parses each script as Python source to find a +# coding cookie / shebang — we override with `BuildBinaryScripts` to do +# a plain byte-copy instead. `STAGED_BINARY_PATH` doesn't exist until +# `BuildWithCargo` runs, which happens during the `build_py` phase. +# Setuptools' build pipeline runs `build_py` before `build_scripts`, so +# by the time `build_scripts` reads this list, the file is on disk. setup( - cmdclass={"build_py": BuildWithCargo}, + cmdclass={ + "build_py": BuildWithCargo, + "build_scripts": BuildBinaryScripts, + }, distclass=BinaryDistribution, + scripts=[str(STAGED_BINARY_PATH)], )