From c1f0e8c2cabd572804a19c2aedb2e78a22bd2dde Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Thu, 5 Nov 2026 19:45:39 +0100 Subject: [PATCH] fix: compare system packs in pipeline step 5 Step 5 built emulator packs one profile at a time, where two profiles never share a path. It now also builds each system pack that gathers several profiles and compares it with verify. The first run found five standalone MSX and ColecoVision packs where verify skipped the data tree of a profile the system filter left without files, and credited msx-emu's copies the pack never wrote. verify now places those trees too: 541 system packs, all equal. --- scripts/pipeline.py | 46 +++++++++++++++++++++++-- scripts/verify.py | 44 ++++++++++++++---------- tests/test_pack_destinations.py | 3 ++ tests/test_system_selection.py | 61 +++++++++++++++++++++++++++++++++ wiki/tools.md | 7 ++-- 5 files changed, 137 insertions(+), 24 deletions(-) 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.