diff --git a/scripts/pipeline.py b/scripts/pipeline.py index 93d6e5210..e4300b034 100644 --- a/scripts/pipeline.py +++ b/scripts/pipeline.py @@ -22,7 +22,7 @@ Steps, numbered as the run prints them: 4c --manifest-targets per-target core lists 4d bundled_only.py --strict copies only an emulator install provides 4e generate_games_index.py install/games/: what the games-on-card mode reads - 5 consistency check verify == pack: counts, paths, every emulator pack + 5 consistency check verify == pack: counts, paths, emulator and system packs 6 --verify-packs pack integrity, extraction and hashes 7 generate_readme.py README.md and CONTRIBUTING.md generate_installer_doc.py run samples and tables of wiki/installer.md @@ -518,7 +518,7 @@ def check_emulator_packs( emulators_dir: str = "emulators", platforms_dir: str = "platforms", db_path: str = "database.json", ) -> bool: - """Build every emulator pack in memory and compare it with verify --emulator. + """Build every emulator and system pack in memory and compare it with verify. Each profile in each build it has (libretro, standalone): the paths the pack fills with the profile's files against the ones verify credits. No @@ -528,9 +528,11 @@ def check_emulator_packs( import verify # noqa: PLC0415 from common import ( # noqa: PLC0415 ProfileSelectionError, + _norm_system_id, build_zip_contents_index, load_database, load_emulator_profiles, + profiles_for_systems, select_emulator_profiles, ) @@ -566,7 +568,45 @@ def check_emulator_packs( print("\n".join(lines)) all_ok = False print(f" {compared} emulator packs compared, {'all equal' if all_ok else 'differences above'}") - return all_ok + # A system pack builds every profile of the system together: a placement + # rule that only goes wrong when two profiles share a path (the bluemsx + # tree beside msx-emu's files) never meets in a one-profile pack. + systems = sorted({ + _norm_system_id(str(sid)) + for profile in profiles.values() + if profile.get("type") not in ("launcher", "alias", "test") + for sid in profile.get("systems") or [] + }) + systems_ok = True + compared = 0 + for sid in systems: + for standalone in (False, True): + matching = profiles_for_systems(profiles, [sid], standalone) + if len(matching) < 2: + continue + record = generate_pack.PackRecord() + with contextlib.redirect_stdout(io.StringIO()), \ + contextlib.redirect_stderr(io.StringIO()): + generate_pack.generate_emulator_pack( + matching, emulators_dir, db, "bios", "", standalone, zip_contents, + offline=True, platforms_dir=platforms_dir, record=record, + system_ids=[sid], + ) + report = verify.verify_emulator( + matching, emulators_dir, db, standalone, platforms_dir=platforms_dir, + system_ids=[sid], + ) + compared += 1 + label = f"--system {sid}{' --standalone' if standalone else ''}" + lines = _set_difference_lines( + label, set(report["pack_destinations"]), set(record.files) + ) + if lines: + print("\n".join(lines)) + systems_ok = False + print(f" {compared} system packs compared, " + f"{'all equal' if systems_ok else 'differences above'}") + return all_ok and systems_ok class _Skipped: diff --git a/scripts/verify.py b/scripts/verify.py index 1d9d78d71..f9e47d8c6 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -2116,6 +2116,30 @@ def verify_emulator( return placed[dest.lower()] = (path, owner, dest) _register_path(dest, placed_files, placed_parents) + + def occupy_trees(emu_name: str, profile: dict) -> None: + """The profile's data trees follow its files in the pack, which + writes them even when the system filter leaves it no file: + bluemsx's Machines/ tree holds msx-emu's coleco.rom path in a + --system colecovision pack.""" + for dd in deliverable(profile.get("data_directories", []), standalone): + cache = (data_registry or {}).get(dd.get("ref", ""), {}).get("local_cache", "") + if not cache or not os.path.isdir(cache): + continue + dd_dest = dd.get("destination", "") + structure = profile.get("pack_structure") + prefix = ( + structure.get("standalone" if standalone else "libretro", "") + if structure else "" + ) + if prefix: + dd_dest = f"{prefix}/{dd_dest}" if dd_dest else prefix + for root, _dirs, names in os.walk(cache): + for fname in names: + src = os.path.join(root, fname) + rel = os.path.relpath(src, cache).replace(os.sep, "/") + occupy(f"{dd_dest}/{rel}" if dd_dest else rel, src, emu_name) + for emu_name, profile in sorted(selected): files = expand_agnostic_entries( expand_directory_entries( @@ -2162,6 +2186,7 @@ def verify_emulator( "ground_truth": [], } ) + occupy_trees(emu_name, profile) continue # Verify archives as units (e.g., neogeo.zip, aes.zip) @@ -2424,24 +2449,7 @@ def verify_emulator( ) > _SEVERITY_ORDER.get(prev_sev, 0): file_severity[dest] = sev - # The profile's data trees follow its files in the pack. - for dd in deliverable(profile.get("data_directories", []), standalone): - cache = (data_registry or {}).get(dd.get("ref", ""), {}).get("local_cache", "") - if not cache or not os.path.isdir(cache): - continue - dd_dest = dd.get("destination", "") - structure = profile.get("pack_structure") - prefix = ( - structure.get("standalone" if standalone else "libretro", "") - if structure else "" - ) - if prefix: - dd_dest = f"{prefix}/{dd_dest}" if dd_dest else prefix - for root, _dirs, names in os.walk(cache): - for fname in names: - src = os.path.join(root, fname) - rel = os.path.relpath(src, cache).replace(os.sep, "/") - occupy(f"{dd_dest}/{rel}" if dd_dest else rel, src, emu_name) + occupy_trees(emu_name, profile) counts = { Severity.OK: 0, diff --git a/tests/test_pack_destinations.py b/tests/test_pack_destinations.py index d5bbd80bd..0cbfb8adb 100644 --- a/tests/test_pack_destinations.py +++ b/tests/test_pack_destinations.py @@ -189,6 +189,9 @@ class EveryEmulatorPackMatchesItsReport(unittest.TestCase): finally: os.chdir(previous) self.assertTrue(ok, out.getvalue()) + # Packs of several profiles too: a placement rule that only goes + # wrong when two profiles share a path never meets in one profile. + self.assertIn("system packs compared, all equal", out.getvalue()) if __name__ == "__main__": diff --git a/tests/test_system_selection.py b/tests/test_system_selection.py index 8623c9c08..8ab0fe7cd 100644 --- a/tests/test_system_selection.py +++ b/tests/test_system_selection.py @@ -424,6 +424,67 @@ class ASystemPackHoldsThatSystem(unittest.TestCase): self.assertEqual(report["total_files"], 2) +class ATreeHoldsAPathForAnotherProfile(unittest.TestCase): + """--system colecovision --standalone: bluemsx keeps no file of the + system but its Machines/ tree is packed, and holds msx-emu's coleco.rom + path. verify skipped the tree of a profile left without files and + credited msx-emu's copy the pack never wrote.""" + + def test_report_and_pack_agree(self): + import hashlib + import io + import shutil + from contextlib import redirect_stdout + + import yaml + from verify import verify_emulator + + common._emulator_profiles_cache.clear() + (REPO_ROOT / "tmp").mkdir(exist_ok=True) + root = Path(tempfile.mkdtemp(dir=REPO_ROOT / "tmp")) + self.addCleanup(shutil.rmtree, root, True) + previous = os.getcwd() + os.chdir(root) + self.addCleanup(os.chdir, previous) + data = b"coleco" * 4 + sha1 = hashlib.sha1(data).hexdigest() + rel = "bios/Coleco/ColecoVision/coleco.rom" + Path(rel).parent.mkdir(parents=True) + Path(rel).write_bytes(data) + tree = Path("data/tree/Machines/COL") + tree.mkdir(parents=True) + (tree / "coleco.rom").write_bytes(data) + db = {"files": {sha1: {"path": rel, "name": "coleco.rom", "size": len(data), + "md5": hashlib.md5(data).hexdigest(), "crc32": ""}}, + "indexes": {"by_md5": {}, "by_name": {"coleco.rom": [sha1]}, + "by_crc32": {}, "by_path_suffix": {}}} + Path("platforms").mkdir() + Path("platforms/_data_dirs.yml").write_text(yaml.safe_dump( + {"data_directories": {"tree": {"local_cache": "data/tree"}}})) + Path("emulators").mkdir() + Path("emulators/atree.yml").write_text(yaml.safe_dump({ + "emulator": "ATree", "type": "libretro", "cores": ["atree"], + "systems": ["colecovision", "msx"], + "data_directories": [{"ref": "tree"}], + "files": [{"name": "msx.rom", "system": "msx"}], + })) + Path("emulators/msxemu.yml").write_text(yaml.safe_dump({ + "emulator": "MsxEmu", "type": "libretro", "cores": ["msxemu"], + "systems": ["colecovision"], + "files": [{"name": "coleco.rom", "path": "Machines/COL/coleco.rom"}], + })) + names = ["atree", "msxemu"] + report = verify_emulator(names, "emulators", db, platforms_dir="platforms", + system_ids=["colecovision"]) + record = builder.PackRecord() + with redirect_stdout(io.StringIO()): + builder.generate_emulator_pack( + names, "emulators", db, "bios", "", offline=True, + platforms_dir="platforms", record=record, system_ids=["colecovision"], + ) + self.assertEqual(sorted(report["pack_destinations"]), sorted(record.files)) + + class ListingsKeepTheirColumns(unittest.TestCase): """--list-systems and --list-emulators started their last column at a position that moved with the file count and the profile type.""" diff --git a/wiki/tools.md b/wiki/tools.md index 6cce3c211..f3af3ee55 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -52,7 +52,7 @@ Pipeline steps: | 4c | Generate target manifests | `--skip-packs` | | 4d | Collected copies only an emulator install provides (`bundled_only.py --strict`, gates the run) | - | | 4e | Games index: what the installer reads for the games on a card (`generate_games_index.py`, fails under its coverage floor) | - (reads the committed install lists) | -| 5 | Consistency check: counts, paths per archive, every emulator pack | `--skip-packs`, or no full pack built (`--source platform` or `truth`) | +| 5 | Consistency check: counts, paths per archive, every emulator and system pack | `--skip-packs`, or no full pack built (`--source platform` or `truth`) | | 6 | Pack integrity (extract + hash) | `--skip-packs`, or no full pack built | | 7 | Generate README and CONTRIBUTING, then the installer page's tables and run samples (`generate_installer_doc.py`) | `--skip-docs` | | 8 | Generate site | `--skip-docs` | @@ -91,8 +91,9 @@ alone, run `python scripts/check_buildbot_system.py`. - **verify.py and generate_pack.py read the same functions.** A count or a path that differs between the report and the pack is a defect, which is what step 5 checks: the counts, the paths each archive holds against the - ones verify credits (`--destinations`), and every emulator pack, each - profile in each build, built in memory against `verify --emulator`. + ones verify credits (`--destinations`), every emulator pack, each + profile in each build, built in memory against `verify --emulator`, and + every system pack that gathers several profiles against `verify --system`. Equal counts once hid four RetroArch paths credited and never written. - **Generators write through `write_if_changed()`.** A new timestamp alone rewrites nothing, so a diff on a generated file always means content.