mirror of
https://github.com/Abdess/retroarch_system.git
synced 2026-10-10 13:33:24 -05:00
refactor: name the gap filter's two kinds of skip
find_undeclared_files decided per entry whether a core requirement can be a gap, through a chain that mixed two skips whose difference is easy to lose. Some record the requirement as settled so no other profile reconsiders it; the rest leave it open, because the same file can be libretro-only in one profile and standalone-only in another and the key carries no emulator. The chain returns a named verdict now, complexity 60 to 45. Twelve tests cover the verdicts and two more cover the distinction end to end: a standalone-only entry seen first must not answer for the profile that needs the file. Collapsing the two skips into one passes every other test in the suite and fails that pair.
This commit is contained in:
1 parent
c313b32347
commit
bf3196ce06
2 files changed
+204
-31
No files matched your search
+48
-31
@@ -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., <user-selected>.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
|
||||
|
||||
@@ -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 ("<region>.png", "disk*.rom", "<user-selected>.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())
|
||||
Reference in new issue
Block a user