diff --git a/scripts/packextras.py b/scripts/packextras.py index fdd74890..add6afd7 100644 --- a/scripts/packextras.py +++ b/scripts/packextras.py @@ -43,7 +43,20 @@ def extra_full_destination(fe: dict, prefix: str) -> str: """ if fe.get("root") == "save": return str(fe.get("pack_path") or "") - dest = sanitize_pack_path(fe.get("destination", fe.get("name", ""))) + return prefixed_destination( + sanitize_pack_path(fe.get("destination", fe.get("name", ""))), prefix + ) + + +def prefixed_destination(dest: str, prefix: str) -> str: + """A core extra's destination from the pack root. + + The one rule the collector, the builder and the manifest place a file + by. DuckStation's standalone `bios/` on Recalbox lands in the + `bios` folder itself: deduplicated at `bios/bios/` but written at + `bios/`, it let PCSX2's scan claim the same path under another + key, and `--region` withdrew one claim while the other kept the file. + """ if not dest or not prefix or dest.startswith(f"{prefix}/"): return dest return f"{prefix}/{dest}" @@ -544,7 +557,7 @@ def _agnostic_scan_extras( seed = system_dir_path(f, seed or fname) folder = seed.rsplit("/", 1)[0] if "/" in seed else "" dest = f"{folder}/{scan_name}" if folder else scan_name - full_dest = f"{extras_prefix}/{dest}" if extras_prefix else dest + full_dest = prefixed_destination(dest, extras_prefix) if full_dest in seen_dests and (taken is None or full_dest not in taken): continue extra = { @@ -705,7 +718,7 @@ def _companion_extras( extras: list[dict] = [] for companion_sha1, companion in companions: companion_dest = f"{folder}/{companion['name']}" if folder else companion["name"] - companion_full = f"{extras_prefix}/{companion_dest}" if extras_prefix else companion_dest + companion_full = prefixed_destination(companion_dest, extras_prefix) if companion_full in seen_dests and (taken is None or companion_full not in taken): continue extra = { @@ -848,7 +861,7 @@ def _archive_prefix_copies( if archive_name not in covered_names: continue dest = f"{prefix}/{archive_name}" - full_dest = f"{extras_prefix}/{dest}" if extras_prefix else dest + full_dest = prefixed_destination(dest, extras_prefix) if full_dest in seen_dests and (taken is None or full_dest not in taken): continue if not by_name.get(archive_name): @@ -927,7 +940,7 @@ class ExtraPlacement: if not slug or self.nested(dest): return None dest = f"{slug}/{dest}" - return dest, (f"{self.prefix}/{dest}" if self.prefix else dest) + return dest, prefixed_destination(dest, self.prefix) def places(self, entry: dict, dest: str) -> list[tuple[str, str]]: """Every (pack-relative, full) destination the pack gives an entry. @@ -1164,7 +1177,8 @@ def _collect_emulator_extras( # Build set of filenames already covered (platform baseline + first pass extras) # Enriched with canonical names from DB via MD5 (handles platform renaming) - covered_names = expand_platform_declared_names(config, db) + platform_names = expand_platform_declared_names(config, db) + covered_names = set(platform_names) for e in extras: covered_names.add(e["name"]) @@ -1211,9 +1225,17 @@ def _collect_emulator_extras( # of the directory core files go to, which the platform may not # declare (Recalbox has its System Cards under pcenginecd/ only, # geargrafx reads them from the root). - full_dest = f"{extras_prefix}/{dest}" if extras_prefix else dest + full_dest = prefixed_destination(dest, extras_prefix) if full_dest in seen_dests and full_dest not in taken: continue + if full_dest in taken and full_dest not in seen and fname not in platform_names: + # Another core's extra holds this path and only that + # extra covers the name: a pack without that core never + # reaches this copy, so this core claims nothing here. + # Batocera's launcher hands DuckStation one named BIOS; + # recorded on duckstation-legacy's scph100.bin, its claim + # kept the file for a target without legacy. + continue # Only files the database indexes: members of a data # directory are not in it and are not duplicated here. if not ( diff --git a/tests/test_install_selection.py b/tests/test_install_selection.py index 27aa7772..59d77ba1 100644 --- a/tests/test_install_selection.py +++ b/tests/test_install_selection.py @@ -1058,6 +1058,90 @@ class AHeldFileTheInstallerCannotFetch(unittest.TestCase): ) +class AStandaloneLayoutBelowTheBiosFolder(unittest.TestCase): + """DuckStation reads `bios/` standalone, which a platform whose + base destination is `bios` places in that folder itself. The collector + deduplicated the path at `bios/bios/` while the builder wrote it + at `bios/`, so another core's claim on the same file took the + path under a second key: `--region north-america` withdrew one key and + the pack kept the European BIOS the installer drops.""" + + def setUp(self): + import common + import generate_db + import generate_pack + + self.gp = generate_pack + tmp = tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") + self.addCleanup(tmp.cleanup) + previous = os.getcwd() + os.chdir(tmp.name) + self.addCleanup(os.chdir, previous) + Path("bios/Sys").mkdir(parents=True) + files = {} + for name in ("us.bin", "eu.bin", "jp.bin"): + payload = name.encode() + Path(f"bios/Sys/{name}").write_bytes(payload) + sha1 = hashlib.sha1(payload).hexdigest() + files[sha1] = {"path": f"bios/Sys/{name}", "name": name, + "size": len(payload), "sha1": sha1, + "md5": hashlib.md5(payload).hexdigest(), + "sha256": hashlib.sha256(payload).hexdigest(), + "crc32": "00000001"} + self.db = {"files": files, "indexes": generate_db.build_indexes(files, {})} + Path("platforms").mkdir() + Path("platforms/p.yml").write_text(json.dumps({ + "platform": "P", "verification_mode": "existence", + "base_destination": "bios", "cores": ["dsa", "other"], + "standalone_cores": ["dsa"], "systems": {"sys": {"files": []}}, + })) + Path("platforms/_registry.yml").write_text( + json.dumps({"platforms": {"p": {"status": "active"}}}) + ) + Path("emulators").mkdir() + Path("emulators/dsa.yml").write_text(json.dumps({ + "emulator": "dsa", "type": "standalone + libretro", "systems": ["sys"], + "files": [ + {"name": name, "standalone_path": f"bios/{name}", "system": "sys", + "region": [region_name], "required": False} + for name, region_name in ( + ("us.bin", "north-america"), ("eu.bin", "europe"), + ("jp.bin", "japan"), + ) + ]})) + Path("emulators/other.yml").write_text(json.dumps({ + "emulator": "other", "type": "libretro", "systems": ["sys"], "files": [ + {"name": name, "system": "sys", "required": False} + for name in ("eu.bin", "jp.bin") + ]})) + common._platform_config_cache.clear() + common._emulator_profiles_cache.clear() + self.addCleanup(common._platform_config_cache.clear) + self.addCleanup(common._emulator_profiles_cache.clear) + self.profiles = common.load_emulator_profiles("emulators") + + def _manifest(self, regions): + return self.gp.generate_manifest( + "p", "platforms", self.db, "bios", "platforms/_registry.yml", + emulators_dir="emulators", emu_profiles=self.profiles, + regions=regions, offline=True, + ) + + def test_one_path_one_entry(self): + full = self._manifest(None) + dests = [f["dest"] for f in full["files"]] + self.assertEqual(sorted(dests), ["eu.bin", "jp.bin", "us.bin"]) + + def test_the_installer_keeps_what_the_pack_keeps(self): + full = self._manifest(None) + built = self._manifest(["north-america"]) + self.assertEqual({f["dest"] for f in built["files"]}, {"us.bin"}) + files, _omitted = install.narrow( + full["files"], full["omitted_files"], [], [], ["north-america"] + ) + self.assertEqual({f["dest"] for f in files}, {f["dest"] for f in built["files"]}) + + class ACoreWithNothingHeldIsAnOmission(unittest.TestCase): """DOSBox serves SC-55/WAVEROM3.BIN, DOSBox Pure wants it and nothing is held for it. A pack built for a target without DOSBox lists it as left