diff --git a/docs/developer-guide.md b/docs/developer-guide.md index 5d406c5cd..60cfd4ec1 100644 --- a/docs/developer-guide.md +++ b/docs/developer-guide.md @@ -183,10 +183,29 @@ This builds the nanobind `_task_interface` extension **and** pre-builds all runt | First time / clean checkout | `pip install --no-build-isolation -e .` | | Runtime C++ source (`src/{arch}/runtime/`, `src/{arch}/platform/`) | Re-run `pip install --no-build-isolation -e .` (or `.` for a non-editable install). Incremental via the cmake caches under `build/cache/` (~1-2s). | | Nanobind bindings (`python/bindings/`) | Re-run `pip install --no-build-isolation -e .` (no rebuild-on-import; `editable.rebuild = false`) | -| Python-only code (`python/*.py`, `simpler_setup/*.py`) | No rebuild needed (editable install) | -| Examples / kernels (`examples/{arch}/`, `tests/st/`) | No rebuild needed, just re-run | +| Nothing — but `HEAD` moved (branch switch, rebase, pull, **or your own commit**) | Re-run `pip install ...`. You may have changed no compiled file, yet the tree moved out from under a binary frozen at install time. `simpler.task_interface` refuses to import on this skew (see below) rather than let struct fields read as 0. | +| Python-only code (`python/*.py`, `simpler_setup/*.py`) | Nothing to recompile (editable install) — but see the row above: once you *commit* it, the import guard keys on `HEAD`, so reinstall before the next import. | +| Examples / kernels (`examples/{arch}/`, `tests/st/`) | Nothing to recompile, just re-run — same commit caveat as the row above | | `pto_isa.pin` changed | Re-run `pip install`. The cmake cache stamp and the `host_runtime` ccache key include the pinned PTO-ISA commit for a2a3 onboard (and a5 onboard when the SDMA overlay is enabled), so a pin bump invalidates stale runtime objects automatically. | +`_task_interface` records the commit it was compiled from, and +`simpler.task_interface` compares it against the working tree at import, +raising when they differ. It is git-based for the same reason +`RuntimeBuilder._build_cache_stamp` is — mtimes do not survive a branch switch — +and deliberately keyed on the whole HEAD rather than on the binding sources +alone: a root `CMakeLists.txt`, `pyproject.toml` or nanobind change alters the +extension without touching `python/bindings/`, and under-detecting here means +silently wrong values. The cost of over-detecting is one ~20 s reinstall, and +120 of the last 200 commits touched the ABI surface anyway — so most of those +reinstalls are ones you owed regardless, merely made visible. + +Verifying that `import simpler` resolves into your worktree does **not** cover +this: under an editable install that only proves the *Python* is live, while the +compiled extension stays at whatever `pip install` produced. + +The check is inert outside a source tree — a wheel has no `.git` to compare +against, and a build made without git carries an empty stamp. + A `pto_isa.pin` bump changes the SDMA headers embedded by `host_runtime.so`. Install-time runtime builds and run-time kernel compilation both read `pto_isa.pin`; use a different ISA revision by updating that file. diff --git a/python/bindings/CMakeLists.txt b/python/bindings/CMakeLists.txt index 4aa4d98d0..75d24dff8 100644 --- a/python/bindings/CMakeLists.txt +++ b/python/bindings/CMakeLists.txt @@ -30,8 +30,25 @@ set(HIERARCHICAL_SOURCES ${HIERARCHICAL_SRC}/worker.cpp ) +# Stamp the source commit into the module so `simpler.task_interface` can refuse +# to run against a binding built from a different tree. Same notion of "which +# sources is this built from" as RuntimeBuilder._build_cache_stamp, and git for +# the same reason: mtimes do not survive a branch switch. Empty when git is +# unavailable (release tarball, no .git), which disables the check. +execute_process( + COMMAND git rev-parse HEAD + WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} + OUTPUT_VARIABLE SIMPLER_BUILD_COMMIT + OUTPUT_STRIP_TRAILING_WHITESPACE + ERROR_QUIET +) + nanobind_add_module(_task_interface ${BINDING_SOURCES} ${HIERARCHICAL_SOURCES}) +target_compile_definitions(_task_interface PRIVATE + SIMPLER_BUILD_COMMIT="${SIMPLER_BUILD_COMMIT}" +) + target_sources(_task_interface PRIVATE ${CMAKE_SOURCE_DIR}/src/common/worker/chip_worker.cpp # Host-side assert_impl / get_stacktrace for the unified Tensor's diff --git a/python/bindings/task_interface.cpp b/python/bindings/task_interface.cpp index 20bab3130..16c109d92 100644 --- a/python/bindings/task_interface.cpp +++ b/python/bindings/task_interface.cpp @@ -562,9 +562,19 @@ void append_cleanup_error(std::string &cleanup_error, const std::string &message // Module definition // ============================================================================ +#ifndef SIMPLER_BUILD_COMMIT +#define SIMPLER_BUILD_COMMIT "" +#endif + NB_MODULE(_task_interface, m) { m.doc() = "Nanobind bindings for task_interface (DataType, Tensor, TaskArgs variants)"; + // Source commit this extension was compiled from; "" when git was + // unavailable at build time. simpler.task_interface compares it against the + // working tree so a binding built from other sources cannot be used + // silently — struct layouts differ and fields read as garbage. + m.attr("__build_commit__") = SIMPLER_BUILD_COMMIT; + // --- DataType enum --- nb::enum_(m, "DataType") .value("FLOAT32", DataType::FLOAT32) diff --git a/python/simpler/task_interface.py b/python/simpler/task_interface.py index 519014323..b18142329 100644 --- a/python/simpler/task_interface.py +++ b/python/simpler/task_interface.py @@ -29,8 +29,10 @@ from dataclasses import dataclass from enum import IntEnum from math import prod +from pathlib import Path from typing import Any +import _task_interface as _ti_module # pyright: ignore[reportMissingImports] from _task_interface import ( # pyright: ignore[reportMissingImports] MAILBOX_ERROR_MSG_SIZE, MAILBOX_OFF_ERROR_MSG, @@ -57,6 +59,71 @@ read_args_from_blob, ) + +def _assert_bindings_match_source_tree() -> None: + """Refuse a `_task_interface` built from a different revision of this tree. + + An editable install pins the compiled extension at install time + (``editable.rebuild = false``) while this file is read live, so switching + branches or rebasing moves the Python source out from under a fixed binary. + Nothing then rebuilds, and a changed struct layout — `CallConfig` losing a + field, say — makes attributes read as 0 with no error at all. That surfaces + much later as a plausible-looking runtime rejection + (``launch_aicpu_num (0) must be in range [1, 4]``) and reads as a product + bug, so it is worth one git call at import to stop. + + Only source-tree installs are checked: a wheel has no ``.git`` to compare + against, and a build with no git available carries an empty stamp. + + A *missing* stamp is not the same as an empty one. The attribute only + disappears on an extension compiled before it existed, which in a checkout + new enough to run this function is by definition a different revision — the + exact case this guards, and the one every already-installed worktree is in + the moment this lands. Treating it like "cannot tell" would let precisely + those through. + """ + import subprocess # noqa: PLC0415 + + repo_root = Path(__file__).resolve().parents[2] + if not (repo_root / ".git").exists(): + return + if not hasattr(_ti_module, "__build_commit__"): + raise ImportError( + "_task_interface predates the build stamp, so it was compiled before the " + "revision you are running and its struct layouts may not match the Python " + "that drives them — fields can read as 0 with no error.\n" + "Rebuild: pip install --no-build-isolation -e ." + ) + built_from: str = _ti_module.__build_commit__ + if not built_from: + return + try: + result = subprocess.run( + ["git", "rev-parse", "HEAD"], + cwd=str(repo_root), + capture_output=True, + text=True, + timeout=5, + check=False, + ) + except Exception: # noqa: BLE001 + return + if result.returncode != 0: + return + head = result.stdout.strip() + if not head or head == built_from: + return + raise ImportError( + f"_task_interface was built from {built_from[:12]}, but this source tree is at " + f"{head[:12]}. The compiled extension does not rebuild on import " + f"(editable.rebuild = false), so its struct layouts may no longer match the " + f"Python that drives them — fields can read as 0 with no error.\n" + f"Rebuild: pip install --no-build-isolation -e ." + ) + + +_assert_bindings_match_source_tree() + __all__ = [ "DataType", "get_element_size",