Add tools/pr_ready.py, CONTRIBUTING.md and a pull request template
pr_ready is the one self-check to run before asking for review: from the files a branch changes it runs rustfmt, clippy with warnings denied, the tests of the touched crates (mapped through cargo metadata), the launcher's own workspace when it is affected, the tokens.css check, the Cargo.lock rule, the Python tests and the public-tree check, and the vanilla harnesses the change can affect, picked by tools/parity.py exactly as CI's parity job picks them. It runs locally or on a fleet machine through ~/fleet/rbuild; on the Mac mini the harnesses use the parity runner's own workspace and scripts. It prints one Markdown block for the pull request body and exits non-zero on any failure. tools/parity.py gains --changed-files for copies without Git history, and tools/ci/vanilla-data.sh accepts the preparation key from outside. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Ludovic Marie
@@ -0,0 +1,19 @@
|
||||
## What and why
|
||||
|
||||
<!-- What this changes, and why. Link the issue it fixes ("Fixes #123"). -->
|
||||
|
||||
## Vanilla reference
|
||||
|
||||
<!-- The vanilla classes and methods this follows (tools/rea_vanilla.py), and the harness that checks it.
|
||||
"Horizon-only" for a feature vanilla doesn't have; "none" for tooling or docs. -->
|
||||
|
||||
## Self-check
|
||||
|
||||
<!-- Paste the block `python3 tools/pr_ready.py` prints (`--base upstream/main` from a fork). -->
|
||||
|
||||
## Screenshots
|
||||
|
||||
<!-- For anything players see: before and after. Delete this section otherwise. -->
|
||||
|
||||
<!-- Please tick "Allow edits by maintainers": we push small fixups ourselves instead of a round trip.
|
||||
See CONTRIBUTING.md. -->
|
||||
@@ -0,0 +1,77 @@
|
||||
# Contributing to Horizon
|
||||
|
||||
Thanks for helping. Horizon is Minecraft: Java Edition 26.3 rewritten in Rust, and the one rule that shapes
|
||||
everything is that **it plays exactly like vanilla**. This page is the short version of how we work; questions are
|
||||
welcome on [Discord](https://discord.gg/HcTeMZxe5E).
|
||||
|
||||
## Build it
|
||||
|
||||
You need Rust (stable), Python 3.10+, JDK 25, and on Linux the ALSA and udev headers (`libasound2-dev libudev-dev`).
|
||||
Horizon never downloads the game: it prepares its resources from your own copy of Minecraft 26.3, as described in
|
||||
[Build from source](README.md#build-from-source).
|
||||
|
||||
```sh
|
||||
cargo run --release -p horizon-prepare -- --prepare --server-jar ~/Downloads/server.jar
|
||||
cargo run --release -p horizon-client
|
||||
```
|
||||
|
||||
The launcher is its own Cargo workspace in `launcher/` (`cd launcher && cargo run`; on NixOS, `nix-shell` first).
|
||||
|
||||
## Vanilla first
|
||||
|
||||
Wherever vanilla has a feature, vanilla is the specification, down to the order of random calls. "Parity" here
|
||||
means Horizon gives the same results as the pinned 26.3 client and server for the same inputs, checked by a
|
||||
**harness**: a `tools/test_*_parity.py` script that runs a small Java oracle against the real game and a Rust
|
||||
probe against Horizon, and compares their outputs. There are about 290 of them.
|
||||
|
||||
- **Read the vanilla code before writing Horizon's.** `tools/rea_vanilla.py` queries the pinned game bytecode:
|
||||
`python3 tools/rea_vanilla.py method net.minecraft.world.level.block.DispenserBlock dispenseFrom`.
|
||||
Say in your pull request which vanilla classes and methods you followed.
|
||||
- **Gameplay changes come with a harness**, new or extended, so the next change can't silently break them.
|
||||
[The parity workflow](docs/parity-workflow.md) shows how harnesses run; copy a nearby one.
|
||||
- **Horizon's own features** (the launcher, the new menus) follow [the design system](docs/design-system.md) and are
|
||||
off by default wherever they would change the vanilla look.
|
||||
- **Everything players run is Rust.** Python and Java are fine for dev tools and harness drivers in `tools/`, never
|
||||
for player-facing code.
|
||||
|
||||
Never commit game files, decompiled source or prepared resources (`.local/`): `tools/check_public_tree.py` rejects
|
||||
them (`git config core.hooksPath .githooks` runs it before every commit).
|
||||
|
||||
## Before asking for review
|
||||
|
||||
Run the self-check and paste the block it prints into your pull request:
|
||||
|
||||
```sh
|
||||
python3 tools/pr_ready.py --base upstream/main # or origin/main if you work on the main repository
|
||||
```
|
||||
|
||||
It looks at what your branch changes and runs what matters: rustfmt, clippy with warnings denied, the tests of the
|
||||
crates you touched (and the launcher's), generated files, `Cargo.lock`, the Python tests, and the vanilla harnesses
|
||||
your change can affect. Without the vanilla data on your machine it lists those harnesses instead; CI runs them.
|
||||
`--quick` skips the harnesses. It exits non-zero when something fails.
|
||||
|
||||
## Pull requests
|
||||
|
||||
- **Small.** One change per pull request, under about 500 lines of code where you can; split bigger work into
|
||||
pieces that each make sense alone. Small pull requests get reviewed and merged the same day.
|
||||
- **Fill in the template**: what and why, the vanilla reference, the pr_ready block, screenshots for anything you
|
||||
can see.
|
||||
- **Tick "Allow edits by maintainers".** When something small is left (a lint, a rebase, a nit) we push the fixup
|
||||
ourselves instead of sending it back to you; only design questions come back.
|
||||
- **Labels** (we set them if you can't): `feature` when players get something new (it appears under "New" in the
|
||||
release notes), `skip-release` when players see no change (tooling, docs, tests), nothing for a fix.
|
||||
`release-now` is for crash fixes that shouldn't wait. pr_ready suggests one.
|
||||
|
||||
Every merge to main is released automatically ([release flow](docs/release-flow.md)), so main always stays playable.
|
||||
|
||||
## Parity checks on forks
|
||||
|
||||
The `verify` check (formatting, lints, tests) runs on every pull request. The `parity` check runs the affected
|
||||
harnesses on a maintainer's machine, which holds the game files, so it never runs a fork's code on its own: a
|
||||
maintainer reads your change, then adds the `run-parity` label, which runs it once on your current commit. After
|
||||
new pushes, we add it again.
|
||||
|
||||
## Talk to us
|
||||
|
||||
[Discord](https://discord.gg/HcTeMZxe5E) for questions and ideas, [issues](https://github.com/ludovic111/horizon/issues)
|
||||
for bugs. If you want to take something on, say so in its issue so nobody else starts the same work.
|
||||
@@ -186,18 +186,14 @@ Online-mode servers (Mojang's session servers) are not supported; skins served o
|
||||
|
||||
## Contributing
|
||||
|
||||
Issues and pull requests are welcome. Before opening a pull request, run the offline checks:
|
||||
Issues and pull requests are welcome: see [CONTRIBUTING.md](CONTRIBUTING.md) for how we work (vanilla first, small pull requests, labels). Before opening a pull request, run the self-check and paste the block it prints:
|
||||
|
||||
```sh
|
||||
git config core.hooksPath .githooks
|
||||
python3 -m unittest discover -s tests -v
|
||||
cargo fmt --all -- --check
|
||||
cargo clippy --workspace --all-targets -- -D warnings
|
||||
cargo test --workspace
|
||||
python3 tools/check_public_tree.py --history
|
||||
python3 tools/pr_ready.py
|
||||
```
|
||||
|
||||
CI runs these checks on GitHub's Linux runners. The vanilla comparisons need the game, which Horizon never downloads, so they run only when started by hand on a self-hosted runner holding the maintainer's own copy. Never commit game files, decompiled source or prepared resources: `tools/check_public_tree.py` rejects them, and tracked Java is allowed only under the project's own `dev/horizonlab` namespace.
|
||||
It runs rustfmt, clippy with warnings denied, the tests of the crates you touched, the Python tests, the public-tree check, and the vanilla harnesses your change can affect. CI runs the same checks on GitHub's Linux runners; the vanilla harnesses need the game, which Horizon never downloads, so they run on a self-hosted runner holding the maintainer's own copy. Never commit game files, decompiled source or prepared resources: `tools/check_public_tree.py` rejects them, and tracked Java is allowed only under the project's own `dev/horizonlab` namespace.
|
||||
|
||||
## Licence
|
||||
|
||||
|
||||
@@ -16,6 +16,8 @@ python3 tools/parity.py --repeat 3 food # run it three times; the runs must ag
|
||||
python3 tools/parity.py --hosts macmini,thinkpad # with other machines (default: tools/parity_hosts.json's default list, empty)
|
||||
python3 tools/parity.py --list # what would run
|
||||
python3 tools/parity.py --status # last result of each harness
|
||||
python3 tools/parity.py --changed origin/main # those a branch's changes can affect (CI's parity job)
|
||||
python3 tools/parity.py --changed-files paths.txt # the same for listed paths, in a copy without Git (tools/pr_ready.py)
|
||||
```
|
||||
|
||||
Each harness runs unmodified under a tracer that records the files it depended on: its Python modules, its Java oracle and the oracle sources javac pulled in, other repository files it read, and the Rust sources of the probes it ran (from Cargo's `.d` dep-info files). A pass stands until one of those files, the pinned vanilla artifacts, `Cargo.lock` or the Rust/Java toolchains change. State and logs are in `.local/parity-runner/`; each harness's own outputs stay in its `.local/<name>/` directory.
|
||||
|
||||
@@ -0,0 +1,116 @@
|
||||
import argparse
|
||||
import unittest
|
||||
from unittest import mock
|
||||
|
||||
from tools import pr_ready
|
||||
|
||||
PACKAGES = {"horizon-core": "crates/horizon-core", "horizon-client": "crates/horizon-client",
|
||||
"horizon-theme": "crates/horizon-theme", "xtask": "xtask"}
|
||||
|
||||
|
||||
def args(host="local", quick=False):
|
||||
return argparse.Namespace(host=host, quick=quick)
|
||||
|
||||
|
||||
class PrReadyTests(unittest.TestCase):
|
||||
def test_paths_map_to_the_crates_that_own_them(self):
|
||||
self.assertEqual(pr_ready.touched_crates({"crates/horizon-core/src/jmath.rs", "crates/horizon-client/build.rs",
|
||||
"xtask/src/main.rs", "tools/parity.py"}, PACKAGES),
|
||||
["horizon-client", "horizon-core", "xtask"])
|
||||
self.assertEqual(pr_ready.touched_crates({"crates/horizon-core/README.md"}, PACKAGES), [])
|
||||
# The generated stylesheet is checked by horizon-theme's tests.
|
||||
self.assertEqual(pr_ready.touched_crates({"website/src/tokens.css"}, PACKAGES), ["horizon-theme"])
|
||||
# A crate whose name prefixes another's directory doesn't own it.
|
||||
self.assertEqual(pr_ready.touched_crates({"crates/horizon-core-extra/x.rs"}, PACKAGES), [])
|
||||
# The workspace manifest and lock touch every crate.
|
||||
self.assertIsNone(pr_ready.touched_crates({"Cargo.lock", "crates/horizon-core/src/lib.rs"}, PACKAGES))
|
||||
|
||||
def test_which_workspace_builds_a_path(self):
|
||||
self.assertTrue(pr_ready.workspace_rust("crates/horizon-theme/fonts/x.ttf"))
|
||||
self.assertTrue(pr_ready.workspace_rust("Cargo.toml"))
|
||||
self.assertFalse(pr_ready.workspace_rust("crates/horizon-client/README.md"))
|
||||
self.assertFalse(pr_ready.workspace_rust("launcher/src/main.rs"))
|
||||
self.assertTrue(pr_ready.launcher_rust("launcher/src/main.rs"))
|
||||
self.assertTrue(pr_ready.launcher_rust("crates/horizon-prepare/src/lib.rs"))
|
||||
self.assertTrue(pr_ready.launcher_rust("crates/horizon-theme/src/lib.rs"))
|
||||
self.assertFalse(pr_ready.launcher_rust("launcher/links.json"))
|
||||
self.assertFalse(pr_ready.launcher_rust("launcher/README.md"))
|
||||
self.assertFalse(pr_ready.launcher_rust("crates/horizon-client/src/game.rs"))
|
||||
|
||||
def test_scope_warns_on_size_and_mixed_areas(self):
|
||||
self.assertEqual(pr_ready.scope_warnings({"crates/horizon-client/src/game.rs", "tools/test_food_parity.py",
|
||||
"mods/horizon/src/test/java/dev/horizonlab/parity/X.java",
|
||||
"docs/native-client.md"}, 120), [])
|
||||
self.assertEqual(pr_ready.scope_warnings({"crates/horizon-theme/src/lib.rs", "launcher/src/theme.rs"}, 10), [])
|
||||
warnings = pr_ready.scope_warnings({"crates/horizon-client/src/game.rs", "launcher/src/main.rs",
|
||||
".github/workflows/ci.yml"}, 900)
|
||||
self.assertEqual(len(warnings), 2)
|
||||
self.assertIn("900 lines of code: over 500", warnings[0])
|
||||
self.assertEqual(warnings[1], "mixes CI, game, launcher")
|
||||
|
||||
def test_documentation_and_tests_are_not_counted_in_the_size(self):
|
||||
self.assertTrue(pr_ready.reviewed("tools/pr_ready.py"))
|
||||
self.assertTrue(pr_ready.reviewed("crates/horizon-client/src/game.rs"))
|
||||
self.assertFalse(pr_ready.reviewed("CONTRIBUTING.md"))
|
||||
self.assertFalse(pr_ready.reviewed("tests/test_pr_ready.py"))
|
||||
self.assertFalse(pr_ready.reviewed("tools/test_food_parity.py"))
|
||||
self.assertFalse(pr_ready.reviewed("crates/horizon-core/tests/jmath.rs"))
|
||||
|
||||
def test_cargo_lock_changes_only_with_a_manifest(self):
|
||||
self.assertIsNone(pr_ready.lock_problem({"crates/horizon-core/src/lib.rs"}))
|
||||
self.assertIsNone(pr_ready.lock_problem({"Cargo.lock", "crates/horizon-client/Cargo.toml"}))
|
||||
self.assertIsNone(pr_ready.lock_problem({"launcher/Cargo.lock", "crates/horizon-theme/Cargo.toml"}))
|
||||
self.assertIn("Cargo.lock", pr_ready.lock_problem({"Cargo.lock", "crates/horizon-core/src/lib.rs"}))
|
||||
|
||||
def test_label_follows_what_players_get(self):
|
||||
self.assertEqual(pr_ready.suggested_label({"tools/pr_ready.py", "CONTRIBUTING.md"}), "`skip-release`")
|
||||
self.assertEqual(pr_ready.suggested_label({"crates/horizon-world/examples/probe.rs"}), "`skip-release`")
|
||||
self.assertIn("feature", pr_ready.suggested_label({"crates/horizon-client/src/game.rs"}))
|
||||
self.assertIn("feature", pr_ready.suggested_label({"launcher/src/main.rs"}))
|
||||
|
||||
def test_affected_harnesses_come_from_the_parity_runner(self):
|
||||
found = {"food": "tools/test_food_parity.py", "datafix#1": "tools/test_datafix_parity.py",
|
||||
"datafix#2": "tools/test_datafix_parity.py"}
|
||||
with mock.patch.object(pr_ready.parity, "harnesses", return_value=found), \
|
||||
mock.patch.object(pr_ready.parity, "load_state", return_value={"harnesses": {
|
||||
"food": {"dependencies": ["crates/horizon-world/src/food.rs"]},
|
||||
"datafix#1": {"dependencies": ["crates/horizon-world/src/datafix.rs"]},
|
||||
"datafix#2": {"dependencies": ["crates/horizon-world/src/datafix.rs"]}}}), \
|
||||
mock.patch.object(pr_ready.parity, "ROOT", pr_ready.parity.Path(".")):
|
||||
self.assertEqual(pr_ready.affected_here({"crates/horizon-world/src/datafix.rs"}), ["datafix"])
|
||||
self.assertEqual(pr_ready.affected_here({"README.md"}), [])
|
||||
self.assertEqual(pr_ready.affected_here({"Cargo.lock"}), ["food", "datafix"])
|
||||
|
||||
def test_harnesses_are_listed_when_they_cannot_run(self):
|
||||
changed = {"crates/horizon-world/src/food.rs"}
|
||||
with mock.patch.object(pr_ready, "affected_here", return_value=["food", "light"]):
|
||||
steps = pr_ready.plan(args(quick=True), "origin/main", changed, {"horizon-world": "crates/horizon-world"})
|
||||
harness = steps[-1]
|
||||
self.assertEqual(harness.status, "list")
|
||||
self.assertIn("2 affected, to run in CI (--quick): food, light", harness.detail)
|
||||
by_name = {step.name: step for step in steps}
|
||||
self.assertIn("tests: horizon-world", by_name)
|
||||
self.assertEqual(by_name["launcher clippy"].status, "skip")
|
||||
self.assertIsNone(by_name["clippy"].status)
|
||||
with mock.patch.object(pr_ready, "affected_here", return_value=["food"]):
|
||||
steps = pr_ready.plan(args(host="macmini"), "origin/main", changed, {"horizon-world": "crates/horizon-world"})
|
||||
self.assertIn("--changed-files", steps[-1].command)
|
||||
self.assertIn("crates/horizon-world/src/food.rs", steps[-1].command)
|
||||
|
||||
def test_summaries_of_check_output(self):
|
||||
output = "test result: ok. 3 passed; 0 failed;\ntest result: FAILED. 4 passed; 1 failed;\n"
|
||||
self.assertEqual(pr_ready.tests_passed(output), "7 passed, 1 failed")
|
||||
self.assertEqual(pr_ready.unittest_passed("....\nRan 12 tests in 1.0s\n\nOK\n"), "12 passed")
|
||||
self.assertEqual(pr_ready.unittest_passed("Ran 12 tests in 1.0s\n\nFAILED (failures=2)\n"),
|
||||
"12 run, FAILED (failures=2)")
|
||||
parity_output = ("3 files changed: 2 harnesses depend on them, 2 without a pass for their current inputs.\n"
|
||||
"\n1 passed, 1 failed in 2.0 min.\n light: /x/.local/parity-runner/logs/light.log\n")
|
||||
self.assertEqual(pr_ready.parity_summary(parity_output), "2 affected, 1 passed, failed: light")
|
||||
self.assertEqual(pr_ready.parity_summary("2 files changed: 0 harnesses depend on them, 0 without a pass for "
|
||||
"their current inputs.\nNothing to run: every harness passed."),
|
||||
"no harness affected")
|
||||
self.assertEqual(pr_ready.duration(125), "2 min 5 s")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -30,7 +30,8 @@ for entry in "$shared"/*; do
|
||||
if [ -L ".local/$name" ] || [ ! -e ".local/$name" ]; then ln -sfn "$entry" ".local/$name"; fi
|
||||
done
|
||||
|
||||
key=$(git ls-files -s crates/horizon-prepare mods/horizon/src/test/java research-lock.json | shasum -a 256 | cut -c1-16)
|
||||
# HORIZON_PREPARE_KEY: the same key, computed where the Git history is (tools/pr_ready.py on another machine).
|
||||
key=${HORIZON_PREPARE_KEY:-$(git ls-files -s crates/horizon-prepare mods/horizon/src/test/java research-lock.json | shasum -a 256 | cut -c1-16)}
|
||||
prepared="$shared/prepared/$key"
|
||||
# Two runners share the vanilla data (~/.actions-runners/horizon-2/_work/horizon/shared-local links to the first
|
||||
# one's): one preparation at a time. A lock older than 30 minutes belongs to a job that died.
|
||||
|
||||
+9
-3
@@ -7,6 +7,7 @@
|
||||
python3 tools/parity.py food light # these harnesses (names without test_/_parity.py)
|
||||
python3 tools/parity.py --failed # the ones that failed last time
|
||||
python3 tools/parity.py --changed origin/main # those a branch's changes can affect (pull requests)
|
||||
python3 tools/parity.py --changed-files F # the same, for paths listed in F (a copy without Git history)
|
||||
python3 tools/parity.py --list # what would run, without running it
|
||||
python3 tools/parity.py --status # last result of every harness
|
||||
|
||||
@@ -1148,6 +1149,8 @@ def main():
|
||||
parser.add_argument("--flake-rounds", type=int, default=3, help="runs required by --flake-check (default 3)")
|
||||
parser.add_argument("--changed", metavar="REF", help="only harnesses the changes since the merge base with REF can "
|
||||
"affect (and that have not passed with their current inputs): the pull-request check")
|
||||
parser.add_argument("--changed-files", metavar="FILE", help="like --changed, for the changed paths listed in FILE, "
|
||||
"one per line: for a copy without Git history (tools/pr_ready.py on another machine)")
|
||||
args = parser.parse_args()
|
||||
|
||||
found = harnesses()
|
||||
@@ -1175,13 +1178,16 @@ def main():
|
||||
chosen = list(found)
|
||||
elif args.failed:
|
||||
chosen = [name for name in found if records.get(name, {}).get("status") == "fail"]
|
||||
elif args.changed:
|
||||
changed = changed_files(args.changed)
|
||||
elif args.changed or args.changed_files:
|
||||
if args.changed_files:
|
||||
changed, since = set(Path(args.changed_files).read_text().split("\n")) - {""}, ""
|
||||
else:
|
||||
changed, since = changed_files(args.changed), f" since the merge base with {args.changed}"
|
||||
if changed is None:
|
||||
parser.error(f"no merge base with {args.changed}")
|
||||
affected = affected_harnesses(found, records, changed)
|
||||
chosen = [name for name in found if name in affected and stale(name, state, hasher, environment)]
|
||||
print(f"{len(changed)} files changed since the merge base with {args.changed}: {len(affected)} harnesses "
|
||||
print(f"{len(changed)} files changed{since}: {len(affected)} harnesses "
|
||||
f"depend on them, {len(chosen)} without a pass for their current inputs.", flush=True)
|
||||
else:
|
||||
chosen = [name for name in found if stale(name, state, hasher, environment)]
|
||||
|
||||
Executable
+406
@@ -0,0 +1,406 @@
|
||||
#!/usr/bin/env python3
|
||||
"""The self-check to run before asking for review: one command, one block to paste into the pull request.
|
||||
|
||||
python3 tools/pr_ready.py # on this machine, against origin/main
|
||||
python3 tools/pr_ready.py --base upstream/main # a fork: against the main repository's main
|
||||
python3 tools/pr_ready.py --host macmini # builds and harnesses on a fleet machine (~/fleet/rbuild)
|
||||
python3 tools/pr_ready.py --quick # without running the vanilla harnesses
|
||||
|
||||
From the files the branch changes since its merge base with --base (committed or not) it decides what to run:
|
||||
rustfmt, clippy with warnings denied and the tests of the touched crates; the launcher (its own workspace) the
|
||||
same way when it, horizon-prepare or horizon-theme changed; website/src/tokens.css against horizon-theme;
|
||||
Cargo.lock changing only with a Cargo.toml; the Python tests and the public-tree check; and the vanilla
|
||||
harnesses the changes can affect, picked as CI's parity job picks them (tools/parity.py --changed). Harnesses run
|
||||
where the vanilla data is (a prepared .local/ here, or the Mac mini); elsewhere they are listed for CI.
|
||||
|
||||
It prints one Markdown block for the pull request's body (stdout) and exits non-zero when a check fails.
|
||||
Progress, and the end of a failed check's output, go to stderr (--verbose: all of it).
|
||||
"""
|
||||
|
||||
import argparse
|
||||
import hashlib
|
||||
import json
|
||||
import os
|
||||
from pathlib import Path
|
||||
import re
|
||||
import shlex
|
||||
import socket
|
||||
import subprocess
|
||||
import sys
|
||||
import time
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
sys.path.insert(0, str(ROOT))
|
||||
FLEET_SHARED = Path.home() / "code" / "horizon-parity" / "shared-local" / "parity-shared"
|
||||
if "PARITY_SHARED" not in os.environ and not (ROOT / ".local" / "minecraft").exists() and FLEET_SHARED.is_dir():
|
||||
# A fleet worktree without vanilla data still knows each harness's dependencies.
|
||||
os.environ["PARITY_SHARED"] = str(FLEET_SHARED)
|
||||
from tools import parity, release_train # noqa: E402
|
||||
|
||||
RBUILD = Path.home() / "fleet" / "rbuild"
|
||||
LINE_LIMIT = 500
|
||||
# What rbuild's login shell needs on each fleet machine before cargo (it adds sccache itself). The ThinkPad hosts
|
||||
# the Claude account pool: three jobs at most.
|
||||
HOST_SETUP = {
|
||||
"macmini": "source ~/.cargo/env; export PATH=/opt/homebrew/bin:$PATH CARGO_BUILD_JOBS=${CARGO_BUILD_JOBS:-10}",
|
||||
"thinkpad": 'export CARGO_BUILD_JOBS=3 PKG_CONFIG_PATH="$(ls -d /nix/store/*-alsa-lib-*-dev/lib/pkgconfig | tail -1):'
|
||||
'$(ls -d /nix/store/*-systemd-*-dev/lib/pkgconfig | tail -1)"',
|
||||
"local": "",
|
||||
}
|
||||
# Hosts that hold the vanilla data, and how a run there gets it: CI's own runner scripts on the parity runner's
|
||||
# workspace (as its second runner does), so the vanilla data, preparations, recorded harness dependencies, passes and
|
||||
# machine slots are CI's; the parity state and probe builds stay per checkout.
|
||||
HARNESS_SETUP = {
|
||||
"macmini": """export RUNNER_WORKSPACE="$HOME/.actions-runners/horizon/_work/horizon" RUNNER_TEMP="$(mktemp -d)"
|
||||
export GITHUB_PATH="$RUNNER_TEMP/path" GITHUB_ENV="$RUNNER_TEMP/env"
|
||||
bash tools/ci/runner-env.sh || exit 1
|
||||
while read -r entry; do PATH="$entry:$PATH"; done < "$GITHUB_PATH"; set -a; . "$GITHUB_ENV"; set +a
|
||||
bash tools/ci/vanilla-data.sh || exit 1
|
||||
state="$HOME/.cache/pr-ready/$(basename "$PWD")"
|
||||
mkdir -p "$state/parity-runner" "$state/parity-target"
|
||||
ln -sfn "$state/parity-runner" .local/parity-runner && ln -sfn "$state/parity-target" .local/parity-target""",
|
||||
}
|
||||
ROOT_RUST = {"Cargo.toml", "Cargo.lock", "rust-toolchain.toml", "rustfmt.toml", "clippy.toml"}
|
||||
TOKENS = "website/src/tokens.css"
|
||||
LAUNCHER_FEATURES = """export features=""
|
||||
if [ "$(uname)" = Darwin ] && ! xcrun metal --version > /dev/null 2>&1; then features="--features gpui/runtime_shaders"; fi
|
||||
in_shell() { if [ "$(uname)" = Linux ] && ! pkg-config --exists xkbcommon 2>/dev/null && command -v nix-shell > /dev/null; then
|
||||
nix-shell shell.nix --run "$1"; else bash -c "$1"; fi; }"""
|
||||
|
||||
|
||||
def is_doc(path):
|
||||
return path.endswith(".md")
|
||||
|
||||
|
||||
def workspace_rust(path):
|
||||
"""Whether a change to path is built by the main Cargo workspace."""
|
||||
return not is_doc(path) and (path.startswith(("crates/", "xtask/", ".cargo/")) or path in ROOT_RUST)
|
||||
|
||||
|
||||
def launcher_rust(path):
|
||||
"""Whether a change to path is built into the launcher (its own workspace)."""
|
||||
return not is_doc(path) and any(path.startswith(p) for p in release_train.LAUNCHER_PATHS) and \
|
||||
path != "launcher/links.json" and not path.startswith("launcher/docs/")
|
||||
|
||||
|
||||
def touched_crates(paths, packages):
|
||||
"""Workspace packages to test for these paths ({name: directory}), or None for the whole workspace."""
|
||||
if any(path in ("Cargo.toml", "Cargo.lock") or path.startswith(".cargo/") for path in paths):
|
||||
return None
|
||||
crates = set()
|
||||
for path in paths:
|
||||
if path == TOKENS:
|
||||
path = packages.get("horizon-theme", "crates/horizon-theme") + "/"
|
||||
if is_doc(path):
|
||||
continue
|
||||
owners = [name for name, directory in packages.items() if path.startswith(directory.rstrip("/") + "/")]
|
||||
if owners:
|
||||
crates.add(max(owners, key=lambda name: len(packages[name])))
|
||||
return sorted(crates)
|
||||
|
||||
|
||||
def area(path):
|
||||
"""The part of the project a path belongs to; None for what goes with any change (docs, tests, harnesses)."""
|
||||
if is_doc(path) or path.startswith(("tests/", "mods/", "docs/")) or path == "research-lock.json" or \
|
||||
(path.startswith("tools/") and not path.startswith("tools/ci/")):
|
||||
return None
|
||||
if path.startswith(("launcher/", "crates/horizon-prepare/")):
|
||||
return "launcher"
|
||||
if path.startswith("website/"):
|
||||
return "website"
|
||||
if path.startswith((".github/", "tools/ci/")):
|
||||
return "CI"
|
||||
if path.startswith("crates/horizon-theme/"):
|
||||
return None # the design system feeds the game, the launcher and the website alike
|
||||
return "game"
|
||||
|
||||
|
||||
def reviewed(path):
|
||||
"""Whether a path's lines count towards a pull request's size: documentation and tests don't."""
|
||||
return not is_doc(path) and not path.startswith("tests/") and not re.search(r"(^|/)tests?/|/test_\w+\.py$", path)
|
||||
|
||||
|
||||
def scope_warnings(paths, lines):
|
||||
"""lines: the changed lines of the reviewed paths (not documentation or tests)."""
|
||||
warnings = []
|
||||
if lines > LINE_LIMIT:
|
||||
warnings.append(f"{lines} lines of code: over {LINE_LIMIT}, split it if it can ship in parts")
|
||||
areas = sorted({area(path) for path in paths} - {None})
|
||||
if len(areas) > 1:
|
||||
warnings.append("mixes " + ", ".join(areas))
|
||||
return warnings
|
||||
|
||||
|
||||
def lock_problem(paths):
|
||||
"""Cargo.lock should change only with a manifest: otherwise it is an accidental `cargo update`."""
|
||||
manifests = any(path.endswith("Cargo.toml") for path in paths)
|
||||
locks = sorted(path for path in paths if path.endswith("Cargo.lock"))
|
||||
return f"{', '.join(locks)} changed without a Cargo.toml" if locks and not manifests else None
|
||||
|
||||
|
||||
def suggested_label(paths, exporter_paths=frozenset()):
|
||||
if any(release_train.player_facing(path, kind, exporter_paths) for path in paths for kind in ("game", "launcher")):
|
||||
return "player-facing: `feature` if players get something new, no label for a fix (`release-now` if urgent)"
|
||||
return "`skip-release`"
|
||||
|
||||
|
||||
def harness_names(names):
|
||||
"""Harness names without shard suffixes, in order."""
|
||||
return list(dict.fromkeys(parity.base_name(name) for name in names))
|
||||
|
||||
|
||||
def affected_here(changed):
|
||||
"""The harnesses the changed files can affect, from the dependencies recorded here, or None if nothing is."""
|
||||
found = parity.harnesses()
|
||||
records = parity.load_state()["harnesses"]
|
||||
known = any(records.get(name, {}).get("dependencies") is not None or
|
||||
(parity.SHARED / "dependencies" / f"{name}.json").exists() for name in found)
|
||||
if not known and not changed & parity.SHARED_INPUTS:
|
||||
return None
|
||||
return harness_names(name for name in found if name in parity.affected_harnesses(found, records, changed))
|
||||
|
||||
|
||||
def short_list(names, limit=12):
|
||||
shown = ", ".join(names[:limit])
|
||||
return shown + (f" and {len(names) - limit} more" if len(names) > limit else "")
|
||||
|
||||
|
||||
def duration(seconds):
|
||||
seconds = int(round(seconds))
|
||||
return f"{seconds // 60} min {seconds % 60} s" if seconds >= 60 else f"{seconds} s"
|
||||
|
||||
|
||||
def git(*arguments):
|
||||
return subprocess.run(["git", *arguments], cwd=ROOT, capture_output=True, text=True, check=True).stdout
|
||||
|
||||
|
||||
def line_counts(base, changed):
|
||||
"""Lines added and removed in each changed path since base, untracked files included: {path: (added, removed)}."""
|
||||
counts = {}
|
||||
for row in git("diff", "--numstat", base).splitlines():
|
||||
plus, minus, path = row.split("\t", 2)
|
||||
if plus != "-":
|
||||
counts[path] = (int(plus), int(minus))
|
||||
for path in git("ls-files", "--others", "--exclude-standard").splitlines():
|
||||
if path in changed:
|
||||
try:
|
||||
counts[path] = (len((ROOT / path).read_text().splitlines()), 0)
|
||||
except (UnicodeDecodeError, OSError):
|
||||
pass
|
||||
return counts
|
||||
|
||||
|
||||
def packages():
|
||||
"""The main workspace's packages and their directories, from Cargo's metadata."""
|
||||
try:
|
||||
output = subprocess.run(["cargo", "metadata", "--no-deps", "--format-version", "1"], cwd=ROOT,
|
||||
capture_output=True, text=True, check=True).stdout
|
||||
except (OSError, subprocess.CalledProcessError) as error:
|
||||
sys.exit(f"pr_ready: cargo metadata failed: {getattr(error, 'stderr', error)}")
|
||||
return {package["name"]: Path(package["manifest_path"]).parent.relative_to(ROOT).as_posix()
|
||||
for package in json.loads(output)["packages"]}
|
||||
|
||||
|
||||
def prepare_key():
|
||||
"""tools/ci/vanilla-data.sh's preparation key, for a copy without Git history."""
|
||||
listing = git("ls-files", "-s", "crates/horizon-prepare", "mods/horizon/src/test/java", "research-lock.json")
|
||||
return hashlib.sha256(listing.encode()).hexdigest()[:16]
|
||||
|
||||
|
||||
class Step:
|
||||
def __init__(self, name, command=None, where="host", skip=None, summary=None):
|
||||
self.name, self.command, self.where, self.skip, self.summary = name, command, where, skip, summary
|
||||
self.status, self.detail, self.seconds, self.output = ("skip", skip, None, "") if skip else (None, "", None, "")
|
||||
|
||||
|
||||
def tests_passed(output):
|
||||
results = re.findall(r"test result: \w+\. (\d+) passed; (\d+) failed", output)
|
||||
passed, failed = (sum(int(result[i]) for result in results) for i in (0, 1))
|
||||
return f"{passed} passed" + (f", {failed} failed" if failed else "") if results else ""
|
||||
|
||||
|
||||
def unittest_passed(output):
|
||||
failed = re.search(r"^(FAILED \(.*\))$", output, re.M)
|
||||
ran = re.search(r"^Ran (\d+) tests?", output, re.M)
|
||||
return f"{ran.group(1) if ran else '?'} run, {failed.group(1)}" if failed else f"{ran.group(1)} passed" if ran else ""
|
||||
|
||||
|
||||
def parity_summary(output):
|
||||
"""tools/parity.py's own account: how many harnesses the change affects, ran and passed, and which failed."""
|
||||
affected = re.search(r"(\d+) harnesses depend on them", output)
|
||||
if affected and affected.group(1) == "0":
|
||||
return "no harness affected"
|
||||
text = f"{affected.group(1)} affected, " if affected else ""
|
||||
if "Nothing to run" in output:
|
||||
return text + "all passed already with these inputs"
|
||||
match = re.search(r"(\d+) passed, (\d+) failed in", output)
|
||||
failed = re.findall(r"^ (\S+): \S+\.log$", output, re.M)
|
||||
skipped = re.findall(r"^skipped on \w+: ([^:]+):", output, re.M)
|
||||
text += f"{match.group(1)} passed" if match else ""
|
||||
if failed:
|
||||
text += f", failed: {', '.join(harness_names(failed))}"
|
||||
if skipped:
|
||||
text += f"; skipped on this platform: {', '.join(harness_names(skipped))}"
|
||||
return text
|
||||
|
||||
|
||||
def plan(args, base, changed, crates_of):
|
||||
"""The checks for these changed files, in the order they run."""
|
||||
rust = any(workspace_rust(path) for path in changed)
|
||||
launcher = any(launcher_rust(path) for path in changed)
|
||||
python = any(path.endswith(".py") or path.startswith(("tests/", "tools/")) for path in changed)
|
||||
crates = touched_crates(changed, crates_of) if rust or TOKENS in changed else []
|
||||
no_rust = "no Rust change"
|
||||
steps = [
|
||||
Step("Cargo.lock", where="git"),
|
||||
Step("Public tree", "python3 tools/check_public_tree.py", where="here"),
|
||||
Step("Python tests", "python3 -m unittest discover -s tests", where="here",
|
||||
skip=None if python else "no Python change", summary=unittest_passed),
|
||||
Step("rustfmt", "cargo fmt --all -- --check", skip=None if rust else no_rust),
|
||||
Step("clippy", "cargo clippy --workspace --all-targets --locked -- -D warnings", skip=None if rust else no_rust),
|
||||
Step("tests" + (": workspace" if crates is None else f": {', '.join(crates)}" if crates else ""),
|
||||
"cargo test --locked " + ("--workspace" if crates is None else " ".join(f"-p {c}" for c in crates or [])),
|
||||
skip=None if crates is None or crates else "no crate touched", summary=tests_passed),
|
||||
Step("tokens.css up to date", "cargo test --locked -p horizon-theme --lib website_tokens_are_up_to_date",
|
||||
skip=None if any(p.startswith(("crates/horizon-theme/", "xtask/")) or p == TOKENS for p in changed)
|
||||
else "theme untouched"),
|
||||
]
|
||||
launcher_skip = None if launcher else "launcher untouched"
|
||||
for name, command in [("launcher rustfmt", "cargo fmt --check"),
|
||||
("launcher clippy", "cargo clippy --all-targets --locked $features -- -D warnings"),
|
||||
("launcher tests", "cargo test --locked --lib $features")]:
|
||||
steps.append(Step(name, f"cd launcher && {LAUNCHER_FEATURES}\nin_shell {shlex.quote(command)}", skip=launcher_skip,
|
||||
summary=tests_passed if name == "launcher tests" else None))
|
||||
harness = Step("Parity", summary=parity_summary)
|
||||
affected = affected_here(changed)
|
||||
local_data = (ROOT / ".local" / "minecraft").exists() and (ROOT / ".local" / "horizon-client").exists()
|
||||
if args.quick or (args.host == "local" and not local_data) or (args.host != "local" and args.host not in HARNESS_SETUP):
|
||||
harness.status = "list"
|
||||
why = "--quick" if args.quick else "no vanilla data here"
|
||||
if affected is None:
|
||||
harness.detail = f"not run ({why}): CI's parity job picks the affected harnesses"
|
||||
elif not affected:
|
||||
harness.detail = "no harness affected"
|
||||
else:
|
||||
harness.detail = f"{len(affected)} affected, to run in CI ({why}): {short_list(affected)}"
|
||||
elif affected == []:
|
||||
harness.status, harness.detail = "pass", "no harness affected"
|
||||
elif args.host == "local":
|
||||
harness.where, harness.command = "here", f"PARITY_HOSTS= python3 tools/parity.py --changed {shlex.quote(base)} --timeout 1500"
|
||||
else:
|
||||
listing = "\n".join(sorted(changed))
|
||||
harness.command = f"""export HORIZON_PREPARE_KEY={prepare_key()}
|
||||
{HARNESS_SETUP[args.host]}
|
||||
cat > "$RUNNER_TEMP/changed" <<'PR_READY_CHANGED'
|
||||
{listing}
|
||||
PR_READY_CHANGED
|
||||
PARITY_HOSTS= python3 tools/parity.py --changed-files "$RUNNER_TEMP/changed" --timeout 1500"""
|
||||
steps.append(harness)
|
||||
return steps
|
||||
|
||||
|
||||
def run_script(steps, host, verbose):
|
||||
"""Runs the steps in one shell on host (here for "local"), each between markers, reading results as they come."""
|
||||
if not steps:
|
||||
return
|
||||
parts = [HOST_SETUP.get(host, ""), "set +e"]
|
||||
for index, step in enumerate(steps):
|
||||
parts.append(f"echo '@@pr_ready:begin:{index}'; started=$(date +%s)\n( {step.command}\n) 2>&1 < /dev/null\n"
|
||||
f"status=$?; echo; echo \"@@pr_ready:end:{index}:$status:$(( $(date +%s) - started ))\"")
|
||||
script = "\n".join(parts)
|
||||
if host == "local":
|
||||
command = ["bash", "-c", script]
|
||||
else:
|
||||
command = [str(RBUILD), host, "--", "bash", "-c", script]
|
||||
current = None
|
||||
with subprocess.Popen(command, cwd=ROOT, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True,
|
||||
errors="replace") as process:
|
||||
for line in process.stdout:
|
||||
marker = re.match(r"@@pr_ready:(begin|end):(\d+)(?::(\d+):(\d+))?$", line.strip())
|
||||
if marker and marker.group(1) == "begin":
|
||||
current = steps[int(marker.group(2))]
|
||||
print(f" {current.name}...", file=sys.stderr, flush=True)
|
||||
elif marker:
|
||||
step = steps[int(marker.group(2))]
|
||||
step.status = "pass" if marker.group(3) == "0" else "fail"
|
||||
step.seconds = int(marker.group(4))
|
||||
finish(step, verbose)
|
||||
current = None
|
||||
elif current is not None:
|
||||
current.output += line
|
||||
if verbose:
|
||||
print(" " + line, end="", file=sys.stderr)
|
||||
elif verbose or host != "local":
|
||||
print(" " + line, end="", file=sys.stderr)
|
||||
for step in steps:
|
||||
if step.status is None:
|
||||
step.status, step.detail = "fail", ("did not finish" if step.output else f"not run: {host} unreachable?")
|
||||
finish(step, verbose)
|
||||
|
||||
|
||||
def finish(step, verbose):
|
||||
if step.status == "pass" and step.summary:
|
||||
step.detail = step.summary(step.output)
|
||||
elif step.status == "fail":
|
||||
step.detail = (step.summary(step.output) if step.summary else "") or step.detail or "failed"
|
||||
if not verbose and step.output:
|
||||
print("".join(step.output.splitlines(keepends=True)[-40:]), end="", file=sys.stderr)
|
||||
took = f" ({duration(step.seconds)})" if step.seconds is not None else ""
|
||||
print(f" {step.name}: {step.status}{took}", file=sys.stderr, flush=True)
|
||||
|
||||
|
||||
MARK = {"pass": "✅", "fail": "❌", "skip": "➖", "list": "➖", "warn": "⚠️"}
|
||||
|
||||
|
||||
def block(args, base, merge_base, changed, counts, steps, label, elapsed):
|
||||
where = socket.gethostname().split(".")[0] if args.host == "local" else args.host
|
||||
flags = "".join(f" --{flag}" for flag in ("quick",) if getattr(args, flag)) + \
|
||||
("" if args.host == "local" else f" --host {args.host}") + ("" if base == "origin/main" else f" --base {base}")
|
||||
added, removed = (sum(count[i] for count in counts.values()) for i in (0, 1))
|
||||
warnings = scope_warnings(changed, sum(sum(count) for path, count in counts.items() if reviewed(path)))
|
||||
lines = [f"**pr_ready** (`tools/pr_ready.py{flags}` on {where}, against {base} at {merge_base[:9]}, "
|
||||
f"{duration(elapsed)})", "", "| Check | Result |", "| --- | --- |",
|
||||
f"| Scope | {'⚠️' if warnings else '✅'} {len(changed)} files, +{added} −{removed}"
|
||||
+ "".join(f"; {warning}" for warning in warnings) + " |"]
|
||||
for step in steps:
|
||||
took = f" ({duration(step.seconds)})" if step.seconds else ""
|
||||
detail = f" {step.detail}" if step.detail else ""
|
||||
lines.append(f"| {step.name} | {MARK[step.status]}{detail}{took} |")
|
||||
lines.append(f"| Label | {label} |")
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
def main():
|
||||
parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter)
|
||||
parser.add_argument("--base", default="origin/main", help="what the pull request merges into (default origin/main)")
|
||||
parser.add_argument("--host", default="local", choices=sorted(HOST_SETUP),
|
||||
help="where builds, tests and harnesses run: local, or a fleet machine through ~/fleet/rbuild")
|
||||
parser.add_argument("--quick", action="store_true", help="list the affected harnesses instead of running them")
|
||||
parser.add_argument("--verbose", action="store_true", help="show every check's output")
|
||||
args = parser.parse_args()
|
||||
if args.host != "local" and not RBUILD.exists():
|
||||
parser.error(f"--host {args.host} needs {RBUILD}; without the fleet, run it here (no --host)")
|
||||
started = time.monotonic()
|
||||
changed = parity.changed_files(args.base)
|
||||
if changed is None:
|
||||
parser.error(f"no merge base with {args.base} (git fetch?)")
|
||||
merge_base = git("merge-base", "HEAD", args.base).strip()
|
||||
counts = line_counts(merge_base, changed)
|
||||
steps = plan(args, args.base, changed, packages() if any(workspace_rust(p) for p in changed) or TOKENS in changed else {})
|
||||
print(f"pr_ready: {len(changed)} files changed since {args.base}; checks on {args.host}", file=sys.stderr, flush=True)
|
||||
|
||||
lock = steps[0]
|
||||
problem = lock_problem(changed)
|
||||
lock.status, lock.detail = ("fail", problem) if problem else ("pass", "unchanged" if not any(
|
||||
p.endswith("Cargo.lock") for p in changed) else "changes with a Cargo.toml")
|
||||
run_script([step for step in steps if step.where == "here" and step.status is None], "local", args.verbose)
|
||||
run_script([step for step in steps if step.where == "host" and step.status is None], args.host, args.verbose)
|
||||
|
||||
label = suggested_label(changed, release_train.exporters())
|
||||
print(block(args, args.base, merge_base, sorted(changed), counts, steps, label,
|
||||
time.monotonic() - started))
|
||||
return 1 if any(step.status == "fail" for step in steps) else 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
Reference in New Issue
Block a user