fix: rank same-named archives by what they hold

The name step only ranked archive copies by members declared
through archive sub-entries, and the contents an archive entry lists
never reached the resolver, so the first copy indexed won. contents
now ranks copies by crc32, or by member name where the code opens
by name, and the report and both extra passes carry it. With no
layout declared, a primary holding every ROM of the others wins;
two revisions keep the index order.
This commit is contained in:
Abdessamad Derraz committed 2026-10-18 02:30:53 +02:00
1 parent da59b20aa6
commit 7d1b8435bc
5 files changed
+161 -7

No files matched your search

+52 -7
View File
@@ -870,7 +870,29 @@ def archive_entry(archive: str, files: list[dict]) -> dict:
def holds_required_members(path: str, members: list[dict]) -> bool:
"""Whether an archive holds every member a profile requires inside it."""
crcs = zip_member_crcs(path)
return all(m["crc32"] in crcs for m in members if m["required"])
names = zip_member_names(path)
return all(_member_held(m, crcs, names) for m in members if m["required"])
def contents_members(file_entry: dict) -> list[dict]:
"""The members an archive entry's ``contents:`` lists, as ranking keys.
A member is its crc32 where the profile declares one, its name where the
code opens it by name alone: Flycast reads epr-21329.ic27 in
hod2bios.zip by filename, and a copy without it has no Japan BIOS.
"""
return [
{"crc32": str(m["crc32"]).lower(), "required": True}
if m.get("crc32") else {"name": str(m["name"]), "required": True}
for m in file_entry.get("contents") or []
if m.get("crc32") or m.get("name")
]
def _member_held(member: dict, crcs: frozenset[str], names: frozenset[str]) -> bool:
if member.get("crc32"):
return member["crc32"] in crcs
return member["name"].rsplit("/", 1)[-1].casefold() in names
def _holding_members(paths: list[str], members: list[dict]) -> list[str]:
@@ -881,16 +903,35 @@ def _holding_members(paths: list[str], members: list[dict]) -> list[str]:
"""
def held(path: str) -> tuple[int, int]:
crcs = zip_member_crcs(path)
return (
sum(1 for m in members if m["required"] and m["crc32"] in crcs),
sum(1 for m in members if m["crc32"] in crcs),
)
names = zip_member_names(path)
found = [m for m in members if _member_held(m, crcs, names)]
return sum(1 for m in found if m["required"]), len(found)
scores = {path: held(path) for path in paths}
best = max(scores.values(), default=(0, 0))
return [path for path in paths if scores[path] == best]
def _covering_archives(paths: list[str]) -> list[str]:
"""Same-named archives, those holding every ROM each other copy holds.
Without a declared layout, a primary that is a superset of the other
primaries misses nothing any of them holds. Two copies that each hold a
ROM the other lacks are different revisions, and nothing here can
choose: the order is kept. Index order alone used to decide, and
deleting an unrelated duplicate flipped which copy a core received.
"""
# A variant is served only when no primary is, so the primaries are
# the copies this choice is between.
pool = [path for path in paths if "/.variants/" not in path] or paths
crcs = {path: zip_member_crcs(path) for path in pool}
covering = [
path for path in pool
if crcs[path] and all(crcs[other] <= crcs[path] for other in pool)
]
return covering + [path for path in paths if path not in covering]
def _by_affinity(paths: list[str], file_entry: dict, dest_hint: str) -> list[str]:
"""Same-named candidates, the likeliest first.
@@ -1231,8 +1272,11 @@ def resolve_local_file(
if candidates:
if zipped_file:
candidates = [p for p in candidates if ".zip" in os.path.basename(p)]
if file_entry.get("members"):
candidates = _holding_members(candidates, file_entry["members"])
members = file_entry.get("members") or contents_members(file_entry)
if members:
candidates = _holding_members(candidates, members)
elif len(candidates) > 1 and name.casefold().endswith(".zip"):
candidates = _covering_archives(candidates)
candidates = _by_affinity(candidates, file_entry, dest_hint)
# A copy outside another emulator's tree comes first: the
# affinity read pcsx2 in bios/Other/armsx2/pcsx2/ and served
@@ -2361,6 +2405,7 @@ from ziptools import ( # noqa: E402,F401
_zip_contents_cache,
_BOUNDED_RATIO_METHODS,
zip_member_crcs,
zip_member_names,
)
from artifacts import ( # noqa: E402,F401
write_if_changed,
+6
View File
@@ -776,6 +776,8 @@ def _collect_emulator_extras(
for identity in PROFILE_IDENTITY_FIELDS:
if f.get(identity) not in (None, "", []):
extra[identity] = f[identity]
if f.get("contents"):
extra["contents"] = f["contents"]
extra["claim_order"] = (2, 0, emu_name)
if _take(extra, full_dest, seen_dests, taken):
extras.append(extra)
@@ -823,6 +825,10 @@ def _report_extra(u: dict, name: str, dest: str) -> dict:
}
if u.get("members"):
extra["members"] = u["members"]
# The archive layout the core opens ranks same-named copies by what
# they hold; without it the first copy indexed won.
if u.get("contents"):
extra["contents"] = u["contents"]
# Keep every reproducible identity constraint carried by the source
# profile. Some profiles expose CRC32/SHA-256 only through their
# validation block; dropping those fields here would let a same-named
+2
View File
@@ -638,6 +638,8 @@ def find_undeclared_files(
"sha1": f.get("sha1"),
"md5": f.get("md5"),
}
if f.get("contents"):
entry["contents"] = f["contents"]
if duplicate:
first.setdefault("other_claims", []).append(entry)
continue
+21
View File
@@ -75,6 +75,27 @@ def _member_crcs(path: str, mtime: float) -> frozenset[str]:
return frozenset()
@functools.lru_cache(maxsize=4096)
def _member_names(path: str, mtime: float) -> frozenset[str]:
try:
with zipfile.ZipFile(path) as archive:
return frozenset(
info.filename.rsplit("/", 1)[-1].casefold()
for info in archive.infolist() if not info.is_dir()
)
except (zipfile.BadZipFile, OSError):
return frozenset()
def zip_member_names(path: str) -> frozenset[str]:
"""Casefolded base name of each member, read from the central directory."""
try:
mtime = os.path.getmtime(path)
except OSError:
return frozenset()
return _member_names(path, mtime)
def zip_member_crcs(path: str) -> frozenset[str]:
"""CRC-32 of each member, read from the central directory."""
try:
+80
View File
@@ -269,5 +269,85 @@ class AnArchiveHashedByItsMembers(unittest.TestCase):
self.assertEqual(marks, {(True, "composite"), (False, None)})
class AnArchiveRankedByWhatItHolds(unittest.TestCase):
"""Two primaries answered hod2bios.zip and the first one indexed won:
Flycast received a set without epr-21329.ic27, its Japan BIOS, once an
unrelated duplicate was deleted and the index order flipped."""
def setUp(self):
directory = tempfile.mkdtemp(dir=REPO_ROOT / "tmp")
self.addCleanup(shutil.rmtree, directory, True)
self.root = Path(directory)
self.roms = {f"epr-213{n}.ic27": bytes([n]) * 16 for n in (29, 30, 31, 32)}
def _db(self, copies: list[tuple[str, list[str]]]) -> dict:
files, by_name = {}, {}
for relative, members in copies:
path = self.root / relative
path.parent.mkdir(parents=True, exist_ok=True)
with zipfile.ZipFile(path, "w") as zf:
for member in members:
zf.writestr(member, self.roms.get(member, member.encode()))
sha1 = hashlib.sha1(path.read_bytes()).hexdigest()
files[sha1] = {"path": str(path), "name": "hod2bios.zip", "sha1": sha1}
by_name.setdefault("hod2bios.zip", []).append(sha1)
return {"files": files, "indexes": {"by_name": by_name, "by_md5": {},
"by_path_suffix": {}, "by_crc32": {}}}
def _resolve(self, entry: dict, db: dict) -> str:
from common import resolve_local_file
path, _status = resolve_local_file(entry, db)
return str(Path(path).relative_to(self.root))
def test_member_names_the_code_opens_choose_the_copy(self):
db = self._db([("bios/Arcade/hod2bios.zip", ["epr-21330.ic27", "epr-21331.ic27"]),
("bios/MAME/hod2bios.zip", list(self.roms))])
entry = {"name": "hod2bios.zip", "contents": [
{"name": name, "size": 16} for name in ("epr-21331.ic27", "epr-21330.ic27",
"epr-21329.ic27")]}
self.assertEqual(self._resolve(entry, db), "bios/MAME/hod2bios.zip")
def test_member_crcs_choose_the_copy(self):
db = self._db([("bios/Arcade/hod2bios.zip", ["epr-21330.ic27", "renamed.ic27"]),
("bios/MAME/hod2bios.zip", ["epr-21330.ic27", "epr-21332.ic27"])])
entry = {"name": "hod2bios.zip", "contents": [
{"name": "other-name.ic27", "crc32": format(zlib.crc32(self.roms[name]), "08x")}
for name in ("epr-21330.ic27", "epr-21332.ic27")]}
self.assertEqual(self._resolve(entry, db), "bios/MAME/hod2bios.zip")
def test_without_a_layout_a_superset_primary_wins(self):
db = self._db([("bios/Arcade/hod2bios.zip", ["epr-21330.ic27"]),
("bios/MAME/hod2bios.zip", list(self.roms))])
self.assertEqual(self._resolve({"name": "hod2bios.zip"}, db),
"bios/MAME/hod2bios.zip")
def test_two_revisions_keep_the_index_order(self):
db = self._db([("bios/Arcade/hod2bios.zip", ["epr-21330.ic27", "epr-21331.ic27"]),
("bios/MAME/hod2bios.zip", ["epr-21330.ic27", "epr-21332.ic27"])])
self.assertEqual(self._resolve({"name": "hod2bios.zip"}, db),
"bios/Arcade/hod2bios.zip")
def test_a_variant_does_not_stop_a_primary_from_covering(self):
db = self._db([("bios/Arcade/hod2bios.zip", ["epr-21330.ic27"]),
("bios/MAME/hod2bios.zip", list(self.roms)),
("bios/Arcade/.variants/hod2bios.zip", ["proto.ic27"])])
self.assertEqual(self._resolve({"name": "hod2bios.zip"}, db),
"bios/MAME/hod2bios.zip")
def test_every_claim_path_carries_the_layout(self):
from packextras import _report_extra
contents = [{"name": "epr-21329.ic27", "size": 16}]
extra = _report_extra({"name": "hod2bios.zip", "contents": contents},
"hod2bios.zip", "dc/hod2bios.zip")
self.assertEqual(extra["contents"], contents)
# The second pass builds its extra from the profile entry itself.
source = (REPO_ROOT / "scripts" / "packextras.py").read_text()
self.assertEqual(source.count('extra["contents"] = '), 2)
report = (REPO_ROOT / "scripts" / "verify.py").read_text()
self.assertIn('entry["contents"] = f["contents"]', report)
if __name__ == "__main__":
unittest.main()