diff --git a/scripts/common.py b/scripts/common.py index 1c28b625a..b8cb33896 100644 --- a/scripts/common.py +++ b/scripts/common.py @@ -1220,6 +1220,19 @@ def archive_entry(archive: str, files: list[dict]) -> dict: return {**own, "members": members} if members else own +def archive_is_optional(entry: dict, archive: str, files: list[dict]) -> bool: + """Optional as a whole: its own entry and every member it carries. + + A --required-only pack leaves it out, and the report of that pack too. + geolith's _Required pack held neocd.zip and irrmaze.zip all the same. + """ + return entry.get("required") is False and all( + member.get("required") is False + for member in files + if member.get("archive") == archive + ) + + def holds_required_members(path: str, members: list[dict]) -> bool: """Whether an archive holds every member a profile requires inside it.""" crcs = zip_member_crcs(path) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index fa91eb16f..e922558c1 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -47,6 +47,7 @@ from common import ( archive_entry, profiles_for_systems, write_text_atomic, + archive_is_optional, artifact_lock, repository_output_lock, ArtifactLockBusy, @@ -1473,18 +1474,6 @@ class DamagedArchiveError(OSError): -def _archive_is_optional(archive_entry: dict, archive_name: str, files: list[dict]) -> bool: - """Optional as a whole: its own entry and every member it carries. - - geolith's _Required pack held neocd.zip and irrmaze.zip all the same. - """ - return archive_entry.get("required") is False and all( - member.get("required") is False - for member in files - if member.get("archive") == archive_name - ) - - def _pack_data_tree( zf: zipfile.ZipFile, local_cache: str, @@ -1673,7 +1662,7 @@ def generate_emulator_pack( ) own_entry = archive_entry(archive_name, files) - if required_only and _archive_is_optional(own_entry, archive_name, files): + if required_only and archive_is_optional(own_entry, archive_name, files): continue local_path, status = resolve_file( own_entry, diff --git a/scripts/verify.py b/scripts/verify.py index 147a8f80d..0f621eb4e 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -43,6 +43,7 @@ from common import ( suggest_names, require_platform, archive_entry, + archive_is_optional, archive_members, profiles_for_systems, PROFILE_IDENTITY_FIELDS, @@ -2056,12 +2057,16 @@ def verify_emulator( regions: list[str] | None = None, platforms_dir: str = _REPO_PLATFORMS, system_ids: list[str] | None = None, + required_only: bool = False, ) -> dict: """Verify files for specific emulator profiles. A region priority list narrows the report the same way a pack built with the same list would be narrowed, through the emulator pack's own drop set. `system_ids` keeps the entries of those systems, as the system pack does. + `required_only` reports on the pack built with --required-only: a loose + file or tree marked ``required: false`` is left out, and an archive when + it and every member it carries are optional, as the builder leaves them. """ from packextras import expand_agnostic_entries from packresolve import resolve_file @@ -2105,7 +2110,7 @@ def verify_emulator( if regions: from packextras import emulator_region_drops - region_drops = emulator_region_drops(selected, standalone, regions, db=db) + region_drops = emulator_region_drops(selected, standalone, regions, required_only, db=db) # The emulator pack walks profiles in name order and keeps the first # file at a path, folding case: a later profile's different file there @@ -2142,6 +2147,8 @@ def verify_emulator( 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): + if required_only and dd.get("required") is False: + continue cache = (data_registry or {}).get(dd.get("ref", ""), {}).get("local_cache", "") if not cache or not os.path.isdir(cache): continue @@ -2170,6 +2177,17 @@ def verify_emulator( ) if system_ids: files = [fe for fe in files if entry_serves_systems(fe, profile, system_ids)] + if required_only: + optional_archives = { + fe["archive"] for fe in files + if fe.get("archive") + and archive_is_optional(archive_entry(fe["archive"], files), fe["archive"], files) + } + files = [ + fe for fe in files + if fe.get("archive") not in optional_archives + and (fe.get("archive") or fe.get("required") is not False) + ] if region_drops: # Loose files only, as the emulator pack: an archive ships whole, # and dropping kronos' regional stvbios.zip members rebuilt the @@ -2185,6 +2203,8 @@ def verify_emulator( for dd in deliverable( profile.get("data_directories", []), standalone ): + if required_only and dd.get("required") is False: + continue ref = dd.get("ref", "") if not ref: continue @@ -2516,6 +2536,7 @@ def verify_system( standalone: bool = False, regions: list[str] | None = None, platforms_dir: str = _REPO_PLATFORMS, + required_only: bool = False, ) -> dict: """Verify files for all emulators supporting given system IDs.""" profiles = load_emulator_profiles(emulators_dir) @@ -2541,7 +2562,7 @@ def verify_system( return verify_emulator( matching, emulators_dir, db, standalone, regions=regions, - platforms_dir=platforms_dir, system_ids=system_ids, + platforms_dir=platforms_dir, system_ids=system_ids, required_only=required_only, ) @@ -2680,6 +2701,7 @@ def _refuse_listing_narrowings( ("--destinations", args.destinations), ("--offline", args.offline), ("--no-game-data", getattr(args, "no_game_data", False)), + ("--required-only", args.required_only), ): if on: parser.error(f"{flag} is incompatible with {listing}") @@ -2708,6 +2730,8 @@ def _refuse_mode_flags(args: argparse.Namespace, parser: argparse.ArgumentParser "--no-game-data requires --platform or --all"), (args.destinations and getattr(args, "region", None), "--destinations describes the full pack and is incompatible with --region"), + (args.destinations and args.required_only, + "--destinations describes the full pack and is incompatible with --required-only"), ): if refused: parser.error(message) @@ -2776,6 +2800,11 @@ def _main(held: contextlib.ExitStack) -> None: help="Report on the pack built with --no-game-data: game data left " "out and named, firmware only", ) + parser.add_argument( + "--required-only", action="store_true", + help="Report on the pack built with --required-only: files the code " + "runs without are left out", + ) parser.add_argument( "--db", default=DEFAULT_DB, help="database file to read (default: %(default)s)", @@ -2867,6 +2896,7 @@ def _main(held: contextlib.ExitStack) -> None: result = verify_emulator( names, args.emulators_dir, db, args.standalone, regions=requested_regions, platforms_dir=args.platforms_dir, + required_only=args.required_only, ) if args.json: result["details"] = _json_details(result["details"], args.verbose) @@ -2881,6 +2911,7 @@ def _main(held: contextlib.ExitStack) -> None: result = verify_system( system_ids, args.emulators_dir, db, args.standalone, regions=requested_regions, platforms_dir=args.platforms_dir, + required_only=args.required_only, ) if args.json: result["details"] = _json_details(result["details"], args.verbose) @@ -2950,6 +2981,7 @@ def _main(held: contextlib.ExitStack) -> None: regions=requested_regions, system_filter=platform_systems, no_game_data=getattr(args, "no_game_data", False), + required_only=args.required_only, ) names = [ load_platform_config(p, args.platforms_dir).get("platform", p) diff --git a/tests/test_system_selection.py b/tests/test_system_selection.py index b70436947..c7016c539 100644 --- a/tests/test_system_selection.py +++ b/tests/test_system_selection.py @@ -430,6 +430,51 @@ class ASystemReportCountsTheClaimsThePackCounts(unittest.TestCase): self.assertEqual(report_owned_by_systems([entry], ["panasonic-3do"]), [entry]) +class VerifyReportsTheRequiredOnlyPack(unittest.TestCase): + """`generate_pack --emulator mame --required-only` held 164 archives and + the only report, `verify --emulator mame`, credited 2302 paths: + --required-only was refused by verify, and the archive rule existed in + the builder only.""" + + def setUp(self): + previous = os.getcwd() + os.chdir(REPO_ROOT) + self.addCleanup(os.chdir, previous) + + def test_the_report_holds_the_paths_the_pack_holds(self): + import contextlib + import io + + from verify import verify_emulator + + db = common.load_database("database.json") + for name in ("beetle_psx", "dolphin", "fbneo", "geolith", "mame"): + with self.subTest(emulator=name): + record = builder.PackRecord() + with contextlib.redirect_stdout(io.StringIO()), \ + contextlib.redirect_stderr(io.StringIO()): + builder.generate_emulator_pack( + [name], "emulators", db, "bios", "", offline=True, + platforms_dir="platforms", record=record, required_only=True, + ) + report = verify_emulator([name], "emulators", db, + platforms_dir="platforms", required_only=True) + full = verify_emulator([name], "emulators", db, platforms_dir="platforms") + self.assertEqual(sorted(report["pack_destinations"]), sorted(record.files)) + self.assertLessEqual(set(report["pack_destinations"]), + set(full["pack_destinations"])) + + def test_every_mode_takes_the_flag(self): + for argv in (["--platform", "lakka"], ["--emulator", "handy"], + ["--system", "atari-lynx"]): + with self.subTest(argv=argv): + proc = subprocess.run( + [sys.executable, "scripts/verify.py", *argv, "--required-only"], + cwd=REPO_ROOT, capture_output=True, text=True, timeout=600, check=False, + ) + self.assertEqual(proc.returncode, 0, proc.stderr[-1500:]) + + class StrictVerifyFailsOnACriticalMiss(unittest.TestCase): """verify exited 0 with a required file missing on an md5 platform, so the pipeline printed "verify OK" over a pack the frontend rejects.""" diff --git a/wiki/advanced-usage.md b/wiki/advanced-usage.md index 814a6d03c..c17cc07ac 100644 --- a/wiki/advanced-usage.md +++ b/wiki/advanced-usage.md @@ -95,6 +95,15 @@ What counts as "required" depends on the platform YAML. For existence-mode platf For MD5-mode platforms (Batocera), all declared files are treated as required unless explicitly marked optional. +An emulator or system pack keeps the files and trees its profiles do not mark +`required: false`, and an archive unless it and every ROM it carries are +optional. `verify.py` takes the same flag and reports on that pack: + +```bash +python scripts/verify.py --platform batocera --required-only +python scripts/verify.py --emulator mame --required-only +``` + ### Region filtering Keep one regional BIOS per system instead of all of them: diff --git a/wiki/tools.md b/wiki/tools.md index 31086a61e..aa9e695a4 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -148,6 +148,7 @@ python scripts/verify.py --platform batocera --system psx # one system of a pla python scripts/verify.py --platform retroarch --target switch # filter by hardware python scripts/verify.py --platform recalbox --region us # regional priority list python scripts/verify.py --platform emuelec --no-game-data # the firmware-only pack, game data named +python scripts/verify.py --emulator mame --required-only # the pack built with --required-only python scripts/verify.py --all --destinations tmp/paths.json # also write the paths each pack holds python scripts/verify.py --all --offline # a release asset the cache lacks counts missing, as in an offline build python scripts/verify.py --list-platforms # list all platforms @@ -258,7 +259,8 @@ own verification mode ([discrepancy detection](verification-modes.md#discrepancy with the closest matches before any work. Without `--platform`, `--system` takes our ids only. - `--required-only`: exclude optional files and the data directories declared - `required: false`, keep only required + `required: false`, keep only required. `verify.py --required-only` reports + on the same file set, for `--platform`, `--emulator` and `--system` - `--no-game-data`: firmware only. Leaves out the platform's entries categorized game data and the cores' `category: game_data` files, and names each one. Tags the pack `_NoGameData`. `verify.py --no-game-data` and