From 7a65a2cf8385c45eef0cd93e1406ea6754a664c8 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Tue, 20 Oct 2026 15:09:16 +0200 Subject: [PATCH] fix: refuse an export without its original System.dat, RomM, Recalbox and RetroBat were rebuilt from our data when their file was not cached: libretro's quoted names, Recalbox's comments and one of its two zxspectrum blocks were lost, and the run said OK. needs_original now follows native_sources, so every format we patch fails without the file it patches and writes nothing. --- scripts/export_native.py | 5 +---- scripts/exporter/base_exporter.py | 13 +++++++++---- tests/test_exporters.py | 25 +++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/scripts/export_native.py b/scripts/export_native.py index 0f9ad98a..526e7478 100644 --- a/scripts/export_native.py +++ b/scripts/export_native.py @@ -472,11 +472,8 @@ def export_platform( originals, missing = collect_originals( exporter, systems, upstream_dir, allow_fetch, scraped ) - if missing and exporter.needs_original(): + if missing: return False, [f"the platform's own file is required: {m}" for m in missing] - messages.extend( - f"original unavailable, written from our data: {m}" for m in missing - ) try: produced = exporter.render(systems, report, originals, scraped) diff --git a/scripts/exporter/base_exporter.py b/scripts/exporter/base_exporter.py index 34bdf9c2..164e27e4 100644 --- a/scripts/exporter/base_exporter.py +++ b/scripts/exporter/base_exporter.py @@ -50,10 +50,15 @@ class BaseExporter(ABC): """ return {} - @staticmethod - def needs_original() -> bool: - """Whether a faithful export requires the platform's own file.""" - return False + @classmethod + def needs_original(cls) -> bool: + """Whether a faithful export requires the platform's own file. + + Any format we patch does: System.dat rebuilt from our data lost + libretro's quoted names, es_bios.xml its comments and one of its two + zxspectrum blocks, and both runs said OK. + """ + return bool(cls.native_sources()) @staticmethod def may_write_nothing() -> bool: diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 1eb64f10..9d4d229b 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -477,6 +477,31 @@ class ExporterRegistry(unittest.TestCase): with self.subTest(platform=name): self.assertTrue(discover_exporters()[name].needs_original()) + def test_every_exporter_with_an_original_needs_it(self): + """System.dat, RomM, Recalbox and RetroBat were rebuilt from our data + when their file was not cached: libretro's quoted names, Recalbox's + comments and its second zxspectrum block were lost, and the run + said OK.""" + for name, exporter in discover_exporters().items(): + if exporter.native_sources(): + with self.subTest(platform=name): + self.assertTrue(exporter.needs_original()) + + def test_an_export_without_its_original_writes_nothing(self): + import export_native # noqa: PLC0415 + + with tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") as tmp: + output = Path(tmp) / "out" + ok, messages = export_native.export_platform( + "retroarch", discover_exporters()["retroarch"], + REPO_ROOT / "dist" / "truth", output, + str(REPO_ROOT / "platforms"), Path(tmp) / "no-cache", False, + ) + if not (REPO_ROOT / "dist" / "truth" / "retroarch.yml").exists(): + self.skipTest("dist/truth not generated") + self.assertFalse(ok, messages) + self.assertFalse(output.exists()) + class SearchOrder(unittest.TestCase): """A list is read in the order the code looks, best first."""