From 79c94468a02178e8ec43d53039ac6de24cb67cd0 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Sun, 4 Oct 2026 21:08:00 +0200 Subject: [PATCH] fix: list in the manifest what the pack ships --- scripts/generate_pack.py | 58 ++++++++++++++++++++++---------- tests/test_pack_counts.py | 71 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 18 deletions(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 39393d82..b6cc6175 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -293,6 +293,24 @@ def download_external(file_entry: dict, dest_path: str) -> bool: PACK_DOCUMENTS = ("README.txt", "manifest.json") +def _inner_rom_check(file_entry: dict, local_path: str) -> str: + """How an archive answers an entry that pins a ROM inside it. + + Batocera, RetroBat and ROCKNIX hash a member of the ZIP, never the ZIP: + the resolver hands back the archive as a mismatch and this decides it. + Returns check_inside_zip's answer for the first accepted MD5 that + matches, else for the last one tried. The pack and the install manifest + both read it, so a file one of them ships is a file the other lists. + """ + declared = [m.strip() for m in file_entry.get("md5", "").split(",") if m.strip()] + result = "not_in_zip" + for candidate in declared or [""]: + result = check_inside_zip(local_path, file_entry["zipped_file"], candidate) + if result == "ok": + break + return result + + def _data_directory_members( pack_systems: dict, data_registry: dict | None, @@ -891,21 +909,8 @@ def generate_pack( ): zf_name = file_entry.get("zipped_file") if zf_name and local_path: - inner_md5_raw = file_entry.get("md5", "") - inner_md5_list = ( - [m.strip() for m in inner_md5_raw.split(",") if m.strip()] - if inner_md5_raw - else [""] - ) - zip_ok = False - last_result = "not_in_zip" - for md5_candidate in inner_md5_list: - last_result = check_inside_zip( - local_path, zf_name, md5_candidate - ) - if last_result == "ok": - zip_ok = True - break + last_result = _inner_rom_check(file_entry, local_path) + zip_ok = last_result == "ok" if zip_ok: status = "zip_exact" file_status.setdefault(dedup_key, "ok") @@ -2952,6 +2957,11 @@ def generate_manifest( manifest_files: list[dict] = [] omitted_by_destination: dict[str, dict] = {} total_size = 0 + # Sizes of the files the pack carries and the installer cannot fetch: the + # collection does not index them, a data directory cache answered. + pack_only_sizes: list[int] = [] + if data_registry is None: + data_registry = load_data_dir_registry(platforms_dir) def manifest_destination(full_destination: str) -> str: if base_dest and full_destination.startswith(f"{base_dest}/"): @@ -3044,8 +3054,16 @@ def generate_manifest( db, bios_dir, zip_contents, + data_dir_registry=data_registry, offline=offline, ) + if ( + status == "hash_mismatch" + and local_path + and file_entry.get("zipped_file") + and _inner_rom_check(file_entry, local_path) == "ok" + ): + status = "zip_exact" # An existence platform never reads the bytes, so a declared # hash the local dump contradicts is not a reason to withhold # the file. Hash platforms would reject it, so they omit it. @@ -3080,6 +3098,12 @@ def generate_manifest( # entry. if not repo_path and not is_release_asset: record_omission(full_dest, file_entry, sys_id, "not_found", None) + if file_size and full_dest not in seen_destinations: + pack_only_sizes.append(file_size) + seen_destinations.add(dedup_key) + _register_path(dedup_key, seen_destinations, seen_parents) + if case_insensitive: + seen_lower.add(dedup_key.lower()) continue entry: dict = { @@ -3129,9 +3153,7 @@ def generate_manifest( # Phase 3: data directories. The installer does not fetch them, so they # stay out of the file list; the pack carries them, so they count toward # what an extraction shows. - if data_registry is None: - data_registry = load_data_dir_registry(platforms_dir) - data_sizes = [ + data_sizes = pack_only_sizes + [ os.path.getsize(src) for src, _dest in _data_directory_members( pack_systems, data_registry, platform_name, base_dest, diff --git a/tests/test_pack_counts.py b/tests/test_pack_counts.py index 096fc0ee..66a3727f 100644 --- a/tests/test_pack_counts.py +++ b/tests/test_pack_counts.py @@ -158,6 +158,77 @@ class ManifestStatesWhatThePackHolds(PackCountFixture): self.assertEqual(errors, []) +class ManifestFollowsTheBuilder(PackCountFixture): + """Two places where the pack carried a file the manifest left out. + + The release record compares each archive with the count its manifest + expects, and the first comparison named both: four `pc6001*.zip` in the + Batocera pack, two MSX2+ ROMs in the Recalbox pack. + """ + + def _platform(self, files: list[dict], mode: str = "md5") -> None: + config = dict(PLATFORM, verification_mode=mode, cores=[]) + config["systems"] = {"demo-system": {"files": files}} + (self.platforms / "demo.yml").write_text(yaml.dump(config)) + + def _pack_names(self) -> set[str]: + out = self.root / "dist" + out.mkdir(exist_ok=True) + zip_path = builder.generate_pack( + "demo", str(self.platforms), self.db, str(self.bios), str(out), + include_extras=True, emulators_dir=str(self.emulators), + emu_profiles=self.profiles, data_registry=self.registry, + offline=True, + ) + with zipfile.ZipFile(zip_path) as archive: + return set(archive.namelist()) | {"manifest.json"} + + def test_an_archive_checked_by_the_rom_inside_it_is_listed(self): + """Batocera pins the MD5 of a ROM inside the ZIP, not of the ZIP. The + builder looked inside, the manifest did not, and an installation by + script lost four required archives the pack shipped.""" + rom = b"rom inside the set" + archive = self.bios / "SystemA" / "set.zip" + with zipfile.ZipFile(archive, "w") as handle: + handle.writestr("basic.rom", rom) + # A second ROM, so the archive is not identified by the first. + handle.writestr("chargen.rom", b"another rom of the set") + raw = archive.read_bytes() + sha1 = hashlib.sha1(raw).hexdigest() + self.db["files"][sha1] = { + "path": str(archive), "name": "set.zip", "size": len(raw), + "sha1": sha1, "md5": hashlib.md5(raw).hexdigest(), + "sha256": hashlib.sha256(raw).hexdigest(), "crc32": "00000000", + } + self.db["indexes"] = generate_db.build_indexes(self.db["files"], {}) + self._platform([{ + "name": "set.zip", "destination": "set.zip", + "md5": hashlib.md5(rom).hexdigest(), "zipped_file": "basic.rom", + }]) + names = self._pack_names() + self.assertIn("set.zip", names) + manifest = self._manifest() + self.assertIn("set.zip", {entry["dest"] for entry in manifest["files"]}) + self.assertEqual(manifest["pack_files"], len(names)) + + def test_a_file_only_a_data_directory_holds_still_counts(self): + """The installer cannot fetch it, so it stays out of the list. The + pack carries it, so it is part of what an extraction shows.""" + (self.data / "Machines").mkdir() + (self.data / "Machines" / "Msx2pe.rom").write_bytes(b"machine rom") + self._platform([{ + "name": "Msx2pe.rom", "destination": "Machines/Msx2pe.rom", + "md5": hashlib.md5(b"machine rom").hexdigest(), + }]) + names = self._pack_names() + self.assertIn("Machines/Msx2pe.rom", names) + manifest = self._manifest() + self.assertNotIn( + "Machines/Msx2pe.rom", {entry["dest"] for entry in manifest["files"]} + ) + self.assertEqual(manifest["pack_files"], len(names)) + + RECORD = { "tag": "v2026.09.04", "packs": {