diff --git a/scripts/common.py b/scripts/common.py index 6f44b40b..f26ed540 100644 --- a/scripts/common.py +++ b/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, diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index e1dce12e..bceb436b 100644 --- a/scripts/generate_pack.py +++ b/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, diff --git a/scripts/verify.py b/scripts/verify.py index 94177870..6b2ab01b 100644 --- a/scripts/verify.py +++ b/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() diff --git a/tests/test_system_selection.py b/tests/test_system_selection.py new file mode 100644 index 00000000..f594bcb3 --- /dev/null +++ b/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()