diff --git a/install.ps1 b/install.ps1 index 25f414a8..16e613e2 100644 --- a/install.ps1 +++ b/install.ps1 @@ -11,7 +11,7 @@ $ErrorActionPreference = "Stop" $defaultInstallUrl = "https://raw.githubusercontent.com/Abdess/retrobios/main/install.py" - $defaultInstallSha256 = "7edd155bb1f50a71007cd3500d6e41003e969dfa3c09570ba517399e94faa428" + $defaultInstallSha256 = "fe8de4eda0919c268d4d2c0d571c7d0dc1cf675d7604263766f5f99a35aa3724" $maximumInstallerBytes = 2MB $installerExit = 0 $installer = if ($PSScriptRoot) { Join-Path $PSScriptRoot "install.py" } else { $null } diff --git a/install.py b/install.py index 7f73526d..9cd3e52e 100644 --- a/install.py +++ b/install.py @@ -1754,6 +1754,8 @@ def _check_omission(entry: dict, dest: str) -> None: raise ValueError(f"invalid omitted required flag for {dest}") if entry.get("reason") not in OMISSION_REASONS: raise ValueError(f"invalid omission reason for {dest}") + if "held" in entry and entry["held"] is not True: + raise ValueError(f"invalid omitted held flag for {dest}") cores = entry.get("cores") if cores is not None and ( not isinstance(cores, list) @@ -2516,8 +2518,10 @@ def narrow( omitted = _filter_by_target(omitted, cores) if regions: # The builder's held set: what the collection serves. An omission - # answers the request but takes no installable file's place. - held = {f["dest"] for f in files} + # answers the request but takes no installable file's place, unless + # the builder holds it (held: true) and only this download lacks it: + # the pack drops the fallback it displaces, and so does this. + held = {f["dest"] for f in files} | {o["dest"] for o in omitted if o.get("held")} dropped = region_drops(files + omitted, regions, systems, held) files = [f for f in files if f["dest"] not in dropped] omitted = [o for o in omitted if o["dest"] not in dropped] diff --git a/install.sh b/install.sh index f5a9af01..6f563d44 100755 --- a/install.sh +++ b/install.sh @@ -18,7 +18,7 @@ esac TEMP_INSTALLER="" TEMP_DIRECTORY="" DEFAULT_INSTALL_URL="https://raw.githubusercontent.com/Abdess/retrobios/main/install.py" -DEFAULT_INSTALL_SHA256="7edd155bb1f50a71007cd3500d6e41003e969dfa3c09570ba517399e94faa428" +DEFAULT_INSTALL_SHA256="fe8de4eda0919c268d4d2c0d571c7d0dc1cf675d7604263766f5f99a35aa3724" MAX_INSTALLER_BYTES=2097152 cleanup() { diff --git a/schemas/install-manifest.schema.json b/schemas/install-manifest.schema.json index 0174cc1b..cca54788 100644 --- a/schemas/install-manifest.schema.json +++ b/schemas/install-manifest.schema.json @@ -170,6 +170,10 @@ "below_platform_folder" ] }, + "held": { + "description": "The pack builder holds this file and the installer cannot download it. Region narrowing counts it held, as the builder does, so it displaces the same fallback.", + "const": true + }, "cores": { "oneOf": [ { diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 6bfd3c0c..5ae48da9 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -3890,7 +3890,7 @@ def _manifest_core_entries( # without the cores served here leaves the slot to it, missing, # and the installer narrowing to those cores has to say so too. entry.setdefault("absent_claims", []).append( - {k: v for k, v in displaced.items() if k != "dest"} + {k: v for k, v in displaced.items() if k not in ("dest", "held")} ) total_size += file_size seen_destinations.add(full_dest) @@ -3937,8 +3937,9 @@ def _manifest_selection( required_only: bool, regions: list[str] | None, undeclared: list[dict] | None = None, -) -> tuple[set[str], SelectionIndex]: - """Destinations a region list removes, and the groups the installer replays. +) -> tuple[set[str], SelectionIndex, set[str]]: + """Destinations a region list removes, the groups the installer replays, + and the destinations the builder holds. Grouped as the pack groups them, so a region the installer applies to the full manifest withdraws what `--region` withdraws from the pack. @@ -3970,7 +3971,7 @@ def _manifest_selection( ) return drops, SelectionIndex.build( region_groups, region_index, pack_systems, extras, required_only - ) + ), held def _record_absent( @@ -4132,6 +4133,9 @@ def generate_manifest( reason: str, cores: list[str] | None, ) -> None: + bare = sanitize_pack_path( + file_entry.get("destination", file_entry.get("name", "")) + ) omitted_by_destination[full_destination] = { "dest": manifest_destination(full_destination), "name": str(file_entry.get("name") or ""), @@ -4139,14 +4143,14 @@ def generate_manifest( "required": bool(file_entry.get("required", True)), "reason": reason, "cores": cores, - # The builder counts an unavailable regional file in its groups: - # the installer must too, or it keeps a fallback the pack drops. - **selection.fields( - sanitize_pack_path( - file_entry.get("destination", file_entry.get("name", "")) - ), - str(file_entry.get("name") or ""), - ), + # An omission keeps its region groups, as the builder groups it: + # a requested file the collection lacks answers the request + # without displacing a held fallback (resolve_region_drops, + # held). One the builder holds and the installer cannot fetch + # does displace it in the pack, so it says so, and the installer + # drops the same fallback. + **selection.fields(bare, str(file_entry.get("name") or "")), + **({"held": True} if bare in builder_held else {}), } # Read by the region grouping and by phase 2 with the same arguments. @@ -4158,7 +4162,7 @@ def generate_manifest( if source != "platform" else None ) - region_drops, selection = _manifest_selection( + region_drops, selection, builder_held = _manifest_selection( config, pack_systems, emulators_dir, db, base_dest, emu_profiles, target_cores, source, required_only, regions, undeclared=undeclared, ) diff --git a/tests/test_install_selection.py b/tests/test_install_selection.py index 2dcfb9d5..1318029b 100644 --- a/tests/test_install_selection.py +++ b/tests/test_install_selection.py @@ -124,6 +124,45 @@ class TheRegionRuleIsTheBuilders(unittest.TestCase): {o["dest"] for o in omitted} - drops, ) + def test_narrow_counts_what_the_builder_holds(self): + """A file the builder holds and the installer cannot download sits in + omitted_files. Counted unheld, it left the installer a fallback the + pack drops. The manifest marks it held, and narrow replays the + builder's held set: installable files plus held omissions.""" + rng = random.Random(20261021) + replayed = 0 + for trial in range(500): + groups, index, entries, requested, held = self._case(rng) + pack_only = {dest for dest in held if rng.random() < 0.3} + files = [dict(e, size=1, cores=None) for e in entries + if e["dest"] in held - pack_only] + omitted = [dict(e, cores=None, **({"held": True} if e["dest"] in pack_only else {})) + for e in entries if e["dest"] not in held - pack_only] + kept, still_omitted = install.narrow(files, omitted, [], [], requested) + drops = region.resolve_region_drops(groups, index, requested, held) + unheld = region.resolve_region_drops( + groups, index, requested, held - pack_only + ) + replayed += drops != unheld + with self.subTest(trial=trial): + self.assertEqual( + {f["dest"] for f in kept}, + {f["dest"] for f in files} - drops, + ) + self.assertEqual( + {o["dest"] for o in still_omitted}, + {o["dest"] for o in omitted} - drops, + ) + # The cases where counting them unheld answers otherwise exist. + self.assertGreater(replayed, 0) + + def test_the_held_flag_is_checked_at_the_boundary(self): + entry = {"name": "a.bin", "system": "s", "required": True, + "reason": "not_found", "cores": None} + install._check_omission({**entry, "held": True}, "a.bin") + with self.assertRaises(ValueError): + install._check_omission({**entry, "held": "yes"}, "a.bin") + def test_a_lone_core_file_leaves_its_systems_drop(self): """Dolphin's GC/EUR/IPL.bin is a core extra no system names, plus the GameCube's own declaration. The builder skips the one-file group, so @@ -706,6 +745,77 @@ class InstallMatchesATargetManifest(unittest.TestCase): +class AHeldFileTheInstallerCannotFetch(unittest.TestCase): + """The builder holds a release asset the manifest run had no copy of: it + counts in the builder's groups and displaces the American fallback for + --region japan, while the manifest can only list it as omitted. Counted + unheld there, the installer kept the fallback the pack 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) + Path("bios/Sys/us.bin").write_bytes(b"us") + sha1 = hashlib.sha1(b"us").hexdigest() + files = {sha1: {"path": "bios/Sys/us.bin", "name": "us.bin", "size": 2, + "sha1": sha1, "md5": hashlib.md5(b"us").hexdigest(), + "sha256": hashlib.sha256(b"us").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", "cores": ["core"], + "systems": {"sys": {"files": []}}, + })) + Path("platforms/_registry.yml").write_text( + json.dumps({"platforms": {"p": {"status": "active"}}}) + ) + Path("emulators").mkdir() + Path("emulators/core.yml").write_text(json.dumps({ + "emulator": "core", "type": "libretro", "systems": ["sys"], "files": [ + # A release asset: held by the builder whether or not this + # run has a copy to describe. + {"name": "jp.bin", "system": "sys", "region": ["japan"], + "storage": "large_file", "required": True}, + {"name": "us.bin", "system": "sys", "region": ["north-america"], + "required": True}, + ]})) + 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_the_installer_drops_the_fallback_the_pack_drops(self): + full = self._manifest(None) + omitted = {o["dest"]: o for o in full["omitted_files"]} + self.assertTrue(omitted["jp.bin"].get("held")) + built = self._manifest(["japan"]) + self.assertNotIn("us.bin", {f["dest"] for f in built["files"]}) + files, still = install.narrow( + full["files"], full["omitted_files"], [], [], ["japan"] + ) + self.assertEqual({f["dest"] for f in files}, {f["dest"] for f in built["files"]}) + self.assertEqual( + {o["dest"] for o in still}, {o["dest"] for o in built["omitted_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