fix: drop manifest entries with no download source

install.py fetches a file from its repo_path or from a release asset.
Resolution can land on a file the database does not index, and the
entry then shipped with neither: a line in the download list that can
only ever fail. Those are recorded as omitted instead, which is what
the installer already knows how to report, and a test holds the
committed manifests to it.

validate_schemas read dist/ while a build was writing it and reported
a half-written pack as 'File is not a zip file'. It takes the shared
lock --verify-packs uses, and says so when a build holds it.
This commit is contained in:
Abdessamad Derraz committed 2026-08-11 14:40:33 +02:00
1 parent 85f3f7c393
commit 94512b5acf
3 files changed
+73 -9

No files matched your search

+17 -9
View File
@@ -1393,18 +1393,14 @@ def generate_pack(
slot_undecidable: list[str] = []
if one_per_slot:
idx = region_mod.build_region_index(emu_profiles or {})
members_by_system = _pack_member_groups(
config, pack_systems, emulators_dir, db, base_dest,
emu_profiles, target_cores, source,
)
slot_groups: dict[str, list[tuple[str, str]]] = {}
for sys_id, members in members_by_system.items():
for dest, name in members:
if dest in region_drops:
continue
tier = ",".join(sorted(region_mod.lookup_regions(idx, dest, name)))
slot_groups.setdefault(f"{sys_id}|{tier}", []).append((dest, name))
slot_groups = {
sys_id: [(d, n) for d, n in members if d not in region_drops]
for sys_id, members in members_by_system.items()
}
slot_drops, slot_undecidable = slot_mod.resolve_slot_drops(
slot_groups, slot_mod.build_slot_index(emu_profiles or {})
)
@@ -3613,6 +3609,18 @@ def generate_manifest(
sha256 = hashes["sha256"]
repo_path = _get_repo_path(sha1, db) if sha1 else ""
is_release_asset = _is_large_file(local_path or "", repo_root)
# An entry needs somewhere to be fetched from. Resolution can
# land on a file the database does not index -- a data
# directory, or a copy added since the last scan -- and then
# neither a repo path nor a release asset exists, so the
# installer would list a file it can never download. Recording
# it as omitted keeps that visible instead of shipping a dead
# entry.
if not repo_path and not is_release_asset:
record_omission(full_dest, file_entry, sys_id, "not_found", None)
continue
entry: dict = {
"dest": dest,
@@ -3623,7 +3631,7 @@ def generate_manifest(
"cores": None,
}
if _is_large_file(local_path or "", repo_root):
if is_release_asset:
entry["storage"] = "release"
entry["release_asset"] = (
os.path.basename(local_path) if local_path else file_entry["name"]
+21
View File
@@ -77,9 +77,30 @@ def _validate_pack_manifests(dist: Path) -> list[str]:
generate_pack.py writes manifest.json inside the archive, not beside it,
so a filesystem glob over dist/ matches nothing and silently validates
zero documents.
Reading a pack while a build is writing it reports "File is not a zip
file" about an archive that is merely half-written, so this takes the same
shared lock --verify-packs does. A build holding the exclusive lock means
the packs on disk are mid-flight and there is nothing stable to validate.
"""
if not dist.is_dir():
return []
sys.path.insert(0, str(ROOT / "scripts"))
from common import ArtifactLockBusy, artifact_lock
try:
with artifact_lock(str(dist), exclusive=False):
return _scan_pack_manifests(dist)
except ArtifactLockBusy:
print(
f"note: {dist.name} is being written; skipping pack manifests",
file=sys.stderr,
)
return []
def _scan_pack_manifests(dist: Path) -> list[str]:
validator = _validator("pack-manifest.schema.json")
def _label(path: Path) -> str:
+35
View File
@@ -779,6 +779,41 @@ class CheckoutCompletenessRegressions(unittest.TestCase):
os.chdir(cwd)
class EveryManifestEntryIsFetchable(unittest.TestCase):
"""An install manifest may not list a file the installer cannot get.
install.py fetches a file either from its repo_path or from a release
asset. An entry carrying neither is a line in the download list that can
only ever fail. It happened when resolution landed on a file the database
does not index, so the hash was computed but the repo lookup came back
empty and the entry shipped anyway.
"""
def test_committed_manifests_all_have_a_source(self):
manifests = sorted((ROOT / "install").glob("*.json"))
self.assertTrue(manifests, "no install manifests to check")
for path in manifests:
with self.subTest(manifest=path.name):
data = json.loads(path.read_text())
orphans = [
entry["dest"]
for entry in data.get("files", [])
if not entry.get("repo_path") and not entry.get("release_asset")
]
self.assertEqual(
orphans, [], f"{path.name} lists unfetchable files: {orphans[:3]}"
)
def test_an_unresolvable_file_is_recorded_as_omitted(self):
"""The reason must be one install.py knows how to report."""
allowed = {"hash_mismatch", "not_found", "external", "user_provided"}
for path in sorted((ROOT / "install").glob("*.json")):
with self.subTest(manifest=path.name):
data = json.loads(path.read_text())
for entry in data.get("omitted_files", []):
self.assertIn(entry.get("reason"), allowed)
class FreshnessGuardMechanics(unittest.TestCase):
"""write_if_changed is what makes `git diff --exit-code` a real check.