diff --git a/exports/recalbox.json b/exports/recalbox.json index b8d10259..676e4e4e 100644 --- a/exports/recalbox.json +++ b/exports/recalbox.json @@ -7,7 +7,7 @@ "files": [ "es_bios.xml" ], - "summary": "357 kept, 87 added, 2 hashes corrected, 127 requirements corrected, 528 additions left out, 102 entries found by content not listed", + "summary": "357 kept, 87 added, 2 hashes corrected, 128 requirements corrected, 528 additions left out, 102 entries found by content not listed", "counts": { "kept": 357, "added": 87, @@ -711,6 +711,15 @@ "old": true, "new": false }, + { + "kind": "requirement_corrected", + "system": "pc98", + "name": "bios.rom", + "destination": "np2kai/bios.rom", + "field": "required", + "old": true, + "new": false + }, { "kind": "requirement_corrected", "system": "pc98", @@ -777,6 +786,15 @@ "old": true, "new": false }, + { + "kind": "requirement_corrected", + "system": "msx1,msx2", + "name": "FMPAC.ROM", + "destination": "msx/FMPAC.ROM", + "field": "required", + "old": true, + "new": false + }, { "kind": "requirement_corrected", "system": "msx1,msx2", @@ -1905,15 +1923,6 @@ "old": true, "new": false }, - { - "kind": "requirement_corrected", - "system": "saturn", - "name": "saturn_bios.bin", - "destination": "saturn/saturn_bios.bin", - "field": "required", - "old": true, - "new": false - }, { "kind": "requirement_corrected", "system": "saturn", @@ -2229,7 +2238,7 @@ "sizes_corrected": [], "sizes_removed": [], "hashes_cleared": [], - "requirements_corrected": 127, + "requirements_corrected": 128, "withheld": [], "found_by_content": 102, "removal_candidates": [ diff --git a/exports/recalbox/es_bios.xml b/exports/recalbox/es_bios.xml index d3418b98..806c9290 100644 --- a/exports/recalbox/es_bios.xml +++ b/exports/recalbox/es_bios.xml @@ -146,7 +146,7 @@ + cd237e16e7e77c06bb58540e9e9fca68,e246140dec5124c5e404869a84caefce" hashMatchMandatory="false" core="libretro/np2kai" mandatory="false" /> - + @@ -579,7 +579,7 @@ - + diff --git a/scripts/export_native.py b/scripts/export_native.py index e5f5f40a..1ccddb68 100644 --- a/scripts/export_native.py +++ b/scripts/export_native.py @@ -535,9 +535,10 @@ def landed_changes(exporter: object, systems: dict) -> LandedChanges: "added", entry, None, None, _added_fields(exporter, entry) )) continue + # Asked of the exporter: a format that names each entry's cores + # reads every entry of those cores, not only the paired one. if ( - (entry.truth or {}).get("required") is not None - and entry.platform.get("required") is not None + entry.platform.get("required") is not None and exporter.states(entry, "required") and exporter.required_value(entry) != bool(entry.platform["required"]) ): diff --git a/scripts/exporter/baseline.py b/scripts/exporter/baseline.py index f55bfb1a..e6f28bef 100644 --- a/scripts/exporter/baseline.py +++ b/scripts/exporter/baseline.py @@ -122,6 +122,10 @@ class NativeFile: # (Recalbox on a .zip): the truth's md5 describes the container and # can be neither compared with it nor written in its place. composite_md5: bool = False + # Every truth entry of the system that answers to this platform entry's + # name, whichever one the pairing took: the element names its cores, and + # each core's own entry for the file says whether that core needs it. + namesakes: list[dict] = field(default_factory=list) @property def origin(self) -> str: @@ -563,6 +567,16 @@ def build_native_model( if candidate.truth is None ] pairs = pair_entries(truth_files, [c.platform or {} for c in candidates]) + for candidate in ( + c + for native_id in target_ids + for c in systems.get(native_id, NativeSystem(native_id)).files + if c.platform is not None + ): + own = str(candidate.name).casefold() + candidate.namesakes.extend( + entry for entry in truth_files if own in _names(entry) + ) for t_index, truth_entry in enumerate(truth_files): name = str(truth_entry["name"]) diff --git a/scripts/exporter/recalbox_exporter.py b/scripts/exporter/recalbox_exporter.py index 490ae90b..71704304 100644 --- a/scripts/exporter/recalbox_exporter.py +++ b/scripts/exporter/recalbox_exporter.py @@ -23,7 +23,7 @@ sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) from common import load_emulator_profiles, parse_untrusted_xml from .base_exporter import BaseExporter -from .baseline import NativeFile, NativeSystem, Report +from .baseline import NativeFile, NativeSystem, Report, by_destination, by_name SOURCE_URL = ( "https://gitlab.com/recalbox/recalbox/-/raw/master/board/recalbox/fsoverlay" @@ -165,27 +165,61 @@ class Exporter(BaseExporter): def required_value(self, fe: NativeFile) -> bool: """Whether the cores the element names need the file. - An element lists its cores, and the merged flag is true when any - core needs the file: cdi/cdibios.zip names libretro/same_cdi, whose - driver declares no such set, and became mandatory because MAME - requires it. Where none of the element's cores reads the file, the - truth says nothing about it and the platform's value stands. + The element lists its cores, and each core's own entry for the file, + found by name, says whether that core needs it. The md5 pairing + takes one entry only: saturn/saturn_bios.bin names yabause, + yabasanshiro and kronos, which require it, and its md5 paired it + with the beetle_saturn entry. The merged flag is true when any core + needs the file: cdi/cdibios.zip names same_cdi, which declares no + such set, and became mandatory because MAME requires it. Where none + of the element's cores declares the file, the platform's value + stands. """ - truth = fe.truth or {} - required_by = truth.get("_required_by") - readers = truth.get("_cores") - if required_by is None or not readers: + entries = [fe.truth] if fe.truth else [] + entries += [ + e for e in fe.namesakes + if e is not fe.truth and self._same_file(e, fe.platform or {}) + ] + if not entries: return fe.required index = self._profile_keys() named = { index.get(core.rsplit("/", 1)[-1].replace("-", "_").lower(), "") for core in fe.cores() - } & set(readers) - if not named: + } - {""} + readers: set[str] = set() + requirers: set[str] = set() + for entry in entries: + cores = set(entry.get("_cores") or ()) & named + readers |= cores + required_by = entry.get("_required_by") + if required_by is None: + if entry.get("required") is not False: + requirers |= cores + else: + requirers |= cores & set(required_by) + if not readers: if fe.platform is not None and fe.platform.get("required") is not None: return bool(fe.platform["required"]) return fe.required - return bool(named & set(required_by)) + return bool(requirers) + + @staticmethod + def _same_file(entry: dict, declared: dict) -> bool: + """Whether a truth entry of that name describes this element's file. + + Its path or its name in the same directory does. A name under an + unrelated directory does too: kronos reads kronos/saturn_bios.bin, + and Recalbox files the same BIOS under saturn/. A name in another + directory of the same tree does not: bluemsx reads + Machines/Shared Roms/KANJI.rom, which is not Recalbox's + Machines/MSX2+ - European/KANJI.rom. + """ + if by_destination(entry, declared) or by_name(entry, declared): + return True + theirs = str(declared.get("destination") or declared.get("name", "")).split("/")[:-1] + ours = str(entry.get("path") or entry.get("name", "")).split("/")[:-1] + return not {d.casefold() for d in theirs} & {d.casefold() for d in ours} def _bios_element(self, fe: NativeFile, native_id: str) -> str: attrs = [f"path={quoteattr(self._path(fe))}"] diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 72025477..24e17ef4 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -1169,6 +1169,57 @@ class RecalboxExport(unittest.TestCase): [("cdimono1.zip", False, True)], ) + def test_mandatory_reads_the_named_cores_not_the_hash_pairing(self): + """saturn/saturn_bios.bin names yabause, yabasanshiro and kronos, + which require it. Its md5 paired it with the beetle_saturn entry, + which does not, and the element went out mandatory="false".""" + md5 = "af5828fdff51384f99b3c4926be27762" + scraped = {"systems": {"sega-saturn": {"native_id": "saturn", "files": [{ + "name": "saturn_bios.bin", "destination": "saturn/saturn_bios.bin", + "md5": md5, "native_system": "saturn", "required": True, + "core": "libretro/yabause,libretro/kronos", + }]}}} + truth = {"systems": {"sega-saturn": {"files": [ + {"name": "saturn_bios.bin", "path": "saturn_bios.bin", "md5": md5, + "required": True, "_cores": ["beetle_saturn", "kronos", "yabause"], + "_required_by": ["beetle_saturn"]}, + {"name": "saturn_bios.bin", "path": "kronos/saturn_bios.bin", + "required": True, "_cores": ["kronos", "yabause"], + "_required_by": ["kronos", "yabause"]}, + ]}}} + systems, report = build_native_model(truth, scraped) + exporter = discover_exporters()["recalbox"]() + declared = next(fe for s in systems.values() for fe in s.files if fe.platform) + self.assertEqual(declared.truth["_required_by"], ["beetle_saturn"]) + produced = exporter.render(systems, report, {}, None)["es_bios.xml"] + element = next( + b for b in ET.fromstring(produced).iter("bios") + if b.get("path") == "saturn/saturn_bios.bin" + ) + self.assertNotEqual(element.get("mandatory"), "false") + self.assertEqual( + [c for c in landed_changes(exporter, systems).records + if c["kind"] == "requirement_corrected"], + [], + ) + + def test_a_namesake_elsewhere_in_the_same_tree_is_another_file(self): + """bluemsx reads Machines/Shared Roms/KANJI.rom, not Recalbox's + Machines/MSX2+ - European/KANJI.rom.""" + scraped = {"systems": {"msx": {"native_id": "msx2", "files": [{ + "name": "KANJI.rom", "destination": "Machines/MSX2+ - European/KANJI.rom", + "md5": "a" * 32, "native_system": "msx2", "required": True, + "core": "libretro/bluemsx", + }]}}} + truth = {"systems": {"msx": {"files": [{ + "name": "KANJI.rom", "path": "Machines/Shared Roms/KANJI.rom", + "md5": "b" * 32, "required": False, "_cores": ["bluemsx"], "_required_by": [], + }]}}} + systems, _ = build_native_model(truth, scraped) + exporter = discover_exporters()["recalbox"]() + declared = next(fe for s in systems.values() for fe in s.files if fe.platform) + self.assertTrue(exporter.required_value(declared)) + def test_an_element_kept_as_written_is_not_counted_lost(self): """The truth takes the entry on sha1 and size and names no md5: the element is written back whole, and the run counted it a platform