diff --git a/scripts/check_release_assets.py b/scripts/check_release_assets.py index 5c934260..a121d26c 100644 --- a/scripts/check_release_assets.py +++ b/scripts/check_release_assets.py @@ -59,9 +59,15 @@ def compare( assets: dict[str, int], notes_current: bool = True, registered: Iterable[str] | None = None, + expected_digests: dict[str, str] | None = None, + asset_digests: dict[str, str] | None = None, ) -> list[Finding]: """Files the release lacks or serves at another size, sorted by kind. + At the same size, the sha256 the release lists for an asset is held + against the collection's: a dump rebuilt at its old size and never + re-uploaded was invisible to a size comparison. + *registered* is every path .gitignore lists; an asset name depends on which other paths share its basename, so it defaults to *expected* only when the caller has nothing wider. A release description that no longer @@ -73,17 +79,41 @@ def compare( findings.append(("notes", "release description", 0, None)) names = asset_names([*(registered or ()), *expected]) for path, size in sorted(expected.items()): - published = next( - (assets[name] for name in _spellings(names[path]) if name in assets), - None, + served = next( + (name for name in _spellings(names[path]) if name in assets), None ) + published = assets[served] if served else None if published is None: findings.append(("missing", path, size, None)) elif published != size: findings.append(("size", path, size, published)) + else: + ours = (expected_digests or {}).get(path) + theirs = (asset_digests or {}).get(served) + if ours and theirs and ours != theirs: + findings.append(("content", path, size, published)) return sorted(findings) +def expected_digests(db: dict, gitignore_text: str) -> dict[str, str]: + """sha256 of each gitignored database path.""" + ignored = set(registered_paths(gitignore_text)) + return { + entry["path"]: entry["sha256"].lower() + for entry in db.get("files", {}).values() + if entry.get("path") in ignored and entry.get("sha256") + } + + +def digests_of(assets_json: list[dict]) -> dict[str, str]: + """sha256 the release lists for each asset, by name.""" + return { + a["name"]: str(a["digest"]).removeprefix("sha256:").lower() + for a in assets_json + if str(a.get("digest") or "").startswith("sha256:") + } + + ASSET_URL = f"https://github.com/{LARGE_FILES_REPO}/releases/download/{LARGE_FILES_RELEASE}/" NOT_INDEXED = "Not indexed" SECTIONS = ( @@ -257,8 +287,9 @@ def cache_hashes(names: set[str], cache_dir: str = ".cache/large") -> dict[str, return hashes -def fetch_release() -> tuple[dict[str, int], str]: - """Name and size of every asset of the large-files release, and its page.""" +def fetch_release() -> tuple[dict[str, int], str, dict[str, str]]: + """Name and size of every asset of the large-files release, its page, + and the sha256 it lists per asset.""" result = subprocess.run( [ "gh", "release", "view", LARGE_FILES_RELEASE, @@ -267,7 +298,11 @@ def fetch_release() -> tuple[dict[str, int], str]: capture_output=True, text=True, check=True, ) data = json.loads(result.stdout) - return {a["name"]: a["size"] for a in data["assets"]}, data.get("body", "") + return ( + {a["name"]: a["size"] for a in data["assets"]}, + data.get("body", ""), + digests_of(data["assets"]), + ) def fetch_assets() -> dict[str, int]: @@ -298,7 +333,7 @@ def main() -> int: db = load_database(args.db) expected = expected_assets(db, gitignore_text) try: - assets, body = fetch_release() + assets, body, asset_digests = fetch_release() except (subprocess.CalledProcessError, FileNotFoundError, KeyError) as exc: print(f"ERROR: cannot list release assets: {exc}", file=sys.stderr) return 2 @@ -315,6 +350,8 @@ def main() -> int: assets, notes_current=rendered.strip() == body.strip(), registered=registered_paths(gitignore_text), + expected_digests=expected_digests(db, gitignore_text), + asset_digests=asset_digests, ) if args.json: @@ -332,6 +369,8 @@ def main() -> int: print(f" MISSING {path} ({size} bytes)") elif kind == "size": print(f" SIZE {path}: local {size} != release {published}") + elif kind == "content": + print(f" CONTENT {path}: same size, other sha256 on the release") else: print(" NOTES release description differs from the rendered page" " (--notes FILE, then gh release edit large-files --notes-file FILE)") diff --git a/scripts/largefiles.py b/scripts/largefiles.py index 458f9fef..fd52a015 100644 --- a/scripts/largefiles.py +++ b/scripts/largefiles.py @@ -7,6 +7,7 @@ from __future__ import annotations import contextlib import http.client +import json import os import sys import re @@ -240,15 +241,58 @@ def _asset_urls(name: str) -> list[tuple[str, str]]: def _replaced_on_release(name: str, cached: str, offline: bool) -> bool: """Whether the release now serves other bytes than the cached copy. - Sizes are compared, as check_release_assets does: a request per asset, - no download. Offline or unreadable, the copy is assumed current. + The release lists a sha256 per asset: a dump rebuilt at the same size + (the eight 251658304-byte DSi NANDs) differs only there, and judged by + size the stale copy was kept for good. Without a digest the sizes are + compared, as check_release_assets does. No download either way; + offline or unreadable, the copy is assumed current. """ if offline: return False + digest = next( + (d for candidate, _url in _asset_urls(name) + if (d := _served_digests().get(candidate))), + None, + ) + if digest: + return compute_hashes(cached, frozenset({"sha256"}))["sha256"] != digest served = _served_size(name) return served is not None and served != os.path.getsize(cached) +_digests_cache: dict[str, str] | None = None + + +def _served_digests() -> dict[str, str]: + """sha256 of each asset the release serves, by asset name; empty when + the API cannot be read.""" + global _digests_cache + if _digests_cache is not None: + return _digests_cache + url = ( + f"https://api.github.com/repos/{LARGE_FILES_REPO}/releases/tags/" + f"{LARGE_FILES_RELEASE}" + ) + headers = {"User-Agent": "retrobios/1.0", "Accept": "application/vnd.github+json"} + if os.environ.get("GITHUB_TOKEN"): + headers["Authorization"] = f"Bearer {os.environ['GITHUB_TOKEN']}" + digests: dict[str, str] = {} + try: + with urllib.request.urlopen( + urllib.request.Request(url, headers=headers), timeout=60 + ) as resp: + release = json.load(resp) + except (urllib.error.URLError, OSError, http.client.HTTPException, ValueError) as exc: + print(f" large files release listing: {exc}", file=sys.stderr) + release = {} + for asset in release.get("assets") or []: + digest = str(asset.get("digest") or "") + if digest.startswith("sha256:"): + digests[asset.get("name", "")] = digest.removeprefix("sha256:").lower() + _digests_cache = digests + return digests + + def _served_size(name: str) -> int | None: """Size of the asset the release serves now, or None when unreadable.""" for candidate, url in _asset_urls(name): diff --git a/tests/test_large_file_cache.py b/tests/test_large_file_cache.py index ab9ef4d0..c91e2de9 100644 --- a/tests/test_large_file_cache.py +++ b/tests/test_large_file_cache.py @@ -679,3 +679,54 @@ class GameDataHasOneDefinition(unittest.TestCase): if __name__ == "__main__": unittest.main() + + +class ASameSizeReuploadIsSeen(unittest.TestCase): + """A DSi NAND rebuilt at the same 251658304 bytes and re-uploaded with + --clobber: judged by size, the stale cache copy was kept for good.""" + + def setUp(self): + self.dir = Path(tempfile.mkdtemp(dir=REPO_ROOT / "tmp")) + self.addCleanup(shutil.rmtree, self.dir, True) + self.cached = self.dir / "dsi_nand.bin" + self.cached.write_bytes(b"OLD NAND") + + def replaced(self, digests: dict, size: int | None) -> bool: + with mock.patch.object(largefiles, "_served_digests", lambda: digests), \ + mock.patch.object(largefiles, "_served_size", lambda name: size): + return largefiles._replaced_on_release("dsi_nand.bin", str(self.cached), False) + + def test_the_digest_decides(self): + same = hashlib.sha256(b"OLD NAND").hexdigest() + other = hashlib.sha256(b"NEW NAND").hexdigest() + self.assertTrue(self.replaced({"dsi_nand.bin": other}, 8)) + self.assertFalse(self.replaced({"dsi_nand.bin": same}, 8)) + + def test_without_a_digest_the_size_decides(self): + self.assertFalse(self.replaced({}, 8)) + self.assertTrue(self.replaced({}, 9)) + + def test_the_release_check_compares_digests_at_equal_size(self): + expected = {"bios/Nintendo/DSi/dsi_nand.bin": 8} + assets = {"dsi_nand.bin": 8} + ours = {"bios/Nintendo/DSi/dsi_nand.bin": "a" * 64} + self.assertEqual( + [f[0] for f in check_release_assets.compare( + expected, assets, expected_digests=ours, + asset_digests={"dsi_nand.bin": "b" * 64}, + )], + ["content"], + ) + self.assertEqual( + check_release_assets.compare( + expected, assets, expected_digests=ours, + asset_digests={"dsi_nand.bin": "a" * 64}, + ), + [], + ) + self.assertEqual( + check_release_assets.digests_of( + [{"name": "x", "digest": "sha256:ABC"}, {"name": "y"}] + ), + {"x": "abc"}, + )