fix: agree on missing files in system packs

The emulator pack recorded only loose files as a path's occupant, so a
path an archive or data tree had filled was reported missing even with
the same bytes in the ZIP, and an archive several profiles ask for was
counted once per profile. verify registered neither, and credited one
profile's file at a path holding another's. Both now record archives
and tree members in pack order; --system snk-neogeo-mvs went from 113
missing in the pack and 51 in verify to 59 on both sides.
This commit is contained in:
Abdessamad Derraz committed 2026-10-17 02:20:25 +02:00
1 parent cb41a52b17
commit e5b8cede92
3 files changed
+133 -10

No files matched your search

+21 -5
View File
@@ -1176,11 +1176,14 @@ def _pack_data_tree(
seen_destinations: set[str],
seen_parents: set[str],
seen_lower: set[str],
packed_at: dict[str, str] | None = None,
) -> int:
"""Write one data directory into an emulator pack, under its shared lock.
A refresh swaps the tree by rename; without the lock a walk half done
read the old tree and wrote files of the new one.
read the old tree and wrote files of the new one. ``packed_at`` records
each member, so a later profile asking for the same bytes there is
served rather than reported missing.
"""
added = 0
with cache_lock(local_cache, shared=True):
@@ -1197,6 +1200,8 @@ def _pack_data_tree(
_register_path(full, seen_destinations, seen_parents)
seen_lower.add(full.lower())
_add_pack_member(zf, src, full)
if packed_at is not None:
packed_at[full.lower()] = src
added += 1
return added
@@ -1294,6 +1299,15 @@ def generate_emulator_pack(
# Loose file packed at each folded destination, to tell a profile asking
# for the same file there from one asking for other bytes.
packed_at: dict[str, str] = {}
# A destination is missing once, however many profiles ask for it:
# verify --emulator counts by destination.
missing_at: set[str] = set()
def report_missing(name: str, destination: str) -> None:
if destination not in missing_at:
missing_at.add(destination)
missing_files.append(name)
data_dir_notices: list[str] = []
data_registry = load_data_dir_registry(
platforms_dir or os.path.join(os.path.dirname(__file__), "..", "platforms")
@@ -1348,9 +1362,10 @@ def generate_emulator_pack(
seen_destinations.add(archive_dest)
_register_path(archive_dest, seen_destinations, seen_parents)
seen_lower.add(archive_dest.lower())
packed_at[archive_dest.lower()] = local_path
total_files += 1
else:
missing_files.append(archive_name)
report_missing(archive_name, archive_dest)
# Pack individual files (skip archived ones). The entries of one
# profile on one destination are the releases the code accepts
@@ -1456,7 +1471,7 @@ def generate_emulator_pack(
):
# The file already at this path, asked for twice.
continue
missing_files.append(alternatives[0]["name"])
report_missing(alternatives[0]["name"], dest)
continue
if chosen is None:
@@ -1465,7 +1480,7 @@ def generate_emulator_pack(
elif all(fe.get("unsourceable") for fe in alternatives):
unsourceable_files.append(alternatives[0]["name"])
else:
missing_files.append(alternatives[0]["name"])
report_missing(alternatives[0]["name"], dest)
continue
fe, local_path, how = chosen
@@ -1535,7 +1550,8 @@ def generate_emulator_pack(
if prefix:
dd_dest = f"{prefix}/{dd_dest}" if dd_dest else prefix
total_files += _pack_data_tree(
zf, local_cache, dd_dest, seen_destinations, seen_parents, seen_lower
zf, local_cache, dd_dest, seen_destinations, seen_parents, seen_lower,
packed_at,
)
# Remove empty ZIP (no files packed and no missing = nothing to ship)
+47 -5
View File
@@ -1466,9 +1466,28 @@ def verify_emulator(
sha1_by_path = {
entry.get("path"): sha1 for sha1, entry in db.get("files", {}).items()
}
# Destination -> (file packed there, its profile, the path as written).
# Archives and data trees take paths too, before and after the loose
# files of their profile, as the pack writes them.
placed: dict[str, tuple[str | None, str, str]] = {}
placed_files: set[str] = set()
placed_parents: set[str] = set()
content_ids: dict[str, str | None] = {}
def content_id(path: str | None) -> str | None:
if not path:
return None
if path not in content_ids:
content_ids[path] = sha1_by_path.get(path) or (
compute_hashes(path)["sha1"] if os.path.isfile(path) else None
)
return content_ids[path]
def occupy(dest: str, path: str | None, owner: str) -> None:
if placement_block(dest, placed_files, set(placed), placed_parents, True):
return
placed[dest.lower()] = (path, owner, dest)
_register_path(dest, placed_files, placed_parents)
for emu_name, profile in sorted(selected):
files = expand_directory_entries(
filter_files_by_mode(profile.get("files", []), standalone),
@@ -1559,6 +1578,8 @@ def verify_emulator(
archive, profile.get("archive_prefix", ""),
profile.get("pack_structure"), standalone,
)
if local_path:
occupy(dest, local_path, emu_name)
dest_to_name[dest] = archive
cur = result["status"]
prev = file_status.get(dest)
@@ -1690,20 +1711,22 @@ def verify_emulator(
)
block_reason = ""
if block and held is not None:
other_sha1, owner, other_dest = placed.get(dest.lower(), (None, "", ""))
other_path, owner, other_dest = placed.get(dest.lower(), (None, "", ""))
same = content_id(held) is not None and content_id(held) == content_id(
other_path
)
if block == "path_conflict":
block_reason = "a packed file occupies its directory"
elif block == "case_twin":
block_reason = (
f"packed only as {other_dest}, from {owner}"
if sha1_by_path.get(held) == other_sha1
if same
else f"the pack holds {owner}'s {other_dest} at this path"
)
elif sha1_by_path.get(held) != other_sha1:
elif not same:
block_reason = f"the pack holds {owner}'s {other_dest} at this path"
elif not block and packed is not None:
placed[dest.lower()] = (sha1_by_path.get(packed), emu_name, dest)
_register_path(dest, placed_files, placed_parents)
occupy(dest, packed, emu_name)
for result, required, hle in alternatives:
if block_reason and result["status"] != Status.MISSING:
result["status"] = Status.MISSING
@@ -1726,6 +1749,25 @@ def verify_emulator(
) > _SEVERITY_ORDER.get(prev_sev, 0):
file_severity[dest] = sev
# The profile's data trees follow its files in the pack.
for dd in filter_files_by_mode(profile.get("data_directories", []), standalone):
cache = (data_registry or {}).get(dd.get("ref", ""), {}).get("local_cache", "")
if not cache or not os.path.isdir(cache):
continue
dd_dest = dd.get("destination", "")
structure = profile.get("pack_structure")
prefix = (
structure.get("standalone" if standalone else "libretro", "")
if structure else ""
)
if prefix:
dd_dest = f"{prefix}/{dd_dest}" if dd_dest else prefix
for root, _dirs, names in os.walk(cache):
for fname in names:
src = os.path.join(root, fname)
rel = os.path.relpath(src, cache).replace(os.sep, "/")
occupy(f"{dd_dest}/{rel}" if dd_dest else rel, src, emu_name)
counts = {
Severity.OK: 0,
Severity.INFO: 0,
+65
View File
@@ -0,0 +1,65 @@
"""A system pack and verify --system name the same missing files.
The pack registered only loose files as occupants: a destination an earlier
profile's archive or data tree had filled was reported missing even when
the bytes were the same and in the ZIP, and an archive several profiles ask
for was counted once per profile. verify, in turn, never registered archives
or trees and credited a profile's file at a path holding another profile's.
`--system snk-neogeo-mvs` said 113 missing where verify said 51.
"""
from __future__ import annotations
import json
import re
import shutil
import subprocess
import sys
import tempfile
import unittest
from pathlib import Path
REPO_ROOT = Path(__file__).resolve().parents[1]
def _run(*argv: str) -> str:
result = subprocess.run(
[sys.executable, *argv], cwd=REPO_ROOT, capture_output=True, text=True,
timeout=1800,
)
if result.returncode != 0:
raise AssertionError(result.stdout + result.stderr)
return result.stdout
class SystemPackMatchesVerify(unittest.TestCase):
SYSTEMS = ("snk-neogeo-mvs", "msx")
def setUp(self):
if not (REPO_ROOT / "database.json").exists():
self.skipTest("database.json is not generated")
self.out = Path(tempfile.mkdtemp(dir=REPO_ROOT / "tmp"))
self.addCleanup(shutil.rmtree, self.out, True)
def test_the_same_files_are_missing(self):
for system in self.SYSTEMS:
with self.subTest(system=system):
packed = _run(
"scripts/generate_pack.py", "--system", system, "--offline",
"--output-dir", str(self.out / system),
)
report = json.loads(_run("scripts/verify.py", "--system", system, "--json"))
pack_missing = sorted(re.findall(r"^ MISSING: (\S+)", packed, re.M))
# One per destination, as the pack reports them; an entry the
# collection cannot hold is counted apart on both sides.
verify_missing = sorted({
(d["destination"], d["name"]) for d in report["details"]
if d["status"] == "missing" and not d.get("unsourceable")
})
verify_missing = sorted(name for _dest, name in verify_missing)
self.assertTrue(pack_missing or verify_missing)
self.assertEqual(pack_missing, verify_missing)
if __name__ == "__main__":
unittest.main()