diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 82a396eb..06340b6b 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -966,13 +966,13 @@ def generate_pack( full_dest = dest else: full_dest = dest - if full_dest in seen_destinations: - continue - # Skip case-insensitive duplicates (Windows/macOS FS safety) - if full_dest.lower() in seen_lower and case_insensitive: - continue - # Skip file/directory path conflicts (e.g., SGB1.sfc file vs SGB1.sfc/ dir) - if _has_path_conflict(full_dest, seen_destinations, seen_parents): + # Taken, a case twin on a case-insensitive filesystem, or a + # file/directory conflict (SGB1.sfc vs SGB1.sfc/): verify replays + # the same rule (packextras.hidden_extras). + if placement_block( + full_dest, seen_destinations, seen_lower, seen_parents, + case_insensitive, + ): continue dest_hint = fe.get("destination", "") @@ -3213,11 +3213,10 @@ def _manifest_core_entries( else: full_dest = dest - if full_dest in seen_destinations: - continue - if case_insensitive and full_dest.lower() in seen_lower: - continue - if _has_path_conflict(full_dest, seen_destinations, seen_parents): + if placement_block( + full_dest, seen_destinations, seen_lower, seen_parents, + case_insensitive, + ): continue dest_hint = fe.get("destination", "") @@ -3989,6 +3988,7 @@ from packpaths import ( # noqa: E402,F401 _register_path, _flat, _resolve_destination, + placement_block, ) from packextras import ( # noqa: E402,F401 _detect_extras_prefix, diff --git a/scripts/packextras.py b/scripts/packextras.py index d6200e36..fb054e5c 100644 --- a/scripts/packextras.py +++ b/scripts/packextras.py @@ -128,6 +128,74 @@ def _slug_for( return _map_emulator_to_slug(profile, platform_systems, norm_map, sys_to_slug) +def hidden_extras( + config: dict, + emulators_dir: str, + db: dict, + profiles: dict, + shipped: dict[str, str], + target_cores: set[str] | None = None, +) -> dict[tuple[str, str, str], str]: + """Report entries the builder holds and still leaves out, with the reason. + + The builder walks the core extras after the platform's own files and + skips a destination that a case-insensitive filesystem folds onto one + already packed, or that a packed file blocks as a directory. RetroPie's + openmsx fmpac.rom gave way to the platform's FMPAC.ROM, other bytes, + while the report counted it in the pack. A twin carrying the same bytes + serves the core on the filesystem that folds the names, so it is not a + file left out. ``shipped`` maps the full destination of each platform + file the pack carries to its local copy. + """ + from packpaths import _register_path, placement_block + + sha1_by_path = { + entry.get("path"): sha1 for sha1, entry in db.get("files", {}).items() + } + base_dest = config.get("base_destination", "") + case_insensitive = bool(config.get("case_insensitive_fs", False)) + seen: set[str] = set() + parents: set[str] = set() + content: dict[str, str | None] = {} + for dest, local in shipped.items(): + _register_path(dest, seen, parents) + content[dest.lower()] = sha1_by_path.get(local) + lower = set(content) if case_insensitive else set() + prefix = _detect_extras_prefix(config, base_dest) + hidden: dict[tuple[str, str, str], str] = {} + for fe in _collect_emulator_extras( + config, emulators_dir, db, set(shipped), base_dest, profiles, + target_cores=target_cores, + ): + dest = sanitize_pack_path(fe.get("destination", fe.get("name", ""))) + if not dest: + continue + full = dest if not prefix or dest.startswith(f"{prefix}/") else f"{prefix}/{dest}" + reason = placement_block(full, seen, lower, parents, case_insensitive) + if reason == "taken": + # Another core's claim on the same path, settled by the claims + # the collector records: not a file left out. + continue + local, _status = resolve_local_file( + fe, db, dest_hint=fe.get("destination", "") + ) + held = sha1_by_path.get(local) if local else None + if reason == "case_twin" and held and content.get(full.lower()) == held: + continue + if reason is not None: + key = ( + fe.get("source_emulator", ""), fe.get("source_name", ""), + fe.get("source_path", ""), + ) + hidden.setdefault(key, reason) + continue + _register_path(full, seen, parents) + content[full.lower()] = held + if case_insensitive: + lower.add(full.lower()) + return hidden + + def unplaceable_extras(config: dict, undeclared: list[dict], profiles: dict) -> set: """Keys of the report entries a slug-based platform cannot place. diff --git a/scripts/packpaths.py b/scripts/packpaths.py index 1359b318..2ea30571 100644 --- a/scripts/packpaths.py +++ b/scripts/packpaths.py @@ -26,6 +26,27 @@ def _has_path_conflict(dest: str, seen_files: set[str], seen_parents: set[str]) return True return False +def placement_block( + dest: str, + seen_files: set[str], + seen_lower: set[str], + seen_parents: set[str], + case_insensitive: bool, +) -> str | None: + """Why a destination cannot take a file in this pack, or None. + + The one rule the builder applies to a core extra and the report replays: + counted apart, the report called a skipped file packed. + """ + if dest in seen_files: + return "taken" + if case_insensitive and dest.lower() in seen_lower: + return "case_twin" + if _has_path_conflict(dest, seen_files, seen_parents): + return "path_conflict" + return None + + def _register_path(dest: str, seen_files: set[str], seen_parents: set[str]) -> None: """Track a file path and its parent directories.""" seen_files.add(dest) diff --git a/scripts/verify.py b/scripts/verify.py index a447f007..9768a032 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -785,6 +785,39 @@ def _mark_unplaced(config: dict, undeclared: list[dict], profiles: dict) -> None u["omitted"] = "no platform slug for its system" +_HIDDEN_REASONS = { + "case_twin": "a packed file of the same name in another case takes its place", + "path_conflict": "a packed file occupies its directory", +} + + +def _mark_hidden( + config: dict, + undeclared: list[dict], + emulators_dir: str, + db: dict, + emu_profiles: dict | None, + shipped: dict[str, str], + target_cores: set[str] | None, +) -> None: + """What the builder holds but cannot place beside the files it packed.""" + from packextras import hidden_extras + + hidden = hidden_extras( + config, emulators_dir, db, + emu_profiles if emu_profiles is not None + else load_emulator_profiles(emulators_dir), + shipped, target_cores, + ) + for u in undeclared: + reason = hidden.get( + (u.get("emulator", ""), u.get("name", ""), u.get("path") or "") + ) + if reason and u.get("in_repo") and u.get("in_pack", True): + u["in_pack"] = False + u["omitted"] = _HIDDEN_REASONS[reason] + + def _twin_index( verify_systems: dict, db: dict, base_dest: str, zip_contents: dict, ) -> tuple[dict[str, int], dict[int, tuple[str, dict]]]: @@ -981,6 +1014,7 @@ def verify_platform( verify_systems, db, base_dest, zip_contents ) + shipped: dict[str, str] = {} for sys_id, system in verify_systems.items(): for file_entry in system.get("files", []): if region_drops and ( @@ -1030,6 +1064,11 @@ def verify_platform( validation_index, ) details.append(result) + if local_path and result["status"] != Status.MISSING: + shipped.setdefault( + f"{base_dest}/{destination}" if base_dest else destination, + local_path, + ) if _twin_unmet(result, held_by): continue @@ -1089,6 +1128,9 @@ def verify_platform( not in region_drops ] _mark_unplaced(config, undeclared, profiles) + _mark_hidden( + config, undeclared, emulators_dir, db, emu_profiles, shipped, target_cores + ) exclusions = find_exclusion_notes( config, emulators_dir, emu_profiles, target_cores=target_cores ) diff --git a/tests/test_hidden_extras.py b/tests/test_hidden_extras.py new file mode 100644 index 00000000..ddc4e70c --- /dev/null +++ b/tests/test_hidden_extras.py @@ -0,0 +1,95 @@ +"""A core file the builder holds but cannot place is not counted packed. + +The builder skips a core extra whose destination a case-insensitive +filesystem folds onto a packed file, or that a packed file blocks as a +directory. verify counted both in "Core files: N in pack": RetroPie's +openmsx fmpac.rom (the pack carries the platform's FMPAC.ROM, other bytes), +ROCKNIX's higan SGB1.sfc/program.rom under the file SGB1.sfc. +""" + +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 packpaths import placement_block # noqa: E402 + + +class OneRule(unittest.TestCase): + def test_reasons(self): + seen = {"bios/FMPAC.ROM", "bios/SGB1.sfc"} + lower = {d.lower() for d in seen} + parents = {"bios"} + cases = { + "bios/FMPAC.ROM": "taken", + "bios/fmpac.rom": "case_twin", + "bios/SGB1.sfc/program.rom": "path_conflict", + "bios/other.rom": None, + } + for dest, reason in cases.items(): + with self.subTest(dest=dest): + self.assertEqual( + placement_block(dest, seen, lower, parents, True), reason + ) + self.assertIsNone( + placement_block("bios/fmpac.rom", seen, lower, parents, False) + ) + + def test_builder_and_report_read_it(self): + for module in ("generate_pack.py", "packextras.py"): + source = (REPO_ROOT / "scripts" / module).read_text(encoding="utf-8") + self.assertIn("placement_block(", source, module) + builder = (REPO_ROOT / "scripts" / "generate_pack.py").read_text( + encoding="utf-8" + ) + self.assertNotIn("if full_dest.lower() in seen_lower and case_insensitive", builder) + + +class TheReportSaysSo(unittest.TestCase): + """Reads the repository: skips when the database is absent.""" + + @classmethod + def setUpClass(cls): + if not (REPO_ROOT / "database.json").exists(): + raise unittest.SkipTest("database.json is not built") + previous = os.getcwd() + os.chdir(REPO_ROOT) + try: + from common import load_database, load_emulator_profiles + from verify import verify_platform + from common import load_platform_config + + db = load_database("database.json") + profiles = load_emulator_profiles("emulators") + config = load_platform_config("retropie", "platforms") + cls.result = verify_platform(config, db, "emulators", profiles) + finally: + os.chdir(previous) + + def _entry(self, emulator: str, path: str) -> dict: + for u in self.result["undeclared_files"]: + if u.get("emulator") == emulator and (u.get("path") or u.get("name")) == path: + return u + self.skipTest(f"{emulator} {path} is not in the cross-reference") + + def test_a_case_twin_with_other_bytes_is_left_out(self): + entry = self._entry("openMSX", "fmpac.rom") + self.assertIs(entry.get("in_pack"), False) + self.assertIn("another case", entry.get("omitted", "")) + + def test_a_twin_with_the_same_bytes_serves_the_core(self): + entry = self._entry("Stella 2023", "KVS1.WAV") + self.assertIsNot(entry.get("in_pack"), False) + + def test_a_directory_under_a_packed_file_is_left_out(self): + entry = self._entry("nSide (SFC Balanced)", "SGB1.sfc/lr35902.boot.rom") + self.assertIs(entry.get("in_pack"), False) + + +if __name__ == "__main__": + unittest.main()