fix: one selection and one name per system

scripts/common.py

@@ -994,6 +994,27 @@ class ProfileSelectionError(ValueError):
     """A named profile cannot answer for itself."""
 
 
+def profiles_for_systems(
+    profiles: dict, system_ids: list[str], standalone: bool = False
+) -> list[str]:
+    """Profile keys that serve any of the systems, spelled either way.
+
+    Compared on the normalized id: stella2014 writes atari_2600 where every
+    other profile writes atari-2600, and an exact match built two different
+    packs under one file name, the second replacing the first.
+    """
+    wanted = {_norm_system_id(sid) for sid in system_ids}
+    matching = []
+    for name, profile in sorted(profiles.items()):
+        if profile.get("type") in ("launcher", "alias", "test"):
+            continue
+        if standalone and "standalone" not in profile.get("type", "libretro"):
+            continue
+        if wanted & {_norm_system_id(sid) for sid in profile.get("systems", [])}:
+            matching.append(name)
+    return matching
+
+
 def select_emulator_profiles(
     profile_names: list[str],
     all_profiles: dict,
@@ -1768,6 +1789,8 @@ from ziptools import (  # noqa: E402,F401
 )
 from artifacts import (  # noqa: E402,F401
     write_if_changed,
+    write_text_atomic,
+    copy_file_atomic,
     ArtifactLockBusy,
     artifact_lock,
     hold_artifact_lock,

scripts/generate_pack.py

@@ -31,6 +31,8 @@ sys.path.insert(0, os.path.dirname(__file__))
 from common import resolve_platform_cores as _platform_cores
 from common import (
     apply_target_overrides,
+    profiles_for_systems,
+    write_text_atomic,
     artifact_lock,
     ArtifactLockBusy,
     build_target_cores_cache,
@@ -1559,16 +1561,7 @@ def generate_system_pack(
 ) -> str | None:
     """Generate a ZIP pack for all emulators supporting given system IDs."""
     profiles = load_emulator_profiles(emulators_dir)
-    matching = []
-    for name, profile in sorted(profiles.items()):
-        if profile.get("type") in ("launcher", "alias", "test"):
-            continue
-        emu_systems = set(profile.get("systems", []))
-        if emu_systems & set(system_ids):
-            ptype = profile.get("type", "libretro")
-            if standalone and "standalone" not in ptype:
-                continue
-            matching.append(name)
+    matching = profiles_for_systems(profiles, system_ids, standalone)
 
     if not matching:
         all_systems: set[str] = set()
@@ -1590,10 +1583,9 @@ def generate_system_pack(
         )
         return None
 
-    # Use system-based ZIP name
-    sys_display = "_".join(
-        _name_part("_".join(w.title() for w in sid.split("-"))) for sid in system_ids
-    )
+    # Named like the platform's --system tag: one spelling of a system is
+    # one file name, and one selection.
+    sys_display = "_".join(_system_display_name(sid) for sid in system_ids)
     # Built in a scratch directory: under its emulator name in output_dir it
     # overwrote, then carried away, an --emulator pack already there.
     os.makedirs(output_dir, exist_ok=True)
@@ -1829,7 +1821,16 @@ def generate_md5_pack(
     if emulator_name and emulators_dir:
         profiles = load_emulator_profiles(emulators_dir, skip_aliases=False)
         if emulator_name in profiles:
-            profile = profiles[emulator_name]
+            # The same gate as --emulator: a core with no standalone build
+            # has no standalone layout, and a pack named for one would be
+            # the libretro pack under a name that promises otherwise.
+            try:
+                (_name, profile), = select_emulator_profiles(
+                    [emulator_name], profiles, standalone
+                )
+            except ProfileSelectionError as exc:
+                print(f"Error: {exc}", file=sys.stderr)
+                return None
             emu_display = profile.get("emulator", emulator_name)
             emu_pack_structure = profile.get("pack_structure")
             for fe in profile.get("files", []):
@@ -2002,9 +2003,7 @@ def generate_target_manifests(targets_dir: str, output_dir: str) -> None:
                 result[alias] = result[target_name]
 
         out_path = Path(output_dir) / f"{yml_file.stem}.json"
-        with open(out_path, "w") as f:
-            json.dump(result, f, indent=2, sort_keys=True)
-            f.write("\n")
+        write_text_atomic(out_path, json.dumps(result, indent=2, sort_keys=True) + "\n")
         count += 1
         print(f"  {yml_file.stem}: {len(result)} targets")
     print(f"Generated {count} target manifest(s) in {output_dir}")
@@ -2106,8 +2105,7 @@ def _write_manifest_if_changed(path: str, manifest: dict) -> None:
             new_cmp = {k: v for k, v in manifest.items() if k != "generated"}
             if old_cmp == new_cmp:
                 return  # no content change, keep existing timestamp
-    with open(path, "w") as f:
-        f.write(new_json)
+    write_text_atomic(path, new_json)
 
 
 def _run_manifest_mode(
@@ -2300,6 +2298,11 @@ def _pack_label(source: str, required_only: bool) -> str:
     return f"[source={source}, required]" if required_only else f"[source={source}]"
 
 
+def _produced(split: bool, zip_path, zip_paths) -> bool:
+    """Whether the build wrote what the run asked for."""
+    return bool(zip_paths if split else zip_path)
+
+
 def _run_platform_packs(
     args,
     groups,
@@ -2402,6 +2405,12 @@ def _run_platform_packs(
             except (FileNotFoundError, OSError, yaml.YAMLError) as e:
                 print(f"  ERROR: {e}")
                 failed.append(representative)
+            else:
+                # A builder that declined (no system left under the target)
+                # produced nothing: the run must say so, not verify whatever
+                # the output directory already held and exit 0.
+                if not _produced(args.split, zip_path, zip_paths):
+                    failed.append(representative)
 
     print("\nVerifying packs and generating manifests...")
     skip_conf = bool(system_filter or args.split)
@@ -3081,6 +3090,18 @@ def _manifest_region_drops(
     )
 
 
+def _record_unplaceable(unplaceable: list[dict], record_omission) -> None:
+    """Held by the collection, nowhere to put it on this platform.
+
+    Said in the manifest, where RomM's DOSBox Pure ROMs were simply absent.
+    """
+    for u in unplaceable:
+        record_omission(
+            u.get("path") or u.get("name", ""), u, u.get("system") or "",
+            "no_platform_slug", [u.get("emulator", "")],
+        )
+
+
 def generate_manifest(
     platform_name: str,
     platforms_dir: str,
@@ -3322,6 +3343,7 @@ def generate_manifest(
                     seen_lower.add(dedup_key.lower())
 
     # Phase 2: core complement (emulator extras)
+    unplaceable: list[dict] = []
     if source != "platform":
         core_files = _collect_emulator_extras(
             config,
@@ -3332,9 +3354,11 @@ def generate_manifest(
             emu_profiles,
             target_cores=target_cores,
             include_all=(source == "truth"),
+            unplaceable=unplaceable,
         )
     else:
         core_files = []
+    _record_unplaceable(unplaceable, record_omission)
     total_size += _manifest_core_entries(
         core_files, config, db, bios_dir, base_dest, repo_root,
         zip_contents, offline, region_drops, case_insensitive,

scripts/verify.py

@@ -34,6 +34,7 @@ import slots
 
 sys.path.insert(0, os.path.dirname(__file__))
 from common import (
+    profiles_for_systems,
     PROFILE_IDENTITY_FIELDS,
     list_available_targets,
     list_platform_system_ids,
@@ -716,6 +717,17 @@ def find_exclusion_notes(
 # Platform verification
 
 
+def _mark_unplaced(config: dict, undeclared: list[dict], profiles: dict) -> None:
+    """What the builder cannot place is not in the pack, held or not."""
+    from packextras import unplaceable_extras
+
+    unplaced = unplaceable_extras(config, undeclared, profiles)
+    for u in undeclared:
+        if (u.get("emulator", ""), u.get("name", ""), u.get("path") or "") in unplaced:
+            u["in_pack"] = False
+            u["omitted"] = "no platform slug for its system"
+
+
 def _twin_index(
     verify_systems: dict, db: dict, base_dest: str, zip_contents: dict,
     data_dir_registry: dict | None,
@@ -1020,6 +1032,7 @@ def verify_platform(
             )
             not in region_drops
         ]
+    _mark_unplaced(config, undeclared, profiles)
     exclusions = find_exclusion_notes(
         config, emulators_dir, emu_profiles, target_cores=target_cores
     )
@@ -1191,17 +1204,27 @@ def _print_undeclared_entry(u: dict, prefix: str, verbose: bool) -> None:
             print(f"      [{'+'.join(checks)}]")
 
 
+def _split_undeclared(undeclared: list[dict]) -> tuple[list[dict], list[dict], list[dict]]:
+    """Game data, firmware the pack places, firmware it holds but cannot place.
+
+    Everything that is not game data is firmware the core loads, archives
+    included: a bios_zip sat in neither list, so a required one that was
+    missing was never printed.
+    """
+    game_data = [u for u in undeclared if u.get("category", "bios") == "game_data"]
+    firmware = [u for u in undeclared if u.get("category", "bios") != "game_data"]
+    unplaced = [u for u in firmware if u["in_repo"] and not u.get("in_pack", True)]
+    placed = [u for u in firmware if u.get("in_pack", True)]
+    return game_data, placed, unplaced
+
+
 def _print_undeclared_section(result: dict, verbose: bool) -> None:
     """Print cross-reference section for undeclared files used by cores."""
     undeclared = result.get("undeclared_files", [])
     if not undeclared:
         return
 
-    # Everything that is not game data is firmware the core loads, archives
-    # included: a bios_zip sat in neither list, so a required one that was
-    # missing was never printed.
-    game_data = [u for u in undeclared if u.get("category", "bios") == "game_data"]
-    bios_files = [u for u in undeclared if u.get("category", "bios") != "game_data"]
+    game_data, bios_files, unplaced = _split_undeclared(undeclared)
 
     req_not_in_repo = [
         u
@@ -1224,6 +1247,8 @@ def _print_undeclared_section(result: dict, verbose: bool) -> None:
     print(
         f"  Core files: {core_in_pack} in pack, {core_missing_req} required missing, {core_missing_opt} optional missing"
     )
+    if unplaced:
+        print(f"  Core files held but not packed: {len(unplaced)} ({unplaced[0]['omitted']})")
 
     for u in req_not_in_repo:
         _print_undeclared_entry(u, "MISSING (required)", verbose)
@@ -1637,16 +1662,7 @@ def verify_system(
 ) -> dict:
     """Verify files for all emulators supporting given system IDs."""
     profiles = load_emulator_profiles(emulators_dir)
-    matching = []
-    for name, profile in sorted(profiles.items()):
-        if profile.get("type") in ("launcher", "alias", "test"):
-            continue
-        emu_systems = set(profile.get("systems", []))
-        if emu_systems & set(system_ids):
-            ptype = profile.get("type", "libretro")
-            if standalone and "standalone" not in ptype:
-                continue  # skip non-standalone in standalone mode
-            matching.append(name)
+    matching = profiles_for_systems(profiles, system_ids, standalone)
 
     if not matching:
         all_systems: set[str] = set()

tests/test_system_selection.py

@@ -0,0 +1,75 @@
+"""One system, two spellings, one pack.
+
+stella2014 and stella2023 write atari_2600 where every other profile writes
+atari-2600. --system matched the spelling exactly and named the ZIP from
+it, so the two spellings built two different packs under one file name,
+the second replacing the first; --from-md5 --emulator X --standalone
+accepted a libretro-only core and named a standalone pack that was the
+libretro pack.
+"""
+
+from __future__ import annotations
+
+import os
+import subprocess
+import sys
+import tempfile
+import unittest
+from pathlib import Path
+
+REPO_ROOT = Path(__file__).resolve().parent.parent
+sys.path.insert(0, str(REPO_ROOT / "scripts"))
+
+import common  # noqa: E402
+import generate_pack as builder  # noqa: E402
+
+
+class SpellingsAgree(unittest.TestCase):
+    def test_both_spellings_select_the_same_profiles(self):
+        profiles = common.load_emulator_profiles(str(REPO_ROOT / "emulators"))
+        dashed = common.profiles_for_systems(profiles, ["atari-2600"])
+        underscored = common.profiles_for_systems(profiles, ["atari_2600"])
+        self.assertEqual(dashed, underscored)
+        self.assertIn("stella2014", dashed)
+        self.assertIn("gopher2600", dashed)
+
+    def test_the_pack_name_normalizes_the_spelling(self):
+        self.assertEqual(
+            builder._system_display_name("atari_2600"), builder._system_display_name("atari-2600")
+        )
+        self.assertEqual(builder._system_display_name("msxturboR"), builder._system_display_name("msxturbor"))
+
+
+class StandaloneNeedsAStandaloneBuild(unittest.TestCase):
+    def test_a_custom_pack_refuses_it_for_a_libretro_core(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            Path(tmp, "emulators").mkdir()
+            Path(tmp, "emulators", "handy.yml").write_text(
+                "emulator: Handy\ntype: libretro\nsystems: [atari-lynx]\nfiles:\n  - name: lynxboot.img\n"
+            )
+            common._emulator_profiles_cache.clear()
+            self.addCleanup(common._emulator_profiles_cache.clear)
+            db = {"files": {}, "indexes": {"by_md5": {}, "by_crc32": {}, "by_name": {}}}
+            result = builder.generate_md5_pack(
+                [("md5", "d8f1206299c48946e6ec5ef96d014eaa")], db, tmp, tmp,
+                emulator_name="handy", emulators_dir=str(Path(tmp, "emulators")), standalone=True,
+            )
+            self.assertIsNone(result)
+            self.assertEqual([p for p in os.listdir(tmp) if p.endswith(".zip")], [])
+
+
+class ADeclinedBuildIsAFailure(unittest.TestCase):
+    def test_a_target_that_removes_every_requested_system_exits_nonzero(self):
+        with tempfile.TemporaryDirectory() as tmp:
+            proc = subprocess.run(
+                [sys.executable, "scripts/generate_pack.py", "--platform", "retroarch",
+                 "--system", "sony-playstation-2", "--target", "switch", "--offline",
+                 "--output-dir", tmp],
+                capture_output=True, text=True, cwd=str(REPO_ROOT), timeout=600, check=False,
+            )
+            self.assertNotEqual(proc.returncode, 0, proc.stdout + proc.stderr)
+            self.assertEqual([p for p in os.listdir(tmp) if p.endswith(".zip")], [])
+
+
+if __name__ == "__main__":
+    unittest.main()