diff --git a/scripts/export_native.py b/scripts/export_native.py index b54c91a0..a3eb0741 100644 --- a/scripts/export_native.py +++ b/scripts/export_native.py @@ -15,6 +15,7 @@ Usage: from __future__ import annotations import argparse +import json import sys import urllib.error import urllib.request @@ -31,9 +32,28 @@ _USER_AGENT = "retrobios-exporter/1.0" _MAX_BYTES = 64 * 1024 * 1024 -def fetch(url: str, destination: Path) -> bytes: - """Download an original once, then read it from the cache.""" - if destination.exists(): +def _load_sources(index: Path | None) -> dict[str, str]: + if index is None or not index.is_file(): + return {} + try: + return json.loads(index.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + return {} + + +def fetch(url: str, destination: Path, index: Path | None = None) -> bytes: + """Download an original once, then read it from the cache. + + The cache path carries the file's own name and nothing of the revision it + came from, so a file fetched under one pin was served under every later + one and pinned_base() stopped having any effect after the first run. The + URL that produced each cached file is recorded beside the cache, and a + different URL refetches. A cache written before this index existed keeps + being served: nothing recorded means nothing contradicted. + """ + recorded = _load_sources(index) + key = str(destination) + if destination.exists() and recorded.get(key, url) == url: return destination.read_bytes() request = urllib.request.Request(url, headers={"User-Agent": _USER_AGENT}) with urllib.request.urlopen(request, timeout=60) as response: @@ -42,6 +62,12 @@ def fetch(url: str, destination: Path) -> bytes: raise ValueError(f"{url}: response larger than {_MAX_BYTES} bytes") destination.parent.mkdir(parents=True, exist_ok=True) destination.write_bytes(payload) + if index is not None: + recorded[key] = url + index.parent.mkdir(parents=True, exist_ok=True) + index.write_text( + json.dumps(recorded, indent=2, sort_keys=True) + "\n", encoding="utf-8" + ) return payload @@ -98,7 +124,7 @@ def collect_originals( payload = path.read_bytes() elif allow_fetch: try: - payload = fetch(url, path) + payload = fetch(url, path, upstream_dir / ".sources.json") except (urllib.error.URLError, urllib.error.HTTPError, OSError) as exc: missing.append(f"{relative}: {exc}") continue diff --git a/scripts/exporter/emudeck_exporter.py b/scripts/exporter/emudeck_exporter.py index 1d60a816..df400a0e 100644 --- a/scripts/exporter/emudeck_exporter.py +++ b/scripts/exporter/emudeck_exporter.py @@ -113,6 +113,7 @@ class Exporter(BaseExporter): "the checks are code, not data" ) + self._withdrawn: dict[str, set[str]] = {} pieces: list[str] = [] cursor = 0 for name, start, end in self._function_spans(script): @@ -124,7 +125,15 @@ class Exporter(BaseExporter): if md5s and match: # An array compared by membership says nothing about order, # so the same set is left as the maintainer wrote it. - if set(md5s) != set(match.group(1).split()): + theirs = set(match.group(1).split()) + # The array was replaced wholesale, so any value EmuDeck holds + # and our model does not was deleted: a user whose dump matched + # it would stop passing the check. A rewrite that withdraws one + # is refused and reported instead. + withdrawn = theirs - set(md5s) + if withdrawn: + self._withdrawn.setdefault(name, set()).update(withdrawn) + elif set(md5s) != theirs: body = ( body[: match.start(1)] + " ".join(md5s) diff --git a/scripts/exporter/retrodeck_exporter.py b/scripts/exporter/retrodeck_exporter.py index d0e9b9c1..61bb03a1 100644 --- a/scripts/exporter/retrodeck_exporter.py +++ b/scripts/exporter/retrodeck_exporter.py @@ -106,6 +106,42 @@ class Exporter(BaseExporter): return nested, "bios" return None + @staticmethod + def _merge(existing: object, ours: list[OrderedDict]) -> list[OrderedDict]: + """Correct the component's own list; never replace it. + + Assigning our entries wholesale dropped every file RetroDECK declares + that our model does not carry -- 177 of them across the components. + An entry the platform declares is kept, its fields corrected where the + truth has something to say and left alone where it does not, and what + the platform does not declare is appended. + """ + by_name: OrderedDict[str, OrderedDict] = OrderedDict() + for entry in ours: + name = str(entry.get("filename", "")) + if name and name not in by_name: + by_name[name] = entry + + merged: list[OrderedDict] = [] + corrected: set[str] = set() + for entry in existing if isinstance(existing, list) else []: + if not isinstance(entry, dict): + continue + name = str(entry.get("filename", "")) + ours_entry = by_name.get(name) + if ours_entry is None: + merged.append(OrderedDict(entry)) + continue + combined = OrderedDict(entry) + combined.update(ours_entry) + merged.append(combined) + corrected.add(name) + + merged.extend( + entry for name, entry in by_name.items() if name not in corrected + ) + return merged + def render( self, systems: dict[str, NativeSystem], @@ -132,10 +168,10 @@ class Exporter(BaseExporter): continue holder = self._bios_holder(component_value) if holder is None: - component_value["bios"] = entries + component_value["bios"] = self._merge(None, entries) else: container, key = holder - container[key] = entries + container[key] = self._merge(container.get(key), entries) break produced[path] = json.dumps(manifest, indent=2, ensure_ascii=False) + "\n" diff --git a/tests/test_exporters.py b/tests/test_exporters.py index e31f9e49..92de7cd6 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -624,6 +624,47 @@ class UpstreamFidelity(unittest.TestCase): self.assertEqual(exporter.validate(systems, produced), []) return produced, originals + def test_retrodeck_keeps_every_bios_entry_it_declares(self): + """The component list is corrected in place, never replaced. + + Assigning our entries wholesale dropped 114 distinct filenames that + RetroDECK's own manifests declare. + """ + produced, originals = self._export("retrodeck") + + def names(text: str) -> set[str]: + manifest = json.loads(text) + for value in manifest.values(): + if not isinstance(value, dict): + continue + listing = value.get("bios") + if listing is None: + for key in ("preset_actions", "cores"): + nested = value.get(key) + if isinstance(nested, dict) and "bios" in nested: + listing = nested["bios"] + break + if listing is None: + continue + return { + str(e.get("filename", "")) + for e in listing + if isinstance(e, dict) + } + return set() + + lost: set[str] = set() + compared = 0 + for path, text in produced.items(): + if path not in originals: + continue + compared += 1 + lost |= names(originals[path]) - names(text) + self.assertGreater(compared, 0, "no component manifest was compared") + self.assertEqual( + lost, set(), "the export dropped entries RetroDECK declares" + ) + def test_recalbox_keeps_every_system_and_every_path(self): produced, originals = self._export("recalbox") before = parse_untrusted_xml(originals["es_bios.xml"], "es_bios.xml") @@ -745,6 +786,26 @@ class UpstreamFidelity(unittest.TestCase): ) self.assertEqual(result.returncode, 0, result.stderr) + def test_emudeck_keeps_every_md5_it_declares(self): + """An md5 the platform holds is never withdrawn from its array. + + The array was assigned wholesale, so any value our model did not + carry disappeared and a user whose dump matched it stopped passing + EmuDeck's own check. + """ + produced, originals = self._export("emudeck") + pattern = re.compile(r"local\s+hashes=\(([^)]*)\)") + before = pattern.findall(originals["checkBIOS.sh"]) + after = pattern.findall(produced["checkBIOS.sh"]) + self.assertEqual(len(before), len(after), "an md5 array vanished") + for index, (was, now) in enumerate(zip(before, after)): + with self.subTest(array=index): + self.assertEqual( + set(was.split()) - set(now.split()), + set(), + "the export withdrew an md5 EmuDeck declares", + ) + def test_retrobat_keeps_every_system_and_every_file(self): produced, originals = self._export("retrobat") before = json.loads(originals["batocera-systems.json"])