diff --git a/.gitignore b/.gitignore index 9d89f45..d0408f9 100644 --- a/.gitignore +++ b/.gitignore @@ -31,10 +31,6 @@ build/ .ruff_cache/ .mypy_cache/ -# Maturin staged native binary (build_wheel.py copies it in, then removes it) -src/template_python_rust_cmd/_bin/* -!src/template_python_rust_cmd/_bin/.gitkeep - # Maturin extension module built in-place by `maturin develop` src/template_python_rust_cmd/_native*.pyd src/template_python_rust_cmd/_native*.so diff --git a/action/cleanup/README.md b/action/cleanup/README.md index 9a5aa5f..1a3c036 100644 --- a/action/cleanup/README.md +++ b/action/cleanup/README.md @@ -28,7 +28,6 @@ line, no copy-paste of the cleanup logic itself. 1. `uv tool uninstall template-python-rust-cmd` (no-op if not installed). 2. `uv cache prune --ci` to drop entries the next job won't reuse. -3. Removes the `_bin/` staging directory if the main action created one. The shell snippets are short by design — anything more complex would move into `ci/gates/cleanup.py` and be invoked via `./ci.sh cleanup`. diff --git a/action/cleanup/action.yml b/action/cleanup/action.yml index d395ba9..8bb3a32 100644 --- a/action/cleanup/action.yml +++ b/action/cleanup/action.yml @@ -27,8 +27,3 @@ runs: shell: bash run: | uv cache prune --ci 2>/dev/null || true - - - name: Remove staged binary - shell: bash - run: | - rm -rf "src/template_python_rust_cmd/_bin" 2>/dev/null || true diff --git a/ci/build_wheel.py b/ci/build_wheel.py index ca070c9..d9e6ca7 100644 --- a/ci/build_wheel.py +++ b/ci/build_wheel.py @@ -2,8 +2,30 @@ # /// script # requires-python = ">=3.11" # /// +"""Build the template-python-rust-cmd wheel and sdist. + +Flow: + 1. Build `template-cli` via cargo (release profile). + 2. Build the maturin wheel + sdist (PyO3 `_native` extension only). + 3. Inject the cargo-built `template-cli[.exe]` into the wheel's + `-.data/scripts/` directory and update RECORD. + 4. Verify the wheel contains both deliverables. + +Why post-process instead of letting maturin handle the binary? + Maturin doesn't ship raw binaries via the wheel scripts mechanism; + it ships `data/` content but treats it as install-time data, not as + Scripts/bin entries. We could go through `[project.scripts]`, but + that generates a pip console-script `.exe` whose `os.execv` is + emulated on Windows as `CreateProcess` + parent-exit — the shim + returns to cmd.exe before the child binary finishes flushing stdout, + so the next shell prompt races ahead of the output. Shipping the + binary as a raw wheel script bypasses the Python launcher entirely. + See fbuild#747 / zackees/template-python-rust-cmd#2 (items 1 + 10). +""" from __future__ import annotations +import base64 +import hashlib import json import os import shutil @@ -14,22 +36,15 @@ from pathlib import Path ROOT = Path(__file__).resolve().parent.parent -PACKAGE_BIN_DIR = ROOT / "src" / "template_python_rust_cmd" / "_bin" +DIST_DIR = ROOT / "dist" +PACKAGE_NAME = "template_python_rust_cmd" CLI_TARGET_NAME = ( "template-cli.exe" if platform.system() == "Windows" else "template-cli" ) -STAGED_BINARY_PATH = PACKAGE_BIN_DIR / CLI_TARGET_NAME # Pin cargo's target directory to a stable home-dir path so PEP 517 # isolated builds reuse cargo's incremental fingerprint cache across -# invocations. Without this, a `pip install .` (or `uv build`) that -# copies the source tree to a temp dir throws `/target/` away -# after each install — every install runs cargo cold (25-30s). -# -# Deliberately separate from `/target/` so iteration on -# bare `cargo check` / `cargo build` doesn't churn the wheel-build -# cache and vice versa. See FastLED/fbuild#743 and -# zackees/template-python-rust-cmd#2 (item 4). +# invocations. See zackees/template-python-rust-cmd#2 (item 4). WHEEL_BUILD_TARGET_DIR = ( Path.home() / ".template-python-rust-cmd" / "cargo-target" / "wheel-build" ) @@ -41,15 +56,11 @@ def run(cmd: list[str]) -> int: def _cargo_target_root() -> Path: - """Return cargo's effective target root, respecting CARGO_TARGET_DIR.""" target_dir = os.environ.get("CARGO_TARGET_DIR") - if target_dir: - return Path(target_dir) - return ROOT / "target" + return Path(target_dir) if target_dir else ROOT / "target" def _iter_cargo_inputs() -> list[Path]: - """Files that, if newer than the staged binary, invalidate the cache.""" patterns = ( "Cargo.toml", "Cargo.lock", @@ -63,37 +74,29 @@ def _iter_cargo_inputs() -> list[Path]: return paths -def _staged_binary_is_up_to_date() -> bool: - """True if the staged binary exists and is newer than every cargo input. +# `cargo_cache_path` is None until the first `build_cli_binary()` / +# mtime-skip resolves the cargo-built binary's location. The wheel +# injection step needs the same path; passing it through the call chain +# keeps `main()` straightforward. +def _cargo_cache_path() -> Path: + return _cargo_target_root() / "release" / CLI_TARGET_NAME - Skips the cargo invocation entirely on no-op reinstalls (version - bumps, lockfile churn, --reinstall-package). Even cargo's "Fresh" - pass walks the workspace and burns wall-clock seconds; an mtime - check is milliseconds. See FastLED/fbuild#743 and - zackees/template-python-rust-cmd#2 (item 6). - """ - if not STAGED_BINARY_PATH.is_file(): + +def _staged_cache_is_up_to_date(cache_path: Path) -> bool: + """True if the cached cargo output is newer than every cargo input.""" + if not cache_path.is_file(): return False - staged_mtime = STAGED_BINARY_PATH.stat().st_mtime + cache_mtime = cache_path.stat().st_mtime for path in _iter_cargo_inputs(): try: - if path.stat().st_mtime > staged_mtime: + if path.stat().st_mtime > cache_mtime: return False except FileNotFoundError: - # Glob race — treat as changed and rebuild. return False return True def _find_cli_executable_from_json(stdout: str) -> Path | None: - """Walk cargo's structured artifact stream for the `template-cli` exec. - - cargo emits one JSON object per line; the artifact we want has - `reason == "compiler-artifact"`, `target.name == "template-cli"`, - and a non-null `executable` field. We keep the *last* match because - cargo emits one artifact per crate target kind and the bin artifact - is what we want (matches `cargo install`'s selection rule). - """ found: Path | None = None for line in stdout.splitlines(): line = line.strip() @@ -115,13 +118,6 @@ def _find_cli_executable_from_json(stdout: str) -> Path | None: def _find_cli_executable_by_search() -> Path | None: - """Fallback when cargo emits no compiler-artifact line for the binary. - - Cargo skips the `compiler-artifact` JSON line for fully-cached - builds (everything `Fresh`), so the primary discovery path returns - None there. Probe `target/release/` and every per-host-triple - subdir of `target/`. See zackees/template-python-rust-cmd#2 (item 8). - """ target_root = _cargo_target_root() candidates = [target_root / "release" / CLI_TARGET_NAME] if target_root.is_dir(): @@ -136,6 +132,19 @@ def _find_cli_executable_by_search() -> Path | None: def build_cli_binary() -> Path: + """Build `template-cli` via cargo and return its on-disk path. + + Fast path: if the cached cargo output is newer than every cargo + input, return it directly without invoking cargo. + """ + cache_path = _cargo_cache_path() + if _staged_cache_is_up_to_date(cache_path): + print( + f" cargo cache up-to-date ({cache_path}); skipping cargo build", + file=sys.stderr, + ) + return cache_path + cmd = [ "cargo", "build", @@ -144,7 +153,6 @@ def build_cli_binary() -> Path: "template-cli", "--message-format=json-render-diagnostics", ] - # stderr passes through; stdout is captured for the artifact stream. proc = subprocess.run( cmd, cwd=ROOT, @@ -168,33 +176,6 @@ def build_cli_binary() -> Path: return binary -def stage_cli_binary(binary: Path) -> Path: - PACKAGE_BIN_DIR.mkdir(parents=True, exist_ok=True) - shutil.copy2(binary, STAGED_BINARY_PATH) - return STAGED_BINARY_PATH - - -def _ensure_staged_cli_binary() -> Path: - """Stage `template-cli` into the package, skipping cargo if cached. - - Fast path: if the staged binary is newer than every cargo input, the - file on disk already reflects the current source — return it without - touching cargo. Slow path: build via cargo and copy. - """ - if _staged_binary_is_up_to_date(): - print( - f" staged binary up-to-date ({STAGED_BINARY_PATH}); skipping cargo", - file=sys.stderr, - ) - return STAGED_BINARY_PATH - return stage_cli_binary(build_cli_binary()) - - -def remove_staged_binary(staged: Path) -> None: - if staged.exists(): - staged.unlink() - - def build_python_artifacts() -> int: cmd = ["uv", "run"] if platform.system() == "Linux": @@ -208,16 +189,98 @@ def build_python_artifacts() -> int: "--interpreter", sys.executable, "--out", - str(ROOT / "dist"), + str(DIST_DIR), ] ) return run(cmd) +def _latest_wheel() -> Path: + wheels = sorted(DIST_DIR.glob(f"{PACKAGE_NAME}-*.whl")) + if not wheels: + raise SystemExit("no wheel found in dist/") + return wheels[-1] + + +def _wheel_distribution_stem(wheel_path: Path) -> str: + """Return `-` from a wheel filename. + + A wheel filename is `{distribution}-{version}(-{build tag})?-{python + tag}-{abi tag}-{platform tag}.whl` (PEP 427). The leading + `-` is what `<...>.data/` and `<...>.dist-info/` + directories are prefixed with inside the archive. + """ + parts = wheel_path.stem.split("-") + return f"{parts[0]}-{parts[1]}" + + +def _record_row(arcname: str, data: bytes) -> str: + """Build a RECORD CSV row matching the wheel spec. + + RECORD is `,sha256=,` per + https://peps.python.org/pep-0376/#record. RECORD's own row uses + empty hash + size fields. + """ + digest = hashlib.sha256(data).digest() + h = base64.urlsafe_b64encode(digest).rstrip(b"=").decode("ascii") + return f"{arcname},sha256={h},{len(data)}" + + +def inject_cli_into_wheel(binary: Path) -> Path: + """Rewrite the wheel: add the CLI at `-.data/scripts/`. + + Files in `-.data/scripts/` are pip's canonical "raw + script" install location — pip drops them straight into the venv's + `Scripts/` (Windows) or `bin/` (POSIX) directory verbatim. `.exe` + files are NOT wrapped (pip only wraps shebang-style text scripts). + This is the same mechanism `cargo-dist` and maturin's "bin" mode + use to ship native binaries via PyPI without a Python shim. + """ + wheel_path = _latest_wheel() + stem = _wheel_distribution_stem(wheel_path) + script_arcname = f"{stem}.data/scripts/{CLI_TARGET_NAME}" + record_arcname = f"{stem}.dist-info/RECORD" + + binary_bytes = binary.read_bytes() + + # Read every existing entry into memory. Wheels are small enough + # that loading the whole thing is fine (maturin emits ~MB-sized + # wheels for projects this size). + with zipfile.ZipFile(wheel_path, "r") as wf: + entries: dict[str, bytes] = {name: wf.read(name) for name in wf.namelist()} + + if record_arcname not in entries: + raise SystemExit( + f"wheel has no {record_arcname}; cannot inject CLI script" + ) + + # Append a RECORD row for the new script. RECORD's own row keeps + # empty hash + size per spec — we preserve that. + record_text = entries[record_arcname].decode("utf-8") + new_row = _record_row(script_arcname, binary_bytes) + "\n" + record_text = record_text.rstrip("\n") + "\n" + new_row + entries[record_arcname] = record_text.encode("utf-8") + entries[script_arcname] = binary_bytes + + # Rewrite the wheel from scratch so deflated sizes and central-dir + # offsets get rebuilt cleanly (in-place append leaves stale entries + # if the same name was already present). + with zipfile.ZipFile(wheel_path, "w", zipfile.ZIP_DEFLATED) as wf: + for name, data in entries.items(): + info = zipfile.ZipInfo(name) + # Stamp Unix executable bit (0o755) on the script so POSIX + # pip installs land it +x. create_system=3 = Unix. + if name == script_arcname: + info.create_system = 3 + info.external_attr = (0o755 << 16) + wf.writestr(info, data) + return wheel_path + + def verify_artifacts() -> int: - dist_dir = ROOT / "dist" - wheels = sorted(dist_dir.glob("template_python_rust_cmd-*.whl")) - sdists = sorted(dist_dir.glob("template_python_rust_cmd-*.tar.gz")) + """Confirm the wheel ships both deliverables: PyO3 + raw CLI script.""" + wheels = sorted(DIST_DIR.glob(f"{PACKAGE_NAME}-*.whl")) + sdists = sorted(DIST_DIR.glob(f"{PACKAGE_NAME}-*.tar.gz")) if not wheels: print("expected at least one wheel in dist/") return 1 @@ -225,12 +288,10 @@ def verify_artifacts() -> int: print("expected an sdist in dist/") return 1 - expected_binary_suffix = ( - "template-cli.exe" if platform.system() == "Windows" else "template-cli" - ) + stem = _wheel_distribution_stem(wheels[-1]) expected_entries = [ - "template_python_rust_cmd/_native", - f"template_python_rust_cmd/_bin/{expected_binary_suffix}", + f"{PACKAGE_NAME}/_native", # PyO3 extension (filename varies by abi) + f"{stem}.data/scripts/{CLI_TARGET_NAME}", # raw CLI script ] with zipfile.ZipFile(wheels[-1]) as archive: names = archive.namelist() @@ -242,13 +303,11 @@ def verify_artifacts() -> int: def main() -> int: - staged = _ensure_staged_cli_binary() - try: - if build_python_artifacts() != 0: - return 1 - return verify_artifacts() - finally: - remove_staged_binary(staged) + binary = build_cli_binary() + if build_python_artifacts() != 0: + return 1 + inject_cli_into_wheel(binary) + return verify_artifacts() if __name__ == "__main__": diff --git a/pyproject.toml b/pyproject.toml index 7a82786..5a4e90c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -13,8 +13,15 @@ license-files = ["LICENSE"] authors = [{ name = "Maintainer", email = "maintainer@example.com" }] dependencies = [] -[project.scripts] -template-python-rust-cmd = "template_python_rust_cmd.cli:main" +# `template-cli` on PATH is the native cargo-built binary itself, +# injected into the wheel's -.data/scripts/ directory by +# ci/build_wheel.py's post-processing step. No Python wrapper sits in +# front of it — `cmd → template-cli → stdout`. We don't use +# `[project.scripts]` (PEP 621 / wheel console-scripts), because on +# Windows pip generates a Python launcher .exe whose `os.execv` is +# emulated as CreateProcess + parent-exit, racing the shell prompt +# ahead of the child's stdout. See fbuild#747 and +# zackees/template-python-rust-cmd#2 (items 1 + 10). [dependency-groups] dev = [ @@ -50,11 +57,14 @@ cache-keys = [ { file = "crates/**/*.rs" }, { file = "crates/**/Cargo.toml" }, ] -# Run the maturin build in the real repo, not a PEP 517 temp copy. -# Isolation forces cargo to discard its incremental fingerprint cache -# every install (the temp copy has a fresh, empty target/), and also -# hides the staged binary from the wheel-side mtime-skip we want to -# land in a follow-up PR. `maturin` already ships in the dev -# dependency group, so the build env always has what it needs. -# See fbuild#743 and zackees/template-python-rust-cmd#2 (item 5). -no-build-isolation-package = ["template-python-rust-cmd"] +# NOTE on `no-build-isolation-package`: fbuild disables PEP 517 +# isolation so its setup.py-based mtime-skip can see the staged binary +# across `pip install` invocations. The maturin backend doesn't fit +# that pattern cleanly — disabling isolation breaks the build env's +# automatic maturin install (uv reuses a build env without maturin, +# and uv's preview `extra-build-dependencies` mechanism is fragile +# here). The CARGO_TARGET_DIR pin in ci/build_wheel.py already gives +# us the cargo-incremental win without needing to touch isolation, +# so we leave isolation on. See zackees/template-python-rust-cmd#2 +# (item 5) — we may revisit if the maturin/uv combo grows a clean +# no-isolation story. diff --git a/src/README.md b/src/README.md index 92a9570..cb9a7ff 100644 --- a/src/README.md +++ b/src/README.md @@ -10,11 +10,18 @@ maturin `python-source`. src/template_python_rust_cmd/ ├── __init__.py # package version + re-exports ├── _native.pyi # typing stub for the PyO3 surface -├── bindings.py # Python wrapper around the extension module -├── cli.py # Python entrypoint; delegates to the native binary -└── _bin/ # staged native executable (gitignored except .gitkeep) +└── bindings.py # Python wrapper around the extension module ``` +The `template-cli` native binary is NOT shipped under this package +directory. It is injected into the wheel's +`-.data/scripts/` directory at build time +(`ci/build_wheel.py`) and pip drops it straight into the venv's +`Scripts/` (Windows) or `bin/` (POSIX) directory at install time — +no Python wrapper sits in front of it. See +`template_python_rust_cmd/README.md` for the rationale (issue #2, +items 1 + 10). + ## Why `src/`-layout instead of flat Standard PEP 517 src-layout avoids the "accidentally importing @@ -28,9 +35,6 @@ for it. - `bindings.py` — the public Python API. Each function should be a near-1:1 reflection of an underlying `_native` call, with type annotations and a one-line docstring. -- `cli.py` — the Python CLI shim. Should stay tiny: locate the - packaged binary, exec it, pass through the exit code. No business - logic. - `__init__.py` — package version, public re-exports. Don't import `_native` directly here; route through `bindings.py`. - `_native.pyi` — optional typing stub mirroring the PyO3 surface. @@ -40,7 +44,6 @@ for it. - `_native*.pyd` / `_native*.so` / `_native*.dylib` — built by maturin, gitignored. -- `_bin/` — staged by `ci/build_wheel.py`, gitignored. - Anything implementing domain logic — that belongs in `template-core`. ## Build modes diff --git a/src/template_python_rust_cmd/README.md b/src/template_python_rust_cmd/README.md index feaabd2..a83723d 100644 --- a/src/template_python_rust_cmd/README.md +++ b/src/template_python_rust_cmd/README.md @@ -8,7 +8,6 @@ The actual Python package. Imported as | Module | Purpose | |---------------|-------------------------------------------------------------------------| | `bindings` | Python wrapper around the PyO3 extension. Public API surface. | -| `cli` | Thin shim; locates `_bin/template-cli` and execs it. Used by the script entry point in `pyproject.toml`. | | `__init__` | Package version (`__version__`) and re-exports. | ## Internal modules @@ -17,7 +16,6 @@ The actual Python package. Imported as |---------------|-------------------------------------------------------------------------| | `_native` | Built by maturin from `crates/template-py`. Never import directly from outside this package — go through `bindings`. | | `_native.pyi` | Optional typing stub for IDEs. | -| `_bin/` | Holds the packaged `template-cli{,.exe}` binary at install time. Staged by `ci/build_wheel.py` and removed afterward. | ## Wrapping the extension @@ -34,18 +32,34 @@ even if the underlying PyO3 decorator's signature changes (e.g., a new keyword argument added at the Rust layer). The wrapper is the unit of API compatibility. -## CLI delegation +## CLI delivery (no Python shim) + +The `template-cli` binary on PATH after `pip install` is the +cargo-built executable itself, **not** a Python launcher. It is +injected into the wheel's `-.data/scripts/` directory by +`ci/build_wheel.py`'s post-processing step. Pip drops files in +`.data/scripts/` straight into the venv's `Scripts/` (Windows) or +`bin/` (POSIX) directory verbatim — `.exe` files are NOT wrapped. + +Why no Python shim? On Windows, `[project.scripts]` generates a pip +console-script `.exe` whose `os.execv` is emulated as `CreateProcess` ++ parent exit. The Python shim returns to cmd.exe **before** the +spawned native binary finishes flushing stdout, so the next shell +prompt races ahead of `template-cli`'s output. Shipping the binary +as a raw wheel script bypasses the Python launcher entirely. See +[fbuild#747](https://github.com/FastLED/fbuild/pull/747) and +[issue #2](https://github.com/zackees/template-python-rust-cmd/issues/2) +items (1) + (10). + +If you want to invoke the CLI from Python code, do it through +`subprocess`: ```python -def main() -> int: - binary = packaged_binary_path() - return subprocess.call([str(binary), *sys.argv[1:]]) -``` +import shutil +import subprocess -`cli.main` is the entry point declared in -`pyproject.toml::project.scripts`. It's deliberately tiny — argv -parsing, exit codes, and behavior all live in the native binary; the -Python shim just hands control over. +subprocess.run([shutil.which("template-cli"), "--help"], check=True) +``` ## Why both an extension AND a binary? @@ -54,7 +68,7 @@ Two different consumption stories: - **In-process API.** A Python program wants to call into Rust without the cost of spawning a subprocess. → Use `bindings.py`. - **Command surface.** A user (or shell script, or CI step) wants to - run `template-cli foo --bar`. → Use the `template-python-rust-cmd` - console script that delegates to the binary. + run `template-cli foo --bar`. → Use the `template-cli` binary + installed by the wheel's raw-script mechanism. `template-core` makes both surfaces honor the same domain behavior. diff --git a/src/template_python_rust_cmd/_bin/.gitkeep b/src/template_python_rust_cmd/_bin/.gitkeep deleted file mode 100644 index 8b13789..0000000 --- a/src/template_python_rust_cmd/_bin/.gitkeep +++ /dev/null @@ -1 +0,0 @@ - diff --git a/src/template_python_rust_cmd/cli.py b/src/template_python_rust_cmd/cli.py deleted file mode 100644 index dbf9c92..0000000 --- a/src/template_python_rust_cmd/cli.py +++ /dev/null @@ -1,20 +0,0 @@ -"""Python console entrypoint that delegates to the packaged Rust binary.""" - -from __future__ import annotations - -import os -import subprocess -import sys -from pathlib import Path - - -def packaged_binary_path() -> Path: - """Return where the packaged Rust executable is expected to live.""" - suffix = ".exe" if os.name == "nt" else "" - return Path(__file__).resolve().parent / "_bin" / f"template-cli{suffix}" - - -def main() -> int: - binary = packaged_binary_path() - completed = subprocess.run([str(binary), *sys.argv[1:]], check=False) - return completed.returncode