From 12d7450f4da109f851a2787b527d8f2f44028a55 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 20 Oct 2026 11:48:17 +0100 Subject: [PATCH 1/3] perf: build the crates sqb shares with the extension once The sqb build resolved several dependency features differently from the extension build, so Cargo compiled polyglot-sql, sqlbuild-analysis, sqlbuild-rules and their dependencies twice in every wheel and uv sync. A new code-free sqlbuild-build-features crate enables the missing features in the sqb build, and make check-build-features fails when the two builds drift apart again. --- Cargo.lock | 16 ++++ Cargo.toml | 14 ++++ Makefile | 8 +- crates/README.md | 3 +- crates/sqlbuild-build-features/Cargo.toml | 32 ++++++++ crates/sqlbuild-build-features/build.rs | 3 + crates/sqlbuild-build-features/src/lib.rs | 3 + crates/sqlbuild-cli/Cargo.toml | 1 + fensu.toml | 1 + scripts/build_features/__init__.py | 1 + scripts/build_features/_helpers/__init__.py | 1 + scripts/build_features/_helpers/drift.py | 24 ++++++ scripts/build_features/_helpers/tree.py | 78 ++++++++++++++++++ scripts/build_features/constants.py | 22 ++++++ scripts/build_features/main/__init__.py | 1 + scripts/build_features/main/check.py | 70 ++++++++++++++++ scripts/build_features/models.py | 23 ++++++ scripts/check_build_features.py | 14 ++++ .../scripts/build_features/__init__.py | 0 .../scripts/build_features/_test_types.py | 22 ++++++ .../scripts/build_features/helpers.py | 56 +++++++++++++ .../scripts/build_features/test_check.py | 79 +++++++++++++++++++ tests/unit/scripts/build_features/__init__.py | 0 .../build_features/_helpers/__init__.py | 0 .../build_features/_helpers/_test_types.py | 10 +++ .../build_features/_helpers/test_tree.py | 72 +++++++++++++++++ 26 files changed, 551 insertions(+), 3 deletions(-) create mode 100644 crates/sqlbuild-build-features/Cargo.toml create mode 100644 crates/sqlbuild-build-features/build.rs create mode 100644 crates/sqlbuild-build-features/src/lib.rs create mode 100644 scripts/build_features/__init__.py create mode 100644 scripts/build_features/_helpers/__init__.py create mode 100644 scripts/build_features/_helpers/drift.py create mode 100644 scripts/build_features/_helpers/tree.py create mode 100644 scripts/build_features/constants.py create mode 100644 scripts/build_features/main/__init__.py create mode 100644 scripts/build_features/main/check.py create mode 100644 scripts/build_features/models.py create mode 100644 scripts/check_build_features.py create mode 100644 tests/integration/scripts/build_features/__init__.py create mode 100644 tests/integration/scripts/build_features/_test_types.py create mode 100644 tests/integration/scripts/build_features/helpers.py create mode 100644 tests/integration/scripts/build_features/test_check.py create mode 100644 tests/unit/scripts/build_features/__init__.py create mode 100644 tests/unit/scripts/build_features/_helpers/__init__.py create mode 100644 tests/unit/scripts/build_features/_helpers/_test_types.py create mode 100644 tests/unit/scripts/build_features/_helpers/test_tree.py diff --git a/Cargo.lock b/Cargo.lock index b9d13af357..97575425aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1217,6 +1217,21 @@ dependencies = [ "sqlbuild-sqltext", ] +[[package]] +name = "sqlbuild-build-features" +version = "0.164.0" +dependencies = [ + "arrayvec", + "bstr", + "either", + "memchr", + "once_cell", + "regex-automata", + "syn 2.0.119", + "syn 3.0.3", + "thiserror 2.0.21", +] + [[package]] name = "sqlbuild-cache" version = "0.164.0" @@ -1240,6 +1255,7 @@ dependencies = [ "rustix", "serde", "serde_json", + "sqlbuild-build-features", "sqlbuild-cache", "sqlbuild-config", "sqlbuild-core", diff --git a/Cargo.toml b/Cargo.toml index 63d94fcfd5..7375ad89eb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,5 +1,6 @@ [workspace] members = [ + "crates/sqlbuild-build-features", "crates/sqlbuild-core", "crates/sqlbuild-cursor", "crates/sqlbuild-state", @@ -26,6 +27,7 @@ resolver = "2" # out of every crate below the polyglot floor. See crates/README.md. [workspace.metadata.native-layers] order = [ + "sqlbuild-build-features", "sqlbuild-core", "sqlbuild-cursor", "sqlbuild-state", @@ -77,6 +79,17 @@ polyglot-sql-function-catalogs = { git = "https://github.com/chio-labs/polyglot. "dialect-duckdb", ] } blake3 = { version = "1", features = ["rayon"] } +# Only sqlbuild-build-features uses these: each enables in the sqb build exactly the features the +# extension build resolves for that crate, so both builds share one compile of every common crate. +arrayvec = "0.7" +bstr = "1" +either = { version = "1", default-features = false, features = ["use_std"] } +memchr = "2" +once_cell = "1" +regex-automata = { version = "0.4", default-features = false, features = ["dfa-search"] } +syn-v2-macro-features = { package = "syn", version = "2", default-features = false, features = ["extra-traits", "fold", "visit-mut"] } +syn-v3-macro-features = { package = "syn", version = "3", default-features = false, features = ["extra-traits", "full"] } +thiserror = "2" # Windows only: `sqb` ignores Ctrl+C while the Python CLI it started handles it. ctrlc = "3" quote = "1" @@ -99,6 +112,7 @@ serde_json = { version = "1", features = ["preserve_order"] } sha2 = "0.10" sqlbuild-analysis = { path = "crates/sqlbuild-analysis" } sqlbuild-attachments = { path = "crates/sqlbuild-attachments" } +sqlbuild-build-features = { path = "crates/sqlbuild-build-features" } sqlbuild-cache = { path = "crates/sqlbuild-cache" } sqlbuild-cli = { path = "crates/sqlbuild-cli" } sqlbuild-config = { path = "crates/sqlbuild-config" } diff --git a/Makefile b/Makefile index 2451eea424..677f6f4f16 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ SHELL := /bin/bash -.PHONY: verify verify-quick verify-pg coverage cli-preview rust-check check-native-layering check-python-boundary check-release-gate check-docs-skill \ +.PHONY: verify verify-quick verify-pg coverage cli-preview rust-check check-native-layering check-build-features check-python-boundary check-release-gate check-docs-skill \ check-e2e-shards test-e2e-duckdb test-e2e-duckdb-build-core \ test-e2e-duckdb-build-incremental \ test-e2e-duckdb-cli-data test-e2e-duckdb-cli test-e2e-duckdb-inspection \ @@ -641,7 +641,7 @@ verify-quick: exit $$status -check-ci: check-e2e-shards check-native-layering check-python-boundary check-release-gate check-docs-skill rust-lint +check-ci: check-e2e-shards check-native-layering check-build-features check-python-boundary check-release-gate check-docs-skill rust-lint uv run ruff format --check . uv run ruff check . uv run ty check src tests @@ -653,6 +653,10 @@ check-native-layering: uv run python -m scripts.check_native_layering +check-build-features: + uv run python -m scripts.check_build_features + + check-python-boundary: uv run python -m scripts.check_python_boundary diff --git a/crates/README.md b/crates/README.md index ec54e19504..8354a427ea 100644 --- a/crates/README.md +++ b/crates/README.md @@ -6,6 +6,7 @@ that can be tested and benchmarked with `cargo test` alone. | Crate | Responsibility | |---|---| +| `sqlbuild-build-features` | No code. Enables, in the `sqb` build, the dependency features the extension build resolves, so Cargo compiles the crates both builds share (polyglot-sql, `sqlbuild-analysis`, `sqlbuild-rules` and the rest) once. `make check-build-features` (part of `make check-ci`) fails when the two builds resolve a shared crate differently and names the features to add. | | `sqlbuild-core` | Constants shared by several crates, the panic boundary that turns an unwinding native failure into a named compiler error, passive identity, diagnostic and SQL value types, text positions, a JSON emitter whose output matches Python's `json.dumps` and `orjson` byte for byte, and run-cost allocation with exact Python `decimal` default-context arithmetic and Snowflake query-history SQL. | | `sqlbuild-cursor` | Microbatch cursor algebra: typed cursor values, grain flooring and stepping, half-open interval clamping, merging and batch splitting, timestamp text normalization and calendar durations, with Python `datetime` results and errors. Zones other than fixed offsets are resolved through a caller-supplied lookup. | | `sqlbuild-state` | Append-only warehouse state rows: content-addressed migration event IDs, typed event SQL literals and statements, and source freshness data-version normalization, hashing, equivalence, age policy and latest-state read SQL, byte-compatible with the rows Python wrote. It also owns execution history: lifecycle event batch digests and conflicts, run projection, event and run cursors, filter and page validation, and the SQLite and PostgreSQL statements the native history backends run. | @@ -33,7 +34,7 @@ sqlbuild-python -> sqlbuild-refactor -> sqlbuild-rules -> sqlbuild-analysis -> sqlbuild-attachments -> sqlbuild-model-config -> sqlbuild-scopes -> sqlbuild-render -> sqlbuild-discovery -> sqlbuild-config -> sqlbuild-sqltext -> sqlbuild-cache -> sqlbuild-pysource -> sqlbuild-state -> sqlbuild-cursor -> sqlbuild-core -sqlbuild-cli -> sqlbuild-cache, sqlbuild-core +sqlbuild-cli -> sqlbuild-cache, sqlbuild-core, sqlbuild-build-features ``` `sqlbuild-scopes` reuses the macro scanner in `sqlbuild-render`; render itself depends only on diff --git a/crates/sqlbuild-build-features/Cargo.toml b/crates/sqlbuild-build-features/Cargo.toml new file mode 100644 index 0000000000..84a5d1ee63 --- /dev/null +++ b/crates/sqlbuild-build-features/Cargo.toml @@ -0,0 +1,32 @@ +[package] +name = "sqlbuild-build-features" +version.workspace = true +description = "No code: makes the sqb build resolve dependency features exactly as the extension build does, so both builds share one compile of every common crate." +edition.workspace = true +license.workspace = true +publish.workspace = true +rust-version.workspace = true + +# Cargo resolves features separately for each build, so without these entries the sqb build +# hashes the crates it shares with the extension differently and compiles polyglot-sql, +# sqlbuild-analysis and sqlbuild-rules a second time. The workspace entries enable in the sqb +# build exactly the features the extension build already resolves; sqlbuild-cli depends on this +# crate, so the extension build sees them too and stays unchanged. `make check-build-features` +# fails when the two builds resolve a shared crate differently. +[dependencies] +arrayvec.workspace = true +bstr.workspace = true +either.workspace = true +memchr.workspace = true +once_cell.workspace = true +regex-automata.workspace = true +thiserror.workspace = true + +# Proc-macro dependencies resolve with build dependencies, so the syn features PyO3's macros use +# are enabled here. The empty build script makes Cargo resolve them. +[build-dependencies] +syn-v2-macro-features.workspace = true +syn-v3-macro-features.workspace = true + +[lints] +workspace = true diff --git a/crates/sqlbuild-build-features/build.rs b/crates/sqlbuild-build-features/build.rs new file mode 100644 index 0000000000..590cd32ad8 --- /dev/null +++ b/crates/sqlbuild-build-features/build.rs @@ -0,0 +1,3 @@ +//! Empty: its presence makes Cargo resolve the build dependencies that align proc-macro features. + +fn main() {} diff --git a/crates/sqlbuild-build-features/src/lib.rs b/crates/sqlbuild-build-features/src/lib.rs new file mode 100644 index 0000000000..e753254da6 --- /dev/null +++ b/crates/sqlbuild-build-features/src/lib.rs @@ -0,0 +1,3 @@ +//! No code: aligns dependency features between the sqb and extension builds (see Cargo.toml). + +#![forbid(unsafe_code)] diff --git a/crates/sqlbuild-cli/Cargo.toml b/crates/sqlbuild-cli/Cargo.toml index 791fafe765..1666867dbf 100644 --- a/crates/sqlbuild-cli/Cargo.toml +++ b/crates/sqlbuild-cli/Cargo.toml @@ -20,6 +20,7 @@ blake3.workspace = true getrandom.workspace = true serde.workspace = true serde_json.workspace = true +sqlbuild-build-features.workspace = true sqlbuild-cache.workspace = true sqlbuild-config.workspace = true sqlbuild-core.workspace = true diff --git a/fensu.toml b/fensu.toml index dad712116c..299971cb8e 100644 --- a/fensu.toml +++ b/fensu.toml @@ -1567,6 +1567,7 @@ remediation = "consume SQLBuild-owned SQL fact rows instead of raw parser types" crate_names = [ "sqlbuild-analysis", "sqlbuild-attachments", + "sqlbuild-build-features", "sqlbuild-cache", "sqlbuild-cli", "sqlbuild-config", diff --git a/scripts/build_features/__init__.py b/scripts/build_features/__init__.py new file mode 100644 index 0000000000..5f4a43650a --- /dev/null +++ b/scripts/build_features/__init__.py @@ -0,0 +1 @@ +"""Check that the sqb and extension builds resolve shared crates identically.""" diff --git a/scripts/build_features/_helpers/__init__.py b/scripts/build_features/_helpers/__init__.py new file mode 100644 index 0000000000..bec8dc89cd --- /dev/null +++ b/scripts/build_features/_helpers/__init__.py @@ -0,0 +1 @@ +"""Build feature alignment helpers.""" diff --git a/scripts/build_features/_helpers/drift.py b/scripts/build_features/_helpers/drift.py new file mode 100644 index 0000000000..e697e7bdaa --- /dev/null +++ b/scripts/build_features/_helpers/drift.py @@ -0,0 +1,24 @@ +"""Compare the crates the sqb and extension builds share.""" + +from __future__ import annotations + +from scripts.build_features.models import FeatureDrift, ResolvedCrate + + +def find_feature_drift( + *, + target: str, + sqb: dict[ResolvedCrate, frozenset[str]], + extension: dict[ResolvedCrate, frozenset[str]], +) -> tuple[FeatureDrift, ...]: + """Return every crate both builds compile on the same side with different features.""" + return tuple( + FeatureDrift( + target=target, + crate=crate, + sqb_features=sqb[crate], + extension_features=extension[crate], + ) + for crate in sorted(sqb.keys() & extension.keys(), key=lambda c: (c.package, c.side)) + if sqb[crate] != extension[crate] + ) diff --git a/scripts/build_features/_helpers/tree.py b/scripts/build_features/_helpers/tree.py new file mode 100644 index 0000000000..a1a6130cc6 --- /dev/null +++ b/scripts/build_features/_helpers/tree.py @@ -0,0 +1,78 @@ +"""Read the features Cargo resolves for each crate of one build.""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +from scripts.build_features.constants import ( + BUILD_DEPENDENCIES_DEPTH_OFFSET, + BUILD_DEPENDENCIES_HEADING, + HOST_SIDE, + PROC_MACRO_MARKER, + TARGET_SIDE, + TREE_FIELD_SEPARATOR, + TREE_FORMAT, + TREE_GLYPHS, + TREE_INDENT_WIDTH, +) +from scripts.build_features.models import ResolvedCrate + + +def run_cargo_tree(*, root: Path, selection: tuple[str, ...], target: str) -> str: + """Return `cargo tree` output for the selected package on one target, without dedupe.""" + completed: subprocess.CompletedProcess[str] = subprocess.run( + ( + "cargo", + "tree", + "--locked", + *selection, + "--target", + target, + "--edges", + "normal,build", + "--format", + TREE_FORMAT, + "--prefix", + "indent", + "--no-dedupe", + ), + cwd=root, + capture_output=True, + text=True, + check=True, + ) + return completed.stdout + + +def parse_cargo_tree(output: str) -> dict[ResolvedCrate, frozenset[str]]: + """Map each crate to its resolved feature lists, with host compiles apart from target ones.""" + resolved: dict[ResolvedCrate, set[str]] = {} + open_ancestors: tuple[tuple[float, bool], ...] = () + for line in output.splitlines(): + body: str = line.lstrip(TREE_GLYPHS) + depth: int = (len(line) - len(body)) // TREE_INDENT_WIDTH + if body.startswith(BUILD_DEPENDENCIES_HEADING): + heading_depth: float = depth + BUILD_DEPENDENCIES_DEPTH_OFFSET + open_ancestors = ( + *_enclosing(ancestors=open_ancestors, depth=heading_depth), + (heading_depth, True), + ) + continue + if TREE_FIELD_SEPARATOR not in body: + continue + open_ancestors = _enclosing(ancestors=open_ancestors, depth=depth) + package, features = body.split(TREE_FIELD_SEPARATOR, 1) + host: bool = PROC_MACRO_MARKER in package or any(on_host for _, on_host in open_ancestors) + name: str = " ".join(package.replace(PROC_MACRO_MARKER, "").split()[:2]) + crate: ResolvedCrate = ResolvedCrate(package=name, side=HOST_SIDE if host else TARGET_SIDE) + resolved.setdefault(crate, set()).add(features.strip()) + open_ancestors = (*open_ancestors, (depth, host)) + return {crate: frozenset(features) for crate, features in resolved.items()} + + +def _enclosing( + *, ancestors: tuple[tuple[float, bool], ...], depth: float +) -> tuple[tuple[float, bool], ...]: + """Keep the ancestors above `depth`; a build-dependencies heading sits half a level down.""" + return tuple(ancestor for ancestor in ancestors if ancestor[0] < depth) diff --git a/scripts/build_features/constants.py b/scripts/build_features/constants.py new file mode 100644 index 0000000000..19c81e0251 --- /dev/null +++ b/scripts/build_features/constants.py @@ -0,0 +1,22 @@ +"""Stable build feature alignment constants.""" + +SQB_PACKAGE: str = "sqlbuild-cli" +EXTENSION_PACKAGE: str = "sqlbuild-python" +EXTENSION_FEATURES: str = "extension-module" +WHEEL_TARGETS: tuple[str, ...] = ( + "x86_64-unknown-linux-gnu", + "aarch64-unknown-linux-gnu", + "x86_64-apple-darwin", + "aarch64-apple-darwin", + "x86_64-pc-windows-msvc", +) +TREE_FORMAT: str = "{p}|{f}" +TREE_INDENT_WIDTH: int = 4 +TREE_GLYPHS: str = "│├└─ " +TREE_FIELD_SEPARATOR: str = "|" +BUILD_DEPENDENCIES_DEPTH_OFFSET: float = 0.5 +BUILD_DEPENDENCIES_HEADING: str = "[build-dependencies]" +PROC_MACRO_MARKER: str = " (proc-macro)" +HOST_SIDE: str = "host" +TARGET_SIDE: str = "target" +FEATURE_CRATE: str = "crates/sqlbuild-build-features/Cargo.toml" diff --git a/scripts/build_features/main/__init__.py b/scripts/build_features/main/__init__.py new file mode 100644 index 0000000000..5975531d03 --- /dev/null +++ b/scripts/build_features/main/__init__.py @@ -0,0 +1 @@ +"""Build feature alignment entrypoints.""" diff --git a/scripts/build_features/main/check.py b/scripts/build_features/main/check.py new file mode 100644 index 0000000000..5af139afee --- /dev/null +++ b/scripts/build_features/main/check.py @@ -0,0 +1,70 @@ +"""Build feature alignment command entrypoint.""" + +from __future__ import annotations + +import argparse +import sys +from pathlib import Path + +from scripts.build_features._helpers.drift import find_feature_drift +from scripts.build_features._helpers.tree import parse_cargo_tree, run_cargo_tree +from scripts.build_features.constants import ( + EXTENSION_FEATURES, + EXTENSION_PACKAGE, + FEATURE_CRATE, + SQB_PACKAGE, + WHEEL_TARGETS, +) +from scripts.build_features.models import FeatureDrift + + +def run_build_features_check(argv: list[str] | None = None) -> int: + """Fail when the sqb build and the extension build would compile a shared crate twice.""" + parser: argparse.ArgumentParser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--root", type=Path, default=Path.cwd()) + arguments: argparse.Namespace = parser.parse_args(argv) + drift: list[FeatureDrift] = [] + for target in WHEEL_TARGETS: + drift.extend( + find_feature_drift( + target=target, + sqb=parse_cargo_tree( + run_cargo_tree( + root=arguments.root, selection=("--package", SQB_PACKAGE), target=target + ) + ), + extension=parse_cargo_tree( + run_cargo_tree( + root=arguments.root, + selection=( + "--package", + EXTENSION_PACKAGE, + "--features", + EXTENSION_FEATURES, + ), + target=target, + ) + ), + ) + ) + if drift: + print( + f"The {SQB_PACKAGE} and {EXTENSION_PACKAGE} builds resolve shared crates with " + f"different features, so Cargo compiles them twice. Enable the missing features " + f"in {FEATURE_CRATE}:", + file=sys.stderr, + ) + for entry in drift: + print( + f"- {entry.target} {entry.crate.side} {entry.crate.package}: " + f"sqb {_features(entry.sqb_features)}, " + f"extension {_features(entry.extension_features)}", + file=sys.stderr, + ) + return 1 + print(f"The {SQB_PACKAGE} and {EXTENSION_PACKAGE} builds share every common crate.") + return 0 + + +def _features(features: frozenset[str]) -> str: + return " | ".join(sorted(entry or "(none)" for entry in features)) diff --git a/scripts/build_features/models.py b/scripts/build_features/models.py new file mode 100644 index 0000000000..32478b80ff --- /dev/null +++ b/scripts/build_features/models.py @@ -0,0 +1,23 @@ +"""Build feature alignment models.""" + +from __future__ import annotations + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class ResolvedCrate: + """A crate version on one side of a build: compiled for the target or for the host.""" + + package: str + side: str + + +@dataclass(frozen=True) +class FeatureDrift: + """A crate both builds compile with different features, so Cargo compiles it twice.""" + + target: str + crate: ResolvedCrate + sqb_features: frozenset[str] + extension_features: frozenset[str] diff --git a/scripts/check_build_features.py b/scripts/check_build_features.py new file mode 100644 index 0000000000..4b053be774 --- /dev/null +++ b/scripts/check_build_features.py @@ -0,0 +1,14 @@ +"""Direct wrapper for the build feature alignment check.""" + +from __future__ import annotations + +from scripts.build_features.main.check import run_build_features_check + + +def main(argv: list[str] | None = None) -> int: + """Check that the sqb and extension builds share every common crate.""" + return run_build_features_check(argv) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/integration/scripts/build_features/__init__.py b/tests/integration/scripts/build_features/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/integration/scripts/build_features/_test_types.py b/tests/integration/scripts/build_features/_test_types.py new file mode 100644 index 0000000000..0f2216725e --- /dev/null +++ b/tests/integration/scripts/build_features/_test_types.py @@ -0,0 +1,22 @@ +"""Build feature alignment cases over real Cargo workspaces.""" + +from dataclasses import dataclass + + +@dataclass(frozen=True) +class BuildFeaturesTestCase: + """A workspace whose sqb and extension crates use a shared crate and a proc-macro.""" + + description: str + extension_shared_features: tuple[str, ...] + extension_host_features: tuple[str, ...] + expected_exit_code: int + expected_drift: tuple[str, ...] + + +@dataclass(frozen=True) +class RepositoryBuildFeaturesTestCase: + """The repository's own sqb and extension builds.""" + + description: str + expected_exit_code: int diff --git a/tests/integration/scripts/build_features/helpers.py b/tests/integration/scripts/build_features/helpers.py new file mode 100644 index 0000000000..325ef69cc0 --- /dev/null +++ b/tests/integration/scripts/build_features/helpers.py @@ -0,0 +1,56 @@ +"""Write small Cargo workspaces shaped like the sqb and extension crates.""" + +import subprocess +from pathlib import Path + +from tests.integration.scripts.build_features._test_types import BuildFeaturesTestCase + +FEATURES: str = "[features]\nextra = []\nmore = []\n" + + +def _write(path: Path, text: str) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + + +def _path_dependency(name: str, features: tuple[str, ...]) -> str: + listed: str = ", ".join(f'"{feature}"' for feature in features) + return f'{name} = {{ path = "../{name}", features = [{listed}] }}\n' + + +def write_workspace(root: Path, test_case: BuildFeaturesTestCase) -> None: + """Write sqb and extension crates sharing a library and a proc-macro, then lock them.""" + _write( + root / "Cargo.toml", + '[workspace]\nresolver = "2"\nmembers = ["crates/*"]\n', + ) + for name in ("shared", "helper"): + _write( + root / "crates" / name / "Cargo.toml", + f'[package]\nname = "{name}"\nversion = "0.1.0"\nedition = "2021"\n\n{FEATURES}', + ) + _write(root / "crates" / name / "src" / "lib.rs", "") + _write( + root / "crates" / "derive" / "Cargo.toml", + '[package]\nname = "derive"\nversion = "0.1.0"\nedition = "2021"\n\n' + '[lib]\nproc-macro = true\n\n[dependencies]\nhelper = { path = "../helper" }\n', + ) + _write(root / "crates" / "derive" / "src" / "lib.rs", "") + _write( + root / "crates" / "sqlbuild-cli" / "Cargo.toml", + '[package]\nname = "sqlbuild-cli"\nversion = "0.1.0"\nedition = "2021"\n\n' + '[dependencies]\nshared = { path = "../shared" }\nderive = { path = "../derive" }\n', + ) + _write(root / "crates" / "sqlbuild-cli" / "src" / "lib.rs", "") + _write( + root / "crates" / "sqlbuild-python" / "Cargo.toml", + '[package]\nname = "sqlbuild-python"\nversion = "0.1.0"\nedition = "2021"\n\n' + "[features]\nextension-module = []\n\n" + '[dependencies]\nsqlbuild-cli = { path = "../sqlbuild-cli" }\n' + f"{_path_dependency('shared', test_case.extension_shared_features)}\n" + "[build-dependencies]\n" + f"{_path_dependency('helper', test_case.extension_host_features)}", + ) + _write(root / "crates" / "sqlbuild-python" / "src" / "lib.rs", "") + _write(root / "crates" / "sqlbuild-python" / "build.rs", "fn main() {}\n") + subprocess.run(("cargo", "generate-lockfile", "--offline"), cwd=root, check=True) diff --git a/tests/integration/scripts/build_features/test_check.py b/tests/integration/scripts/build_features/test_check.py new file mode 100644 index 0000000000..26e3ba7229 --- /dev/null +++ b/tests/integration/scripts/build_features/test_check.py @@ -0,0 +1,79 @@ +"""Run the build feature alignment check with Cargo over real workspaces.""" + +import shutil +from pathlib import Path + +import pytest + +from scripts.build_features.main.check import run_build_features_check +from tests.integration.scripts.build_features._test_types import ( + BuildFeaturesTestCase, + RepositoryBuildFeaturesTestCase, +) +from tests.integration.scripts.build_features.helpers import write_workspace + +pytestmark: pytest.MarkDecorator = pytest.mark.skipif( + shutil.which("cargo") is None, reason="Cargo is not installed" +) + + +@pytest.mark.parametrize( + "test_case", + [ + BuildFeaturesTestCase( + description="both builds resolve the shared crates identically", + extension_shared_features=(), + extension_host_features=(), + expected_exit_code=0, + expected_drift=(), + ), + BuildFeaturesTestCase( + description="the extension enables a feature of a shared target crate", + extension_shared_features=("extra",), + extension_host_features=(), + expected_exit_code=1, + expected_drift=( + "x86_64-unknown-linux-gnu target shared v0.1.0: sqb (none), extension extra", + ), + ), + BuildFeaturesTestCase( + description="the extension enables a feature of a crate a shared proc-macro uses", + extension_shared_features=(), + extension_host_features=("more",), + expected_exit_code=1, + expected_drift=( + "x86_64-unknown-linux-gnu host helper v0.1.0: sqb (none), extension more", + ), + ), + ], + ids=lambda case: case.description, +) +def test_given_workspace_when_checking_build_features_then_reports_crates_compiled_twice( + tmp_path: Path, capsys: pytest.CaptureFixture[str], test_case: BuildFeaturesTestCase +) -> None: + write_workspace(tmp_path, test_case) + + exit_code: int = run_build_features_check(["--root", str(tmp_path)]) + + errors: str = capsys.readouterr().err + assert exit_code == test_case.expected_exit_code + for drift in test_case.expected_drift: + assert f"- {drift}" in errors + + +@pytest.mark.parametrize( + "test_case", + [ + RepositoryBuildFeaturesTestCase( + description="the repository sqb and extension builds share every common crate", + expected_exit_code=0, + ) + ], + ids=lambda case: case.description, +) +def test_given_repository_when_checking_build_features_then_builds_share_crates( + test_case: RepositoryBuildFeaturesTestCase, +) -> None: + repository: Path = Path(__file__).resolve().parents[4] + + assert run_build_features_check(["--root", str(repository)]) == test_case.expected_exit_code diff --git a/tests/unit/scripts/build_features/__init__.py b/tests/unit/scripts/build_features/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit/scripts/build_features/_helpers/__init__.py b/tests/unit/scripts/build_features/_helpers/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit/scripts/build_features/_helpers/_test_types.py b/tests/unit/scripts/build_features/_helpers/_test_types.py new file mode 100644 index 0000000000..101b705adc --- /dev/null +++ b/tests/unit/scripts/build_features/_helpers/_test_types.py @@ -0,0 +1,10 @@ +from dataclasses import dataclass + +from scripts.build_features.models import ResolvedCrate + + +@dataclass(frozen=True) +class ParseTreeTestCase: + description: str + output: str + expected_crates: dict[ResolvedCrate, frozenset[str]] diff --git a/tests/unit/scripts/build_features/_helpers/test_tree.py b/tests/unit/scripts/build_features/_helpers/test_tree.py new file mode 100644 index 0000000000..8434bf2336 --- /dev/null +++ b/tests/unit/scripts/build_features/_helpers/test_tree.py @@ -0,0 +1,72 @@ +import pytest + +from scripts.build_features._helpers.tree import parse_cargo_tree +from scripts.build_features.models import ResolvedCrate +from tests.unit.scripts.build_features._helpers._test_types import ParseTreeTestCase + +CLI: ResolvedCrate = ResolvedCrate(package="sqlbuild-cli v0.1.0", side="target") + + +@pytest.mark.parametrize( + "test_case", + [ + ParseTreeTestCase( + description="normal dependencies compile for the target", + output=( + "sqlbuild-cli v0.1.0 (/work/crates/sqlbuild-cli)|\n" + "├── memchr v2.8.3|alloc,std\n" + "└── regex v1.12.2|default,std\n" + " └── memchr v2.8.3|alloc,std\n" + ), + expected_crates={ + CLI: frozenset({""}), + ResolvedCrate(package="memchr v2.8.3", side="target"): frozenset({"alloc,std"}), + ResolvedCrate(package="regex v1.12.2", side="target"): frozenset({"default,std"}), + }, + ), + ParseTreeTestCase( + description="proc-macros and their dependencies compile for the host", + output=( + "sqlbuild-cli v0.1.0 (/work/crates/sqlbuild-cli)|\n" + "├── serde v1.0.229|default,derive\n" + "│ └── serde_derive v1.0.229 (proc-macro)|default\n" + "│ └── syn v2.0.119|full\n" + "└── syn v2.0.119|parsing\n" + ), + expected_crates={ + CLI: frozenset({""}), + ResolvedCrate(package="serde v1.0.229", side="target"): frozenset( + {"default,derive"} + ), + ResolvedCrate(package="serde_derive v1.0.229", side="host"): frozenset({"default"}), + ResolvedCrate(package="syn v2.0.119", side="host"): frozenset({"full"}), + ResolvedCrate(package="syn v2.0.119", side="target"): frozenset({"parsing"}), + }, + ), + ParseTreeTestCase( + description="build dependencies compile for the host until the next sibling", + output=( + "sqlbuild-cli v0.1.0 (/work/crates/sqlbuild-cli)|\n" + "├── blake3 v1.8.7|std\n" + "│ ├── cfg-if v1.0.4|\n" + "│ [build-dependencies]\n" + "│ └── cc v1.4.4|\n" + "│ └── shlex v2.0.1|std\n" + "└── libc v0.2.189|std\n" + ), + expected_crates={ + CLI: frozenset({""}), + ResolvedCrate(package="blake3 v1.8.7", side="target"): frozenset({"std"}), + ResolvedCrate(package="cfg-if v1.0.4", side="target"): frozenset({""}), + ResolvedCrate(package="cc v1.4.4", side="host"): frozenset({""}), + ResolvedCrate(package="shlex v2.0.1", side="host"): frozenset({"std"}), + ResolvedCrate(package="libc v0.2.189", side="target"): frozenset({"std"}), + }, + ), + ], + ids=lambda case: case.description, +) +def test_given_cargo_tree_output_when_parsing_then_splits_host_and_target_features( + test_case: ParseTreeTestCase, +) -> None: + assert parse_cargo_tree(test_case.output) == test_case.expected_crates From c4fdda92d93d8895a0271be11710a864fbdd8c54 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 20 Oct 2026 13:40:50 +0100 Subject: [PATCH 2/3] fix: read cargo tree without colour in the build features check --- scripts/build_features/_helpers/tree.py | 2 ++ tests/integration/scripts/build_features/test_check.py | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/build_features/_helpers/tree.py b/scripts/build_features/_helpers/tree.py index a1a6130cc6..0ef1441b53 100644 --- a/scripts/build_features/_helpers/tree.py +++ b/scripts/build_features/_helpers/tree.py @@ -36,6 +36,8 @@ def run_cargo_tree(*, root: Path, selection: tuple[str, ...], target: str) -> st "--prefix", "indent", "--no-dedupe", + "--color", + "never", ), cwd=root, capture_output=True, diff --git a/tests/integration/scripts/build_features/test_check.py b/tests/integration/scripts/build_features/test_check.py index 26e3ba7229..b14d09ae33 100644 --- a/tests/integration/scripts/build_features/test_check.py +++ b/tests/integration/scripts/build_features/test_check.py @@ -49,9 +49,13 @@ ids=lambda case: case.description, ) def test_given_workspace_when_checking_build_features_then_reports_crates_compiled_twice( - tmp_path: Path, capsys: pytest.CaptureFixture[str], test_case: BuildFeaturesTestCase + tmp_path: Path, + capsys: pytest.CaptureFixture[str], + monkeypatch: pytest.MonkeyPatch, + test_case: BuildFeaturesTestCase, ) -> None: write_workspace(tmp_path, test_case) + monkeypatch.setenv("CARGO_TERM_COLOR", "always") exit_code: int = run_build_features_check(["--root", str(tmp_path)]) From 5754573efd37d41b3b4f7e8d49d1f168cfb9d20e Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Tue, 20 Oct 2026 13:42:51 +0100 Subject: [PATCH 3/3] ci: let the macOS sqb wheel build reuse the extension's crates --- .github/workflows/build-distributions.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/build-distributions.yml b/.github/workflows/build-distributions.yml index 0055d23eb5..ce6ed51bfb 100644 --- a/.github/workflows/build-distributions.yml +++ b/.github/workflows/build-distributions.yml @@ -135,6 +135,13 @@ jobs: manylinux: auto - uses: PyO3/maturin-action@v1 if: inputs.prebuilt-run-id == '' + # On macOS maturin links the extension with these flags for every crate, which changes + # their build hash. Passing the same flags here lets sqb reuse the crates the extension + # build just compiled instead of compiling them again; sqb has no undefined symbols, and + # the smoke test below runs it. + env: + RUSTFLAGS: >- + ${{ runner.os == 'macOS' && '-C link-arg=-undefined -C link-arg=dynamic_lookup' || '' }} with: command: build args: >-