chore: merge audit fixes (mame)

scripts/scraper/_hash_merge.py

@@ -595,12 +595,82 @@ def _zip_scalar(name: str) -> str:
     return name if re.fullmatch(r"[a-z0-9_]+\.zip", name) else _yaml_scalar(name)
 
 
+_PLAIN_SYSTEM = re.compile(r"^[a-z][a-z0-9-]*$")
+
+
+def _system_lines(systems: list[str]) -> list[str]:
+    """`system:` for one system, `systems:` for several, nothing for none."""
+    def scalar(value: str) -> str:
+        return value if _PLAIN_SYSTEM.match(value) else _yaml_scalar(value)
+
+    if not systems:
+        return []
+    if len(systems) == 1:
+        return [f"    system: {scalar(systems[0])}"]
+    return ["    systems:", *(f"      - {scalar(s)}" for s in systems)]
+
+
+def _entry_systems(entry: dict) -> list[str]:
+    """The systems an entry states, in the shape _system_lines writes."""
+    if entry.get("system"):
+        return [str(entry["system"])]
+    return [str(s) for s in entry.get("systems") or []]
+
+
+def _section_names(text: str) -> set[str]:
+    """Entry names inside the generated device section."""
+    start = _device_section_start(text)
+    if start is None:
+        return set()
+    end = _FILES_END.search(text, text.index("\n", start) + 1)
+    section = text[start : end.start() if end else len(text)]
+    return {
+        m.group(1).strip('"').strip("'")
+        for m in re.finditer(r"^  - name:\s*(.+?)\s*$", section, re.M)
+    }
+
+
+def _set_section_systems(text: str, systems: dict[str, list[str]]) -> str:
+    """Rewrite the system fields of the named entries of the device section.
+
+    Any `system:` or `systems:` the entry holds is dropped and the new one
+    written right after its category, where a written entry keeps it.
+    """
+    if not systems:
+        return text
+    start = _device_section_start(text)
+    if start is None:
+        return text
+    head, section = text[:start], text[start:]
+    out: list[str] = []
+    current = ""
+    in_list = False
+    for line in section.split("\n"):
+        entry_head = re.match(r"^  - name:\s*(.+?)\s*$", line)
+        if entry_head:
+            current = entry_head.group(1).strip('"').strip("'")
+        if current in systems:
+            if in_list and line.startswith("      - "):
+                continue
+            in_list = False
+            if re.match(r"^    system:", line):
+                continue
+            if re.match(r"^    systems:\s*$", line):
+                in_list = True
+                continue
+        out.append(line)
+        if current in systems and re.match(r"^    category:", line):
+            out.extend(_system_lines(systems[current]))
+    return head + "\n".join(out)
+
+
 def _format_device_entry(name: str, device: dict, role: str) -> str:
     """One generated device entry, in the field order of the written ones."""
     lines = [
         f"  - name: {_zip_scalar(name)}",
         "    required: false",
         "    category: bios_zip",
+        *_system_lines(device.get("systems") or []),
     ]
     if device.get("source_line"):
         lines.append(f'    source_ref: "{_build_source_ref(device, role)}"')
