diff --git a/scripts/verify.py b/scripts/verify.py index a8cc1427..1b0a246a 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -338,6 +338,49 @@ def _name_in_index( return False +def _candidate_verdict( + file_entry: dict, + fname: str, + is_standalone: bool, + include_all: bool, + declared_names: set, +) -> str: + """Whether a profile entry can be a gap, and whether it is settled. + + Three answers. "keep" means it is a candidate. "settled" means something + has answered for it -the platform declares it, or the profile documents + it as unsourceable -so the same requirement reached from another profile + must not be reconsidered. "skip" means it does not apply in this context, + which leaves it open for a profile where it does: the same file can be + libretro-only here and standalone-only there. + """ + if file_entry.get("unsourceable"): + return "settled" + # Placeholders stand for a family of files, not a file. + if "<" in fname or ">" in fname or "*" in fname: + return "skip" + # An explicit null path means the user imports it through the UI. + if "path" in file_entry and file_entry["path"] is None: + return "skip" + file_mode = file_entry.get("mode") + if file_mode == "standalone" and not is_standalone: + return "skip" + if file_mode == "libretro" and is_standalone: + return "skip" + # Read from somewhere other than the system directory: not a BIOS gap. + load_from = file_entry.get("load_from", "") + if load_from and load_from != "system_dir": + return "skip" + # Filename-agnostic entries are answered by the builder's own scan. + if file_entry.get("agnostic"): + return "skip" + if not include_all: + archive = file_entry.get("archive") + if fname in declared_names or (archive and archive in declared_names): + return "settled" + return "keep" + + def find_undeclared_files( config: dict, emulators_dir: str, @@ -425,42 +468,16 @@ def find_undeclared_files( ) if not fname or seen_key in seen_files: continue - # Skip unsourceable files (documented reason, not a gap) - if f.get("unsourceable"): + verdict = _candidate_verdict( + f, fname, is_standalone, include_all, declared_names + ) + if verdict == "settled": seen_files.add(seen_key) continue - # Skip pattern placeholders (e.g., .bin) - if "<" in fname or ">" in fname or "*" in fname: - continue - # Skip UI-imported files with explicit path: null (not resolvable by pack) - if "path" in f and f["path"] is None: - continue - # Mode filtering: skip files incompatible with platform's usage - file_mode = f.get("mode") - if file_mode == "standalone" and not is_standalone: - continue - if file_mode == "libretro" and is_standalone: - continue - # Skip files loaded from non-system directories (save_dir, content_dir) - load_from = f.get("load_from", "") - if load_from and load_from != "system_dir": - continue - - # Skip agnostic files (filename-agnostic, handled by agnostic scan) - if f.get("agnostic"): + if verdict == "skip": continue archive = f.get("archive") - - # Skip files declared by the platform (by name or archive) - if not include_all: - if fname in declared_names: - seen_files.add(seen_key) - continue - if archive and archive in declared_names: - seen_files.add(seen_key) - continue - seen_files.add(seen_key) # Archived files are grouped by archive diff --git a/tests/test_native_mode.py b/tests/test_native_mode.py index 3e144f37..51f9f974 100644 --- a/tests/test_native_mode.py +++ b/tests/test_native_mode.py @@ -259,3 +259,159 @@ class GapAnalysisAgreesWithTheBuilder(unittest.TestCase): if __name__ == "__main__": unittest.main() + + +class CandidateVerdict(unittest.TestCase): + """Whether a profile entry can be a gap, and whether it is settled. + + The filter chain mixed two kinds of skip. A settled entry is answered for + -the platform declares it, the profile calls it unsourceable -so the same + requirement reached from another profile must not be reconsidered. A + skipped one merely does not apply here: the same file can be libretro-only + in one profile and standalone-only in another, and recording it from the + profile that cannot use it would hide it from the one that can. + """ + + def verdict(self, entry, *, standalone=False, include_all=False, + declared=frozenset()): + from verify import _candidate_verdict + + return _candidate_verdict( + entry, entry.get("name", ""), standalone, include_all, set(declared) + ) + + def test_a_plain_requirement_is_a_candidate(self): + self.assertEqual(self.verdict({"name": "bios.bin"}), "keep") + + def test_a_declared_file_is_settled(self): + self.assertEqual( + self.verdict({"name": "bios.bin"}, declared={"bios.bin"}), "settled" + ) + + def test_a_declared_archive_settles_its_entry(self): + self.assertEqual( + self.verdict( + {"name": "rom.bin", "archive": "neogeo.zip"}, + declared={"neogeo.zip"}, + ), + "settled", + ) + + def test_include_all_ignores_what_the_platform_declares(self): + """Ground-truth mode reports the core's needs, not the platform's list.""" + self.assertEqual( + self.verdict( + {"name": "bios.bin"}, declared={"bios.bin"}, include_all=True + ), + "keep", + ) + + def test_an_unsourceable_entry_is_settled(self): + self.assertEqual( + self.verdict({"name": "font.rom", "unsourceable": "in a paid package"}), + "settled", + ) + + def test_a_placeholder_names_a_family_not_a_file(self): + for name in (".png", "disk*.rom", ".bin"): + self.assertEqual(self.verdict({"name": name}), "skip", name) + + def test_a_null_path_means_the_user_imports_it(self): + self.assertEqual(self.verdict({"name": "key.bin", "path": None}), "skip") + + def test_mode_mismatches_are_skipped_but_left_open(self): + """Not settled: the profile that can use the file must still see it.""" + self.assertEqual( + self.verdict({"name": "b.bin", "mode": "standalone"}, standalone=False), + "skip", + ) + self.assertEqual( + self.verdict({"name": "b.bin", "mode": "libretro"}, standalone=True), + "skip", + ) + self.assertEqual( + self.verdict({"name": "b.bin", "mode": "standalone"}, standalone=True), + "keep", + ) + + def test_a_file_read_from_elsewhere_is_not_a_bios_gap(self): + self.assertEqual( + self.verdict({"name": "save.bin", "load_from": "save_dir"}), "skip" + ) + self.assertEqual( + self.verdict({"name": "b.bin", "load_from": "system_dir"}), "keep" + ) + + def test_an_agnostic_entry_is_answered_by_the_builders_scan(self): + self.assertEqual( + self.verdict({"name": "any.bin", "agnostic": True}), "skip" + ) + + +class SkippingIsNotSettling(unittest.TestCase): + """A profile that cannot use a file must not answer for one that can. + + The key recording a settled requirement carries the name, path, system and + variant, not the emulator, so two profiles can reach the same key. If the + one where the entry does not apply records it, the entry vanishes from the + report for the profile that does need it. + """ + + def _report(self): + import hashlib + import tempfile + + import common + from verify import find_undeclared_files + + tmp = tempfile.TemporaryDirectory() + root = Path(tmp.name) + (root / "emulators").mkdir() + rom = root / "shared.bin" + rom.write_bytes(b"SHARED REQUIREMENT") + sha1 = hashlib.sha1(rom.read_bytes()).hexdigest() + db = { + "files": {sha1: {"path": str(rom), "name": "shared.bin", + "size": rom.stat().st_size, "sha1": sha1, + "md5": hashlib.md5(rom.read_bytes()).hexdigest()}}, + "indexes": {"by_name": {"shared.bin": [sha1]}, "by_md5": {}, + "by_sha256": {}, "by_crc32": {}, "by_path_suffix": {}}, + } + # "a_" sorts first, so the profile that cannot use the file is seen + # before the one that can. + for slug, mode in (("a_standalone_only", "standalone"), ("b_libretro", None)): + entry = " - name: shared.bin\n system: demo-system\n" + if mode: + entry += f" mode: {mode}\n" + (root / "emulators" / f"{slug}.yml").write_text( + f"emulator: {slug}\n" + "type: libretro\n" + f"display_name: {slug}\n" + "systems: [demo-system]\n" + f"cores: [{slug}]\n" + "files:\n" + entry + ) + try: + common._emulator_profiles_cache.clear() + profiles = common.load_emulator_profiles(str(root / "emulators")) + config = { + "platform": "Demo", "verification_mode": "existence", + "cores": ["a_standalone_only", "b_libretro"], "systems": {}, + } + found = find_undeclared_files( + config, str(root / "emulators"), db, profiles, data_names=set() + ) + return {(u["emulator"], u["name"]) for u in found} + finally: + common._emulator_profiles_cache.clear() + tmp.cleanup() + + def test_the_profile_that_needs_the_file_still_reports_it(self): + reported = self._report() + self.assertIn( + ("b_libretro", "shared.bin"), reported, + "a standalone-only entry seen first must not settle the requirement", + ) + + def test_the_profile_that_cannot_use_it_does_not_report_it(self): + self.assertNotIn(("a_standalone_only", "shared.bin"), self._report())