diff --git a/AGENTS.md b/AGENTS.md index 19da985d..d496081d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -70,7 +70,9 @@ When adding the first vendored patch to a previously-rolling engine: pin in the **Delivery model (vLLM):** patches reach the container by **volume-mounting into the pinned *stock* `vllm/vllm-openai` image** (python sidecars / site-package overlays / install scripts — see `delivery_mechanism` in `scripts/lib/profiles/patches.yml`), **not** by baking a custom image. The older baked-image path (`ghcr.io/noonghunna/vllm-club3090`, which shipped the release images through `club-v0.8.3`) is **retired** — no compose or engine-pin references it, and the `dockerfile_bake` `delivery:` block in `patches.yml` is legacy/test-only. The GHCR package is kept as historical release artifacts (users pinned to a `club-v0.8.x` tag can still pull); it is not deleted and not produced by anything in-repo. ### File encoding in scripts — always `encoding="utf-8"` -Any Python read of a repo source file (compose YAML, profile YAML, `baselines.yml`, …) — including python heredocs inside shell scripts — MUST pass `encoding="utf-8"`. `Path.read_text()` / `open()` default to the **locale** encoding, and community rigs run non-UTF-8 locales (minimal VMs / containers with `LC_ALL=C` → `ANSI_X3.4-1968`). Repo files are full of unicode (`— × → ⚠` in compose headers), so a bare read that works on the dev machine crashes `switch.sh`/`launch.sh` on those rigs (#599). Two corollaries: +Any Python read of a repo source file (compose YAML, profile YAML, `baselines.yml`, …) — including python heredocs inside shell scripts — MUST pass `encoding="utf-8"`. `Path.read_text()` / `open()` default to the **locale** encoding, and community rigs run non-UTF-8 locales (minimal VMs / containers with `LC_ALL=C` → `ANSI_X3.4-1968`). Repo files are full of unicode (`— × → ⚠` in compose headers), so a bare read that works on the dev machine crashes `switch.sh`/`launch.sh` on those rigs (#599). Corollaries: +- **Writes too:** a *piped* stdout under the C locale defaults to ASCII — printing a status note with unicode raises `UnicodeEncodeError`. Python emit blocks on launcher paths pin it with `sys.stdout.reconfigure(encoding="utf-8")`. +- **The launcher table path is python-STDLIB-ONLY** — no PyYAML, no pip deps (community VMs ship bare python3; #584's `ModuleNotFoundError: yaml`). The `--json` contract path may require PyYAML but must fail with a `Fix:` hint, not a traceback. `test-registry-emit-no-yaml` guards both plus the locale cases. - **Repro before claiming fixed:** modern Python coerces `C` → `C.UTF-8`, so plain `LC_ALL=C` won't reproduce — use `PYTHONUTF8=0 PYTHONCOERCECLOCALE=0 LC_ALL=C`. - **Don't blind-`2>/dev/null` launcher derive paths** — swallowing the traceback hid this exact class for months; capture stderr and surface it on failure instead. diff --git a/scripts/lib/registry-emit.sh b/scripts/lib/registry-emit.sh index e11cec12..a8c42822 100755 --- a/scripts/lib/registry-emit.sh +++ b/scripts/lib/registry-emit.sh @@ -9,11 +9,33 @@ registry_variant_rows() { python3 - "$root" <<'PY_EMIT' from __future__ import annotations +import os import re import sys from pathlib import Path -import yaml +# PyYAML is OPTIONAL on this path (the switch.sh/launch.sh table derivation): +# community rigs run minimal VMs without python3-yaml (#584 — ryan's Proxmox +# box), and the ONLY thing this block used yaml for is pulling container_name +# out of each compose — which the regex fallback below handles for our own +# compose files. The `--json` contract path (c3 cockpit / baselines join) +# still requires PyYAML. CLUB3090_EMIT_NO_YAML=1 forces the fallback so the +# no-yaml guard test can exercise it on rigs where yaml IS installed. +try: + import yaml +except Exception: + yaml = None +if os.environ.get("CLUB3090_EMIT_NO_YAML") == "1": + yaml = None + +# Non-UTF-8 locales (LC_ALL=C VMs, #599/#584) also break the WRITE side: a +# piped stdout defaults to the locale codec → UnicodeEncodeError printing the +# unicode in status notes. Reads were fixed in #599; pin the output too. +for _stream in (sys.stdout, sys.stderr): + try: + _stream.reconfigure(encoding="utf-8") + except Exception: + pass root = Path(sys.argv[1]) sys.path.insert(0, str(root)) @@ -35,8 +57,27 @@ def switch_engine(key: str) -> str: return "llamacpp" if prefix == "llamacpp" else prefix +# First non-comment `container_name:` line — the regex fallback for rigs +# without PyYAML. Our compose files are single-service (or first-service-wins, +# matching the yaml path's dict-order iteration), so this is equivalent for +# every checked-in compose; test-registry-emit-no-yaml asserts that parity. +_CONTAINER_RX = re.compile(r"""^\s*container_name:\s*(['"]?)(.+?)\1\s*$""", re.M) + + +def _unwrap_env_default(raw: str) -> str: + match = re.fullmatch(r"\$\{[^}:]+:-(.+)\}", raw) + return match.group(1) if match else raw + + def container_name(compose_path: str) -> str: path = root / compose_path + if yaml is None: + try: + text = path.read_text(encoding="utf-8") + except Exception as exc: + raise RuntimeError(f"could not read compose yaml: {exc}") from exc + m = _CONTAINER_RX.search(text) + return _unwrap_env_default(m.group(2).strip()) if m else "" try: data = yaml.safe_load(path.read_text(encoding="utf-8")) or {} except Exception as exc: @@ -46,9 +87,7 @@ def container_name(compose_path: str) -> str: raw = service.get("container_name") if not raw: continue - raw = str(raw) - match = re.fullmatch(r"\$\{[^}:]+:-(.+)\}", raw) - return match.group(1) if match else raw + return _unwrap_env_default(str(raw)) return "" @@ -390,6 +429,28 @@ import os import sys from pathlib import Path +# PyYAML is REQUIRED on the --json contract path (profiles + baselines join — +# load_profiles() below imports it too, so check FIRST and fail with the fix, +# not a bare ModuleNotFoundError traceback). The switch.sh/launch.sh table +# path runs stdlib-only (regex container_name fallback, #584) — only +# c3-cockpit consumers need PyYAML. +try: + import yaml # noqa: E402,F401 +except Exception: + print( + "registry-emit --json requires PyYAML (profiles/baselines join).\n" + "Fix: sudo apt install python3-yaml (or: pip install pyyaml)", + file=sys.stderr, + ) + raise SystemExit(3) + +# Pin output to UTF-8 regardless of locale (see the table-path note, #599/#584). +for _stream in (sys.stdout, sys.stderr): + try: + _stream.reconfigure(encoding="utf-8") + except Exception: + pass + root = Path(sys.argv[1]) sys.path.insert(0, str(root)) @@ -421,7 +482,7 @@ profiles = load_profiles() # BENCHMARKS.md) directly. --- import re as _re # noqa: E402 -import yaml as _yaml # noqa: E402 +_yaml = yaml # required-import guard at the top of this block (#584) _bl_path = root / "scripts" / "lib" / "profiles" / "baselines.yml" _baselines = {} diff --git a/scripts/tests/test-registry-emit-no-yaml.sh b/scripts/tests/test-registry-emit-no-yaml.sh new file mode 100755 index 00000000..455d39ce --- /dev/null +++ b/scripts/tests/test-registry-emit-no-yaml.sh @@ -0,0 +1,58 @@ +#!/usr/bin/env bash +# test-registry-emit-no-yaml.sh — the switch.sh/launch.sh table derivation must +# be python-STDLIB-ONLY and locale-proof (#584, ryan's Proxmox VM): +# +# 1. PyYAML absent → the regex container_name fallback must produce output +# BYTE-IDENTICAL to the yaml parse (CLUB3090_EMIT_NO_YAML=1 forces the +# fallback on rigs where yaml IS installed — i.e. CI and this rig). +# 2. Non-UTF-8 locale (LC_ALL=C + PYTHONUTF8=0 + PYTHONCOERCECLOCALE=0 — the +# #599 repro recipe; modern python silently coerces C→C.UTF-8 without the +# extra vars) → reads AND writes must still work: compose headers carry +# unicode, and a piped stdout under the C locale would otherwise +# UnicodeEncodeError printing status notes. +# 3. Both at once — the actual community-rig configuration that broke. +# +# The --json contract path (c3 / baselines join) legitimately requires PyYAML +# and must fail with an actionable Fix: line, not a bare traceback. +set -euo pipefail +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +# shellcheck source=../lib/registry-emit.sh +source "$ROOT/scripts/lib/registry-emit.sh" + +fail() { echo "FAIL: $*" >&2; exit 1; } + +base="$(registry_variant_rows "$ROOT")" || fail "baseline emit errored" +[[ -n "$base" ]] || fail "baseline emit produced no rows" + +# 1 — yaml-less parity +noyaml="$(CLUB3090_EMIT_NO_YAML=1 registry_variant_rows "$ROOT")" \ + || fail "yaml-less emit errored (regex container_name fallback broken)" +diff <(printf '%s\n' "$base") <(printf '%s\n' "$noyaml") >/dev/null \ + || fail "yaml-less output drifts from the yaml parse (container_name regex fallback wrong for some compose)" + +# 2 — C-locale (reads #599 + writes) +clocale="$(LC_ALL=C PYTHONUTF8=0 PYTHONCOERCECLOCALE=0 registry_variant_rows "$ROOT")" \ + || fail "C-locale emit errored (encoding regression — reads need encoding=utf-8, writes need stdout reconfigure)" +diff <(printf '%s\n' "$base") <(printf '%s\n' "$clocale") >/dev/null \ + || fail "C-locale output drifts from the UTF-8 run" + +# 3 — the ryan-rig combination: no PyYAML + ASCII locale +ryan="$(CLUB3090_EMIT_NO_YAML=1 LC_ALL=C PYTHONUTF8=0 PYTHONCOERCECLOCALE=0 registry_variant_rows "$ROOT")" \ + || fail "no-yaml + C-locale emit errored (the #584 community-rig configuration)" +diff <(printf '%s\n' "$base") <(printf '%s\n' "$ryan") >/dev/null \ + || fail "no-yaml + C-locale output drifts" + +# 4 — the --json path must refuse WITHOUT yaml via an actionable message. +# Simulate by hiding yaml with a poisoned import stub on PYTHONPATH. +stubdir="$(mktemp -d)" +trap 'rm -rf "$stubdir"' EXIT +printf 'raise ImportError("PyYAML hidden by test-registry-emit-no-yaml")\n' > "$stubdir/yaml.py" +set +e +json_err="$(PYTHONPATH="$stubdir" bash "$ROOT/scripts/lib/registry-emit.sh" --json 2>&1 >/dev/null)" +json_rc=$? +set -e +[[ $json_rc -ne 0 ]] || fail "--json without PyYAML should exit non-zero" +grep -q "requires PyYAML" <<<"$json_err" || fail "--json no-yaml error lacks the actionable message (got: ${json_err:0:200})" +grep -q "Fix:" <<<"$json_err" || fail "--json no-yaml error lacks a Fix: hint" + +echo "PASS: launcher table path is stdlib-only + locale-proof; --json fails actionably without PyYAML"