project.yaml's version is the Atlas disk cache's only invalidation signal, and nothing enforced that changing canvas GENERATION also moved it. It broke five times -- 0.4.2 lake_margin_q, 0.4.3 coast_warp_px, 0.4.4 the extent inversion, 0.4.5 the Global sentinel, 0.4.6 one-course-per-river -- each bumped only after someone noticed a wrong map. The failure is invisible to its author: it needs a warm cache to reproduce, so a cold checkout looks fine. T-1239 is the last one, and it took eight days. tooling/canvas_sources.py is the path registry; tooling/check-canvas-version rejects a push that touches those paths without moving project.yaml's version line. Wired into the pre-push hook, `make check-canvas-version`, and, for the parsing units, `make test-tooling`. Verified against real history rather than a synthetic branch: run over4e503c356-- the commit that actually caused T-1239 -- the gate rejects and names the three files. Run over the commits that DID bump (bdea71953,39f0fd8c5, and T-1239's own fix), it passes. The registry is globbed, not hand-listed. step_canvas.rs imports ten sibling modules and those import more, so a traced closure would be stale within a month, and stale here is silent. It over-includes on purpose: a false positive costs one bump and one round of cache misses, a false negative costs another week of a wrong map -- the ticket's own ruling. Two deliberate calls worth naming. The registry includes ITSELF, which closes the narrowing hole: remove a path and change that same path in one push, and the gate still fires because the registry file is in the set. And there is no override flag -- it would be reached for exactly when someone is certain their change is harmless, which is the reasoning behind all five regressions. Version bumped 0.4.6 -> 0.4.7 with NO canvas-generation change: self-inclusion means adding the registry trips its own rule. Spent rather than special-cased, because the first exception is how a rule like this dies. The units cover the property no branch run can show -- that editing project.yaml's comment block, which quotes old version NUMBERS directly above the field, is not a bump -- plus a registry-coverage test naming the files each of the five known regressions touched, so a future narrowing past them fails loudly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
165 lines
6.0 KiB
Python
Executable File
165 lines
6.0 KiB
Python
Executable File
#!/usr/bin/env python3
|
|
"""Fail if a push changes canvas generation without moving project.yaml's version.
|
|
|
|
`project.yaml`'s `version:` is the Atlas disk cache's only invalidation signal.
|
|
Change how a canvas is generated without moving it and every warm cache keeps
|
|
serving canvases built by code that no longer exists — silently, and only on
|
|
machines that have a warm cache, so the author never sees it. That has happened
|
|
five times (see tooling/canvas_sources.py for the roll-call); T-1239 is what the
|
|
last one cost.
|
|
|
|
The rule: if the push touches anything in the canvas-generation registry, the
|
|
`version:` line in project.yaml must change in the SAME range.
|
|
|
|
Deliberately no override flag. The ticket's ruling (T-1242) is that a false
|
|
positive is cheap — one version bump, one round of cache misses — and a false
|
|
negative is another week of a wrong map. An escape hatch would be reached for
|
|
exactly when someone is sure their change is harmless, which is the state of mind
|
|
that produced all five regressions.
|
|
|
|
Exit: 0 = fine (or nothing relevant in range), 1 = version bump required.
|
|
"""
|
|
|
|
import argparse
|
|
import subprocess
|
|
import sys
|
|
from pathlib import Path
|
|
|
|
sys.path.insert(0, str(Path(__file__).resolve().parent))
|
|
from canvas_sources import relative_paths # noqa: E402
|
|
|
|
REPO_ROOT = Path(__file__).resolve().parent.parent
|
|
DEFAULT_BASE = "origin/main"
|
|
|
|
|
|
def git(*args: str) -> str | None:
|
|
"""Run a git command, returning stdout, or None if it failed."""
|
|
result = subprocess.run(
|
|
["git", "-C", str(REPO_ROOT), *args],
|
|
capture_output=True,
|
|
text=True,
|
|
)
|
|
if result.returncode != 0:
|
|
return None
|
|
return result.stdout
|
|
|
|
|
|
def changed_files(commit_range: str) -> list[str] | None:
|
|
out = git("diff", "--name-only", commit_range)
|
|
if out is None:
|
|
return None
|
|
return [line for line in out.splitlines() if line]
|
|
|
|
|
|
def diff_has_version_bump(diff_text: str) -> bool:
|
|
"""Does this project.yaml diff actually move the `version:` field?
|
|
|
|
Pure, so the property is testable without constructing git history
|
|
(tooling/test_canvas_version_check.py).
|
|
|
|
Matched on the diff body rather than on the file appearing in --name-only:
|
|
project.yaml carries a long comment block documenting past bumps — including
|
|
lines that quote old version NUMBERS — so editing that commentary, or any
|
|
other field in the file, must NOT count as bumping the version.
|
|
|
|
Requires the ADDED side: a lone deletion means the field was removed, not
|
|
moved. Diff context/metadata lines such as `+++ b/project.yaml` must not
|
|
match either, which is why this anchors on `+version:` exactly.
|
|
"""
|
|
for line in diff_text.splitlines():
|
|
if line.startswith("+++"):
|
|
continue # diff header, not content
|
|
if line.startswith("+version:"):
|
|
return True
|
|
return False
|
|
|
|
|
|
def version_line_changed(commit_range: str) -> bool:
|
|
"""Did project.yaml's `version:` line itself change in this range?"""
|
|
out = git("diff", "-U0", commit_range, "--", "project.yaml")
|
|
if out is None:
|
|
return False
|
|
return diff_has_version_bump(out)
|
|
|
|
|
|
def main() -> int:
|
|
parser = argparse.ArgumentParser(
|
|
description="Require a project.yaml version bump alongside canvas-generation changes"
|
|
)
|
|
parser.add_argument(
|
|
"--base",
|
|
default=DEFAULT_BASE,
|
|
help=f"Base ref to compare against (default: {DEFAULT_BASE})",
|
|
)
|
|
parser.add_argument(
|
|
"--head",
|
|
default="HEAD",
|
|
help="Head ref to compare (default: HEAD)",
|
|
)
|
|
args = parser.parse_args()
|
|
|
|
# Three-dot: what HEAD added since the merge base, matching the systems.db
|
|
# stamp check's own convention in .config/hooks/pre-push.
|
|
commit_range = f"{args.base}...{args.head}"
|
|
|
|
if git("rev-parse", "--verify", args.base) is None:
|
|
# No base to compare against (fresh clone, no remote yet). Skipping is
|
|
# correct rather than failing: there is no "range" to judge.
|
|
print(
|
|
f"check-canvas-version: {args.base} not found — skipping (nothing to compare)"
|
|
)
|
|
return 0
|
|
|
|
changed = changed_files(commit_range)
|
|
if changed is None:
|
|
print(
|
|
f"check-canvas-version: could not diff {commit_range} — skipping",
|
|
file=sys.stderr,
|
|
)
|
|
return 0
|
|
|
|
registry = set(relative_paths())
|
|
touched = sorted(set(changed) & registry)
|
|
|
|
if not touched:
|
|
print("check-canvas-version: no canvas-generation changes in range — OK")
|
|
return 0
|
|
|
|
if version_line_changed(commit_range):
|
|
print(
|
|
f"check-canvas-version: OK — {len(touched)} canvas-generation file(s) "
|
|
"changed and project.yaml's version moved with them"
|
|
)
|
|
return 0
|
|
|
|
shown = touched[:10]
|
|
remainder = len(touched) - len(shown)
|
|
print(
|
|
"check-canvas-version: canvas generation changed without a version bump\n"
|
|
"\n"
|
|
f" Range: {commit_range}\n"
|
|
" Changed canvas-generation files:\n"
|
|
+ "".join(f" {p}\n" for p in shown)
|
|
+ (f" ... and {remainder} more\n" if remainder else "")
|
|
+ "\n"
|
|
"project.yaml's `version:` is the Atlas disk cache's ONLY invalidation\n"
|
|
"signal. Without a bump, every warm cache keeps serving canvases built by\n"
|
|
"the code you just changed — silently, and only on machines that have a\n"
|
|
"warm cache, so you will not see it on a cold checkout.\n"
|
|
"\n"
|
|
"Fix: bump `version:` in project.yaml (scheme 0.{phase}.{n}), add a line to\n"
|
|
"the comment block above it saying what the old entries carried, and mirror\n"
|
|
"the new value into client/project.godot's config/version.\n"
|
|
"\n"
|
|
"If you are certain this change cannot alter canvas bytes, bump it anyway:\n"
|
|
"the cost is one round of cache misses. That trade is the point — this has\n"
|
|
"shipped broken five times, most recently T-1239, which took eight days to\n"
|
|
"find.",
|
|
file=sys.stderr,
|
|
)
|
|
return 1
|
|
|
|
|
|
if __name__ == "__main__":
|
|
sys.exit(main())
|