diff --git a/CLAUDE.md b/CLAUDE.md index 0091bd9..0d3dc12 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -13,7 +13,7 @@ Read `docs/STEERING.md` before non-trivial work (new operations, transport or fr - A model property that reads a device field checks the field is PRESENT (`protocol.field_present`) before reporting it. Most of this schema sits in synthetic `oneof`s, so protobuf returns `""` or `0` for a field the unit never sent, and reporting that as the answer is the guess the rule above forbids. Never cache a reply that came back incomplete - a retry has to be able to recover. - Anything the model caches is valid only while its connection is. A closed `Device` refuses reads rather than answering from cache, because a model that reports the unit's state through an object with no unit behind it is the failure the whole layer exists to avoid. - `import hid` appears exactly once, lazily, inside `session.open_device()`. Never import `hid` at module scope. A new module that needs it imports it inside the function that opens the device; `tests/test_import_cleanliness.py` walks the whole package and proves it. -- Never gitignore or delete `pyquadcortex/protocol/proto/*_pb2.py` - the generated bindings are committed on purpose (ADR-0001, written before the proto directory was moved). Regenerate only via `scripts/compile_protos.sh`, and bump the `protobuf` pin in `pyproject.toml` in the same commit as regenerated bindings. Read the gencode version in the regenerated diff before committing it: an older `grpcio-tools` in the venv silently emits older gencode, which still imports and quietly walks the pin backwards. CI's `build` job runs `scripts/check_artifacts.py`, which proves the bindings are inside the wheel and the sdist. +- Never gitignore or delete `pyquadcortex/protocol/proto/*_pb2.py` - the generated bindings are committed on purpose (ADR-0001, written before the proto directory was moved). Regenerate only via `scripts/compile_protos.sh`, and bump the `protobuf` pin in `pyproject.toml` in the same commit as regenerated bindings. The `grpcio-tools` floor in the dev extra is part of that same commit: `grpcio-tools` carries its own protoc, so the installed version decides the gencode, and an older one emits older gencode that still imports and quietly walks the pin backwards (ADR-0008). Both directions are now guarded - the script refuses to write a downgrade, and `tests/test_packaging.py` proves the committed gencode equals the pin floor - so trust the failure and fix the cause rather than working around either. Never read the floor off `grpcio-tools` metadata; 1.82.1 declares `protobuf>=7.35.1` and emits 7.35.0. Run the compiler and read the stamp. CI's `build` job runs `scripts/check_artifacts.py`, which proves the bindings are inside the wheel and the sdist. - New operations follow `docs/architecture.md` "How to add a new operation": register the type, add a thin client method (no HID, no bytes, no sleeps in `protocol/client.py`), add an offline test asserting the exact wire shape, then verify on hardware and update the coverage table in `docs/protocol.md`. - Grid mutations use the row/column-keyed pattern (`set_param` / `set_bypass`) - never extend the wholesale `write_preset` path. - Docstrings state their evidence: confirmed on hardware vs inferred from the schema. When you verify something on hardware, record it (docstring + coverage table) in the same change. diff --git a/changelog.md b/changelog.md index 1d894d6..95bbce2 100644 --- a/changelog.md +++ b/changelog.md @@ -108,6 +108,29 @@ audit and the model design were both written against. The correction is in control it cannot yet drive and refuses it, rather than omitting it or guessing - is ADR-0007. +### Regenerating the protobuf bindings can no longer walk the pin backwards + +Nothing you install changes. This is about the generated bindings that ship in +the wheel, and it matters to anyone who regenerates them. + +`grpcio-tools` carries its own copy of protoc, so whichever version is installed +decides the gencode written into the bindings. The dev extra's floor was +`>=1.68`, low enough that `pip install -e ".[dev]"` could resolve to a generator +emitting gencode 7.35.0 against bindings committed at 7.35.1 - and lower still +through a venv that picked up `grpcio-tools` some other way, or the script's +fallback to a system `protoc`, which no floor constrains. So regenerating could +silently downgrade them. Nothing caught it: the protobuf runtime only checks +`runtime >= gencode`, so older bindings import cleanly and pass the whole suite +while `pyproject.toml`'s pin no longer describes them. + +The floor is now `grpcio-tools>=1.83.0`, the oldest release whose protoc emits +gencode 7.35.1, and it moves in the same commit as any gencode bump. +`scripts/compile_protos.sh` refuses to install bindings older than the committed +ones and leaves the tree untouched when it does; `tests/test_packaging.py` +checks on every PR that the committed gencode and the pin floor are the same +number. The bindings themselves are unchanged - regenerating is its own change +with its own pin bump (ADR-0001, ADR-0008). + ## 0.40.0 - 2026-08-10 ### The lane/mixer level span is -40..+12 dB, not -100..+30 diff --git a/contributing.md b/contributing.md index 009a792..c41c57f 100644 --- a/contributing.md +++ b/contributing.md @@ -58,6 +58,11 @@ scripts/compile_protos.sh If you regenerate with a newer `protobuf`, the runtime pin in `pyproject.toml` must be raised to match the generated code, or imports will fail for everyone else. + +Regenerating with an *older* generator is the quieter mistake, so the script +checks for it: if your `grpcio-tools` would write older gencode than what is +committed, it refuses and leaves the bindings alone. Reinstall the dev extra +(`pip install -U -e ".[dev]"`) to get a generator at or above the pinned floor. See [docs/architecture.md](docs/architecture.md) for the details. ## Running the tests diff --git a/docs/ADR.md b/docs/ADR.md index 79f66d6..8d94ed6 100644 --- a/docs/ADR.md +++ b/docs/ADR.md @@ -90,3 +90,23 @@ Records are append-only once `Decided` and built upon: a shipped decision is nev - Finding the MODE wire path becomes a prerequisite of M3's device-settings Epic, not of M1. **No tempo surface ships at M1**, so nothing in this record is user-visible yet. - Design principle 3 keeps its meaning and gains a boundary: omission is for behaviour we do not understand, refusal is for behaviour we understand and cannot yet drive. A record that says which one applies is now expected of anything the model leaves out. - This does not license modelling controls on a hunch. It applies where the unit's behaviour is confirmed and only the message is missing; a control we have not understood on the hardware is still omitted. + +## ADR-0008: The generator floor joins the bindings/pin unit, with a gate at regeneration and a CI check on the pin + +- **Status:** Decided (2026-08-12) +- **Decision:** The `grpcio-tools` floor in the dev extra is part of what ADR-0001 calls one unit, alongside the committed bindings and the `protobuf` runtime pin. `scripts/compile_protos.sh` refuses to write bindings whose gencode is older than the ones already in the tree, and `tests/test_packaging.py` asserts on every PR that the committed gencode and the pin floor are the same number. +- **Context:** ADR-0001 couples the bindings to the runtime pin, but nothing enforced the coupling. The protobuf runtime validates `runtime >= gencode` and nothing else, so bindings written by an older generator import cleanly and pass the whole suite. The generator is `grpcio-tools`, which carries its own protoc, so the installed version silently decides the gencode. With the floor at `>=1.68`, `pip install -e ".[dev]"` resolved at its lowest to grpcio-tools 1.82.1, whose protoc emits gencode 7.35.0 against committed bindings at 7.35.1 - one patch backwards, no failure anywhere. Older generators fall much further (1.68.0 emits 5.28.1) and are reachable through a venv that acquired `grpcio-tools` separately, or through the script's fallback to a system `protoc`, which no floor constrains at all. +- **Options:** + - **(a) Gate in the script and prove the committed state in a test** - chosen. Two guards, two jobs. + - **(b) The script gate alone.** It never runs in CI, so nothing polices what lands on main, and it sees only gencode that arrives through the script - not a hand edit, an IDE-run protoc, or a pin bumped without regenerating. + - **(c) The test alone.** It fires after the bindings are already overwritten, and it cannot say "your generator is too old" because the offline suite has no generator to ask. + - **(d) A test that runs the installed generator and compares.** It would couple the offline suite to `grpcio-tools` and fail for contributors who never regenerate, which teaches people to ignore it. + - **(e) Pin `grpcio-tools` exactly.** Over-constrains every dev environment for one file's sake, and still proves nothing about the committed tree. +- **Open Questions:** None. +- **Rationale:** Prevention and detection are different jobs and neither covers the other. The script is the only place that can stop the downgrade before it reaches the tree, and it is where the mistake is actually made, so that is where the explanation belongs. The test is the only guard that runs on every PR, needs no toolchain, and holds for gencode that arrived by any route at all. The floor belongs in the same commit as the pin for the same reason the pin belongs with the bindings: all three describe one generated artifact, and the one that is easiest to forget is the one nothing was watching. +- **Consequences:** + - The dev extra's floor moves with the gencode. It is `grpcio-tools>=1.83.0` today, for bindings at gencode 7.35.1. + - The floor cannot be read off `grpcio-tools` metadata: 1.82.1 declares `protobuf>=7.35.1` and still emits gencode 7.35.0. Finding the right floor means running candidate versions and reading the stamp they write. + - `compile_protos.sh` generates into a temporary directory and installs into the package only after the check passes, so a refusal leaves the tree exactly as it was. + - The pin floor and the committed gencode must be equal, not merely compatible. A floor above the gencode still imports for every user, which is precisely the drift ADR-0001 exists to prevent, so the test treats it as a failure rather than a curiosity. + - What CI proves is the bindings-to-pin half. Nothing machine-checks the `grpcio-tools` floor itself, because deciding whether a floor is high enough means running that generator, and the offline suite has none. A gencode bump that updates the bindings and the pin but forgets the floor therefore leaves CI green. The script is what catches it, one regeneration later, and its refusal names the stale floor as a cause - so the residual exposure is a delay, not a silent pass. diff --git a/docs/STEERING.md b/docs/STEERING.md index ca4d799..4c3f2f9 100644 --- a/docs/STEERING.md +++ b/docs/STEERING.md @@ -46,7 +46,7 @@ The protocol layer is stateless between calls: every read is a live exchange, an - `tests/` - the fully offline suite and its fixtures - `examples/` - runnable scripts, also used as hardware-verification shapes - `docs/` - protocol record, architecture, coverage, this file -- `scripts/` - `compile_protos.sh` +- `scripts/` - `compile_protos.sh`, `check_artifacts.py`, `generate_models.py` - `.github/workflows/` - CI ## 5. Patterns in Use @@ -61,7 +61,7 @@ The protocol layer is stateless between calls: every read is a live exchange, an ## 6. Constraints - **Runtime dependencies are exactly `hid` and `protobuf`.** The wheel installs with no compiler, no protoc, no build step. -- **The protobuf runtime pin is coupled to the committed gencode.** The runtime validates `runtime >= gencode` at import time; a mismatch is a hard `ImportError` for every user. Currently gencode 7.35.1, pinned `>=7.35.1,<8` (see ADR-0001). +- **The protobuf runtime pin is coupled to the committed gencode, and so is the generator floor.** The runtime validates `runtime >= gencode` at import time; a mismatch is a hard `ImportError` for every user. Currently gencode 7.35.1, pinned `>=7.35.1,<8` (see ADR-0001). The generator is `grpcio-tools`, which carries its own protoc and so decides the gencode by which version is installed, hence the `grpcio-tools>=1.83.0` floor in the dev extra. Older gencode still imports, so both guards are explicit: `scripts/compile_protos.sh` refuses to write a downgrade, and `tests/test_packaging.py` proves the committed gencode and the pin floor are the same number (see ADR-0008). - **Python >= 3.11.** - **The default test suite runs fully offline.** No test imports `hid`, touches hardware, or needs `DYLD_LIBRARY_PATH`; CI runs the real suite on plain runners for every PR (see ADR-0002). A separate hardware-in-the-loop suite - state-neutral on success, best-effort restore on failure, never run in CI - lives in `tests/hardware/` and runs only under `pytest --hardware` (see ADR-0005). Its modules must stay import-safe offline: `tests/test_scene_echo_predicates.py` imports `tests/hardware/test_write_echo.py` to exercise its predicates with no unit attached, which is the only way a predicate that can never match gets caught cheaply. - **Wire baseline: CorOS / Cortex Control 4.0.1, firmware d14e.** The protocol is unversioned, so no behavior is guaranteed across firmware updates; [`architecture.md`](architecture.md) has the re-verification checklist. @@ -80,6 +80,7 @@ Decisions for this area are recorded in [`ADR.md`](ADR.md): | ADR-0005 | A hardware-in-the-loop integration suite, state-neutral on success | | ADR-0006 | The domain model takes the top-level namespace; the protocol layer moves to `pyquadcortex.protocol` | | ADR-0007 | The model may represent a control whose wire path is still open | +| ADR-0008 | The generator floor joins the bindings/pin unit, with a gate at regeneration and a CI check on the pin | ## 8. Open Questions @@ -119,6 +120,25 @@ Single-device, single-connection USB HID at interactive rates (129-byte reports) ## Change Log +### 2026-08-12 - The generator floor joins the bindings/pin unit (ADR-0008) + +**What changed:** +- The dev extra's `grpcio-tools` floor went from `>=1.68` to `>=1.83.0`, with the reason written next to it. `grpcio-tools` ships its own protoc, so the installed version decides the gencode stamped into the committed bindings. The old floor let `pip install -e ".[dev]"` resolve to 1.82.1, which emits gencode 7.35.0 against bindings committed at 7.35.1; the script's system-`protoc` fallback has no floor at all +- `scripts/compile_protos.sh` now generates into a temporary directory, compares the gencode it produced against the committed one, and refuses to install a downgrade. On refusal the tree is untouched +- `tests/test_packaging.py` proves the committed state on every PR: all bindings from one generator, the pin floor equal to the committed gencode, the pin's ceiling one major above it +- ADR.md: ADR-0008. Section 6's pin constraint says the floor is part of the same unit, and section 4 lists the two scripts added since it was last written + +**Why:** +- Found while working the PR #17 review. ADR-0001 makes the bindings and the pin one unit, but nothing enforced it: protobuf validates `runtime >= gencode` and nothing else, so bindings regenerated by an older generator import cleanly and pass the whole suite while walking the pin backwards +- The floor is not derivable from package metadata. `grpcio-tools` 1.82.1 declares `protobuf>=7.35.1` and still emits gencode 7.35.0, so 1.83.0 was found by running each candidate and reading the stamp it writes + +**Scope of impact:** +- **Updated:** `pyproject.toml`, `scripts/compile_protos.sh`, `tests/test_packaging.py`, ADR.md, STEERING.md, CLAUDE.md, architecture.md, contributing.md, changelog.md +- **Not updated (intentionally):** the bindings themselves - regenerating is its own change with its own pin bump (ADR-0001), and this one deliberately leaves the generated files byte-identical + +**Downstream to consider:** +- The floor now moves with every gencode bump. `compile_protos.sh` prints the number to put in the pin when the gencode moves up, but the `grpcio-tools` floor is the maintainer's to raise + ### 2026-08-11 - The namespace flip lands, and ADR-0007 **What changed:** diff --git a/docs/architecture.md b/docs/architecture.md index fe1bbc0..6de70da 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -347,7 +347,9 @@ scripts/compile_protos.sh It prefers the version-matched generator from the dev extra (`grpcio-tools`, hence `.venv/bin/python -m grpc_tools.protoc`) and falls back -to a system `protoc`. Output goes to `pyquadcortex/protocol/proto/`. +to a system `protoc`. It generates into a temporary directory first and copies +into `pyquadcortex/protocol/proto/` only after the gencode check below passes, +so a refusal leaves the tree untouched. **The runtime pin must match the gencode version.** The protobuf runtime validates at import time that `runtime >= gencode` (see the @@ -358,8 +360,34 @@ a newer generator, bump that lower bound to the new gencode version in the same commit; if you cross a major version, bump the upper bound too. A mismatch is a hard `ImportError` for every user, not a warning. -Commit regenerated bindings together with the `.proto` change and the pyproject -pin, so the tree is never internally inconsistent. +**The generator floor moves with it.** `grpcio-tools` bundles its own protoc, so +whichever version is installed is what decides the gencode. That makes an *older* +generator the quiet failure: `runtime >= gencode` is still satisfied, so bindings +regenerated backwards import fine and pass every test while the pin no longer +describes them. `pyproject.toml`'s dev extra therefore floors `grpcio-tools` at +the oldest release whose protoc emits the committed gencode - `>=1.83.0` for +gencode 7.35.1 - and that floor is raised in the same commit as any gencode bump. + +The floor cannot be read off package metadata. `grpcio-tools` releases do not +track `protobuf` releases, and the declared dependency is a runtime floor rather +than the gencode stamp: 1.82.1 requires `protobuf>=7.35.1` and still emits +gencode 7.35.0. Find the floor by running candidates and reading the stamp: + +```bash +printf 'syntax = "proto3";\nmessage Ping { int32 n = 1; }\n' > /tmp/ping.proto +python -m grpc_tools.protoc -I /tmp --python_out=/tmp /tmp/ping.proto +grep "Protobuf Python Version" /tmp/ping_pb2.py +``` + +Two guards keep this honest, and they cover different routes (ADR-0008): + +| Guard | Catches | When | +|---|---|---| +| `scripts/compile_protos.sh` | a generator that would write older gencode than what is committed - it refuses and writes nothing | at regeneration, before the tree changes | +| `tests/test_packaging.py` | committed gencode that disagrees with itself or with the pin, however it got there | every PR, no protoc needed | + +Commit regenerated bindings together with the `.proto` change, the pyproject +pin and the generator floor, so the tree is never internally inconsistent. ## Testing philosophy diff --git a/pyproject.toml b/pyproject.toml index ad27842..7381d20 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,7 +36,16 @@ dependencies = [ ] [project.optional-dependencies] -dev = ["pytest>=8", "grpcio-tools>=1.68"] +# grpcio-tools carries its own protoc, so the version installed here is what +# decides the gencode stamped into the committed bindings. The floor is the +# oldest release whose protoc emits gencode 7.35.1, matching the bindings in +# the tree: an older one regenerates them BACKWARDS and still imports, because +# protobuf only checks runtime >= gencode (ADR-0001). Raise this in the same +# commit as any gencode bump. +# Ask the compiler, never the metadata: grpcio-tools versions do not track +# protobuf ones, and 1.82.1 declares `protobuf>=7.35.1` while still emitting +# gencode 7.35.0. +dev = ["pytest>=8", "grpcio-tools>=1.83.0"] [project.scripts] qcctl = "pyquadcortex.protocol.cli:main" diff --git a/scripts/compile_protos.sh b/scripts/compile_protos.sh index 712c41e..4b099e3 100755 --- a/scripts/compile_protos.sh +++ b/scripts/compile_protos.sh @@ -18,14 +18,117 @@ if [ ! -f "$OUT/__init__.py" ]; then echo " a fresh one is missing the sys.path shim the bindings need." >&2 exit 1 fi + +# Generate into a scratch directory, not straight into OUT. The gencode version +# is a property of the generator and is only knowable by reading its output, so +# the downgrade check below has to happen after protoc runs but before the +# committed bindings are overwritten. Staging is what buys that gap. +STAGE="$(mktemp -d)" +trap 'rm -rf "$STAGE"' EXIT + # Prefer the version-matched generator from the venv dev extra (grpcio-tools); # fall back to system protoc for environments without the venv. if [ -x "$HERE/.venv/bin/python" ]; then - "$HERE/.venv/bin/python" -m grpc_tools.protoc -I "$PROTO_SRC" --python_out="$OUT" Preset.proto ProductionAutomation.proto + "$HERE/.venv/bin/python" -m grpc_tools.protoc -I "$PROTO_SRC" --python_out="$STAGE" Preset.proto ProductionAutomation.proto else - protoc -I "$PROTO_SRC" --python_out="$OUT" Preset.proto ProductionAutomation.proto + protoc -I "$PROTO_SRC" --python_out="$STAGE" Preset.proto ProductionAutomation.proto +fi + +# protoc exiting 0 having written nothing would otherwise reach the gate as an +# unexpanded glob, sail through it with nothing to compare, and die at the `cp` +# with a bare "No such file or directory". Name the actual problem here. +if [ ! -f "$STAGE/Preset_pb2.py" ]; then + echo "error: the generator exited 0 but wrote no *_pb2.py into $STAGE." >&2 + echo " Check whether the .proto files grew a \`package\` statement:" >&2 + echo " protoc then writes into a subdirectory named after it, and this" >&2 + echo " script (and the package's flat import layout) expect neither." >&2 + exit 1 +fi + +# Every generated file carries the version of the generator that wrote it: +# # Protobuf Python Version: 7.35.1 +# Both helpers below are single awk processes on purpose. The obvious spellings +# pipe into `head -1`, and under `set -o pipefail` a producer that gets SIGPIPE +# when head exits early fails the whole pipeline - which, inside `$(...)`, comes +# back as an empty string and reads as "not older". A gate that fails open is +# worse than no gate. +gencode_of() { + [ -f "$1" ] || return 0 + awk '/^# Protobuf Python Version: /{print $NF; exit}' "$1" +} + +# 0 when $1 is strictly older than $2, comparing dot-separated fields +# numerically. Absent fields count as 0, so 7.35 is older than 7.35.1. +older_than() { + awk -v a="$1" -v b="$2" 'BEGIN { + fields = split(a, left, ".") + if (split(b, right, ".") > fields) fields = split(b, right, ".") + for (i = 1; i <= fields; i++) { + if (left[i] + 0 < right[i] + 0) exit 0 + if (left[i] + 0 > right[i] + 0) exit 1 + } + exit 1 + }' +} + +# THE GATE. protobuf only validates `runtime >= gencode`, so bindings written by +# an OLDER generator import perfectly and pass every test - they just quietly +# walk the pin backwards, which is the one thing ADR-0001 exists to prevent. +# Refuse rather than write. Whoever ran this wanted new bindings, so leaving the +# old ones in place is not silent either: they get this message instead. +DOWNGRADES="" +MOVED="" +for staged in "$STAGE"/*_pb2.py; do + name="$(basename "$staged")" + new="$(gencode_of "$staged")" + # No file in the tree yet means a binding that is new in this change. There is + # nothing to compare it against, and the pin check in tests/test_packaging.py + # covers it once it lands. + [ -f "$OUT/$name" ] || continue + # "Missing" and "present but unstamped" are NOT the same answer, and reading + # them as one is how this gate would let the worst case through: bindings from + # a pre-stamp protoc carry no version line at all, so treating that as "new + # file, nothing to compare" would wave in any generator at all. Refuse and say + # so. Deleting the file is the deliberate way to say "yes, replace this". + existing="$(gencode_of "$OUT/$name")" + if [ -z "$existing" ]; then + DOWNGRADES="$DOWNGRADES + $name: the copy in the tree carries no version stamp, so nothing here can + tell whether $new replaces it or downgrades it. Delete it and re-run if + replacing it is what you mean." + elif [ -z "$new" ]; then + DOWNGRADES="$DOWNGRADES + $name: tree has $existing, this generator stamps no version at all" + elif older_than "$new" "$existing"; then + DOWNGRADES="$DOWNGRADES + $name: tree has $existing, this generator emits $new" + elif [ "$new" != "$existing" ]; then + MOVED="$new" + fi +done + +if [ -n "$DOWNGRADES" ]; then + # "the tree", not "committed": the baseline is the file on disk in OUT, which + # is not necessarily what is in HEAD. + echo "error: this generator would DOWNGRADE the gencode in the tree.$DOWNGRADES" >&2 + echo "" >&2 + echo " Nothing was written. Older gencode still imports - protobuf only" >&2 + echo " checks runtime >= gencode - so this would pass the whole suite" >&2 + echo " while breaking the ADR-0001 coupling between the bindings and" >&2 + echo " the protobuf pin." >&2 + echo "" >&2 + echo " grpcio-tools carries its own protoc, so the fix is a newer one:" >&2 + echo " pip install -U -e '.[dev]' # or: uv pip install -U -e '.[dev]'" >&2 + echo "" >&2 + echo " If that already gives the version above, then the grpcio-tools" >&2 + echo " floor in pyproject.toml is stale - find the release whose protoc" >&2 + echo " emits the gencode above and raise the floor to it." >&2 + exit 1 fi + +cp "$STAGE"/*_pb2.py "$OUT/" echo "Generated bindings in $OUT" + # "Nothing changed" is a result worth seeing rather than assuming: it means the # schema edit did not reach the bindings, or protoc wrote somewhere else. if git -C "$HERE" rev-parse --git-dir >/dev/null 2>&1; then @@ -34,7 +137,14 @@ if git -C "$HERE" rev-parse --git-dir >/dev/null 2>&1; then echo "note: the bindings are byte-identical to what was already committed." else echo "$CHANGED" - echo "note: bump the protobuf pin in pyproject.toml in this same commit" - echo " (ADR-0001: the gencode and the runtime pin are one unit)." + if [ -n "$MOVED" ]; then + echo "note: the gencode moved UP to $MOVED. Set protobuf>=$MOVED in" + echo " pyproject.toml in this same commit, and raise the grpcio-tools" + echo " floor to the release you just used (ADR-0001: the gencode, the" + echo " runtime pin and the generator floor are one unit)." + else + echo "note: bump the protobuf pin in pyproject.toml in this same commit" + echo " (ADR-0001: the gencode and the runtime pin are one unit)." + fi fi fi diff --git a/tests/test_packaging.py b/tests/test_packaging.py index 53422c2..a0310ca 100644 --- a/tests/test_packaging.py +++ b/tests/test_packaging.py @@ -6,12 +6,18 @@ """ import importlib import pathlib +import re import subprocess import sys import tomllib ROOT = pathlib.Path(__file__).resolve().parent.parent PYPROJECT = tomllib.loads((ROOT / "pyproject.toml").read_text()) +BINDINGS = ROOT / "pyquadcortex" / "protocol" / "proto" + +#: Every generated file names the generator that wrote it, on a line reading +#: `# Protobuf Python Version: 7.35.1`. +GENCODE_STAMP = re.compile(r"^# Protobuf Python Version: *(\S+)", re.MULTILINE) def test_the_console_script_is_still_qcctl(): @@ -84,3 +90,94 @@ def test_the_generated_bindings_are_where_the_package_imports_them_from(): proto = ROOT / "pyquadcortex" / "protocol" / "proto" for name in ("__init__.py", "Preset_pb2.py", "ProductionAutomation_pb2.py"): assert (proto / name).is_file(), f"{name} is missing from {proto}" + + +def _committed_gencode() -> dict[str, str]: + """The generator version stamped into each committed binding.""" + stamps = {} + for path in sorted(BINDINGS.glob("*_pb2.py")): + found = GENCODE_STAMP.search(path.read_text()) + assert found, ( + f"{path.name} carries no `# Protobuf Python Version:` line, so " + f"nothing here can tell which generator wrote it") + stamps[path.name] = found.group(1) + assert stamps, f"no generated bindings found in {BINDINGS}" + return stamps + + +def _protobuf_pin() -> str: + """The `protobuf` requirement string from pyproject's runtime deps.""" + for requirement in PYPROJECT["project"]["dependencies"]: + # Anchored on a version operator, not a word boundary: `protobuf-stubs` + # is a real package name and `protobuf\b` would happily match it. + if re.match(r"protobuf\s*[<>=!~]", requirement): + return requirement + raise AssertionError("pyproject no longer depends on protobuf at all") + + +def _bound(pin: str, operator: str) -> str | None: + """The version in `pin`'s `operator` clause, e.g. `>=` -> "7.35.1".""" + for clause in pin.split(","): + clause = clause.strip().removeprefix("protobuf").strip() + if clause.startswith(operator): + return clause[len(operator):].strip() + return None + + +def _the_gencode() -> str: + """The one gencode version the committed bindings agree on. + + Both files come out of the same protoc run, so two different stamps mean one + was regenerated on its own - and since the sibling import ties them + together, the descriptors they build are no longer known to agree. + """ + stamps = _committed_gencode() + assert len(set(stamps.values())) == 1, ( + f"the committed bindings carry different gencode versions: {stamps}. " + f"Regenerate them together with scripts/compile_protos.sh") + return next(iter(stamps.values())) + + +def test_all_the_committed_bindings_came_from_one_generator(): + assert _the_gencode() + + +def test_the_protobuf_pin_floor_is_exactly_the_committed_gencode(): + """ADR-0001's whole claim: the bindings and the pin are one unit. + + Nothing at runtime enforces this. protobuf validates `runtime >= gencode` + and nothing else, so both ways of drifting stay quiet until they reach a + user, and regenerating with an older generator is the easy accident: + `scripts/compile_protos.sh` refuses that one, and this catches gencode that + arrived by any other route, on every PR, with no protoc installed. + """ + gencode = _the_gencode() + pin = _protobuf_pin() + floor = _bound(pin, ">=") + assert floor is not None, f"the protobuf pin {pin!r} has no `>=` lower bound" + assert floor == gencode, ( + f"pyproject pins protobuf>={floor} but the committed bindings are " + f"gencode {gencode}.\n" + f" floor below gencode: every user who installs protobuf=={floor} " + f"gets a hard ImportError.\n" + f" floor above gencode: the bindings were regenerated by an older " + f"generator. That still imports, which is exactly why it needs " + f"catching here.\n" + f"Move whichever one is wrong, in this commit (ADR-0001).") + + +def test_the_protobuf_pin_stops_below_the_next_gencode_major(): + """A major bump is the case where an unchanged pin does reach users. + + protobuf gencode is only guaranteed against a runtime of the same major, so + an upper bound left behind a major-crossing regeneration lets pip resolve a + runtime that cannot load the bindings at all. + """ + gencode = _the_gencode() + pin = _protobuf_pin() + ceiling = _bound(pin, "<") + assert ceiling is not None, f"the protobuf pin {pin!r} has no `<` upper bound" + expected = str(int(gencode.split(".")[0]) + 1) + assert ceiling.split(".")[0] == expected, ( + f"the committed bindings are gencode {gencode}, so the pin should stop " + f"below protobuf {expected}, not {ceiling}")