diff --git a/scripts/common.py b/scripts/common.py index 80bdb2b4..8c9d91f3 100644 --- a/scripts/common.py +++ b/scripts/common.py @@ -1829,6 +1829,17 @@ def declared_root(config: dict) -> str: return root if count / len(roots) > 0.9 else "" +def merged_platform_declarations( + configs: list[dict], db: dict +) -> dict[str, list[tuple[int | None, str]]]: + """`platform_declarations` of several platforms, one index.""" + merged: dict[str, list[tuple[int | None, str]]] = {} + for config in configs: + for name, places in platform_declarations(config, db).items(): + merged.setdefault(name, []).extend(places) + return merged + + def expand_platform_declared_names(config: dict, db: dict) -> set[str]: """The names a platform declares, enriched as `platform_declarations`.""" return set(platform_declarations(config, db)) diff --git a/scripts/cross_reference.py b/scripts/cross_reference.py index 5ef821c5..8865728b 100644 --- a/scripts/cross_reference.py +++ b/scripts/cross_reference.py @@ -32,7 +32,7 @@ from common import ( resolve_local_file, runs_standalone, ) -from validation import outside_gap_scope +from validation import declaration_settles, outside_gap_scope yaml = require_yaml() @@ -70,23 +70,27 @@ def load_platform_files( return declared, platform_data_dirs -def declared_names(platforms_dir: str, platforms: list[str] | None, db: dict) -> set[str]: - """Every name the platforms declare, under every spelling the database knows. +def declared_names( + platforms_dir: str, platforms: list[str] | None, db: dict +) -> dict[str, list[tuple[int | None, str]]]: + """Every name the platforms declare, with each declaration's size and slot. The report judged "undeclared" per normalized system id, which does not bring bk and elektronika-bk, or nintendo-sgb and nintendo-super-game-boy, together: 646 entries whose exact name a platform declares were counted - undeclared, while verify and the site read one flat set. + undeclared, while verify and the site read one flat index. """ - from common import expand_platform_declared_names + from common import merged_platform_declarations - names: set[str] = set() - for platform_name in platforms or list_registered_platforms( - platforms_dir, include_archived=True - ): - config = load_platform_config(platform_name, platforms_dir) - names.update(expand_platform_declared_names(config, db)) - return names + return merged_platform_declarations( + [ + load_platform_config(platform_name, platforms_dir) + for platform_name in platforms or list_registered_platforms( + platforms_dir, include_archived=True + ) + ], + db, + ) def _build_supplemental_index( @@ -405,10 +409,7 @@ def _cross_reference_profile( # Check platform declaration: by name, by an alias the core answers # to (RetroArch lists amiberry's cd32.rom as kick40060.CD32), or by # archive. - answers = {fname, *f.get("aliases", [])} - in_platform = bool(answers & platform_names) - if not in_platform and archive: - in_platform = archive in platform_names + in_platform = declaration_settles(f, fname, platform_names, effective_path) if in_platform: seen_files.add(seen_key) @@ -506,7 +507,7 @@ def cross_reference( db: dict, platform_data_dirs: dict[str, set[str]] | None = None, data_names: set[str] | None = None, - all_declared: set[str] | None = None, + all_declared: dict[str, list[tuple[int | None, str]]] | None = None, standalone_cores: set[str] | None = None, ) -> dict: """Compare emulator profiles against platform declarations. @@ -519,8 +520,8 @@ def cross_reference( The boolean ``in_repo`` is derived: ``source != "missing"``. - When *all_declared* is provided (flat set of every filename declared by - any platform for any system), it is used for the ``in_platform`` check + When *all_declared* is provided (every filename declared by any platform + for any system, with its sizes and slots), it is used for the ``in_platform`` check instead of the per-system lookup. This is appropriate for the global gap analysis page where "undeclared" means "no platform declares it at all". """ diff --git a/scripts/generate_site.py b/scripts/generate_site.py index eca1b022..1cbf45d9 100644 --- a/scripts/generate_site.py +++ b/scripts/generate_site.py @@ -530,15 +530,17 @@ def build_emulator_gap_report( The gap analysis page and the published gaps export must not compute this twice and drift; both read this one report. """ - from common import _norm_system_id, expand_platform_declared_names + from common import _norm_system_id, merged_platform_declarations from cross_reference import cross_reference as run_cross_reference - all_declared: set[str] = set() + # Enriched with alias resolution, with each declaration's size and slot: + # a name alone settled DOSBox's 32 KiB SC-55 ROM1.BIN on Galaksija's. + all_declared = merged_platform_declarations( + [cov["config"] for cov in coverages.values()], db + ) declared: dict[str, set[str]] = {} for _name, cov in coverages.items(): config = cov["config"] - # Enrich with alias resolution (MD5 -> SHA1 -> canonical name + aliases) - all_declared.update(expand_platform_declared_names(config, db)) for sys_id, system in config.get("systems", {}).items(): for fe in system.get("files", []): fname = fe.get("name", "") diff --git a/scripts/validation.py b/scripts/validation.py index 0dc0f320..8e1a5da3 100644 --- a/scripts/validation.py +++ b/scripts/validation.py @@ -8,8 +8,9 @@ truth data for reporting. from __future__ import annotations import os +from collections.abc import Mapping -from common import compute_hashes, size_fits +from common import compute_hashes, name_match_size_ok, sanitize_pack_path, size_fits from hashing import parse_md5_list from nativemode import digest_algorithm, hash_mismatch_excludes_file from ziptools import check_inside_zip @@ -487,6 +488,54 @@ def outside_gap_scope(file_entry: dict, is_standalone: bool) -> bool: ) +def declaration_settles( + file_entry: dict, + fname: str, + declared_names: Mapping[str, list[tuple[int | None, str]]] | set[str], + dest: str = "", +) -> bool: + """Whether a platform declaration meets a core's requirement. + + A platform declaring the name the core reads has met the requirement + where the declaration fills the entry's own slot, whose content the slot + arbitration decides, or elsewhere at a size the core accepts: the builder + copies that file to the core's path. Galaksija's 4 KiB ROM1.BIN does not + stand for DOSBox's 32 KiB SC-55 one. The builder copies nothing for an + alias, so an alias answers only where the core reads it: quasi88 reads + n88sub.rom or disk.rom in quasi88/, and System.dat names quasi88/disk.rom. + RetroDECK's root MSX.ROM is not CLK's MSX/MSX.ROM, and RetroBat's 16 KiB + MSX DISK.ROM at the root is not the 256-byte Disk II ROM gsplus also + accepts as c600.rom. The archive answers by its name. A bare set of names + carries no slot or size and answers by name. + + Read by verify and by the gap report, which settled on the name alone + and left DOSBox's SC-55 ROMs off the gaps page. + """ + if file_entry.get("archive") in declared_names: + return True + slot = sanitize_pack_path(dest or fname).lower() + directory = slot if dest.endswith("/") else slot.rpartition("/")[0] + for name in (fname, *file_entry.get("aliases", [])): + if name not in declared_names: + continue + if not isinstance(declared_names, Mapping): + return True + if name == fname: + if any( + where == slot or name_match_size_ok(file_entry, size) + for size, where in declared_names[name] + ): + return True + continue + read_at = f"{directory}/{name}".lower() if directory else name.lower() + if any( + where == read_at and name_match_size_ok(file_entry, size) + for size, where in declared_names[name] + ): + return True + return False + + def filter_files_by_mode(files: list[dict], standalone: bool) -> list[dict]: """Filter file entries by libretro/standalone mode.""" result = [] diff --git a/scripts/verify.py b/scripts/verify.py index 2850a3f8..1b46639e 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -73,6 +73,7 @@ from nativemode import ( reads_file_contents, ) from validation import ( + declaration_settles, existence_discrepancy, destination_owners, validated_choice, @@ -414,40 +415,10 @@ def _candidate_verdict( return "settled" if outside_gap_scope(file_entry, is_standalone): return "skip" - if not include_all: - # A platform declaring the name the core reads has met the - # requirement where the declaration fills the entry's own slot, - # whose content the slot arbitration decides, or elsewhere at a size - # the core accepts: the builder copies that file to the core's path. - # Galaksija's 4 KiB ROM1.BIN does not stand for DOSBox's 32 KiB SC-55 - # one. The builder copies nothing for an alias, so an alias answers - # only where the core reads it: quasi88 reads n88sub.rom or disk.rom - # in quasi88/, and System.dat names quasi88/disk.rom. RetroDECK's - # root MSX.ROM is not CLK's MSX/MSX.ROM, and RetroBat's 16 KiB MSX - # DISK.ROM at the root is not the 256-byte Disk II ROM gsplus also - # accepts as c600.rom. The archive answers by its name. - if file_entry.get("archive") in declared_names: - return "settled" - slot = sanitize_pack_path(dest or fname).lower() - directory = slot if dest.endswith("/") else slot.rpartition("/")[0] - for name in (fname, *file_entry.get("aliases", [])): - if name not in declared_names: - continue - if not isinstance(declared_names, Mapping): - return "settled" - if name == fname: - if any( - where == slot or name_match_size_ok(file_entry, size) - for size, where in declared_names[name] - ): - return "settled" - continue - read_at = f"{directory}/{name}".lower() if directory else name.lower() - if any( - where == read_at and name_match_size_ok(file_entry, size) - for size, where in declared_names[name] - ): - return "settled" + if not include_all and declaration_settles( + file_entry, fname, declared_names, dest + ): + return "settled" return "keep" diff --git a/tests/test_gap_settlement.py b/tests/test_gap_settlement.py new file mode 100644 index 00000000..2d7b0953 --- /dev/null +++ b/tests/test_gap_settlement.py @@ -0,0 +1,50 @@ +"""The gap report settles a core file as verify does: slot or size. + +cross_reference counted a core file declared as soon as a platform named +it. RetroArch names Galaksija's 4 KiB galaksija/rom1.bin, so DOSBox's +32 KiB SC-55/ROM1.BIN left the gaps page while verify listed it undeclared. +""" + +from __future__ import annotations + +import os +import sys +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(REPO_ROOT / "scripts")) + +from cross_reference import cross_reference # noqa: E402 +from validation import declaration_settles # noqa: E402 + +SC55 = {"name": "ROM1.BIN", "path": "SC-55/ROM1.BIN", "size": 32768, + "validation": ["size"], "required": True} + + +class OneRule(unittest.TestCase): + def test_a_same_named_file_of_another_size_does_not_settle(self): + declared = {"ROM1.BIN": [(4096, "galaksija/rom1.bin")]} + self.assertFalse(declaration_settles(SC55, "ROM1.BIN", declared, SC55["path"])) + declared["ROM1.BIN"].append((32768, "rom1.bin")) + self.assertTrue(declaration_settles(SC55, "ROM1.BIN", declared, SC55["path"])) + + def test_the_gap_report_reads_it(self): + profiles = {"dosbox": {"emulator": "DOSBox", "type": "libretro", + "systems": ["dos"], "files": [SC55]}} + report = cross_reference( + profiles, {}, {"files": {}, "indexes": {}}, + all_declared={"ROM1.BIN": [(4096, "galaksija/rom1.bin")]}, + ) + self.assertEqual( + [g["name"] for g in report["dosbox"]["gap_details"]], ["ROM1.BIN"] + ) + + def test_both_sides_call_it(self): + for module in ("verify.py", "cross_reference.py"): + source = (REPO_ROOT / "scripts" / module).read_text(encoding="utf-8") + self.assertIn("declaration_settles(", source, module) + + +if __name__ == "__main__": + unittest.main()