diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 000000000..b6443a4e5 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,19 @@ +## What and why + + + +## Vanilla reference + + + +## Self-check + + + +## Screenshots + + + + diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 000000000..6fdfeb6b4 --- /dev/null +++ b/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. diff --git a/README.md b/README.md index c766eba04..02efe2016 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/docs/parity-workflow.md b/docs/parity-workflow.md index fc35c9e61..dde4b5d41 100644 --- a/docs/parity-workflow.md +++ b/docs/parity-workflow.md @@ -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//` directory. diff --git a/tests/test_pr_ready.py b/tests/test_pr_ready.py new file mode 100644 index 000000000..0e53cf7f5 --- /dev/null +++ b/tests/test_pr_ready.py @@ -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() diff --git a/tools/ci/vanilla-data.sh b/tools/ci/vanilla-data.sh index 5234a48f5..5276c54e9 100755 --- a/tools/ci/vanilla-data.sh +++ b/tools/ci/vanilla-data.sh @@ -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. diff --git a/tools/parity.py b/tools/parity.py index 85d079c14..5809f913e 100644 --- a/tools/parity.py +++ b/tools/parity.py @@ -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)] diff --git a/tools/pr_ready.py b/tools/pr_ready.py new file mode 100755 index 000000000..980080284 --- /dev/null +++ b/tools/pr_ready.py @@ -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())