fix: compare release digests, not only sizes

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)")

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):

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"},
+        )