fix: replay held omissions in install regions

The builder keeps a held regional fallback when the requested file is
missing (since 2026-10-17), and the comment beside the manifest's
omissions still said the opposite. A file the builder holds but the
manifest cannot download sat in omitted_files counted unheld, so the
installer kept a fallback the pack drops. Such an omission now carries
held: true and narrow() counts it held, as the builder does.
This commit is contained in:
Abdessamad Derraz committed 2026-10-21 11:18:53 +02:00
1 parent 7ca6adca56
commit ef69687fe1
6 files changed
+139 -17

No files matched your search

+1 -1
View File
@@ -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 }
+6 -2
View File
@@ -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]
+1 -1
View File
@@ -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() {
+4
View File
@@ -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": [
{
+17 -13
View File
@@ -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,
)
+110
View File
@@ -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