feat: read required from romload for MAME sets

A set whose every ROM is NO_DUMP or ROM_OPTIONAL only warns in
handle_missing_file, so the machine runs without it. The parser now
keeps undumped and optional ROMs for that decision, the merge and the
release profiles write required false for such sets, and the text
patcher rewrites the required line it used to leave. A curated false
over a set with dumped ROMs is kept, except over no contents, which is
what an undumped set leaves once a later release dumps it.

pSxMAME builds mamedev/mame itself at 0.133 and left the current
refresh, which would have moved it onto 0.289 refs. Release profiles
keep their citations double-quoted on regeneration.

scripts/scraper/_hash_merge.py

@@ -40,6 +40,25 @@ def mame_ref_role(profile: dict[str, Any]) -> str:
     return ""
 
 
+def settled_required(entry: dict, set_data: dict) -> bool:
+    """The required flag a bios_zip entry carries after a refresh.
+
+    set_data["required"] is what romload.cpp does with the set missing: False
+    when every ROM is NO_DUMP or ROM_OPTIONAL, so the machine runs without
+    it, and nothing outranks that. True only says the machine itself stops;
+    whether the core needs that machine is the profile's call, so a curated
+    false is kept. The exception is a false over no contents, which is
+    what an undumped set leaves: once a ROM is dumped, the set is required.
+    """
+    current = entry.get("required", True)
+    code = set_data.get("required")
+    if code is False:
+        return False
+    if code is True and current is False and not entry.get("contents"):
+        return True
+    return current
+
+
 def merge_mame_profile(
     profile_path: str,
     hashes_path: str,
@@ -48,11 +67,11 @@ def merge_mame_profile(
 ) -> dict[str, Any]:
     """Merge MAME bios_zip entries from upstream hash data.
 
-    Preserves system, note, required per entry. Updates contents and
-    source_ref from the hashes JSON. New sets are only added when
-    add_new=True (main profile). Entries not in the hash data are
-    left untouched (the scraper only covers MACHINE_IS_BIOS_ROOT sets,
-    not all machine ROM sets).
+    Preserves system and note per entry. Updates contents and source_ref
+    from the hashes JSON, and required where the source settles it (see
+    settled_required). New sets are only added when add_new=True (main
+    profile). Entries not in the hash data are left untouched (the scraper
+    only covers MACHINE_IS_BIOS_ROOT sets, not all machine ROM sets).
 
     If write=True, backs up existing profile to .old.yml before writing.
     """
@@ -93,13 +112,14 @@ def merge_mame_profile(
             entry["contents"] = contents
             if source_ref:
                 entry["source_ref"] = source_ref
+            entry["required"] = settled_required(existing_by_name[set_name], set_data)
             updated_bios.append(entry)
             matched_names.add(set_name)
         elif add_new:
             # New BIOS set, only added to the main profile
             entry = {
                 "name": f"{set_name}.zip",
-                "required": True,
+                "required": settled_required({}, set_data),
                 "category": "bios_zip",
                 "system": None,
                 "source_ref": source_ref,
@@ -245,7 +265,9 @@ def _diff_mame(
         # rots, which is exactly what profile_sync then reports as drift.
         ref_moved = bool(new_ref) and old_entry.get("source_ref") != new_ref
 
-        if _contents_differ(old_contents, new_contents) or ref_moved:
+        required_moved = old_entry.get("required", True) != settled_required(old_entry, set_data)
+
+        if _contents_differ(old_contents, new_contents) or ref_moved or required_moved:
             updated.append(set_name)
         else:
             unchanged += 1
@@ -435,7 +457,7 @@ def _patch_scalar(text: str, key: str, value: str) -> str:
 
 
 def _patch_bios_entries(text: str, files: list[dict]) -> str:
-    """Patch contents and source_ref for existing bios_zip entries in-place.
+    """Patch contents, source_ref and required of bios_zip entries in-place.
 
     Processes entries in reverse order to preserve line offsets.
     Each entry's "owned" lines are: the `- name:` line plus all indented
@@ -499,6 +521,13 @@ def _patch_bios_entries(text: str, files: list[dict]) -> str:
                 lines.insert(last_owned + 1, f'    source_ref: "{source_ref}"')
                 last_owned += 1
 
+        # The source settles required for a set the machine runs without
+        if "required" in fe:
+            for j in range(start_line + 1, last_owned + 1):
+                if re.match(r"^    required:", lines[j]):
+                    lines[j] = f"    required: {str(fe['required']).lower()}"
+                    break
+
         # Remove existing contents block if present
         contents_start = None
         contents_end = None

scripts/scraper/mame_hash_scraper.py

@@ -11,6 +11,7 @@ import argparse
 import json
 import os
 import logging
+import re
 import shutil
 import subprocess
 import sys
@@ -41,6 +42,7 @@ from ._hash_merge import (
     merge_mame_devices,
     merge_mame_profile,
     merge_mame_support,
+    settled_required,
 )
 from .mame_devices import (
     attribute_softlists,
@@ -241,12 +243,18 @@ def _find_mame_profiles() -> list[Path]:
             # declare mamedev/mame upstream but build from their own fork at
             # another release: refreshing their refs from current MAME moves
             # them onto paths their tree does not have, such as the 0.289 move
-            # of neogeo.cpp from src/mame/neogeo/ to src/mame/snk/.
+            # of neogeo.cpp from src/mame/neogeo/ to src/mame/snk/. A profile
+            # that builds mamedev/mame itself at a pinned commit, as pSxMAME
+            # does at 0.133, is a snapshot of that tree just the same.
             if (
                 isinstance(upstream, str)
                 and upstream.rstrip("/") == "https://github.com/mamedev/mame"
                 and isinstance(source, str)
                 and source.rstrip("/") in _MAME_TREES
+                and not (
+                    source.rstrip("/") == "https://github.com/mamedev/mame"
+                    and data.get("source_commit")
+                )
             ):
                 profiles.append(path)
         except (yaml.YAMLError, OSError) as exc:
@@ -427,7 +435,8 @@ def release_profile(
     BIOS root sets and their ROMs change between releases, so the current
     MAME profile does not describe them. Each set keeps the system and the
     required flag the current profile gives it, while the contents and the line
-    are the release's own.
+    are the release's own, and so is a set the release runs without: recel is
+    undumped at 0.258 and dumped later.
     """
     version = release.removeprefix("mame")
     version = f"{version[0]}.{version[1:]}" if version.isdigit() else version
@@ -440,7 +449,7 @@ def release_profile(
         name = f"{set_name}.zip"
         entry: dict[str, Any] = {
             "name": name,
-            "required": known.get(name, {}).get("required", True),
+            "required": settled_required(known.get(name, {}), data),
             "category": "bios_zip",
         }
         if known.get(name, {}).get("system"):
@@ -471,6 +480,9 @@ def release_profile(
     }
 
 
+_QUOTE_REF = re.compile(r'^(\s*source_ref: )([^"\'\n].*)$', re.MULTILINE)
+
+
 def _write_release(args: argparse.Namespace) -> None:
     """Write the profile of one MAME release, read from its own tag."""
     try:
@@ -487,6 +499,9 @@ def _write_release(args: argparse.Namespace) -> None:
         args.built_by,
     )
     text = yaml.safe_dump(profile, sort_keys=False, allow_unicode=True, width=100)
+    # Citations are written double-quoted in every profile: left bare, each
+    # regeneration rewrote all of them.
+    text = _QUOTE_REF.sub(r'\g<1>"\g<2>"', text)
     write_text_atomic(str(args.profile), text)
     print(f"{args.profile}: {len(profile['files'])} BIOS root sets at {args.release}")
 

scripts/scraper/mame_parser.py

@@ -3,7 +3,8 @@
 Extracts BIOS root sets and ROM definitions from MAME driver sources.
 Handles GAME/GAMEL/SYST/COMP/CONS macros with MACHINE_IS_BIOS_ROOT flag,
 ROM_START/ROM_END blocks, ROM_LOAD variants, ROM_REGION, ROM_SYSTEM_BIOS,
-NO_DUMP filtering, and BAD_DUMP flagging.
+NO_DUMP filtering, BAD_DUMP and ROM_OPTIONAL flagging, and whether a missing
+set stops the machine.
 """
 
 from __future__ import annotations
@@ -66,6 +67,8 @@ _CRC_SHA = re.compile(
 _NO_DUMP = re.compile(r"\bNO_DUMP\b")
 _BAD_DUMP = re.compile(r"\bBAD_DUMP\b")
 _ROM_BIOS = re.compile(r"ROM_BIOS\s*\(\s*(\d+)\s*\)")
+# ROM_LOAD_OPTIONAL expands to ROMX_LOAD(..., ROM_OPTIONAL) (src/emu/romentry.h)
+_OPTIONAL = re.compile(r"\bROM_OPTIONAL\b|\bROMX?_LOAD\w*_OPTIONAL\b")
 
 
 def strip_comments(source: str) -> str:
@@ -161,12 +164,15 @@ def find_bios_root_sets(source: str, filename: str) -> dict[str, dict]:
     return results
 
 
-def parse_rom_block(source: str, set_name: str) -> list[dict]:
+def parse_rom_block(
+    source: str, set_name: str, keep_no_dump: bool = False,
+) -> list[dict]:
     """Parse ROM definitions for a given set name.
 
     Finds the ROM_START(set_name)...ROM_END block, expands local
     #define macros that contain ROM_LOAD/ROM_REGION calls, then
-    extracts all ROM entries. Skips NO_DUMP, flags BAD_DUMP.
+    extracts all ROM entries. Skips NO_DUMP unless keep_no_dump, which
+    returns them flagged no_dump. Flags BAD_DUMP and ROM_OPTIONAL.
     """
     pattern = re.compile(
         r"ROM_START\s*\(\s*" + re.escape(set_name) + r"\s*\)",
@@ -187,7 +193,22 @@ def parse_rom_block(source: str, set_name: str) -> list[dict]:
     macros = _collect_rom_macros(source)
     block = _expand_macros(block, macros, depth=5)
 
-    return _parse_rom_entries(block)
+    return _parse_rom_entries(block, keep_no_dump)
+
+
+def set_is_required(entries: list[dict]) -> bool | None:
+    """Whether MAME refuses to run the machine when the set is missing.
+
+    handle_missing_file (src/emu/romload.cpp) counts a missing ROM_OPTIONAL
+    as a warning and a missing NO_DUMP as known bad. Any other ROM raises
+    an error, and display_rom_load_results throws
+    EMU_ERR_MISSING_FILES on errors. A set whose every ROM is optional or
+    undumped warns and the machine runs. None when the block holds no
+    ROM_LOAD at all: nothing was read, so nothing is decided.
+    """
+    if not entries:
+        return None
+    return any(not e.get("no_dump") and not e.get("optional") for e in entries)
 
 
 def parse_mame_source_tree(base_path: str) -> dict[str, dict]:
@@ -213,12 +234,15 @@ def parse_mame_source_tree(base_path: str) -> dict[str, dict]:
 
                 bios_sets = find_bios_root_sets(content, rel_path)
                 for set_name, info in bios_sets.items():
-                    roms = parse_rom_block(content, set_name)
+                    entries = parse_rom_block(content, set_name, keep_no_dump=True)
                     results[set_name] = {
                         "source_file": info["source_file"],
                         "source_line": info["source_line"],
-                        "roms": roms,
+                        "roms": [e for e in entries if not e.get("no_dump")],
                     }
+                    required = set_is_required(entries)
+                    if required is not None:
+                        results[set_name]["required"] = required
 
     return results
 
@@ -353,7 +377,7 @@ def _split_macro_args(inner: str) -> list[str]:
     return args
 
 
-def _parse_rom_entries(block: str) -> list[dict]:
+def _parse_rom_entries(block: str, keep_no_dump: bool = False) -> list[dict]:
     """Parse ROM entries from a ROM block (content between ROM_START and ROM_END).
 
     Uses regex scanning over the entire block (not line-by-line) to handle
@@ -398,7 +422,8 @@ def _parse_rom_entries(block: str) -> list[dict]:
                 context_end = m.end() + 200
             context = block[context_start : min(context_end, len(block))]
 
-            if _NO_DUMP.search(context):
+            no_dump = bool(_NO_DUMP.search(context))
+            if no_dump and not keep_no_dump:
                 continue
 
             rom_name = m.group(1)
@@ -435,6 +460,10 @@ def _parse_rom_entries(block: str) -> list[dict]:
                 entry["bios_index"] = bios_index
                 entry["bios_label"] = bios_label
                 entry["bios_description"] = bios_description
+            if _OPTIONAL.search(context):
+                entry["optional"] = True
+            if no_dump:
+                entry["no_dump"] = True
 
             roms.append(entry)
 

tests/test_hash_merge.py

@@ -170,6 +170,85 @@ class TestMameMerge(unittest.TestCase):
             self.assertEqual(entry["system"], "snk-neogeo-mvs")
             self.assertFalse(entry["required"])
 
+    def test_merge_sets_required_false_for_a_set_the_machine_runs_without(self) -> None:
+        profile = _make_mame_profile()
+        profile["files"].append({
+            "name": "mac2bios.zip", "required": True, "category": "bios_zip",
+            "note": "kept",
+        })
+        hashes = _make_mame_hashes()
+        hashes["bios_sets"]["mac2bios"] = {
+            "source_file": "src/mame/seta/macs.cpp", "source_line": 776,
+            "roms": [], "required": False,
+        }
+        with tempfile.TemporaryDirectory() as td:
+            p = Path(td)
+            profile_path = _write_yaml(p / "mame.yml", profile)
+            hashes_path = _write_json(p / "hashes.json", hashes)
+
+            diff = compute_diff(profile_path, hashes_path, mode="mame")
+            self.assertIn("mac2bios", diff["updated"])
+
+            result = merge_mame_profile(profile_path, hashes_path)
+            entry = next(f for f in result["files"] if f["name"] == "mac2bios.zip")
+            self.assertFalse(entry["required"])
+            self.assertEqual(entry["note"], "kept")
+
+    def test_written_profile_carries_the_settled_required(self) -> None:
+        """The text patcher rewrote contents and source_ref but left required."""
+        hashes = _make_mame_hashes()
+        hashes["bios_sets"] = {"mac2bios": {
+            "source_file": "src/mame/seta/macs.cpp", "source_line": 776,
+            "roms": [], "required": False,
+        }}
+        with tempfile.TemporaryDirectory() as td:
+            p = Path(td)
+            profile_path = str(p / "mame.yml")
+            Path(profile_path).write_text(
+                'emulator: MAME\ncore_version: "0.289"\nfiles:\n'
+                "  - name: mac2bios.zip\n    required: true\n    category: bios_zip\n"
+                '    source_ref: "upstream src/mame/seta/macs.cpp:776"\n',
+                encoding="utf-8",
+            )
+            hashes_path = _write_json(p / "hashes.json", hashes)
+            merge_mame_profile(profile_path, hashes_path, write=True)
+            written = yaml.safe_load(Path(profile_path).read_text(encoding="utf-8"))
+        entry = next(f for f in written["files"] if f["name"] == "mac2bios.zip")
+        self.assertIs(entry["required"], False)
+
+    def test_merge_keeps_a_curated_false_over_a_fatal_set(self) -> None:
+        profile = _make_mame_profile()
+        profile["files"][0]["required"] = False
+        hashes = _make_mame_hashes()
+        hashes["bios_sets"]["neogeo"]["required"] = True
+        with tempfile.TemporaryDirectory() as td:
+            p = Path(td)
+            profile_path = _write_yaml(p / "mame.yml", profile)
+            hashes_path = _write_json(p / "hashes.json", hashes)
+            result = merge_mame_profile(profile_path, hashes_path)
+            entry = next(f for f in result["files"] if f["name"] == "neogeo.zip")
+            self.assertFalse(entry["required"])
+
+    def test_merge_restores_required_once_an_undumped_set_is_dumped(self) -> None:
+        profile = _make_mame_profile()
+        profile["files"].append({
+            "name": "recel.zip", "required": False, "category": "bios_zip",
+            "contents": [],
+        })
+        hashes = _make_mame_hashes()
+        hashes["bios_sets"]["recel"] = {
+            "source_file": "src/mame/pinball/recel.cpp", "source_line": 546,
+            "roms": [{"name": "a2361.b1", "size": 1024, "crc32": "eeb8f3f0"}],
+            "required": True,
+        }
+        with tempfile.TemporaryDirectory() as td:
+            p = Path(td)
+            profile_path = _write_yaml(p / "mame.yml", profile)
+            hashes_path = _write_json(p / "hashes.json", hashes)
+            result = merge_mame_profile(profile_path, hashes_path)
+            entry = next(f for f in result["files"] if f["name"] == "recel.zip")
+            self.assertTrue(entry["required"])
+
     def test_merge_adds_new_bios_set(self) -> None:
         hashes = _make_mame_hashes()
         hashes["bios_sets"]["pgm"] = {
@@ -405,6 +484,30 @@ class AReleaseProfileDescribesItsOwnRelease(unittest.TestCase):
         self.assertNotIn("system", files["newset.zip"])
         self.assertEqual(files["neogeo.zip"]["source_ref"], "upstream src/mame/neogeo/neogeo.cpp:10")
 
+    def test_a_set_the_release_runs_without_is_not_required(self):
+        """recel is undumped at 0.258: the current profile's true does not apply."""
+        from scripts.scraper.mame_hash_scraper import release_profile
+
+        bios_sets = {
+            "recel": {"source_file": "src/mame/pinball/recel.cpp", "source_line": 505,
+                      "roms": [], "required": False},
+            "neogeo": {"source_file": "src/mame/neogeo/neogeo.cpp", "source_line": 10,
+                       "roms": [{"name": "sp-s2.sp1", "size": 131072, "crc32": "9036d879"}],
+                       "required": True},
+        }
+        reference = {"files": [
+            {"name": "recel.zip", "category": "bios_zip", "required": True,
+             "contents": [{"name": "a2361.b1", "size": 1024, "crc32": "eeb8f3f0"}]},
+            {"name": "neogeo.zip", "category": "bios_zip", "required": True},
+        ]}
+        profile = release_profile(
+            "mame0258", bios_sets, "a" * 40, "b" * 40, ["mame0258"], reference,
+            ["recalbox"],
+        )
+        files = {f["name"]: f for f in profile["files"]}
+        self.assertFalse(files["recel.zip"]["required"])
+        self.assertTrue(files["neogeo.zip"]["required"])
+
     def test_a_release_profile_is_left_out_of_the_current_refresh(self):
         from scripts.scraper.mame_hash_scraper import _find_mame_profiles
 
@@ -412,6 +515,12 @@ class AReleaseProfileDescribesItsOwnRelease(unittest.TestCase):
         self.assertIn("mame.yml", names)
         self.assertFalse({"mame0258.yml", "mame0278.yml"} & names)
 
+    def test_a_pinned_build_of_the_mame_tree_is_left_out(self):
+        """pSxMAME builds mamedev/mame at 0.133: refreshed, it would carry 0.289 refs."""
+        from scripts.scraper.mame_hash_scraper import _find_mame_profiles
+
+        self.assertNotIn("psxmame.yml", {path.name for path in _find_mame_profiles()})
+
 
 class TestFbneoMerge(unittest.TestCase):
     """Tests for merge_fbneo_profile."""

tests/test_mame_parser.py

@@ -10,6 +10,7 @@ from scripts.scraper.mame_parser import (
     find_bios_root_sets,
     parse_mame_source_tree,
     parse_rom_block,
+    set_is_required,
 )
 
 # Standard GAME macro with MACHINE_IS_BIOS_ROOT, multiple ROM entries, BIOS variants
@@ -51,6 +52,35 @@ ROM_END
 GAME( 2000, testnd, 0, testnd, testnd, test_state, init_test, ROT0, "Test", "Test ND", MACHINE_IS_BIOS_ROOT )
 """
 
+# Every ROM undumped, as macs.cpp declares mac2bios: the machine runs without the set
+ALL_NODUMP_FIXTURE = """\
+#define MACS2_BIOS \\
+	ROM_REGION( 0x400000, "bios", 0 ) \\
+	ROM_LOAD16_BYTE( "macs2os_l.bin", 0x00000, 0x80000, NO_DUMP ) \\
+	ROM_LOAD16_BYTE( "macs2os_h.bin", 0x00001, 0x80000, NO_DUMP )
+
+ROM_START( mac2bios )
+	MACS2_BIOS
+
+	ROM_REGION( 0x1000000, "maincpu", 0 )
+	ROM_COPY( "bios",   0x000000, 0x000000, 0x400000 )
+ROM_END
+
+GAME( 1995, mac2bios, 0, macs, macs_m, macs_state, init_macs2, ROT0, "I'Max", "Multi Amenity Cassette System 2 BIOS", MACHINE_IS_BIOS_ROOT | MACHINE_NOT_WORKING )
+"""
+
+# Every ROM optional, through both spellings romentry.h offers, plus one undumped
+ALL_OPTIONAL_FIXTURE = """\
+ROM_START( testopt )
+    ROM_REGION( 0x10000, "maincpu", 0 )
+    ROM_LOAD_OPTIONAL( "opt1.rom", 0x00000, 0x4000, CRC(aabbccdd) SHA1(1122334455667788990011223344556677889900) )
+    ROMX_LOAD( "opt2.rom", 0x04000, 0x4000, CRC(aabbccde) SHA1(1122334455667788990011223344556677889901), ROM_OPTIONAL )
+    ROM_LOAD( "gone.rom", 0x08000, 0x4000, NO_DUMP )
+ROM_END
+
+GAME( 2000, testopt, 0, testopt, testopt, test_state, init_test, ROT0, "Test", "Test Optional", MACHINE_IS_BIOS_ROOT )
+"""
+
 # ROM_LOAD with BAD_DUMP
 BADDUMP_FIXTURE = """\
 ROM_START( testbd )
@@ -284,6 +314,47 @@ ROM_END
         self.assertEqual(roms[0]["sha1"], "aabbccddeeff00112233aabbccddeeff00112233")
 
 
+class TestSetIsRequired(unittest.TestCase):
+    """A set is required only if romload.cpp errors when it is missing."""
+
+    def test_all_no_dump_set_is_not_required(self) -> None:
+        entries = parse_rom_block(ALL_NODUMP_FIXTURE, "mac2bios", keep_no_dump=True)
+        self.assertEqual([e["name"] for e in entries], ["macs2os_l.bin", "macs2os_h.bin"])
+        self.assertTrue(all(e["no_dump"] for e in entries))
+        self.assertIs(set_is_required(entries), False)
+
+    def test_optional_and_no_dump_set_is_not_required(self) -> None:
+        entries = parse_rom_block(ALL_OPTIONAL_FIXTURE, "testopt", keep_no_dump=True)
+        self.assertEqual(len(entries), 3)
+        self.assertTrue(entries[0]["optional"] and entries[1]["optional"])
+        self.assertIs(set_is_required(entries), False)
+
+    def test_one_dumped_rom_makes_the_set_required(self) -> None:
+        entries = parse_rom_block(NODUMP_FIXTURE, "testnd", keep_no_dump=True)
+        self.assertIs(set_is_required(entries), True)
+
+    def test_bad_dump_is_still_fatal(self) -> None:
+        entries = parse_rom_block(BADDUMP_FIXTURE, "testbd", keep_no_dump=True)
+        self.assertIs(set_is_required(entries), True)
+
+    def test_no_rom_read_decides_nothing(self) -> None:
+        self.assertIsNone(set_is_required([]))
+
+    def test_no_dump_stays_out_of_the_default_parse(self) -> None:
+        self.assertEqual(parse_rom_block(ALL_NODUMP_FIXTURE, "mac2bios"), [])
+
+    def test_tree_records_required_per_set(self) -> None:
+        with tempfile.TemporaryDirectory() as tmpdir:
+            mame_dir = os.path.join(tmpdir, "src", "mame", "seta")
+            os.makedirs(mame_dir)
+            with open(os.path.join(mame_dir, "macs.cpp"), "w") as f:
+                f.write(ALL_NODUMP_FIXTURE + NEOGEO_FIXTURE)
+            results = parse_mame_source_tree(tmpdir)
+            self.assertIs(results["mac2bios"]["required"], False)
+            self.assertEqual(results["mac2bios"]["roms"], [])
+            self.assertIs(results["neogeo"]["required"], True)
+
+
 class TestParseMameSourceTree(unittest.TestCase):
     """Tests for parse_mame_source_tree."""