diff --git a/.github/workflows/platforms-mirror-check.yml b/.github/workflows/platforms-mirror-check.yml new file mode 100644 index 000000000..c643158a5 --- /dev/null +++ b/.github/workflows/platforms-mirror-check.yml @@ -0,0 +1,44 @@ +name: Platforms Mirror Check + +# `packages/ts/src/platforms.ts` is the release source of truth for the published +# `@dirsql/lib-*` sub-packages; the node distcheck flow keeps a hand-maintained +# Python subset of it. Nothing checked the correspondence, so it drifted in both +# directions -- a field on the Python side justified as mirroring a TypeScript +# property that never existed, and TypeScript-only fields with no counterpart. +# This holds the two tables to the subset on every change to either, so a new +# published target cannot be half-added. +# +# Deliberately its own workflow rather than a job in internals-distcheck-ci.yml: +# that lane's path filter does not watch packages/ts, which is exactly where the +# half-added-target case starts. + +on: + pull_request: + branches: [main] + paths: + - 'packages/ts/src/platforms.ts' + - 'internals/distcheck/src/distcheck/node_flow/platforms.py' + - 'internals/checks/src/checks/platforms_mirror/**' + - '.github/workflows/platforms-mirror-check.yml' + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + platforms-mirror: + name: Platforms mirror + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - name: Install uv + uses: astral-sh/setup-uv@v7 + with: + python-version: "3.13" + + - name: Assert the two platform tables agree on the fields they share + run: uv run --project internals/checks dirsql-checks platforms-mirror diff --git a/docs/howto/search-by-meaning.md b/docs/howto/search-by-meaning.md index 1d5e15885..c62c0a4f5 100644 --- a/docs/howto/search-by-meaning.md +++ b/docs/howto/search-by-meaning.md @@ -3,10 +3,10 @@ Ask a question in plain language and get the closest documents back — even when they share no keywords with it. Install [`dirsql-plugin-embeddings`](../plugins.md#dirsql-plugin-embeddings) and -semantic search becomes plain SQL: the plugin's `embed()` function turns -text into vectors, [`sqlite-vec`](https://github.com/asg017/sqlite-vec)'s -`vec_distance_cosine()` measures distance, and `ORDER BY … LIMIT` does the -ranking. No config, no API keys, no services — the model runs locally. +semantic search becomes plain SQL: the plugin's `embed()` function turns text +into vectors and loads [`sqlite-vec`](https://github.com/asg017/sqlite-vec), +which does the distance math. No API keys, no services — the model runs +locally. Suppose short notes live in `notes/*.md`: @@ -40,7 +40,32 @@ The first-ever run downloads the model (on the order of a hundred megabytes, with progress on stderr); after that it loads from the local cache. Results print one `pathdistance` line per match, closest first. -## The SQL behind it +## Which shape + +The one-liner embeds every matched file on every run. That is the right trade +for a question you ask once, and the wrong one for a corpus you search +repeatedly — where a stored [`vec0` index](./search-indexes.md#vector-search-vec0) +embeds each file once, at ingest, and a query embeds only the question: + +| | This page: path-table | [Vector index](./search-indexes.md#vector-search-vec0) | +|---|---|---| +| Setup | none | a `[[table]]` with a `ddl` batch | +| Freshness | always current — the walk is the read | watcher-maintained; survives restarts under [`--persist`](./persist.md) | +| Per query | one `embed()` round trip **per matched file**, plus a walk that reads every file's content | one `embed()` round trip **total**, plus an in-process KNN scan | +| Top-k | `ORDER BY … LIMIT k` | `MATCH … AND k = …` | +| Good for | a one-off question, a small corpus, an ad-hoc glob | a corpus you query repeatedly | + +The index only pays off when the table outlives the query — under `--persist`, +or inside a long-running [`dirsql server`](../reference/cli.md). A one-shot +`dirsql query` against an ephemeral index rebuilds the table, and therefore +re-embeds the corpus, before it answers; that is strictly more work than the +subquery below. The full recipe — the width probe, the `ddl` batch, both +triggers, and what a model-id edit costs — is +[Add a search index to a table](./search-indexes.md#vector-search-vec0). + +The rest of this page is the zero-setup shape. + +## The SQL behind the one-liner The one-liner generates and runs ordinary `dirsql` SQL, and you can write it yourself when you want more than ranked paths — a different projection, a @@ -79,7 +104,9 @@ Reading the query inside-out: and its distance are NULL too — and SQLite sorts NULLs *first* ascending, so without this line the unrankable files take the top-k slots. 4. `vec_distance_cosine(...)` computes cosine distance between the two - vectors; `ORDER BY distance LIMIT 3` keeps the three nearest. + vectors; `ORDER BY distance LIMIT 3` keeps the three nearest. There is no + `vec0` table here, so `sqlite-vec`'s `MATCH … AND k = …` does not apply — + for a plain expression, `ORDER BY … LIMIT k` *is* its documented top-k. Structured files compose with SQL's JSON operators — embed one field instead of the whole file: @@ -93,24 +120,19 @@ ORDER BY vec_distance_cosine(emb, embed('local private models')) LIMIT 10 ``` -::: tip Top-k is `LIMIT k` -If you know `sqlite-vec` you may reach for its `MATCH … AND k = 10` idiom. -That syntax belongs to `sqlite-vec`'s `vec0` virtual table, which the table -above does not use: a `[[table]]`'s own `name` is always a per-file row table. -For plain expressions, `sqlite-vec`'s own documented pattern is exactly what -this guide uses: `ORDER BY vec_distance_cosine(...) LIMIT k`. To get the `vec0` -idiom instead, declare the `vec0` table alongside the row table in the same -[`ddl` batch](../reference/config.md#batch-ddl) and fill it from a trigger. -::: +The same projection works as an `on-file` hook feeding the indexed shape: +parse the field in the hook, store it as a column, and the trigger embeds it +once instead of on every query. ## Repeat runs are cheap Computed vectors are cached on disk, keyed on content and model ([vector cache](../plugins.md#vector-cache)) — re-running a search over -unchanged files skips the model entirely and re-embeds only what changed. -And the plugin costs nothing when idle: a query that never calls `embed()` -spawns no worker and loads no model -([zero cost when unused](../plugins.md#zero-cost-when-unused)). +unchanged files skips the model entirely and re-embeds only what changed. That +takes the *inference* out of the shape above, but not the walk or the per-file +round trip; only a stored index removes those. And the plugin costs nothing +when idle: a query that never calls `embed()` spawns no worker and loads no +model ([zero cost when unused](../plugins.md#zero-cost-when-unused)). ## How `embed()` gets into SQL diff --git a/docs/howto/search-indexes.md b/docs/howto/search-indexes.md index f3c137bac..eb165466e 100644 --- a/docs/howto/search-indexes.md +++ b/docs/howto/search-indexes.md @@ -155,8 +155,10 @@ ddl = ''' CREATE TABLE notes (slug TEXT, title TEXT, body TEXT); -- Width must equal what the probe printed for the model id named below. -CREATE VIRTUAL TABLE notes_vec USING vec0(embedding float[512]); -CREATE TRIGGER notes_vi AFTER INSERT ON notes BEGIN +CREATE VIRTUAL TABLE notes_vec + USING vec0(embedding float[512] distance_metric=cosine); +CREATE TRIGGER notes_vi AFTER INSERT ON notes +WHEN new.body IS NOT NULL BEGIN INSERT INTO notes_vec(rowid, embedding) VALUES (new.rowid, embed(new.body, 'minishlab/potion-retrieval-32M')); END; @@ -169,6 +171,29 @@ END; The delete side is an ordinary `DELETE`, not FTS5's `'delete'` command row — `vec0` is a normal-looking table that way. +Two clauses in there are load-bearing: + +- **`distance_metric=cosine`.** `vec0` defaults to L2, which is not the metric + the rest of dirsql's semantic search uses — `vec_distance_cosine()` backs + both the plugin's one-liner and + [Search documents by meaning](./search-by-meaning.md). The two agree only for + vectors of equal length, so as soon as documents embed to vectors of + different magnitude they rank differently: against three notes, L2 returned + `a, c, b` where cosine returned `a, b, c`. Declaring the metric makes + `distance` the number `vec_distance_cosine()` would compute. +- **`WHEN new.body IS NOT NULL`.** `embed(NULL)` is `NULL`, and `vec0` rejects + a NULL vector outright — so without the guard a single row whose text is + missing fails the **whole** table load, not just its own insert: + + ``` + dirsql query: failed to load config: SQLite error: Inserted vector for the + "embedding" column is invalid: Input must have type BLOB (compact format) or + TEXT (JSON), found NULL + ``` + + With it, that row still lands in `notes`; only its vector is skipped. FTS5 + needs no such guard — it indexes a NULL happily. + Query it with `sqlite-vec`'s KNN form, joined back on `rowid`: ```bash diff --git a/internals/checks/e2e-attestations/claude-issues-1003-1004-kfn8ug.json b/internals/checks/e2e-attestations/claude-issues-1003-1004-kfn8ug.json new file mode 100644 index 000000000..f9872eb2e --- /dev/null +++ b/internals/checks/e2e-attestations/claude-issues-1003-1004-kfn8ug.json @@ -0,0 +1,7 @@ +{ + "command": "uv run python -m pytest tests/e2e -x -q", + "ran_at": 1787281290, + "exit_code": 0, + "commit": "c9a387f5ccf1974a5d0153252b5487c662f80e67", + "branch": "claude/issues-1003-1004-kfn8ug" +} diff --git a/internals/checks/src/checks/cli.py b/internals/checks/src/checks/cli.py index 6afb41581..89ee15221 100644 --- a/internals/checks/src/checks/cli.py +++ b/internals/checks/src/checks/cli.py @@ -12,6 +12,7 @@ from checks.changelog_gate.cli import cli as changelog_gate from checks.declared_deps.cli import cli as declared_deps from checks.npm_binary_extension_load.cli import cli as npm_binary_extension_load +from checks.platforms_mirror.cli import cli as platforms_mirror from checks.preflight.cli import cli as preflight from checks.pytest_gate.cli import cli as pytest_gate from checks.release_globs.cli import cli as release_globs @@ -27,6 +28,7 @@ def main() -> None: main.add_command(changelog_gate, name="changelog-gate") main.add_command(declared_deps, name="declared-deps") main.add_command(npm_binary_extension_load, name="npm-binary-extension-load") +main.add_command(platforms_mirror, name="platforms-mirror") main.add_command(preflight, name="preflight") main.add_command(pytest_gate, name="pytest-gate") main.add_command(release_globs, name="release-globs") diff --git a/internals/checks/src/checks/cli_test.py b/internals/checks/src/checks/cli_test.py index 24fbb0007..6d73e99e9 100644 --- a/internals/checks/src/checks/cli_test.py +++ b/internals/checks/src/checks/cli_test.py @@ -30,6 +30,10 @@ def test_declared_deps_is_registered(): assert "declared-deps" in main.commands +def test_platforms_mirror_is_registered(): + assert "platforms-mirror" in main.commands + + def test_preflight_is_registered(): assert "preflight" in main.commands diff --git a/internals/checks/src/checks/platforms_mirror/__init__.py b/internals/checks/src/checks/platforms_mirror/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/internals/checks/src/checks/platforms_mirror/cli.py b/internals/checks/src/checks/platforms_mirror/cli.py new file mode 100644 index 000000000..e35a6189c --- /dev/null +++ b/internals/checks/src/checks/platforms_mirror/cli.py @@ -0,0 +1,29 @@ +"""The platforms-mirror check -- repo-only (#1004). + +Backs `dirsql-checks platforms-mirror`: the node distcheck flow's platform table +and the TypeScript release source of truth must agree on the fields they share. +""" + +from __future__ import annotations + +import click + +from checks.platforms_mirror.decide import PYTHON_FILE, TYPESCRIPT_FILE +from checks.platforms_mirror.gate import run + + +@click.command() +@click.option( + "--python", + "python_path", + default=PYTHON_FILE, + help="The node distcheck flow's platform table.", +) +@click.option( + "--typescript", + "typescript_path", + default=TYPESCRIPT_FILE, + help="The release source of truth for published sub-packages.", +) +def cli(python_path: str, typescript_path: str) -> None: + raise SystemExit(run(python_path, typescript_path)) diff --git a/internals/checks/src/checks/platforms_mirror/cli_test.py b/internals/checks/src/checks/platforms_mirror/cli_test.py new file mode 100644 index 000000000..9da7e0ef6 --- /dev/null +++ b/internals/checks/src/checks/platforms_mirror/cli_test.py @@ -0,0 +1,32 @@ +"""Colocated unit tests for the platforms-mirror command (isolation -- no +`CliRunner`). Driven through `.callback`; `run` is mocked at its import site. +""" + +from unittest import mock + +import pytest + +from checks.platforms_mirror.cli import cli + + +def invoke(**kwargs): + with mock.patch("checks.platforms_mirror.cli.run", **kwargs) as run: + with pytest.raises(SystemExit) as exc_info: + cli.callback(python_path="p.py", typescript_path="t.ts") + return run, exc_info.value.code + + +def test_exits_with_runs_return_code(): + run, code = invoke(return_value=0) + run.assert_called_once_with("p.py", "t.ts") + assert code == 0 + + +def test_propagates_a_drifted_mirror_as_exit_one(): + assert invoke(return_value=1)[1] == 1 + + +def test_defaults_to_the_two_committed_platform_tables(): + python_path, typescript_path = cli.params + assert python_path.default == "internals/distcheck/src/distcheck/node_flow/platforms.py" + assert typescript_path.default == "packages/ts/src/platforms.ts" diff --git a/internals/checks/src/checks/platforms_mirror/decide.py b/internals/checks/src/checks/platforms_mirror/decide.py new file mode 100644 index 000000000..0c20b1a52 --- /dev/null +++ b/internals/checks/src/checks/platforms_mirror/decide.py @@ -0,0 +1,135 @@ +"""Pure decision logic for the platforms-mirror check (#1004). + +`internals/distcheck`'s `Platform` dataclass is a hand-maintained subset of +`packages/ts/src/platforms.ts`, which is the release source of truth for the +published `@dirsql/lib-*` sub-packages. The subset framing is deliberate -- +distcheck needs only enough of each target to reconstruct the host's +sub-package and find its staged addon -- but it was asserted by a comment and +checked by nothing, and it drifted both ways: `exe` sat on the Python side +justified as mirroring TypeScript, which has never had the field. + +So the invariant here is the subset, stated as data: `SHARED` names every +Python field and where its value comes from, both tables carry the same set of +targets, and a Python field outside `SHARED` is drift by construction. +`triple` and `libc` staying TypeScript-only is the subset working as intended, +not a gap. +""" + +from __future__ import annotations + +PYTHON_FILE = "internals/distcheck/src/distcheck/node_flow/platforms.py" +TYPESCRIPT_FILE = "packages/ts/src/platforms.ts" + +# Python field -> the TypeScript property it is sourced from. `slug` is the +# sub-package name minus its prefix, which is what `librarySlug()` returns. +SHARED = { + "node_platform": "nodePlatform", + "node_arch": "nodeArch", + "slug": "libName", + "os": "os", + "cpu": "cpu", +} +LIB_PREFIX = "@dirsql/lib-" + + +def _slug(lib_name: str) -> str: + return lib_name.removeprefix(LIB_PREFIX) + + +# Python field -> how its TypeScript value is derived, for the fields that are +# not a straight read. Keyed rather than branched so the field name is never +# compared, only looked up. +DERIVE = {"slug": _slug} + + +def key(node_platform: str, node_arch: str) -> str: + return f"{node_platform}-{node_arch}" + + +def unmirrored_fields(fields) -> list[str]: + """One message per dataclass field with no TypeScript source.""" + return [ + f"Platform.{field} has no counterpart in {TYPESCRIPT_FILE}. platforms.py " + f"holds a deliberate subset of the published target ({', '.join(SHARED)}), " + f"so a field the mirror cannot source is drift: delete it, or add the " + f"property to the TypeScript `Platform` interface and to SHARED here." + for field in fields + if field not in SHARED + ] + + +def typescript_value(field: str, row: dict): + """``row``'s value for a Python field name.""" + value = row.get(SHARED[field]) + derive = DERIVE.get(field) + if derive is None or not isinstance(value, str): + return value + return derive(value) + + +def prefix_problems(rows) -> list[str]: + """One message per `libName` that `librarySlug()` would throw on.""" + return [ + f"{key(row.get('nodePlatform'), row.get('nodeArch'))}: libName " + f"{row.get('libName')!r} does not start with {LIB_PREFIX!r}, so " + f"`librarySlug()` throws on it and the Python `slug` cannot be derived. " + f"Fix the name in {TYPESCRIPT_FILE}." + for row in rows + if not str(row.get("libName", "")).startswith(LIB_PREFIX) + ] + + +def missing_rows(python_keys, typescript_rows) -> list[str]: + """One message per published target with no row in platforms.py.""" + return [ + f"{key(row['nodePlatform'], row['nodeArch'])} is published by platforms.ts " + f"but has no row in platforms.py. Add " + f"Platform({row['nodePlatform']!r}, {row['nodeArch']!r}, " + f"{_slug(str(row.get('libName', '')))!r}, {row.get('os')!r}, " + f"{row.get('cpu')!r}) to PLATFORMS in {PYTHON_FILE}, or the node distcheck " + f"flow cannot resolve the new target on that host." + for row in typescript_rows + if key(row["nodePlatform"], row["nodeArch"]) not in python_keys + ] + + +def stray_rows(typescript_keys, python_rows) -> list[str]: + """One message per platforms.py row that is not a published target.""" + return [ + f"{key(row['node_platform'], row['node_arch'])} has a row in platforms.py " + f"but is not a published target in platforms.ts. Either add the target to " + f"PLATFORMS in {TYPESCRIPT_FILE} or drop the row from {PYTHON_FILE}: " + f"distcheck must not resolve a sub-package the release never publishes." + for row in python_rows + if key(row["node_platform"], row["node_arch"]) not in typescript_keys + ] + + +def field_problems(python_row: dict, typescript_row: dict) -> list[str]: + """One message per shared field whose two values disagree.""" + problems = [] + for field in SHARED: + found = python_row.get(field) + expected = typescript_value(field, typescript_row) + if found != expected: + problems.append( + f"{key(python_row['node_platform'], python_row['node_arch'])}: " + f"{field} is {found!r} in platforms.py, {expected!r} in " + f"platforms.ts. platforms.ts is the release source of truth -- " + f"change {PYTHON_FILE} to match." + ) + return problems + + +def problems(fields, python_rows, typescript_rows) -> list[str]: + """Every way the two tables disagree, most structural first.""" + found = unmirrored_fields(fields) + prefix_problems(typescript_rows) + by_key = {key(row["nodePlatform"], row["nodeArch"]): row for row in typescript_rows} + python_keys = {key(row["node_platform"], row["node_arch"]) for row in python_rows} + found += missing_rows(python_keys, typescript_rows) + found += stray_rows(set(by_key), python_rows) + for row in python_rows: + counterpart = by_key.get(key(row["node_platform"], row["node_arch"])) + if counterpart is not None: + found += field_problems(row, counterpart) + return found diff --git a/internals/checks/src/checks/platforms_mirror/decide_test.py b/internals/checks/src/checks/platforms_mirror/decide_test.py new file mode 100644 index 000000000..a33e8a275 --- /dev/null +++ b/internals/checks/src/checks/platforms_mirror/decide_test.py @@ -0,0 +1,160 @@ +"""Colocated unit tests for the platforms-mirror decision logic (isolation -- pure +dicts in, messages out). +""" + +from checks.platforms_mirror.decide import ( + DERIVE, + SHARED, + _slug, + field_problems, + missing_rows, + prefix_problems, + problems, + stray_rows, + typescript_value, + unmirrored_fields, +) + +FIELDS = list(SHARED) + +PY_LINUX = { + "node_platform": "linux", + "node_arch": "x64", + "slug": "linux-x64-gnu", + "os": ["linux"], + "cpu": ["x64"], +} +TS_LINUX = { + "triple": "x86_64-unknown-linux-gnu", + "nodePlatform": "linux", + "nodeArch": "x64", + "libName": "@dirsql/lib-linux-x64-gnu", + "os": ["linux"], + "cpu": ["x64"], + "libc": ["glibc"], +} +PY_WIN = { + "node_platform": "win32", + "node_arch": "x64", + "slug": "win32-x64-msvc", + "os": ["win32"], + "cpu": ["x64"], +} +TS_WIN = { + "nodePlatform": "win32", + "nodeArch": "x64", + "libName": "@dirsql/lib-win32-x64-msvc", + "os": ["win32"], + "cpu": ["x64"], +} + + +def describe_unmirrored_fields(): + def it_passes_the_declared_subset(): + assert unmirrored_fields(FIELDS) == [] + + def it_names_a_field_with_no_typescript_source(): + (message,) = unmirrored_fields([*FIELDS, "exe"]) + assert message.startswith("Platform.exe has no counterpart in ") + assert "packages/ts/src/platforms.ts" in message + + def it_does_not_demand_the_typescript_only_fields(): + assert "triple" not in SHARED + assert "libc" not in SHARED + + +def describe_typescript_value(): + def it_strips_the_sub_package_prefix_off_the_slug(): + assert typescript_value("slug", TS_LINUX) == "linux-x64-gnu" + + def it_reads_other_fields_straight_through(): + assert typescript_value("cpu", TS_LINUX) == ["x64"] + + def it_leaves_a_lib_name_without_the_prefix_alone(): + assert typescript_value("slug", {"libName": "@dirsql/nope"}) == "@dirsql/nope" + + def it_derives_only_the_fields_that_need_it(): + assert set(DERIVE) == {"slug"} + assert _slug("@dirsql/lib-darwin-arm64") == "darwin-arm64" + + def it_does_not_strip_a_prefix_off_a_straight_read_field(): + row = {"nodePlatform": "@dirsql/lib-linux"} + assert typescript_value("node_platform", row) == "@dirsql/lib-linux" + + def it_returns_none_for_a_missing_property(): + assert typescript_value("slug", {}) is None + + +def describe_prefix_problems(): + def it_passes_well_formed_names(): + assert prefix_problems([TS_LINUX, TS_WIN]) == [] + + def it_names_a_lib_name_library_slug_would_throw_on(): + (message,) = prefix_problems([{**TS_WIN, "libName": "@dirsql/win32-x64-msvc"}]) + assert message.startswith("win32-x64: libName '@dirsql/win32-x64-msvc'") + assert "@dirsql/lib-" in message + + def it_names_a_row_with_no_lib_name_at_all(): + assert len(prefix_problems([{"nodePlatform": "win32", "nodeArch": "x64"}])) == 1 + + +def describe_missing_rows(): + def it_passes_when_every_published_target_has_a_row(): + assert missing_rows({"linux-x64"}, [TS_LINUX]) == [] + + def it_suggests_the_row_to_add(): + (message,) = missing_rows(set(), [TS_LINUX]) + assert message.startswith("linux-x64 is published by platforms.ts") + assert "Platform('linux', 'x64', 'linux-x64-gnu', ['linux'], ['x64'])" in message + + +def describe_stray_rows(): + def it_passes_when_every_row_is_published(): + assert stray_rows({"linux-x64"}, [PY_LINUX]) == [] + + def it_names_a_row_the_release_never_publishes(): + (message,) = stray_rows(set(), [PY_LINUX]) + assert message.startswith("linux-x64 has a row in platforms.py") + assert "packages/ts/src/platforms.ts" in message + + +def describe_field_problems(): + def it_passes_a_row_that_agrees(): + assert field_problems(PY_LINUX, TS_LINUX) == [] + + def it_names_the_field_and_both_values(): + (message,) = field_problems({**PY_LINUX, "cpu": ["arm64"]}, TS_LINUX) + assert message == ( + "linux-x64: cpu is ['arm64'] in platforms.py, ['x64'] in platforms.ts. " + "platforms.ts is the release source of truth -- change " + "internals/distcheck/src/distcheck/node_flow/platforms.py to match." + ) + + def it_reports_every_disagreeing_field(): + drifted = {**PY_LINUX, "cpu": ["arm64"], "slug": "nope"} + assert len(field_problems(drifted, TS_LINUX)) == 2 + + def it_reports_a_value_that_sorts_after_the_typescript_one(): + # The pair above drifts *below* platforms.ts; this one drifts above, so + # neither ordering can stand in for the inequality. + (message,) = field_problems({**PY_LINUX, "slug": "zzz"}, TS_LINUX) + assert message.startswith("linux-x64: slug is 'zzz' in platforms.py") + + +def describe_problems(): + def it_passes_two_tables_that_mirror(): + assert problems(FIELDS, [PY_LINUX, PY_WIN], [TS_LINUX, TS_WIN]) == [] + + def it_reports_the_structural_problems_before_the_field_ones(): + found = problems( + [*FIELDS, "exe"], + [PY_LINUX, {**PY_WIN, "cpu": ["arm64"]}], + [TS_LINUX, TS_WIN], + ) + assert found[0].startswith("Platform.exe") + assert found[-1].startswith("win32-x64: cpu") + + def it_compares_only_the_rows_present_on_both_sides(): + found = problems(FIELDS, [PY_LINUX, PY_WIN], [TS_LINUX]) + assert len(found) == 1 + assert found[0].startswith("win32-x64 has a row in platforms.py") diff --git a/internals/checks/src/checks/platforms_mirror/gate.py b/internals/checks/src/checks/platforms_mirror/gate.py new file mode 100644 index 000000000..45ae7bebe --- /dev/null +++ b/internals/checks/src/checks/platforms_mirror/gate.py @@ -0,0 +1,48 @@ +"""Orchestration for the platforms-mirror check (#1004). + +Reads both platform tables off disk and holds them to the subset invariant in +`decide.py`. A table this cannot parse fails the check rather than passing on an +empty read: the whole point is that nothing silently stops comparing. +""" + +from __future__ import annotations + +from collections.abc import Callable + +from checks.platforms_mirror.decide import problems +from checks.platforms_mirror.parse import ParseError, python_table, typescript_table + + +def read(path: str) -> str: + with open(path, encoding="utf-8") as handle: + return handle.read() + + +def run( + python_path: str, + typescript_path: str, + source: Callable[[str], str] = read, + echo: Callable[[str], None] = print, +) -> int: + try: + fields, python_rows = python_table(source(python_path)) + typescript_rows = typescript_table(source(typescript_path)) + except ParseError as error: + echo(f"::error::platforms-mirror could not read a platform table: {error}") + return 1 + found = problems(fields, python_rows, typescript_rows) + for problem in found: + echo(f"::error::{problem}") + if found: + echo( + f"platforms-mirror: {len(found)} problem(s). platforms.ts is the release " + f"source of truth for the published sub-packages; platforms.py mirrors the " + f"subset named in " + f"internals/checks/src/checks/platforms_mirror/decide.py's SHARED." + ) + return 1 + echo( + f"ok platforms-mirror: {python_path} mirrors the shared fields of " + f"{typescript_path} across {len(python_rows)} published target(s)." + ) + return 0 diff --git a/internals/checks/src/checks/platforms_mirror/gate_test.py b/internals/checks/src/checks/platforms_mirror/gate_test.py new file mode 100644 index 000000000..072470dd6 --- /dev/null +++ b/internals/checks/src/checks/platforms_mirror/gate_test.py @@ -0,0 +1,94 @@ +"""Colocated unit tests for the platforms-mirror gate (#1004). + +Isolation: the reader is injected, so nothing here touches the repo's real +platforms.py / platforms.ts. The reader itself is exercised against a scratch +file. +""" + +from checks.platforms_mirror.gate import read, run + +PYTHON = '''\ +from dataclasses import dataclass + + +@dataclass(frozen=True) +class Platform: + node_platform: str + node_arch: str + slug: str + os: list[str] + cpu: list[str] +{extra} + +PLATFORMS: tuple[Platform, ...] = ( + Platform("linux", "x64", "linux-x64-gnu", ["linux"], ["x64"]{value}), +) +''' + +TYPESCRIPT = """\ +export const PLATFORMS: readonly Platform[] = [ + { + triple: "x86_64-unknown-linux-gnu", + nodePlatform: "linux", + nodeArch: "x64", + libName: "@dirsql/lib-linux-x64-gnu", + os: ["linux"], + cpu: ["x64"], + libc: ["glibc"], + }, +]; +""" + + +def sources(python=None, typescript=TYPESCRIPT): + text = { + "p.py": PYTHON.format(extra="", value="") if python is None else python, + "t.ts": typescript, + } + return lambda path: text[path] + + +def invoke(**kwargs): + lines = [] + code = run("p.py", "t.ts", source=sources(**kwargs), echo=lines.append) + return code, "\n".join(lines) + + +def describe_read(): + def it_reads_a_file_as_utf8_text(tmp_path): + path = tmp_path / "platforms.ts" + path.write_text("// caffè\n", encoding="utf-8") + assert read(str(path)) == "// caffè\n" + + +def describe_run(): + def it_reports_the_target_count_when_the_subset_agrees(): + code, out = invoke() + assert code == 0 + assert out == ( + "ok platforms-mirror: p.py mirrors the shared fields of t.ts across " + "1 published target(s)." + ) + + def it_fails_on_a_field_with_no_typescript_counterpart(): + code, out = invoke(python=PYTHON.format(extra=" exe: bool\n", value=", False")) + assert code == 1 + assert "::error::Platform.exe has no counterpart" in out + assert "platforms-mirror: 1 problem(s)." in out + + def it_names_decide_py_so_the_shared_subset_can_be_found(): + code, out = invoke(python=PYTHON.format(extra=" exe: bool\n", value=", False")) + assert code == 1 + assert "platforms_mirror/decide.py's SHARED" in out + + def it_fails_when_the_python_table_cannot_be_read(): + code, out = invoke(python="x = 1\n") + assert code == 1 + assert "::error::platforms-mirror could not read a platform table:" in out + assert "no `class Platform`" in out + + def it_fails_when_the_typescript_table_cannot_be_read(): + code, out = invoke(typescript="export const OTHER = [];\n") + assert code == 1 + assert "could not read a platform table:" in out + assert "no `PLATFORMS" in out diff --git a/internals/checks/src/checks/platforms_mirror/parse.py b/internals/checks/src/checks/platforms_mirror/parse.py new file mode 100644 index 000000000..0a6c3e99a --- /dev/null +++ b/internals/checks/src/checks/platforms_mirror/parse.py @@ -0,0 +1,157 @@ +"""Reading the two platform tables as source text (#1004). + +Neither side is imported: `internals/checks` depends on neither +`internals/distcheck` nor a node toolchain, and a check that executes the file +it audits can be fooled by the file. The Python table comes out of `ast`; the +TypeScript one is stripped of comments, normalized to JSON, and handed to +`json`'s own decoder, which finds the end of the array so nothing here counts +brackets. + +Every shape this cannot read raises `ParseError` rather than returning an empty +table -- a mirror check that silently sees no rows passes forever. +""" + +from __future__ import annotations + +import ast +import json +import re + +CLASS_NAME = "Platform" +TABLE_NAME = "PLATFORMS" +ROW_CALLS = frozenset({CLASS_NAME}) + +# One alternation of "things a naive scan would cut in half": the three string +# forms first, so a `//` inside a string is matched as string rather than as the +# comment that follows it. +_TOKENS = re.compile( + r'"(?:\\.|[^"\\])*"' r"|'(?:\\.|[^'\\])*'" r"|`(?:\\.|[^`\\])*`" r"|//[^\n]*" r"|/\*.*?\*/", + re.S, +) + + +class ParseError(Exception): + """A platform table that could not be read in the shape this check expects.""" + + +def _dataclass_fields(tree: ast.Module) -> list[str]: + classes = {node.name: node for node in tree.body if isinstance(node, ast.ClassDef)} + declaration = classes.get(CLASS_NAME) + if declaration is None: + raise ParseError( + f"no `class {CLASS_NAME}` at module level; this check reads the mirrored " + f"field list off its annotations." + ) + fields = [ + statement.target.id + for statement in declaration.body + if isinstance(statement, ast.AnnAssign) and isinstance(statement.target, ast.Name) + ] + if not fields: + raise ParseError( + f"class {CLASS_NAME} declares no annotated fields; this check reads the " + f"mirrored field list off them." + ) + return fields + + +def _assigned_names(node: ast.stmt) -> list[str]: + """The module-level names ``node`` binds, empty when it binds none.""" + if isinstance(node, ast.AnnAssign): + targets = [node.target] + elif isinstance(node, ast.Assign): + targets = node.targets + else: + return [] + return [target.id for target in targets if isinstance(target, ast.Name)] + + +def _table_elements(tree: ast.Module) -> list[ast.expr]: + bindings = {name: node for node in tree.body for name in _assigned_names(node)} + assignment = bindings.get(TABLE_NAME) + if assignment is None: + raise ParseError(f"no module-level `{TABLE_NAME} = (...)` assignment.") + if not isinstance(assignment.value, (ast.Tuple, ast.List)): + raise ParseError( + f"{TABLE_NAME} is not a tuple or list literal; this check reads its rows " + f"statically and cannot evaluate a computed table." + ) + return list(assignment.value.elts) + + +def _call_name(element: ast.expr): + """The name of a plain `Name(...)` call, else ``None``.""" + if isinstance(element, ast.Call) and isinstance(element.func, ast.Name): + return element.func.id + return None + + +def _literal(node: ast.expr): + try: + return ast.literal_eval(node) + except ValueError as error: + raise ParseError(f"a {CLASS_NAME}(...) argument is not a literal: {error}") from error + + +def _row(element: ast.expr, fields: list[str]) -> dict: + if _call_name(element) not in ROW_CALLS: + raise ParseError(f"every {TABLE_NAME} entry must be a literal `{CLASS_NAME}(...)` call.") + if len(element.args) > len(fields): + raise ParseError( + f"a {CLASS_NAME}(...) row passes {len(element.args)} positional arguments but " + f"{CLASS_NAME} declares {len(fields)} fields." + ) + row = dict(zip(fields, (_literal(argument) for argument in element.args))) + for keyword in element.keywords: + if keyword.arg is None: + raise ParseError(f"a {CLASS_NAME}(...) row splats **kwargs; read it statically.") + row[keyword.arg] = _literal(keyword.value) + return row + + +def python_table(source: str) -> tuple[list[str], list[dict]]: + """`(field names, rows)` from a `platforms.py`-shaped module.""" + tree = ast.parse(source) + fields = _dataclass_fields(tree) + return fields, [_row(element, fields) for element in _table_elements(tree)] + + +def _requoted(text: str) -> str: + """A TypeScript string literal as a JSON one.""" + if text.startswith('"'): + return text + return json.dumps(text[1:-1].replace('\\"', '"').replace("\\'", "'")) + + +def _without_comments(source: str) -> str: + def keep(match: re.Match) -> str: + text = match.group(0) + return "" if text.startswith(("//", "/*")) else _requoted(text) + + return _TOKENS.sub(keep, source) + + +def _as_json(text: str) -> str: + keyed = re.sub(r"([{,]\s*)([A-Za-z_$][\w$]*)\s*:", r'\1"\2":', text) + return re.sub(r",(\s*[}\]])", r"\1", keyed) + + +def typescript_table(source: str) -> list[dict]: + """Rows of the `PLATFORMS` array in a `platforms.ts`-shaped module.""" + cleaned = _without_comments(source) + marker = re.search(rf"\b{TABLE_NAME}\b[^=;]*=\s*(\[)", cleaned) + if marker is None: + raise ParseError(f"no `{TABLE_NAME} = [...]` assignment in the TypeScript source.") + try: + # `raw_decode` stops at the end of the array, so nothing here has to find + # the closing bracket or care what follows it. + rows, _ = json.JSONDecoder().raw_decode(_as_json(cleaned[marker.start(1) :])) + except json.JSONDecodeError as error: + raise ParseError( + f"`{TABLE_NAME}` is not a plain array of object literals ({error}); this check " + f"reads it as data and cannot evaluate spreads, computed keys or identifiers, " + f"and cannot read an unterminated array, string or comment." + ) from error + if not all(isinstance(row, dict) for row in rows): + raise ParseError(f"every `{TABLE_NAME}` entry must be an object literal.") + return rows diff --git a/internals/checks/src/checks/platforms_mirror/parse_test.py b/internals/checks/src/checks/platforms_mirror/parse_test.py new file mode 100644 index 000000000..a7b601bd3 --- /dev/null +++ b/internals/checks/src/checks/platforms_mirror/parse_test.py @@ -0,0 +1,163 @@ +"""Colocated unit tests for the platform-table readers (isolation -- pure text in, +data out; nothing here touches the repo's real platforms.py / platforms.ts). +""" + +import pytest + +from checks.platforms_mirror.parse import ParseError, python_table, typescript_table + +PYTHON = '''\ +from dataclasses import dataclass + + +@dataclass(frozen=True) +class Platform: + node_platform: str + node_arch: str + slug: str + + +PLATFORMS: tuple[Platform, ...] = ( + Platform("linux", "x64", "linux-x64-gnu"), + Platform("win32", "x64", slug="win32-x64-msvc", node_arch="x64"), +) +''' + +TYPESCRIPT = """\ +// Leading comment with a "quoted" brace { and a stray ]. +export const PLATFORMS: readonly Platform[] = [ + { + /* block comment */ + triple: "x86_64-unknown-linux-gnu", + nodePlatform: "linux", + os: ["linux"], // trailing comment + }, +]; +""" + + +def describe_python_table(): + def it_reads_the_annotated_fields_and_the_rows(): + fields, rows = python_table(PYTHON) + assert fields == ["node_platform", "node_arch", "slug"] + assert rows[0] == {"node_platform": "linux", "node_arch": "x64", "slug": "linux-x64-gnu"} + + def it_lets_keywords_override_positionals(): + assert python_table(PYTHON)[1][1] == { + "node_platform": "win32", + "node_arch": "x64", + "slug": "win32-x64-msvc", + } + + def it_reads_a_plain_assignment_without_an_annotation(): + source = PYTHON.replace("PLATFORMS: tuple[Platform, ...] =", "PLATFORMS =") + assert len(python_table(source)[1]) == 2 + + @pytest.mark.parametrize("name", ["Machine", "Target"]) + def it_rejects_a_module_whose_only_class_is_named_otherwise(name): + with pytest.raises(ParseError, match="no `class Platform`"): + python_table(f"class {name}:\n slug: str\n\n\nPLATFORMS = ()\n") + + def it_rejects_a_module_with_no_class_at_all(): + with pytest.raises(ParseError, match="no `class Platform`"): + python_table("PLATFORMS = ()\n") + + def it_rejects_a_platform_class_with_no_annotated_fields(): + with pytest.raises(ParseError, match="declares no annotated fields"): + python_table("class Platform:\n pass\n\n\nPLATFORMS = ()\n") + + @pytest.mark.parametrize("name", ["OTHERS", "TARGETS"]) + def it_rejects_a_table_bound_under_another_name(name): + with pytest.raises(ParseError, match="no module-level"): + python_table(f"class Platform:\n slug: str\n\n\n{name} = ()\n") + + def it_rejects_a_module_with_no_table(): + with pytest.raises(ParseError, match="no module-level"): + python_table("class Platform:\n slug: str\n") + + def it_reads_a_table_bound_alongside_another_name(): + source = "class Platform:\n slug: str\n\n\nOTHERS = PLATFORMS = (Platform('a'),)\n" + assert python_table(source)[1] == [{"slug": "a"}] + + def it_rejects_a_computed_table(): + source = PYTHON.replace("PLATFORMS: tuple[Platform, ...] = (", "PLATFORMS = tuple(") + with pytest.raises(ParseError, match="not a tuple or list literal"): + python_table(source.replace(")\n", ")\n", 1)) + + @pytest.mark.parametrize("row", ["1", "Machine('a')", "Target('a')"]) + def it_rejects_a_row_that_is_not_a_platform_call(row): + with pytest.raises(ParseError, match="literal `Platform"): + python_table(f"class Platform:\n slug: str\n\n\nPLATFORMS = ({row},)\n") + + def it_accepts_a_row_that_fills_every_field_positionally(): + source = "class Platform:\n slug: str\n\n\nPLATFORMS = (Platform('a'),)\n" + assert python_table(source)[1] == [{"slug": "a"}] + + def it_rejects_a_row_with_more_positionals_than_fields(): + with pytest.raises(ParseError, match="positional arguments"): + python_table('class Platform:\n slug: str\n\n\nPLATFORMS = (Platform("a", "b"),)\n') + + def it_rejects_a_row_that_splats_keywords(): + with pytest.raises(ParseError, match="splats"): + python_table("class Platform:\n slug: str\n\n\nPLATFORMS = (Platform(**row),)\n") + + def it_rejects_a_non_literal_argument(): + with pytest.raises(ParseError, match="not a literal"): + python_table("class Platform:\n slug: str\n\n\nPLATFORMS = (Platform(slug),)\n") + + +def describe_typescript_table(): + def it_reads_the_array_through_comments_and_trailing_commas(): + assert typescript_table(TYPESCRIPT) == [ + { + "triple": "x86_64-unknown-linux-gnu", + "nodePlatform": "linux", + "os": ["linux"], + } + ] + + def it_normalizes_single_quoted_strings(): + assert typescript_table("const PLATFORMS = [{ os: ['linux'] }];") == [{"os": ["linux"]}] + + def it_unwraps_a_single_quoted_string_without_trimming_its_contents(): + assert typescript_table("const PLATFORMS = [{ os: ['aba'] }];") == [{"os": ["aba"]}] + + def it_unwraps_a_template_literal(): + assert typescript_table("const PLATFORMS = [{ os: [`aba`] }];") == [{"os": ["aba"]}] + + def it_keeps_a_double_quoted_string_verbatim(): + assert typescript_table('const PLATFORMS = [{ os: ["aba"] }];') == [{"os": ["aba"]}] + + def it_finds_the_array_when_a_comment_mentions_a_bracket(): + source = "// see PLATFORMS = [ elsewhere\nconst PLATFORMS = [{ os: ['a'] }];" + assert typescript_table(source) == [{"os": ["a"]}] + + def it_keeps_an_escape_sequence_inside_a_string(): + assert typescript_table(r'const PLATFORMS = [{ os: "a\"b" }];') == [{"os": 'a"b'}] + + def it_rejects_a_source_with_no_table(): + with pytest.raises(ParseError, match="no `PLATFORMS"): + typescript_table("export const OTHER = [];\n") + + @pytest.mark.parametrize( + "source", + [ + "const PLATFORMS = [{ os: [] }\n", + "const PLATFORMS = [ /* open\n", + 'const PLATFORMS = [{ os: "open\n', + 'const PLATFORMS = [{ os: "open\\', + "const PLATFORMS = [ // open", + ], + ids=["unclosed-array", "unclosed-block-comment", "unclosed-string", "trailing-backslash", "comment-to-eof"], + ) + def it_rejects_a_table_it_cannot_read_to_the_end(source): + with pytest.raises(ParseError, match="not a plain array"): + typescript_table(source) + + def it_rejects_an_entry_that_is_not_an_object_literal(): + with pytest.raises(ParseError, match="object literal"): + typescript_table('const PLATFORMS = ["linux"];') + + def it_rejects_a_table_it_cannot_read_as_data(): + with pytest.raises(ParseError, match="not a plain array"): + typescript_table("const PLATFORMS = [{ ...spread }];") diff --git a/internals/checks/tests/e2e/platforms_mirror_test.py b/internals/checks/tests/e2e/platforms_mirror_test.py new file mode 100644 index 000000000..e953e5430 --- /dev/null +++ b/internals/checks/tests/e2e/platforms_mirror_test.py @@ -0,0 +1,98 @@ +"""E2E test for `dirsql-checks platforms-mirror` through the real CLI. + +No mocking of any kind: spawns the packaged `dirsql-checks` console script as a +subprocess, once against real scratch files and once against the repo's own +`platforms.py` / `platforms.ts` through the command's default paths. +""" + +from __future__ import annotations + +import shutil +import subprocess +from pathlib import Path + +REPO = Path(__file__).resolve().parents[4] + +PYTHON_SOURCE = '''\ +from dataclasses import dataclass + + +@dataclass(frozen=True) +class Platform: + node_platform: str + node_arch: str + slug: str + os: list[str] + cpu: list[str] +{extra} + +PLATFORMS: tuple[Platform, ...] = ( + Platform("linux", "x64", "linux-x64-gnu", ["linux"], ["x64"]{value}), +) +''' + +TYPESCRIPT_SOURCE = """\ +export const PLATFORMS: readonly Platform[] = [ + { + triple: "x86_64-unknown-linux-gnu", + nodePlatform: "linux", + nodeArch: "x64", + libName: "@dirsql/lib-linux-x64-gnu", + os: ["linux"], + cpu: ["x64"], + libc: ["glibc"], + }, +]; +""" + + +def _cli() -> str: + dirsql_checks = shutil.which("dirsql-checks") + assert dirsql_checks is not None, ( + "`dirsql-checks` console script not on PATH -- run " + "`uv run --project internals/checks pytest tests/e2e` " + "or `uv sync --project internals/checks`" + ) + return dirsql_checks + + +def _write(tmp_path, *, stray_field: bool): + python = tmp_path / "platforms.py" + python.write_text( + PYTHON_SOURCE.format( + extra=" exe: bool\n" if stray_field else "", + value=", False" if stray_field else "", + ) + ) + typescript = tmp_path / "platforms.ts" + typescript.write_text(TYPESCRIPT_SOURCE) + return ["--python", str(python), "--typescript", str(typescript)] + + +def _invoke(arguments, cwd=None): + return subprocess.run( + [_cli(), "platforms-mirror", *arguments], + capture_output=True, + text=True, + timeout=60, + cwd=cwd, + ) + + +def describe_dirsql_checks_platforms_mirror(): + def it_exits_zero_when_the_subset_agrees(tmp_path): + proc = _invoke(_write(tmp_path, stray_field=False)) + + assert proc.returncode == 0, f"stdout={proc.stdout!r} stderr={proc.stderr!r}" + assert "ok platforms-mirror" in proc.stdout + + def it_exits_nonzero_and_names_the_unmirrored_field(tmp_path): + proc = _invoke(_write(tmp_path, stray_field=True)) + + assert proc.returncode == 1, f"stdout={proc.stdout!r} stderr={proc.stderr!r}" + assert "Platform.exe" in proc.stdout + + def it_holds_the_repos_own_files_through_the_default_paths(): + proc = _invoke([], cwd=REPO) + + assert proc.returncode == 0, f"stdout={proc.stdout!r} stderr={proc.stderr!r}" diff --git a/internals/checks/tests/integration/platforms_mirror_test.py b/internals/checks/tests/integration/platforms_mirror_test.py new file mode 100644 index 000000000..c680d275e --- /dev/null +++ b/internals/checks/tests/integration/platforms_mirror_test.py @@ -0,0 +1,165 @@ +"""Integration tests for the platforms-mirror check against real files. + +Exercises `gate.run` with its default collaborators -- a real `ast` parse of a +real `platforms.py` and a real parse of a real `platforms.ts` -- rather than the +packaged `dirsql-checks` CLI. + +The reproduction is #1004 exactly: `Platform.exe` sat on the Python dataclass for +the stated reason that it "mirrors packages/ts/src/platforms.ts", while the +TypeScript source of truth has never carried the field. A comment asserted the +correspondence and nothing checked it, which is the same mechanism behind #972 +and #996. The last case here is the one that matters day to day: the repo's own +two files, held to the invariant on every run. +""" + +from __future__ import annotations + +from pathlib import Path + +from checks.platforms_mirror.gate import run + +REPO = Path(__file__).resolve().parents[4] +PYTHON = REPO / "internals" / "distcheck" / "src" / "distcheck" / "node_flow" / "platforms.py" +TYPESCRIPT = REPO / "packages" / "ts" / "src" / "platforms.ts" + +ANNOTATIONS = { + "node_platform": "str", + "node_arch": "str", + "slug": "str", + "os": "list[str]", + "cpu": "list[str]", + "exe": "bool", +} + +SUBSET = ("node_platform", "node_arch", "slug", "os", "cpu") + +LINUX = { + "node_platform": "linux", + "node_arch": "x64", + "slug": "linux-x64-gnu", + "os": ["linux"], + "cpu": ["x64"], + "exe": False, +} +WINDOWS = { + "node_platform": "win32", + "node_arch": "x64", + "slug": "win32-x64-msvc", + "os": ["win32"], + "cpu": ["x64"], + "exe": True, +} + +PYTHON_SOURCE = '''\ +from dataclasses import dataclass + + +@dataclass(frozen=True) +class Platform: +{fields} + + @property + def name(self) -> str: + return f"@dirsql/lib-{{self.slug}}" + + +PLATFORMS: tuple[Platform, ...] = ( +{rows} +) +''' + +TYPESCRIPT_SOURCE = """\ +export interface Platform {{ + triple: string; + nodePlatform: NodeJS.Platform; + nodeArch: NodeJS.Architecture; + libName: string; + os: string[]; + cpu: string[]; + libc?: string[]; +}} + +export const PLATFORMS: readonly Platform[] = [ +{rows} +]; +""" + + +def write(tmp_path, python_rows, typescript_rows, fields=SUBSET): + python = tmp_path / "platforms.py" + python.write_text( + PYTHON_SOURCE.format( + fields="\n".join(f" {name}: {ANNOTATIONS[name]}" for name in fields), + rows="\n".join( + f" Platform({', '.join(repr(row[name]) for name in fields)})," + for row in python_rows + ), + ) + ) + typescript = tmp_path / "platforms.ts" + typescript.write_text( + TYPESCRIPT_SOURCE.format( + rows="\n".join( + " {\n" + f' triple: "{row["node_arch"]}-{row["node_platform"]}",\n' + f' nodePlatform: "{row["node_platform"]}",\n' + f' nodeArch: "{row["node_arch"]}",\n' + f' libName: "{row.get("lib_name", "@dirsql/lib-" + row["slug"])}",\n' + f' os: {list(row["os"])!r},\n' + f' cpu: {list(row["cpu"])!r},\n' + " }," + for row in typescript_rows + ).replace("'", '"') + ) + ) + return str(python), str(typescript) + + +def describe_run_against_real_platform_files(): + def it_passes_when_the_declared_subset_agrees(tmp_path, capsys): + assert run(*write(tmp_path, [LINUX, WINDOWS], [LINUX, WINDOWS])) == 0 + assert "ok platforms-mirror" in capsys.readouterr().out + + def it_rejects_a_python_field_with_no_typescript_counterpart(tmp_path, capsys): + # #1004: `exe` justified by a mirror that never had it. + code = run(*write(tmp_path, [LINUX, WINDOWS], [LINUX, WINDOWS], fields=(*SUBSET, "exe"))) + + assert code == 1 + out = capsys.readouterr().out + assert "::error::Platform.exe" in out + assert "no counterpart in packages/ts/src/platforms.ts" in out + + def it_rejects_a_published_target_missing_from_the_python_rows(tmp_path, capsys): + # A new target half-added: published by the TS source of truth, invisible + # to the node distcheck flow. + code = run(*write(tmp_path, [LINUX], [LINUX, WINDOWS])) + + assert code == 1 + assert "::error::win32-x64 is published by platforms.ts" in capsys.readouterr().out + + def it_rejects_a_python_row_that_is_not_a_published_target(tmp_path, capsys): + code = run(*write(tmp_path, [LINUX, WINDOWS], [LINUX])) + + assert code == 1 + assert "::error::win32-x64 has a row in platforms.py" in capsys.readouterr().out + + def it_rejects_a_shared_field_whose_values_disagree(tmp_path, capsys): + drifted = {**WINDOWS, "cpu": ["arm64"]} + code = run(*write(tmp_path, [LINUX, drifted], [LINUX, WINDOWS])) + + assert code == 1 + out = capsys.readouterr().out + assert "::error::win32-x64: cpu is ['arm64'] in platforms.py" in out + assert "['x64'] in platforms.ts" in out + + def it_rejects_a_lib_name_that_does_not_carry_the_sub_package_prefix(tmp_path, capsys): + stray = {**WINDOWS, "lib_name": "@dirsql/win32-x64-msvc"} + code = run(*write(tmp_path, [LINUX, WINDOWS], [LINUX, stray])) + + assert code == 1 + assert "@dirsql/lib-" in capsys.readouterr().out + + +def describe_run_against_the_repos_own_files(): + def it_holds_the_committed_platform_tables_to_the_invariant(capsys): + assert run(str(PYTHON), str(TYPESCRIPT)) == 0, capsys.readouterr().out diff --git a/internals/distcheck/src/distcheck/node_flow/platforms.py b/internals/distcheck/src/distcheck/node_flow/platforms.py index 66e8bbaa0..3f4188330 100644 --- a/internals/distcheck/src/distcheck/node_flow/platforms.py +++ b/internals/distcheck/src/distcheck/node_flow/platforms.py @@ -18,7 +18,6 @@ class Platform: slug: str os: list[str] cpu: list[str] - exe: bool @property def name(self) -> str: @@ -35,15 +34,16 @@ def addon_name(self) -> str: return f"dirsql.{self.slug}.node" -# Mirror of PLATFORMS in packages/ts/src/platforms.ts (the fields the node distcheck -# flow needs). Keep in lockstep with that file -- it is the release source of -# truth for the published sub-packages. +# The subset of PLATFORMS in packages/ts/src/platforms.ts this flow needs -- that +# file is the release source of truth for the published sub-packages, and its +# `triple` and `libc` reconstruct nothing here. `dirsql-checks platforms-mirror` +# holds the two tables to that subset, in both directions. PLATFORMS: tuple[Platform, ...] = ( - Platform("linux", "x64", "linux-x64-gnu", ["linux"], ["x64"], False), - Platform("linux", "arm64", "linux-arm64-gnu", ["linux"], ["arm64"], False), - Platform("darwin", "x64", "darwin-x64", ["darwin"], ["x64"], False), - Platform("darwin", "arm64", "darwin-arm64", ["darwin"], ["arm64"], False), - Platform("win32", "x64", "win32-x64-msvc", ["win32"], ["x64"], True), + Platform("linux", "x64", "linux-x64-gnu", ["linux"], ["x64"]), + Platform("linux", "arm64", "linux-arm64-gnu", ["linux"], ["arm64"]), + Platform("darwin", "x64", "darwin-x64", ["darwin"], ["x64"]), + Platform("darwin", "arm64", "darwin-arm64", ["darwin"], ["arm64"]), + Platform("win32", "x64", "win32-x64-msvc", ["win32"], ["x64"]), ) _ARCH = { diff --git a/internals/distcheck/src/distcheck/node_flow/platforms_test.py b/internals/distcheck/src/distcheck/node_flow/platforms_test.py index 8047a891b..8569279e7 100644 --- a/internals/distcheck/src/distcheck/node_flow/platforms_test.py +++ b/internals/distcheck/src/distcheck/node_flow/platforms_test.py @@ -27,7 +27,6 @@ def test_find_host_platform_linux_x64(): assert p.addon_name == "dirsql.linux-x64-gnu.node" assert p.os == ["linux"] assert p.cpu == ["x64"] - assert p.exe is False def test_find_host_platform_arch_is_significant(): @@ -36,10 +35,9 @@ def test_find_host_platform_arch_is_significant(): def test_find_host_platform_windows_addon_has_no_exe_suffix(): - # The `.exe` suffix belonged to the standalone binary, which is no longer - # published (#739); an addon is a `.node` on every platform. + # An addon is a `.node` on every platform; the `.exe` suffix belonged to the + # standalone binary, which is no longer published (#739). p = find_host_platform("win32", "x64") - assert p.exe is True assert p.addon_name == "dirsql.win32-x64-msvc.node" @@ -58,13 +56,6 @@ def test_every_platform_addon_name_matches_slug(): assert p.addon_name == f"dirsql.{p.slug}.node" -def test_only_windows_rows_flag_exe(): - # `exe` still marks the Windows row (it mirrors packages/ts/src/platforms.ts), - # but nothing derives a filename from it now that no binary is published. - for p in PLATFORMS: - assert p.exe is (p.node_platform == "win32") - - def test_platform_is_immutable(): p = find_host_platform("linux", "x64") with pytest.raises(AttributeError):