From 8e41ddc2975599cc1a5aa4ebc0a3eb0927e4b5f6 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Mon, 5 Oct 2026 22:05:10 +0200 Subject: [PATCH] fix: ship the archive every member declaration accepts --- scripts/generate_pack.py | 26 ++++++-- tests/test_archive_choice.py | 113 +++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 4 deletions(-) create mode 100644 tests/test_archive_choice.py diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 5d709258..d83bdbe1 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -480,16 +480,34 @@ def _preferred_entries( ] if not constrained: continue - best = None - for fe in constrained: - _lp, _st = resolve_file( + resolved = [ + (fe, *resolve_file( fe, db, bios_dir, zip_contents, data_dir_registry=data_registry, offline=offline, - ) + )) + for fe in constrained + ] + members = [fe for fe in entries if fe.get("zipped_file")] + if members: + # One archive declared once per ROM it must hold: the archive + # to ship is the one the most declarations accept, not the first + # that answers one of them (adam_fdc.zip: a one-member MAME set + # answered first, the eight-member set sat beside it). + def accepted(path: str | None) -> int: + if not path or not path.endswith(".zip"): + return -1 + return sum(_inner_rom_check(fe, path) == "ok" for fe in members) + + fe, path, _st = max(resolved, key=lambda item: accepted(item[1])) + if accepted(path) > 0: + preferred_entries[full] = id(fe) + continue + best = None + for fe, _lp, _st in resolved: if _lp and _st == "md5_exact": best = fe break diff --git a/tests/test_archive_choice.py b/tests/test_archive_choice.py new file mode 100644 index 00000000..718ba7ad --- /dev/null +++ b/tests/test_archive_choice.py @@ -0,0 +1,113 @@ +"""An archive declared once per member ships as the copy holding them all. + +Batocera checks adam_fdc.zip through eight zipped_file declarations. The +first hash-exact answer was a one-member MAME set, so the pack shipped it and +seven of the eight members Batocera checks were missing. +""" + +from __future__ import annotations + +import hashlib +import os +import sys +import tempfile +import unittest +import zipfile +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(REPO_ROOT / "scripts")) + + +class PreferredArchive(unittest.TestCase): + def test_the_archive_most_declarations_accept_wins(self): + import generate_pack + + roms = {f"r{i}.bin": bytes([i]) * 32 for i in range(3)} + with tempfile.TemporaryDirectory() as tmp: + previous = os.getcwd() + os.chdir(tmp) + self.addCleanup(os.chdir, previous) + Path("bios/a").mkdir(parents=True) + Path("bios/b").mkdir(parents=True) + with zipfile.ZipFile("bios/a/set.zip", "w") as zf: + zf.writestr("r0.bin", roms["r0.bin"]) + with zipfile.ZipFile("bios/b/set.zip", "w") as zf: + for name, data in roms.items(): + zf.writestr(name, data) + files, by_md5, by_name = {}, {}, {} + for path in ("bios/a/set.zip", "bios/b/set.zip"): + data = Path(path).read_bytes() + sha1, md5 = hashlib.sha1(data).hexdigest(), hashlib.md5(data).hexdigest() + files[sha1] = {"path": path, "name": "set.zip", "sha1": sha1, "md5": md5, + "size": len(data)} + by_md5[md5] = sha1 + by_name.setdefault("set.zip", []).append(sha1) + db = {"files": files, "indexes": {"by_name": by_name, "by_md5": by_md5, + "by_path_suffix": {}, "by_crc32": {}}} + member_md5 = {n: hashlib.md5(d).hexdigest() for n, d in roms.items()} + one_member = files[by_name["set.zip"][0]]["md5"] + declarations = [{"name": "set.zip", "destination": "set.zip", "md5": one_member}] + declarations += [ + {"name": "set.zip", "destination": "set.zip", "zipped_file": name, "md5": md5} + for name, md5 in member_md5.items() + ] + systems = {"s": {"files": declarations}} + preferred = generate_pack._preferred_entries( + systems, db, "bios", "", False, {}, None, True) + chosen = next(fe for fe in declarations if id(fe) == preferred["set.zip"]) + path, _status = generate_pack.resolve_file(chosen, db, "bios", {}, offline=True) + self.assertEqual(path, "bios/b/set.zip") + + +class IntegrityChecksEveryMember(unittest.TestCase): + """A pack holding one ROM of an archive declared per ROM fails its check. + + The check accepted the destination as soon as one declaration passed, + so a Batocera pack missing seven of adam_fdc.zip's eight ROMs verified. + """ + + def test_a_missing_member_is_an_error_when_the_repository_holds_it(self): + import yaml + + import packverify + + roms = {f"r{i}.bin": bytes([i]) * 32 for i in range(3)} + with tempfile.TemporaryDirectory() as tmp: + previous = os.getcwd() + os.chdir(tmp) + self.addCleanup(os.chdir, previous) + Path("bios/b").mkdir(parents=True) + with zipfile.ZipFile("bios/b/set.zip", "w") as zf: + for name, data in roms.items(): + zf.writestr(name, data) + data = Path("bios/b/set.zip").read_bytes() + sha1 = hashlib.sha1(data).hexdigest() + db = {"files": {sha1: {"path": "bios/b/set.zip", "name": "set.zip", "sha1": sha1, + "md5": hashlib.md5(data).hexdigest(), "size": len(data)}}, + "indexes": {"by_name": {"set.zip": [sha1]}, "by_md5": {}, + "by_path_suffix": {}, "by_crc32": {}}} + Path("platforms").mkdir() + config = {"platform": "P", "verification_mode": "md5", "base_destination": "bios", + "cores": [], "systems": {"s": {"files": [ + {"name": "set.zip", "destination": "set.zip", "zipped_file": n, + "md5": hashlib.md5(d).hexdigest()} for n, d in roms.items()]}}} + Path("platforms/p.yml").write_text(yaml.safe_dump(config), encoding="utf-8") + Path("emulators").mkdir() + import io + + inner = io.BytesIO() + with zipfile.ZipFile(inner, "w") as zf: + zf.writestr("r0.bin", roms["r0.bin"]) + with zipfile.ZipFile("pack.zip", "w") as zf: + zf.writestr("bios/set.zip", inner.getvalue()) + ok, *_rest, = packverify.verify_pack_against_platform( + "pack.zip", "p", "platforms", db, emulators_dir="emulators", emu_profiles={}) + errors = _rest[2] + self.assertFalse(ok) + self.assertEqual(sorted(e.split(": ")[1] for e in errors), + ["r1.bin not found inside ZIP", "r2.bin not found inside ZIP"]) + + +if __name__ == "__main__": + unittest.main()