mirror of
https://github.com/Abdess/retroarch_system.git
synced 2026-10-21 03:09:08 -05:00
fix: isolate an unreadable original per platform
A cached bios_db.json.zip that was not a zip raised BadZipFile out of run(): --all stopped and the platforms after it were never exported. fetch() also cached any 200 reply, so an error page replaced a good archive and was served on every later run, and an IncompleteRead or an oversized reply escaped the same way. An archive is now read before it is cached, and an unreadable file fails its own platform with the --refresh-cache remedy.
This commit is contained in:
1 parent
194ed23ded
commit
4da40c471d
2 files changed
+106
-20
No files matched your search
+47
-20
@@ -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
|
||||
|
||||
|
||||
@@ -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"<html>not a zip</html>")
|
||||
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"<html>error</html>")
|
||||
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")
|
||||
|
||||
Reference in new issue
Block a user