From 144cd46df112f24d8f7d8ce685a5195459198944 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 6 Nov 2026 14:55:23 +0100 Subject: [PATCH] fix: count one export line per addition path --- scripts/exporter/recalbox_exporter.py | 29 ++++++++---- scripts/exporter/retrodeck_exporter.py | 19 ++++++-- scripts/exporter/systemdat_exporter.py | 15 +++++- tests/test_exporters.py | 64 ++++++++++++++++++++++++++ 4 files changed, 113 insertions(+), 14 deletions(-) diff --git a/scripts/exporter/recalbox_exporter.py b/scripts/exporter/recalbox_exporter.py index bad223329..161c9a9bb 100644 --- a/scripts/exporter/recalbox_exporter.py +++ b/scripts/exporter/recalbox_exporter.py @@ -221,12 +221,19 @@ class Exporter(BaseExporter): 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: + def _bios_element( + self, fe: NativeFile, native_id: str, also: tuple[NativeFile, ...] = () + ) -> str: + """``also`` are further additions at fe's path: Bios.cpp reads one + element per path, and its md5 and core lists take every one of them.""" + group = (fe, *also) + md5 = dict.fromkeys(h for f in group for h in f.hashes("md5")) + cores = dict.fromkeys(c for f in group for c in f.cores()) attrs = [f"path={quoteattr(self._path(fe))}"] - attrs.append(f'md5={quoteattr(",".join(fe.hashes("md5")))}') - attrs.append(f'core={quoteattr(",".join(fe.cores()))}') + attrs.append(f'md5={quoteattr(",".join(md5))}') + attrs.append(f'core={quoteattr(",".join(cores))}') - if not self.required_value(fe): + if not any(self.required_value(f) for f in group): attrs.append('mandatory="false"') elif fe.native("mandatory_declared", None) is True: attrs.append('mandatory="true"') @@ -343,11 +350,17 @@ class Exporter(BaseExporter): } ours: dict[str, tuple[NativeSystem, dict[str, tuple[NativeFile, str]]]] = {} for system in systems.values(): - writable = [fe for fe in system.files if self.writable(fe)] - if writable: + # Two additions at one path (two cores, two dumps) are one + # element: keyed by path, the second replaced the first and its + # md5 left the file while both were counted added. + at_path: dict[str, list[NativeFile]] = {} + for fe in system.files: + if self.writable(fe): + at_path.setdefault(self._path(fe), []).append(fe) + if at_path: ours[system.native_id] = (system, { - self._path(fe): (fe, self._bios_element(fe, system.native_id)) - for fe in writable + path: (group[0], self._bios_element(group[0], system.native_id, tuple(group[1:]))) + for path, group in at_path.items() }) def element(attrs: dict[str, str]) -> str: diff --git a/scripts/exporter/retrodeck_exporter.py b/scripts/exporter/retrodeck_exporter.py index b742b796c..b9a88a027 100644 --- a/scripts/exporter/retrodeck_exporter.py +++ b/scripts/exporter/retrodeck_exporter.py @@ -215,6 +215,7 @@ class Exporter(BaseExporter): ours: list[OrderedDict], additions: list[OrderedDict] | None = None, appended: set[tuple[str, str]] | None = None, + held_back: list[int] | None = None, ) -> list[OrderedDict]: """Correct the component's own list, never replace it. @@ -301,13 +302,19 @@ class Exporter(BaseExporter): for e in merged for system in (_systems_of(e) or [""]) } - for entry in additions or []: + for index, entry in enumerate(additions or []): key = (str(entry.get("filename", "")), str(entry.get("system", "")), _paths_key(entry)) if key[0] and key not in held: held.add(key) merged.append(entry) + elif held_back is not None: + held_back.append(index) return merged + def lands(self, fe: NativeFile) -> bool: + """An addition the list already held is not written a second time.""" + return self.writable(fe) and id(fe) not in getattr(self, "_held_back", set()) + def render( self, systems: dict[str, NativeSystem], @@ -317,6 +324,7 @@ class Exporter(BaseExporter): ) -> dict[str, str]: self._place_additions(systems) self._appended: set[int] = set() + self._held_back: set[int] = set() grouped = self._by_component(systems) produced: dict[str, str] = {} @@ -331,25 +339,28 @@ class Exporter(BaseExporter): continue entries = [self._entry(fe) for fe in files if fe.platform is not None] - additions = [self._entry(fe) for fe in files if fe.platform is None] + added = [fe for fe in files if fe.platform is None] + additions = [self._entry(fe) for fe in added] appended: set[tuple[str, str]] = set() + held_back: list[int] = [] for component_value in manifest.values(): if not isinstance(component_value, dict): continue holder = self._bios_holder(component_value) if holder is None: component_value["bios"] = self._merge( - None, entries, additions, appended + None, entries, additions, appended, held_back ) else: container, key = holder written = container.get(key) - merged = self._merge(written, entries, additions, appended) + merged = self._merge(written, entries, additions, appended, held_back) # A single object stays one while nothing is added to it. container[key] = ( merged[0] if isinstance(written, dict) and len(merged) == 1 else merged ) break + self._held_back.update(id(added[index]) for index in held_back) # A declared entry whose revision the platform does not hold is # written beside theirs: a new line, not a corrected value. self._appended.update( diff --git a/scripts/exporter/systemdat_exporter.py b/scripts/exporter/systemdat_exporter.py index a42aa6c86..3bb02de04 100644 --- a/scripts/exporter/systemdat_exporter.py +++ b/scripts/exporter/systemdat_exporter.py @@ -175,6 +175,9 @@ class Exporter(BaseExporter): ours: dict[str, dict[str, str]] = {} added: dict[str, set[str]] = {} + # A rom name is one line: an addition whose name an earlier entry + # took is not written, and was counted added. + self._shadowed: set[int] = set() for system, files in self.exportable(systems): rendered: dict[str, str] = {} for fe in files: @@ -194,7 +197,11 @@ class Exporter(BaseExporter): sha1 = fe.record_hash("sha1") if sha1: parts.append(f"sha1 {sha1}") - rendered.setdefault(name, f"\trom ( {' '.join(parts)} )") + if name in rendered: + if fe.platform is None: + self._shadowed.add(id(fe)) + continue + rendered[name] = f"\trom ( {' '.join(parts)} )" if fe.platform is None: added.setdefault(system.native_id, set()).add(name) if rendered: @@ -241,7 +248,11 @@ class Exporter(BaseExporter): return {self.native_filename(): "\n".join(lines)} def lands(self, fe: NativeFile) -> bool: - return self.writable(fe) and id(fe) not in getattr(self, "_untitled", set()) + return ( + self.writable(fe) + and id(fe) not in getattr(self, "_untitled", set()) + and id(fe) not in getattr(self, "_shadowed", set()) + ) def validate( self, diff --git a/tests/test_exporters.py b/tests/test_exporters.py index 8dc24b4ff..8d340d6e9 100644 --- a/tests/test_exporters.py +++ b/tests/test_exporters.py @@ -827,6 +827,70 @@ class SearchOrder(unittest.TestCase): self.assertEqual(entry.priority, 2) +class TwoAdditionsAtOnePath(unittest.TestCase): + """Two cores declaring two dumps of one file the platform lacks: both + were counted added while the render wrote one. Recalbox's md5 list takes + both, a System.dat name and a RetroDECK path hold one.""" + + A, B = "615a" + "0" * 28, "ef49" + "0" * 28 + + def _addition(self, md5: str, core: str, system: str = "dos") -> NativeFile: + return NativeFile( + name="MT32_CONTROL.ROM", destination="mt32/MT32_CONTROL.ROM", native_system=system, + truth={"name": "MT32_CONTROL.ROM", "path": "mt32/MT32_CONTROL.ROM", "md5": md5, + "crc32": md5[:8], "required": True, "_cores": [core]}, + runners=[(False, core)], + ) + + def _added(self, exporter, systems) -> list[str]: + return [c["name"] for c in landed_changes(exporter, systems).records + if c["kind"] == "added"] + + def test_recalbox_writes_both_md5_in_one_element(self): + systems = {"dos": NativeSystem("dos", files=[ + self._addition(self.A, "dosbox_pure"), self._addition(self.B, "dosbox_core")])} + exporter = discover_exporters()["recalbox"]() + produced = exporter.render(systems, Report(), {})["es_bios.xml"] + elements = [e for e in ET.fromstring(produced).iter("bios")] + self.assertEqual(len(elements), 1) + self.assertEqual(elements[0].get("md5").split(","), [self.A, self.B]) + self.assertEqual(elements[0].get("core"), "libretro/dosbox_pure,libretro/dosbox_core") + self.assertEqual(exporter.validate(systems, {"es_bios.xml": produced}), []) + self.assertEqual(len(self._added(exporter, systems)), 2) + + def test_system_dat_counts_the_line_it_wrote(self): + systems = {"DOS": NativeSystem("DOS", files=[ + self._addition(self.A, "dosbox_pure", "DOS"), + self._addition(self.B, "dosbox_core", "DOS")])} + exporter = discover_exporters()["retroarch"]() + original = 'clrmamepro (\n\tname "System"\n)\n\ngame (\n\tname "System"\n' \ + '\tcomment "System"\n\n\tcomment "DOS"\n\trom ( name a.bin size 1 crc 00000000 )\n)\n' + produced = exporter.render(systems, Report(), {"System.dat": original})["System.dat"] + self.assertEqual(produced.count("MT32_CONTROL.ROM"), 1) + self.assertEqual(len(self._added(exporter, systems)), 1) + + def test_retrodeck_counts_the_entry_it_appended(self): + theirs = NativeFile( + name="scph1001.bin", destination="scph1001.bin", native_system="psx", + platform={"name": "scph1001.bin", "md5": "1" * 32, "component": "duckstation"}) + + def addition(md5: str) -> NativeFile: + return NativeFile( + name="scph5501.bin", destination="scph5501.bin", native_system="psx", + truth={"name": "scph5501.bin", "md5": md5, "required": True, + "_cores": ["duckstation"]}, + runners=[(True, "duckstation")]) + + systems = {"psx": NativeSystem("psx", files=[theirs, addition(self.A), addition(self.B)])} + manifest = json.dumps({"duckstation": {"name": "DuckStation", "bios": [ + {"filename": "scph1001.bin", "md5": "1" * 32, "system": "psx"}]}}) + exporter = discover_exporters()["retrodeck"]() + produced = exporter.render(systems, Report(), {"duckstation/component_manifest.json": manifest}) + bios = json.loads(produced["duckstation/component_manifest.json"])["duckstation"]["bios"] + self.assertEqual([e["filename"] for e in bios], ["scph1001.bin", "scph5501.bin"]) + self.assertEqual(self._added(exporter, systems), ["scph5501.bin"]) + + class AdditionOrder(unittest.TestCase): """What we add is listed the way the code looks for it."""