From 56f0af94f23ad34c9bf7a84e412a01d776ebfb82 Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Fri, 30 Oct 2026 11:02:20 -0400 Subject: [PATCH] ci: fail when cargo remove drops the dev profile's package entries --- .github/CONTRIBUTING.md | 2 ++ .github/NEW_PROJECT.md | 2 +- tools/policy.sh | 5 ++++- tools/policy/cargo.rego | 23 +++++++++++++++++++++++ tools/policy/cargo_test.rego | 17 +++++++++++++++++ tools/policy/fixtures_test.rego | 6 +++++- 6 files changed, 52 insertions(+), 3 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index b4b20a35c..7516975e3 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -51,6 +51,8 @@ pnpm exec ast-grep test --skip-snapshot-tests && pnpm exec ast-grep scan ast-grep's rules (`sgconfig.yml`, in `tools/ast-grep/rules/` with their cases in `tools/ast-grep/tests/`) include `no-direct-pds-read`, which fails a record read that skips HappyView, in TypeScript, Svelte or Rust. Its exceptions are in the rule. clippy (`clippy.toml`) bans jacquard's typed requests for the same reads. +`cargo remove` also deletes the root `Cargo.toml`'s `[profile.dev.package.*]` entries, with their comments, for every crate no member depends on directly: its clean-up after a removal ([rust-lang/cargo#11194](https://github.com/rust-lang/cargo/pull/11194), since Cargo 1.67) only keeps the entries for direct dependencies. `tools/policy.sh` fails while one is missing and its crate is still in `Cargo.lock`, so put it back with `git checkout -p Cargo.toml`. A new entry goes in `dev_optimised` in `tools/policy/cargo.rego` too. + CI runs a tool's checks only when its files (or the shared ones) change. Each workflow ends with a "… passed" check that reports either way, and those are the checks `main` requires before a merge. On a draft pull request, the Rust workspace's and the website's checks wait until you mark it ready for review, so open a pull request as a draft while it isn't next to merge (a stacked one, say). Their "… passed" checks fail on a draft that has something for them to check, since none of it ran. Marking it ready starts a new run, and if one doesn't show up, re-run the workflow. The slow suites run weekly in the `*-scheduled.yml` workflows, and you can start them on any branch from the Actions tab. A weekly run that fails opens an issue labelled `scheduled-failure`, and the next weekly run that passes closes it. On a pull request, each tool's workflow keeps one comment up to date with its coverage: the Rust workspace's, with the Hegel property tests' alone beside it, and the website's and the extension's against `main`, with each changed file's. The website's and the extension's comments also show how the change moves what they ship against `main`: the website's client JavaScript and CSS, and the extension's zip. A pull request from a fork can't be commented on, so it finds the same in the run's summary. The website's Lighthouse scores are in the run's summary too, when the change touches what its pages are built from. diff --git a/.github/NEW_PROJECT.md b/.github/NEW_PROJECT.md index 162c68602..9eb6528ef 100644 --- a/.github/NEW_PROJECT.md +++ b/.github/NEW_PROJECT.md @@ -4,7 +4,7 @@ This covers what the repository runs on and every place a new project has to be Most of the steps are checked, so a missed one fails CI with a message that says what to add: -- `tools/policy.sh` (the Repo checks workflow) runs Rego policies in `tools/policy/` over the config files with [conftest](https://www.conftest.dev). They check each TypeScript app's scripts, `package.json` fields and git hooks; that every workflow on pull requests (but the benchmarks, the release plan and dependency review) has the `changes`, work and `passed` jobs and no `paths` in its trigger, and that one of them runs when each app, package, crate or plugin changes; that `.github/rulesets/protect-main.json` requires every `passed` check; that the labels, labeler and issue forms know every app and every file gets a label; that Rust packages share the workspace's settings; that lexicons are filed by NSID; and that an app with a Dockerfile has a Railway service in `.railway/railway.ts`. It needs Docker, and Node with the root's dev dependencies locally. The policies' own tests have to cover every line. +- `tools/policy.sh` (the Repo checks workflow) runs Rego policies in `tools/policy/` over the config files with [conftest](https://www.conftest.dev). They check each TypeScript app's scripts, `package.json` fields and git hooks; that every workflow on pull requests (but the benchmarks, the release plan and dependency review) has the `changes`, work and `passed` jobs and no `paths` in its trigger, and that one of them runs when each app, package, crate or plugin changes; that `.github/rulesets/protect-main.json` requires every `passed` check; that the labels, labeler and issue forms know every app and every file gets a label; that Rust packages share the workspace's settings and the dev profile keeps its entries for the crates in `Cargo.lock`; that lexicons are filed by NSID; and that an app with a Dockerfile has a Railway service in `.railway/railway.ts`. It needs Docker, and Node with the root's dev dependencies locally. The policies' own tests have to cover every line. - The website's tests check that every folder in `apps/` has a tool page and is named in the root README, CONTRIBUTING, SECURITY, the website's README and the `web: tools` label (`apps/web/src/lib/tools/docs.test.ts`). - locallibrary's build fails while a folder in `plugins/` isn't one of its dependencies (`apps/locallibrary/build.rs`), and its tests check that every one ships and is in its docs (`apps/locallibrary/tests/it/plugins.rs`). - clippy's `cargo_common_metadata` wants full metadata on every crate that isn't `publish = false`, and the release workflow's plan fails while it would release anything but locallibrary. diff --git a/tools/policy.sh b/tools/policy.sh index 0153a2c8a..4f8cb7103 100755 --- a/tools/policy.sh +++ b/tools/policy.sh @@ -24,6 +24,9 @@ while IFS= read -r file; do files+=("$file"); done < <(list -- package.json pnpm scratch=$(mktemp -d) trap 'rm -rf "$scratch"' EXIT list | jq -R -s '{files: split("\n")[:-1]}' > "$scratch/tracked-files.json" +# Cargo.lock under a name conftest parses as TOML, for the policy that the dev profile keeps its +# entries for the crates still in it. +cp Cargo.lock "$scratch/Cargo.lock.toml" # The Railway project as Railway's SDK evaluates .railway/railway.ts, for the railway policy. node --experimental-strip-types --input-type=module \ -e 'const { default: railway } = await import(process.argv[1]); process.stdout.write(JSON.stringify(railway()));' \ @@ -42,4 +45,4 @@ if [ -n "$uncovered" ]; then echo "$uncovered" exit 1 fi -run "$conftest" test --combine --all-namespaces --no-color --policy tools/policy "${files[@]}" /scratch/tracked-files.json /scratch/railway-project.json +run "$conftest" test --combine --all-namespaces --no-color --policy tools/policy "${files[@]}" /scratch/tracked-files.json /scratch/Cargo.lock.toml /scratch/railway-project.json diff --git a/tools/policy/cargo.rego b/tools/policy/cargo.rego index 62cba8118..01775e780 100644 --- a/tools/policy/cargo.rego +++ b/tools/policy/cargo.rego @@ -58,3 +58,26 @@ deny contains msg if { object.get(lib.file(path)["package"], "readme", "") != "README.md" msg := sprintf("%s: set `readme = \"README.md\"`, the crate's docs", [path]) } + +# The crates the dev profile optimises, each a `[profile.dev.package.]` in Cargo.toml (which +# says why). `cargo remove` deletes the entries for crates no member depends on directly +# (.github/CONTRIBUTING.md, "Checks"), so they're listed here as well. Only a crate still in +# Cargo.lock has to keep its entry, so one that really goes away doesn't trip this. +dev_optimised := {"aes", "blake3", "clap_builder", "hmac", "hybrid-array", "inout", "jsonschema", "referencing", "ring", "sha2", "symphonia-codec-pcm", "symphonia-core"} + +# tools/policy.sh gives conftest Cargo.lock as /scratch/Cargo.lock.toml, so it parses it as TOML. +locked := {p.name | some p in lib.file("/scratch/Cargo.lock.toml")["package"]} + +dev_packages := object.get(lib.file("Cargo.toml"), ["profile", "dev", "package"], {}) + +deny contains msg if { + some name in (dev_optimised & locked) + not dev_packages[name] + msg := sprintf("Cargo.toml: put back [profile.dev.package.%s]; `cargo remove` deletes it (git diff Cargo.toml shows what went)", [name]) +} + +deny contains msg if { + some name, _ in dev_packages + not name in dev_optimised + msg := sprintf("Cargo.toml: add \"%s\" to dev_optimised in tools/policy/cargo.rego, so the policies notice if it goes missing", [name]) +} diff --git a/tools/policy/cargo_test.rego b/tools/policy/cargo_test.rego index 1072548bf..48b45d94d 100644 --- a/tools/policy/cargo_test.rego +++ b/tools/policy/cargo_test.rego @@ -53,3 +53,20 @@ test_crate_without_readme if { broken := json.remove(crate, ["package/readme"]) "crates/loclib-x/Cargo.toml: set `readme = \"README.md\"`, the crate's docs" in cargo.deny with input as fixtures.with_file("crates/loclib-x/Cargo.toml", broken) } + +without_dev_package(name) := fixtures.with_file("Cargo.toml", json.remove(fixtures.file("Cargo.toml"), [sprintf("profile/dev/package/%s", [name])])) + +test_dev_package_dropped if { + "Cargo.toml: put back [profile.dev.package.sha2]; `cargo remove` deletes it (git diff Cargo.toml shows what went)" in cargo.deny with input as without_dev_package("sha2") +} + +test_dev_package_gone_from_lock if { + lock := {"path": "/scratch/Cargo.lock.toml", "contents": {"package": [{"name": "tool"}]}} + count(cargo.deny) == 0 with input as array.concat([f | some f in without_dev_package("sha2"); f.path != lock.path], [lock]) +} + +test_dev_package_not_listed if { + crate := fixtures.file("Cargo.toml") + broken := json.patch(crate, [{"op": "add", "path": "/profile/dev/package/digest", "value": {"opt-level": 3}}]) + "Cargo.toml: add \"digest\" to dev_optimised in tools/policy/cargo.rego, so the policies notice if it goes missing" in cargo.deny with input as fixtures.with_file("Cargo.toml", broken) +} diff --git a/tools/policy/fixtures_test.rego b/tools/policy/fixtures_test.rego index fa92ba0e0..0c826c238 100644 --- a/tools/policy/fixtures_test.rego +++ b/tools/policy/fixtures_test.rego @@ -47,7 +47,11 @@ repo := [ "deploy": {"healthcheckPath": "/healthz", "multiRegionConfig": {"us-east4": {"numReplicas": 2}}}, }]}}, {"path": "apps/tool/Cargo.toml", "contents": {"package": object.union({"name": "tool"}, package_fields), "lints": {"workspace": true}}}, - {"path": "Cargo.toml", "contents": {"workspace": {"members": ["apps/tool", "crates/*", "plugins/*", "vendor/pdb"]}}}, + {"path": "Cargo.toml", "contents": { + "workspace": {"members": ["apps/tool", "crates/*", "plugins/*", "vendor/pdb"]}, + "profile": {"dev": {"package": {"sha2": {"opt-level": 3}}}}, + }}, + {"path": "/scratch/Cargo.lock.toml", "contents": {"package": [{"name": "sha2"}, {"name": "tool"}]}}, {"path": "crates/loclib-x/Cargo.toml", "contents": {"package": object.union({"name": "loclib-x", "readme": "README.md"}, package_fields), "lints": {"workspace": true}}}, {"path": "plugins/p/Cargo.toml", "contents": {"package": object.union({"name": "locallibrary-plugin-p", "publish": false}, package_fields), "lints": {"workspace": true}}}, {"path": "vendor/pdb/Cargo.toml", "contents": {"package": {"name": "pdb", "edition": "2021", "rust-version": {"workspace": true}}, "lints": {"workspace": true}}},