diff --git a/scripts/export_native.py b/scripts/export_native.py index 526e7478..b50c7ef5 100644 --- a/scripts/export_native.py +++ b/scripts/export_native.py @@ -18,11 +18,15 @@ from __future__ import annotations import argparse import contextlib import functools +import http.client import re import json import sys -import urllib.error +import tarfile import urllib.request +import zipfile +import zlib +from collections.abc import Callable from dataclasses import dataclass, field from pathlib import Path @@ -82,8 +86,26 @@ def load_sources(upstream_dir: Path) -> dict[str, str]: return _load_sources(upstream_dir / SOURCES_INDEX) +class OriginalUnreadable(ValueError): + """A platform's file that is not what its format says it is.""" + + +def _read_archive(unpack: Callable[[bytes], dict[str, str]], payload: bytes) -> dict[str, str]: + """Unpack an original, naming the remedy when its bytes are not one.""" + try: + return unpack(payload) + except (zipfile.BadZipFile, tarfile.TarError, EOFError, zlib.error, KeyError) as exc: + raise OriginalUnreadable( + f"archive unreadable ({exc}): --refresh-cache downloads it again" + ) from exc + + def fetch( - url: str, destination: Path, index: Path | None = None, refresh: bool = False + url: str, + destination: Path, + index: Path | None = None, + refresh: bool = False, + check: Callable[[bytes], object] | None = None, ) -> bytes: """Download an original once, then read it from the cache. @@ -122,7 +144,11 @@ def fetch( with urllib.request.urlopen(request, timeout=60) as response: payload = response.read(_MAX_BYTES + 1) if len(payload) > _MAX_BYTES: - raise ValueError(f"{url}: response larger than {_MAX_BYTES} bytes") + raise OriginalUnreadable(f"{url}: response larger than {_MAX_BYTES} bytes") + # An error page served with 200 replaced a good archive and was then + # served on every run: what does not read is never cached. + if check is not None: + check(payload) destination.parent.mkdir(parents=True, exist_ok=True) with locked(): write_bytes_atomic(str(destination), payload) @@ -261,26 +287,27 @@ def collect_originals( index = upstream_dir / SOURCES_INDEX originals: dict[str, str] = {} missing: list[str] = [] + unpack = getattr(exporter, "unpack", None) for relative, url in wanted.items(): path = root / relative - payload: bytes | None = None - if allow_fetch: - try: - payload = fetch(url, path, index, refresh) - except (urllib.error.URLError, urllib.error.HTTPError, OSError) as exc: - missing.append(f"{relative}: {exc}") + archive = callable(unpack) and relative.endswith((".zip", ".tar.gz")) + check = (lambda raw: _read_archive(unpack, raw)) if archive else None + try: + if allow_fetch: + payload = fetch(url, path, index, refresh, check) + elif path.exists(): + payload = path.read_bytes() + else: + missing.append(f"{relative}: absent and fetching is off") continue - elif path.exists(): - payload = path.read_bytes() - else: - missing.append(f"{relative}: absent and fetching is off") - continue - - unpack = getattr(exporter, "unpack", None) - if callable(unpack) and relative.endswith((".zip", ".tar.gz")): - originals.update(unpack(payload)) - else: - originals[relative] = payload.decode("utf-8", errors="replace") + if archive: + originals.update(_read_archive(unpack, payload)) + else: + originals[relative] = payload.decode("utf-8", errors="replace") + # One platform's unreadable file fails that platform: raised, it + # stopped --all before the platforms after it. + except (OSError, http.client.HTTPException, OriginalUnreadable) as exc: + missing.append(f"{relative}: {exc}") return originals, missing diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 9d4d229b..b3e4dc8d 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -15,6 +15,7 @@ and skip when it has not been populated (python scripts/export_native.py from __future__ import annotations import difflib +import io import json import os import re @@ -23,6 +24,7 @@ import sys import tempfile import unittest import xml.etree.ElementTree as ET +import zipfile from collections import OrderedDict from pathlib import Path @@ -709,6 +711,18 @@ class _OneFileExporter: return {"list.txt": self._url} +class _ArchiveExporter(_OneFileExporter): + """One whose original is an archive it unpacks, as MiSTer's is.""" + + def native_sources(self) -> dict[str, str]: + return {"db.zip": self._url} + + @staticmethod + def unpack(raw: bytes) -> dict[str, str]: + with zipfile.ZipFile(io.BytesIO(raw)) as archive: + return {"db.json": archive.read("db.json").decode("utf-8")} + + class CachedOriginal(unittest.TestCase): """The cached original is the file the platform data was read from.""" @@ -792,6 +806,51 @@ class CachedOriginal(unittest.TestCase): fetch(url, target, self.index) self.assertEqual(held, [True]) + def test_an_unreadable_cached_archive_fails_its_platform_only(self): + """A cached bios_db.json.zip that was not a zip raised BadZipFile + out of run(): --all stopped and the next platforms were never + exported.""" + target = self.cache / "plat" / "db.zip" + target.parent.mkdir(parents=True) + target.write_bytes(b"not a zip") + originals, missing = collect_originals( + _ArchiveExporter("file:///nowhere/db.zip"), {}, self.cache, False + ) + self.assertEqual(originals, {}) + self.assertEqual(len(missing), 1) + self.assertIn("--refresh-cache", missing[0]) + + def test_a_download_that_is_not_the_archive_keeps_the_cache(self): + """fetch() cached any 200 reply, so an error page replaced a good + archive and was served on every later run.""" + good = self.root / "db.zip" + with zipfile.ZipFile(good, "w") as archive: + archive.writestr("db.json", "{}") + url = good.as_uri() + exporter = _ArchiveExporter(url) + collect_originals(exporter, {}, self.cache, True) + cached = (self.cache / "plat" / "db.zip").read_bytes() + good.write_bytes(b"error") + originals, missing = collect_originals(exporter, {}, self.cache, True, refresh=True) + self.assertEqual(originals, {}) + self.assertIn("--refresh-cache", missing[0]) + self.assertEqual((self.cache / "plat" / "db.zip").read_bytes(), cached) + + def test_an_incomplete_read_is_reported(self): + import http.client # noqa: PLC0415 + from unittest import mock # noqa: PLC0415 + + import export_native # noqa: PLC0415 + + def cut(*_args, **_kwargs): + raise http.client.IncompleteRead(b"par") + + with mock.patch.object(export_native, "fetch", cut): + originals, missing = collect_originals( + _OneFileExporter("https://example.invalid/list.txt"), {}, self.cache, True + ) + self.assertEqual((originals, len(missing)), ({}, 1)) + def test_the_index_names_a_file_the_same_from_any_directory(self): target = self.cache / "plat" / "list.txt" self.assertEqual(source_key(target, self.index), "plat/list.txt")