fix: count what each export format writes

scripts/export_native.py

@@ -267,16 +267,28 @@ def export_platform(
         path.parent.mkdir(parents=True, exist_ok=True)
         path.write_text(content, encoding="utf-8")
 
-    # Only what the format can state: a correction to a field the file has
-    # no place for is not a change the maintainer will find in the diff, and
-    # counting it would announce work the export did not do.
-    carried = exporter.carries()
-    applied = [
-        correction
-        for correction in report.hashes_corrected
-        if correction.rsplit(" ", 1)[-1] in carried
-    ]
-    requirements = len(report.required_corrected) if "required" in carried else 0
+    # Only what the format states: a correction to a field the file has no
+    # place for, or one its render keeps out, is not a change the maintainer
+    # will find in the diff, and counting it would announce work the export
+    # did not do. A hash written where the platform left none changes the
+    # check from existence to content, so it is counted too.
+    applied: list[str] = []
+    filled: list[str] = []
+    requirements = 0
+    for system in systems.values():
+        for entry in system.files:
+            for field_name in entry.corrections:
+                if not exporter.states(entry, field_name):
+                    continue
+                if field_name == "required":
+                    requirements += 1
+                else:
+                    applied.append(f"{entry.native_system}/{entry.name} {field_name}")
+            filled.extend(
+                f"{entry.native_system}/{entry.name} {field_name}"
+                for field_name in entry.filled
+                if exporter.states(entry, field_name)
+            )
 
     landed = 0
     refused = 0
@@ -300,6 +312,8 @@ def export_platform(
             f"{report.files_kept} kept, {landed} added, "
             f"{len(applied)} hashes corrected, {requirements} requirements corrected"
         )
+        if filled:
+            summary += f", {len(filled)} hashes filled"
         if refused:
             summary += f", {refused} the format cannot state"
         if lost:
@@ -309,6 +323,10 @@ def export_platform(
         messages.append(f"hash corrected: {correction}")
     if len(applied) > 5:
         messages.append(f"and {len(applied) - 5} more hash corrections")
+    for fill in filled[:5]:
+        messages.append(f"hash filled: {fill}")
+    if len(filled) > 5:
+        messages.append(f"and {len(filled) - 5} more hash fills")
 
     messages.extend(f"INVALID: {issue}" for issue in issues[:10])
     if len(issues) > 10:

scripts/exporter/base_exporter.py

@@ -133,6 +133,15 @@ class BaseExporter(ABC):
         require = require or cls.requires()
         return not require or bool(fe.hash(require))
 
+    def states(self, fe: NativeFile, field_name: str) -> bool:
+        """Whether the written file carries this entry's corrected field.
+
+        The summary counts what lands, read entry by entry: a correction the
+        model makes and the format keeps out of the file is not a change the
+        maintainer will find in the diff.
+        """
+        return field_name in self.carries()
+
     def outcome(
         self,
         systems: dict[str, NativeSystem],

scripts/exporter/baseline.py

@@ -48,6 +48,8 @@ class NativeFile:
     platform: dict | None = None
     truth: dict | None = None
     corrections: list[str] = field(default_factory=list)
+    # Hash fields the platform left empty and the truth fills.
+    filled: list[str] = field(default_factory=list)
 
     @property
     def origin(self) -> str:
@@ -104,7 +106,16 @@ class NativeFile:
         return None
 
     def size(self) -> int | None:
-        for entry in (self.truth, self.platform):
+        """Size of the content the written hashes describe.
+
+        hashes() keeps the platform's values when the truth has none, so the
+        truth's size is only taken when the truth also speaks for the hash:
+        a name-matched 480-byte fbneo boot.bin turned RomM's Dreamcast
+        boot.bin into size 480 beside its 2 MB md5, which never verifies.
+        """
+        truth_hashed = any(_hash_values(self.truth or {}, f) for f in HASH_FIELDS)
+        order = (self.truth, self.platform) if truth_hashed else (self.platform, self.truth)
+        for entry in order:
             if entry and entry.get("size"):
                 return int(entry["size"])
         return None
@@ -310,6 +321,8 @@ def build_native_model(
                         report.hashes_corrected.append(
                             f"{matched.native_system}/{matched.name} {field_name}"
                         )
+                    elif ours and not theirs:
+                        matched.filled.append(field_name)
                 t_req = truth_entry.get("required")
                 p_req = (matched.platform or {}).get("required")
                 if (
@@ -323,6 +336,18 @@ def build_native_model(
                     )
                 continue
 
+            # A file another core's entry already claimed is still the
+            # platform's file, not a new one: scph101.bin, declared by two
+            # PSX cores, was written into RetroDECK's manifest a second time.
+            declared = [
+                candidate
+                for native_id in target_ids
+                for candidate in systems.get(native_id, NativeSystem(native_id)).files
+                if candidate.platform is not None
+            ]
+            if any(by_destination(c) or by_name(c) for c in declared):
+                continue
+
             # The truth knows a file the platform does not declare.
             native_id = target_ids[0]
             system = systems.get(native_id)

scripts/exporter/emudeck_exporter.py

@@ -18,7 +18,7 @@ sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
 from scraper.emudeck_scraper import FUNCTION_HASH_MAP, _RE_FUNC, _RE_LOCAL_HASHES
 
 from .base_exporter import BaseExporter
-from .baseline import NativeSystem, Report
+from .baseline import NativeFile, NativeSystem, Report, _hash_values
 
 SOURCE_URL = (
     "https://raw.githubusercontent.com/dragoonDorise/EmuDeck/main"
@@ -71,6 +71,27 @@ class Exporter(BaseExporter):
         """
         return False
 
+    @staticmethod
+    def _entry_md5s(fe: NativeFile) -> list[str]:
+        """The values one platform entry puts in the array, corrected in place.
+
+        An entry keeps its own values unless the truth contradicts all of
+        them, and then the truth's replace them. An entry without a hash
+        adds nothing, and the truth's extra accepted revisions are not
+        appended: either would grow an array other consumers read too.
+        """
+        theirs = _hash_values(fe.platform or {}, "md5")
+        if not theirs:
+            return []
+        if "md5" in fe.corrections:
+            return fe.hashes("md5")
+        return theirs
+
+    def states(self, fe: NativeFile, field_name: str) -> bool:
+        if field_name in fe.filled:
+            return False
+        return super().states(fe, field_name)
+
     @classmethod
     def _md5s(cls, systems: dict[str, NativeSystem], system_id: str) -> list[str]:
         """Every MD5 the system accepts, in a stable order, deduplicated."""
@@ -81,7 +102,7 @@ class Exporter(BaseExporter):
             for fe in system.files:
                 if not cls.writable(fe):
                     continue
-                for value in fe.hashes("md5"):
+                for value in cls._entry_md5s(fe):
                     if _MD5.match(value) and value not in seen:
                         seen.append(value)
         return seen

scripts/exporter/retrobat_exporter.py

@@ -10,7 +10,7 @@ import json
 from collections import OrderedDict
 
 from .base_exporter import BaseExporter
-from .baseline import NativeSystem, Report
+from .baseline import NativeFile, NativeSystem, Report
 
 SOURCE_URL = (
     "https://raw.githubusercontent.com/RetroBat-Official/emulatorlauncher/master"
@@ -41,6 +41,24 @@ class Exporter(BaseExporter):
     def native_sources() -> dict[str, str]:
         return {"batocera-systems.json": SOURCE_URL}
 
+    @staticmethod
+    def _md5(fe: NativeFile) -> str:
+        """The one md5 the entry is checked against, empty for existence.
+
+        RetroBat writes an empty md5 to check a file by existence. Filling it
+        turns that into a content check on a single value: right when the
+        truth accepts exactly one image, wrong when the core takes several
+        and the field can hold only the first.
+        """
+        if "md5" in fe.filled and len(fe.hashes("md5")) != 1:
+            return ""
+        return fe.hash("md5")
+
+    def states(self, fe: NativeFile, field_name: str) -> bool:
+        if field_name == "md5" and "md5" in fe.filled:
+            return bool(self._md5(fe))
+        return super().states(fe, field_name)
+
     def render(
         self,
         systems: dict[str, NativeSystem],
@@ -71,7 +89,7 @@ class Exporter(BaseExporter):
             bios_files = []
             for fe in files:
                 entry: OrderedDict[str, str] = OrderedDict()
-                entry["md5"] = fe.hash("md5")
+                entry["md5"] = self._md5(fe)
                 declared = fe.native("native_path", "")
                 entry["file"] = str(declared) if declared else f"bios/{fe.destination}"
                 bios_files.append(entry)

scripts/exporter/retrodeck_exporter.py

@@ -19,6 +19,10 @@ COMPONENTS_REPO = "RetroDECK/components"
 COMPONENTS_BRANCH = "main"
 RAW_BASE = f"https://raw.githubusercontent.com/{COMPONENTS_REPO}/{COMPONENTS_BRANCH}"
 MANIFEST = "component_manifest.json"
+# Labels that only say yes or no, so a corrected requirement can be written
+# over them. Every other label is a sentence ("At least one BIOS file
+# required", "Required for some Japanese games.") and is the platform's own.
+PLAIN_LABELS = ("Required", "Optional")
 
 
 class Exporter(BaseExporter):
@@ -42,6 +46,42 @@ class Exporter(BaseExporter):
         # one from BIOS data alone would throw the component away.
         return True
 
+    @classmethod
+    def writable(cls, fe: NativeFile, require: str = "") -> bool:
+        """An addition lands only in a manifest it can be placed in."""
+        if fe.platform is None and not fe.native("component", ""):
+            return False
+        return super().writable(fe, require)
+
+    def states(self, fe: NativeFile, field_name: str) -> bool:
+        if field_name == "required":
+            label = str(fe.native("required_label", ""))
+            return not label or label in PLAIN_LABELS
+        return super().states(fe, field_name)
+
+    @staticmethod
+    def _place_additions(systems: dict[str, NativeSystem]) -> None:
+        """Give each addition the component its system already lives in.
+
+        A component is one emulator's manifest, and only the platform's
+        entries name it. A system whose entries all sit in one component
+        takes the addition there; one spread over several, or with none of
+        its own, cannot say which manifest is meant, and the addition is
+        left out and counted as such.
+        """
+        for system in systems.values():
+            components = {
+                str(fe.native("component", ""))
+                for fe in system.files
+                if fe.platform is not None and fe.native("component", "")
+            }
+            if len(components) != 1:
+                continue
+            component = components.pop()
+            for fe in system.files:
+                if fe.platform is None and fe.truth is not None:
+                    fe.truth = {**fe.truth, "component": component}
+
     @staticmethod
     def component_url(component: str) -> str:
         return f"{RAW_BASE}/{component}/{MANIFEST}"
@@ -73,11 +113,13 @@ class Exporter(BaseExporter):
         # RetroDECK words the requirement in prose ("Required", "At least one
         # BIOS file required"), so the platform's own wording is kept and a
         # boolean is only rendered when there is none to keep.
-        label = fe.native("required_label", "")
-        if label:
-            entry["required"] = str(label)
+        label = str(fe.native("required_label", ""))
+        if label and label not in PLAIN_LABELS:
+            entry["required"] = label
         elif fe.required:
             entry["required"] = "Required"
+        elif label:
+            entry["required"] = "Optional"
         destination = fe.destination
         if destination and destination not in (fe.name, f"bios/{fe.name}"):
             directory = destination.rsplit("/", 1)[0]
@@ -116,29 +158,47 @@ class Exporter(BaseExporter):
         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()
+        # Keyed by name AND system: the retroarch manifest declares
+        # ATARIOSB.ROM for atari5200 and atari800 with different md5 lists,
+        # and a name-only key wrote the first of ours over both.
+        by_key: OrderedDict[tuple[str, str], OrderedDict] = OrderedDict()
         for entry in ours:
-            name = str(entry.get("filename", ""))
-            if name and name not in by_name:
-                by_name[name] = entry
+            key = (str(entry.get("filename", "")), str(entry.get("system", "")))
+            if key[0] and key not in by_key:
+                by_key[key] = entry
 
         merged: list[OrderedDict] = []
-        corrected: set[str] = set()
+        corrected: set[tuple[str, 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:
+            declared = entry.get("system")
+            # One entry can serve several systems: neogeo.zip is declared
+            # once for neogeo, fbneo and arcade.
+            systems = (
+                [str(s) for s in declared] if isinstance(declared, list)
+                else [str(declared)] if declared else []
+            )
+            keys = [(name, s) for s in systems if (name, s) in by_key]
+            if not systems:
+                # An entry without a system matches ours only when one
+                # system alone declares the name.
+                named = [k for k in by_key if k[0] == name]
+                keys = named if len(named) == 1 else []
+            if not keys:
                 merged.append(OrderedDict(entry))
                 continue
             combined = OrderedDict(entry)
-            combined.update(ours_entry)
+            combined.update(
+                (field, value) for field, value in by_key[keys[0]].items()
+                if not (field == "system" and declared)
+            )
             merged.append(combined)
-            corrected.add(name)
+            corrected.update(keys)
 
         merged.extend(
-            entry for name, entry in by_name.items() if name not in corrected
+            entry for key, entry in by_key.items() if key not in corrected
         )
         return merged
 
@@ -149,6 +209,7 @@ class Exporter(BaseExporter):
         originals: dict[str, str],
         scraped: dict | None = None,
     ) -> dict[str, str]:
+        self._place_additions(systems)
         grouped = self._by_component(systems)
         produced: dict[str, str] = {}
 

tests/test_export_counts.py

@@ -0,0 +1,139 @@
+"""An export writes what its summary announces, and nothing else.
+
+RetroDECK announced 1474 additions and wrote none, counted 161 requirement
+corrections while keeping the platform's prose labels, and wrote one
+system's ATARIOSB.ROM over another's. RetroBat and EmuDeck turned empty or
+partial hash lists into content checks nobody counted, and a size was taken
+from a file other than the one the hash describes.
+"""
+
+from __future__ import annotations
+
+import json
+import sys
+import unittest
+from collections import OrderedDict
+from pathlib import Path
+
+REPO_ROOT = Path(__file__).resolve().parents[1]
+sys.path.insert(0, str(REPO_ROOT / "scripts"))
+
+from exporter.baseline import NativeFile, NativeSystem, build_native_model  # noqa: E402
+from exporter.emudeck_exporter import Exporter as EmuDeck  # noqa: E402
+from exporter.retrobat_exporter import Exporter as RetroBat  # noqa: E402
+from exporter.retrodeck_exporter import Exporter as RetroDeck  # noqa: E402
+
+A = "a" * 32
+B = "b" * 32
+C = "c" * 32
+
+
+class RetroDeckWritesWhatItCounts(unittest.TestCase):
+    def test_same_name_in_two_systems_stays_two_entries(self):
+        existing = [
+            {"filename": "ATARIOSB.ROM", "system": "atari5200", "md5": f"{A},{B}"},
+            {"filename": "ATARIOSB.ROM", "system": "atari800", "md5": B},
+        ]
+        ours = [
+            OrderedDict(filename="ATARIOSB.ROM", system="atari800", md5=C),
+        ]
+        merged = RetroDeck._merge(existing, ours)
+        by_system = {e["system"]: e["md5"] for e in merged}
+        self.assertEqual(by_system, {"atari5200": f"{A},{B}", "atari800": C})
+        self.assertEqual(len(merged), 2)
+
+    def test_a_list_of_systems_is_kept(self):
+        existing = [{"filename": "neogeo.zip", "system": ["neogeo", "fbneo"], "md5": A}]
+        ours = [OrderedDict(filename="neogeo.zip", system="fbneo", md5=B)]
+        merged = RetroDeck._merge(existing, ours)
+        self.assertEqual(len(merged), 1)
+        self.assertEqual(merged[0]["system"], ["neogeo", "fbneo"])
+        self.assertEqual(merged[0]["md5"], B)
+
+    def test_prose_label_is_neither_rewritten_nor_counted(self):
+        exporter = RetroDeck()
+        prose = NativeFile(
+            "x.bin", "bios/x.bin", "psx",
+            platform={"required": True, "required_label": "At least one BIOS file required"},
+            truth={"required": False},
+        )
+        plain = NativeFile(
+            "y.bin", "bios/y.bin", "psx",
+            platform={"required": True, "required_label": "Required"},
+            truth={"required": False},
+        )
+        self.assertFalse(exporter.states(prose, "required"))
+        self.assertEqual(RetroDeck._entry(prose)["required"], "At least one BIOS file required")
+        self.assertTrue(exporter.states(plain, "required"))
+        self.assertEqual(RetroDeck._entry(plain)["required"], "Optional")
+
+    def test_addition_goes_to_the_system_component(self):
+        declared = NativeFile("a.bin", "bios/a.bin", "psx",
+                              platform={"component": "duckstation", "md5": A})
+        added = NativeFile("b.bin", "bios/b.bin", "psx", truth={"md5": B})
+        spread = NativeSystem("amiga", files=[
+            NativeFile("k1", "bios/k1", "amiga", platform={"component": "retroarch"}),
+            NativeFile("k2", "bios/k2", "amiga", platform={"component": "puae"}),
+            NativeFile("k3", "bios/k3", "amiga", truth={"md5": C}),
+        ])
+        systems = {"psx": NativeSystem("psx", files=[declared, added]), "amiga": spread}
+        manifest = json.dumps({"duckstation": {"name": "DuckStation", "bios": []}})
+        produced = RetroDeck().render(
+            systems, None, {"duckstation/component_manifest.json": manifest}
+        )
+        written = json.loads(produced["duckstation/component_manifest.json"])
+        names = [e["filename"] for e in written["duckstation"]["bios"]]
+        self.assertIn("b.bin", names)
+        self.assertFalse(RetroDeck.writable(spread.files[2]))
+
+
+class HashesTheFormatCannotHold(unittest.TestCase):
+    def test_retrobat_fills_only_a_single_accepted_value(self):
+        exporter = RetroBat()
+        single = NativeFile("s.bin", "s.bin", "nds", platform={"md5": ""},
+                            truth={"md5": A}, filled=["md5"])
+        several = NativeFile("m.bin", "m.bin", "nds", platform={"md5": ""},
+                             truth={"md5": [A, B]}, filled=["md5"])
+        self.assertEqual(RetroBat._md5(single), A)
+        self.assertTrue(exporter.states(single, "md5"))
+        self.assertEqual(RetroBat._md5(several), "")
+        self.assertFalse(exporter.states(several, "md5"))
+
+    def test_emudeck_never_grows_an_array(self):
+        unhashed = NativeFile("scph5501.bin", "scph5501.bin", "psx",
+                              platform={"md5": ""}, truth={"md5": C}, filled=["md5"])
+        extra = NativeFile("scph1001.bin", "scph1001.bin", "psx",
+                           platform={"md5": A}, truth={"md5": [A, B]})
+        corrected = NativeFile("scph7001.bin", "scph7001.bin", "psx",
+                               platform={"md5": B}, truth={"md5": C},
+                               corrections=["md5"])
+        systems = {"psx": NativeSystem("psx", files=[unhashed, extra, corrected])}
+        self.assertEqual(EmuDeck._md5s(systems, "psx"), [A, C])
+        self.assertFalse(EmuDeck().states(unhashed, "md5"))
+
+
+class ModelKeepsOneFileOneEntry(unittest.TestCase):
+    def test_size_describes_the_hash_written(self):
+        fe = NativeFile("boot.bin", "dc/boot.bin", "dc",
+                        platform={"size": 2097152, "md5": A}, truth={"size": 480})
+        self.assertEqual(fe.size(), 2097152)
+        hashed = NativeFile("boot.bin", "dc/boot.bin", "dc",
+                            platform={"size": 2097152, "md5": A},
+                            truth={"size": 480, "md5": B})
+        self.assertEqual(hashed.size(), 480)
+
+    def test_second_core_entry_for_a_declared_file_adds_nothing(self):
+        scraped = {"systems": {"psx": {"files": [
+            {"name": "scph101.bin", "destination": "bios/scph101.bin", "md5": A},
+        ]}}}
+        truth = {"systems": {"psx": {"files": [
+            {"name": "scph101.bin", "path": "scph101.bin", "md5": A},
+            {"name": "scph101.bin", "path": "scph101.bin", "md5": B},
+        ]}}}
+        systems, report = build_native_model(truth, scraped)
+        self.assertEqual(report.files_added, 0)
+        self.assertEqual(len(systems["psx"].files), 1)
+
+
+if __name__ == "__main__":
+    unittest.main()