@@ -632,6 +702,11 @@ def merge_mame_devices(
     has none. A set it lacks is written under the generated section, in name
     order, with required false. An entry of another category under a device
     name is reported, never touched.
+
+    A device carrying `systems` (attribute_systems) has them written on its
+    entry when the entry is in the generated section: `system:` for one,
+    `systems:` for several, neither for none. An entry above the section
+    keeps the system a reader gave it.
     """
     path = Path(profile_path)
     text = path.read_text(encoding="utf-8")
@@ -646,7 +721,9 @@ def merge_mame_devices(
     }
     contents_patches: list[dict] = []
     ref_patches: dict[str, str] = {}
+    system_patches: dict[str, list[str]] = {}
     new_blocks: dict[str, str] = {}
+    generated = _section_names(text)
     for set_name, device in sorted(devices.items()):
         name = f"{set_name}.zip"
         existing = entries.get(name)
@@ -666,10 +743,18 @@ def merge_mame_devices(
         if not old.get("source_ref") and device.get("source_line"):
             ref_patches[name] = _build_source_ref(device, role)
             changed = True
+        if (
+            "systems" in device
+            and name in generated
+            and _entry_systems(old) != list(device["systems"])
+        ):
+            system_patches[name] = list(device["systems"])
+            changed = True
         report["updated" if changed else "unchanged"].append(set_name)
 
-    if write and (contents_patches or ref_patches or new_blocks):
+    if write and (contents_patches or ref_patches or system_patches or new_blocks):
         patched = _add_source_refs(text, ref_patches)
+        patched = _set_section_systems(patched, system_patches)
         patched = _patch_bios_entries(patched, contents_patches)
         patched = _insert_device_entries(patched, new_blocks)
         shutil.copy2(path, path.with_suffix(PROFILE_BACKUP_SUFFIX))

scripts/scraper/mame_devices.py

@@ -8,6 +8,9 @@ isdevice="yes", its ROMs and the source file defining the device. The lines
 cited for a set are the ROM_START block of that file declaring those ROMs, read
 at the release tag. A set whose block cannot be told apart is left without
 lines and reported.
+
+The same document says which machines load each set, so a set is filed under
+the systems of those machines (attribute_systems).
 """
 
 from __future__ import annotations
@@ -20,7 +23,11 @@ from collections.abc import Callable, Iterator
 from concurrent.futures import ThreadPoolExecutor
 from contextlib import contextmanager
 from pathlib import Path
+from typing import NamedTuple
 
+from common import yaml_load
+
+from ._hash_merge import _device_section_start
 from .mame_parser import _collect_rom_macros, _expand_macros, _parse_rom_entries
 
 LISTXML_CHUNK = 8 * 1024 * 1024
@@ -31,6 +38,8 @@ _ATTR = re.compile(rb'(\w+)="([^"]*)"')
 _DESCRIPTION = re.compile(rb"<description>(.*?)</description>", re.S)
 _BIOSSET = re.compile(rb"<biosset\b([^>]*)>")
 _ROM = re.compile(rb"<rom\b([^>]*)>")
+_DEVICE_REF = re.compile(rb'<device_ref\b[^>]*\bname="([^"]+)"')
+_SLOT_OPTION = re.compile(rb'<slotoption\b[^>]*\bdevname="([^"]+)"')
 
 _ROM_START = re.compile(r"ROM_START\s*\(\s*(\w+)\s*\)")
 _ROM_END = re.compile(r"\bROM_END\b")
@@ -97,14 +106,40 @@ def open_listxml(path: Path) -> Iterator[io.BufferedIOBase]:
             yield stream
 
 
-def scan_device_sets(stream: io.BufferedIOBase) -> tuple[str, dict[str, dict]]:
-    """Release tag of a -listxml document and its device sets with dumped ROMs.
+class MachineLinks(NamedTuple):
+    """What one listxml element says about the sets it loads.
+
+    *romof* is the parent the element names. *uses* are the devices it can
+    instantiate: every subdevice of its default configuration, each device a
+    selectable slot option adds, and for a device its parent ROM device,
+    whose set the device's ROMs are also searched in.
+    """
+
+    romof: str
+    isdevice: bool
+    uses: tuple[str, ...]
+
+
+class Release(NamedTuple):
+    tag: str
+    devices: dict[str, dict]
+    machines: dict[str, MachineLinks]
+
+
+def scan_release(stream: io.BufferedIOBase) -> Release:
+    """Release tag, device sets with dumped ROMs and every element's links.
 
     The document is read through a sliding window, never whole: the 0.289
-    listxml is 311 MB.
+    listxml is 311 MB. The links are what infoxml.cpp writes: romof is the
+    driver's parent (src/frontend/mame/infoxml.cpp:761-765) or a device's
+    parent ROM device (856-858), device_ref every subdevice of the default
+    configuration (786, 1010-1015), slotoption each selectable card of a
+    slot (2129-2172). device_t::searchpath puts the parent ROM device after
+    the device itself (src/emu/device.cpp:140-152).
     """
     tag = ""
     devices: dict[str, dict] = {}
+    machines: dict[str, MachineLinks] = {}
     buffer = b""
     while chunk := stream.read(LISTXML_CHUNK):
         buffer += chunk
@@ -115,13 +150,24 @@ def scan_device_sets(stream: io.BufferedIOBase) -> tuple[str, dict[str, dict]]:
             end = buffer.find(b"</machine>", match.end())
             if end == -1:
                 break
+            name = match.group(1).decode()
             attrs = _attrs(match.group(2))
-            if attrs.get("isdevice") == "yes":
-                body = buffer[match.end() : end]
+            body = buffer[match.end() : end]
+            isdevice = attrs.get("isdevice") == "yes"
+            romof = attrs.get("romof", "")
+            uses = {
+                ref.decode()
+                for ref in (*_DEVICE_REF.findall(body), *_SLOT_OPTION.findall(body))
+            }
+            if isdevice and romof:
+                uses.add(romof)
+            uses.discard(name)
+            machines[name] = MachineLinks(romof, isdevice, tuple(sorted(uses)))
+            if isdevice:
                 roms = _device_roms(body)
                 if roms:
                     description = _DESCRIPTION.search(body)
-                    devices[match.group(1).decode()] = {
+                    devices[name] = {
                         "description": html.unescape(description.group(1).decode())
                         if description else "",
                         "sourcefile": attrs.get("sourcefile", ""),
@@ -131,7 +177,86 @@ def scan_device_sets(stream: io.BufferedIOBase) -> tuple[str, dict[str, dict]]:
         buffer = buffer[position:] if position else buffer[-(2 * LISTXML_CHUNK) :]
     if not tag:
         raise ValueError("no <mame build=...> header: not a MAME -listxml document")
-    return tag, devices
+    return Release(tag, devices, machines)
+
+
+def scan_device_sets(stream: io.BufferedIOBase) -> tuple[str, dict[str, dict]]:
+    """Release tag of a -listxml document and its device sets with dumped ROMs."""
+    release = scan_release(stream)
+    return release.tag, release.devices
+
+
+def profile_seeds(text: str) -> dict[str, str]:
+    """Set name to system, for the bios_zip entries above the device section.
+
+    These are the entries a reader wrote, the seeds attribute_systems starts
+    from. The generated section is left out: its systems derive from these,
+    and reading them back would carry a previous release's answer forward.
+    """
+    start = _device_section_start(text)
+    written = yaml_load(text if start is None else text[:start]) or {}
+    seeds: dict[str, str] = {}
+    for entry in written.get("files") or []:
+        name = str(entry.get("name") or "")
+        if entry.get("category") == "bios_zip" and name.endswith(".zip") and entry.get("system"):
+            seeds[name[:-4]] = str(entry["system"])
+    return seeds
+
+
+def _seed_system(
+    name: str, machines: dict[str, MachineLinks], seeds: dict[str, str]
+) -> str:
+    """System a profile entry gives this element, through its romof chain.
+
+    A clone names its parent and a parent its BIOS root set, so a Neo Geo
+    game reaches neogeo and takes its system. A device is never walked: its
+    romof is a parent ROM device, not the machine it belongs to. The driver
+    file decides nothing: one file holds machines of several systems
+    (neogeo.cpp declares the MVS and the AES, atari400.cpp the 800 and the
+    5200), and filing a machine under the wrong one would take its devices
+    away from its own system.
+    """
+    seen: set[str] = set()
+    while name and name not in seen:
+        seen.add(name)
+        if name in seeds:
+            return seeds[name]
+        links = machines.get(name)
+        if links is None or links.isdevice:
+            return ""
+        name = links.romof
+    return ""
+
+
+def attribute_systems(
+    machines: dict[str, MachineLinks], seeds: dict[str, str]
+) -> dict[str, list[str]]:
+    """Systems whose machines can load each set.
+
+    *seeds* maps a set name to the system a profile entry states for it.
+    Each element _seed_system places passes its system to every device it
+    can instantiate, followed through those devices' own links. One walk per
+    system, so the cost is the size of the graph times the number of
+    systems.
+    """
+    starts: dict[str, list[str]] = {}
+    for name in machines:
+        system = _seed_system(name, machines, seeds)
+        if system:
+            starts.setdefault(system, []).append(name)
+    systems: dict[str, set[str]] = {}
+    for system, nodes in starts.items():
+        seen = set(nodes)
+        stack = list(nodes)
+        while stack:
+            links = machines.get(stack.pop())
+            for used in links.uses if links else ():
+                if used not in seen:
+                    seen.add(used)
+                    stack.append(used)
+        for name in seen:
+            systems.setdefault(name, set()).add(system)
+    return {name: sorted(found) for name, found in systems.items()}
 
 
 def source_path(sourcefile: str) -> str:

scripts/scraper/mame_hash_scraper.py

@@ -16,6 +16,7 @@ import subprocess
 import sys
 import urllib.error
 import urllib.request
+from collections import Counter
 from datetime import datetime, timezone
 from pathlib import Path
 from typing import Any
@@ -33,7 +34,13 @@ from ._hash_merge import (
     merge_mame_devices,
     merge_mame_profile,
 )
-from .mame_devices import locate_sources, open_listxml, scan_device_sets
+from .mame_devices import (
+    attribute_systems,
+    locate_sources,
+    open_listxml,
+    profile_seeds,
+    scan_release,
+)
 from .mame_parser import parse_mame_source_tree
 
 log = logging.getLogger(__name__)
@@ -474,9 +481,10 @@ def _merge_devices(args: argparse.Namespace) -> None:
     have to be the lines of the profile's upstream_commit.
     """
     profile_path = _EMULATORS_DIR / "mame.yml"
-    profile = yaml_load(profile_path.read_text(encoding="utf-8")) or {}
+    text = profile_path.read_text(encoding="utf-8")
+    profile = yaml_load(text) or {}
     with open_listxml(args.listxml) as stream:
-        tag, devices = scan_device_sets(stream)
+        tag, devices, machines = scan_release(stream)
     version = _parse_version_tag(tag)
     if str(profile.get("core_version")) != version:
         raise RuntimeError(
@@ -494,6 +502,9 @@ def _merge_devices(args: argparse.Namespace) -> None:
         devices,
         lambda path: upstream.fetch_file(repo, commit, path, upstream.CACHE_DIR),
     )
+    systems = attribute_systems(machines, profile_seeds(text))
+    for set_name, device in devices.items():
+        device["systems"] = systems.get(set_name, [])
     report = merge_mame_devices(str(profile_path), devices, write=not args.dry_run)
 
     print(f"mame-devices: {len(devices)} device ROM sets in the {tag} listxml")
@@ -503,6 +514,13 @@ def _merge_devices(args: argparse.Namespace) -> None:
     print(f"    = {len(report['unchanged'])} unchanged")
     for set_name in report["conflict"]:
         print(f"    ! {set_name}.zip is listed under another category, left as written")
+    by_count = Counter(len(device["systems"]) for device in devices.values())
+    print(
+        f"  systems: {by_count[1]} sets under one system, "
+        f"{sum(n for k, n in by_count.items() if k > 1)} under several, "
+        f"{by_count[0]} loaded by no machine a profile entry places "
+        "(they serve every system of the profile)"
+    )
     if unplaced:
         print(f"  {len(unplaced)} without source lines:")
         for set_name, reason in sorted(unplaced.items()):