fix: compare pack paths, not only counts

Step 5 compared (ok, total) and the core file count. RetroArch, Lakka
and RetroPie read 526/526 on both sides while verify credited four paths
below SGB1.sfc and SGB2.sfc that the pack never writes: a declared file
blocks them as a directory. verify now places the platform files in the
builder's order with the same path rules, and a user_provided or
external declaration keeps its slot on both sides, so no core file takes
it in the report only.

verify --destinations writes the paths each pack holds by the report,
and step 5 compares them with the archive members (notes and data tree
members set aside). Every emulator profile, in each build it has, is
built in memory (PackRecord, nothing read or written) and compared with
verify --emulator path by path. Cost measured here: about 1 s per
platform archive and 11 s for the 930 emulator packs.

scripts/generate_pack.py

@@ -106,6 +106,20 @@ DEFAULT_BIOS_DIR = "bios"
 
 
 
+class PackRecord:
+    """A pack build kept in memory: the paths it writes, no archive.
+
+    ``members`` is every path written, ``files`` the ones the build counts as
+    the profile's files (archives and loose files, not trees or notes). The
+    pipeline builds every emulator pack this way and compares ``files`` with
+    what verify --emulator credits, without reading or compressing a byte.
+    """
+
+    def __init__(self) -> None:
+        self.members: list[str] = []
+        self.files: list[str] = []
+
+
 def _add_pack_member(zf: zipfile.ZipFile, source_path: str, arcname: str) -> None:
     """Add a file to the pack with metadata that does not depend on the build.
 
@@ -117,6 +131,9 @@ def _add_pack_member(zf: zipfile.ZipFile, source_path: str, arcname: str) -> Non
     two consecutive builds. The content is streamed rather than read whole,
     because some members are firmware images of several hundred megabytes.
     """
+    if isinstance(zf, PackRecord):
+        zf.members.append(arcname)
+        return
     info = zipfile.ZipInfo(filename=arcname, date_time=_FIXED_DATE_TIME)
     info.compress_type = zipfile.ZIP_DEFLATED
     # The executable bit is the one permission worth carrying: the RetroDECK
@@ -143,6 +160,9 @@ def _write_generated_member(zf: zipfile.ZipFile, arcname: str, text: str) -> Non
     a different hash on every run. Pinning them to the epoch the archive
     rebuilder already uses makes a pack a function of its inputs alone.
     """
+    if isinstance(zf, PackRecord):
+        zf.members.append(arcname)
+        return
     info = zipfile.ZipInfo(filename=arcname, date_time=_FIXED_DATE_TIME)
     info.compress_type = zipfile.ZIP_DEFLATED
     info.external_attr = 0o100644 << 16
@@ -895,6 +915,13 @@ def generate_pack(
                     ) as tmp:
                         tmp_path = tmp.name
 
+                    # The slot is the platform's file whether the download
+                    # arrives or not: verify cannot fetch, and a core file
+                    # filling it on a failed download made the two differ.
+                    seen_destinations.add(dedup_key)
+                    _register_path(dedup_key, seen_destinations, seen_parents)
+                    if case_insensitive:
+                        seen_lower.add(dedup_key.lower())
                     try:
                         if download_external(file_entry, tmp_path):
                             extract = file_entry.get("extract", False)
@@ -902,10 +929,6 @@ def generate_pack(
                                 _extract_zip_to_archive(tmp_path, _flat(full_dest, base_dest, flatten), zf)
                             else:
                                 _add_pack_member(zf, tmp_path, _flat(full_dest, base_dest, flatten))
-                            seen_destinations.add(dedup_key)
-                            _register_path(dedup_key, seen_destinations, seen_parents)
-                            if case_insensitive:
-                                seen_lower.add(dedup_key.lower())
                             file_status.setdefault(dedup_key, "ok")
                             total_files += 1
                         else:
@@ -1232,6 +1255,9 @@ def _add_zip_to_pack(
     declare the hash of a ROM inside the ZIP (checkInsideZip), which a
     content-preserving rebuild does not affect.
     """
+    if isinstance(target_zf, PackRecord):
+        target_zf.members.append(dest_path)
+        return
     if (
         file_entry
         and not file_entry.get("zipped_file")
@@ -1379,6 +1405,7 @@ def generate_emulator_pack(
     pack_name: str | None = None,
     system_ids: list[str] | None = None,
     final_dir: str | None = None,
+    record: PackRecord | None = None,
 ) -> str | None:
     """Generate a ZIP pack for specific emulator profiles.
 
@@ -1387,6 +1414,8 @@ def generate_emulator_pack(
     the 255 bytes a file name may hold. `system_ids` keeps only the entries
     of those systems, as a platform pack narrows core extras. `final_dir`
     is where the caller moves the archive, the path the report names.
+    `record` builds the pack in memory instead (PackRecord): nothing is
+    written or read, and the paths are kept on it.
     """
     all_profiles = load_emulator_profiles(emulators_dir, skip_aliases=False)
     if zip_contents is None:
@@ -1411,7 +1440,8 @@ def generate_emulator_pack(
         display_names, [n for n, _p in selected], f"{narrow_tags}_BIOS_Pack.zip"
     )
     zip_path = os.path.join(output_dir, zip_name)
-    os.makedirs(output_dir, exist_ok=True)
+    if record is None:
+        os.makedirs(output_dir, exist_ok=True)
 
     region_drops: set[str] = (
         emulator_region_drops(selected, standalone, regions, required_only, db)
@@ -1449,7 +1479,10 @@ def generate_emulator_pack(
         platforms_dir or os.path.join(os.path.dirname(__file__), "..", "platforms")
     )
 
-    with _writing_pack(zip_path) as zf:
+    with (
+        contextlib.nullcontext(record) if record is not None
+        else _writing_pack(zip_path)
+    ) as zf:
         for emu_name, profile in sorted(selected):
             pack_structure = profile.get("pack_structure")
             files = expand_agnostic_entries(
@@ -1514,6 +1547,8 @@ def generate_emulator_pack(
                     seen_lower.add(archive_dest.lower())
                     packed_at[archive_dest.lower()] = local_path
                     total_files += 1
+                    if record is not None:
+                        record.files.append(archive_dest)
                 else:
                     report_missing(archive_name, archive_dest)
 
@@ -1641,12 +1676,16 @@ def generate_emulator_pack(
                     ) as tmp:
                         tmp_path = tmp.name
                     try:
-                        if download_external(fe, tmp_path):
+                        # Built in memory, nothing is fetched: verify --emulator
+                        # cannot fetch either and reports the file missing.
+                        if record is None and download_external(fe, tmp_path):
                             _add_pack_member(zf, tmp_path, dest)
                             seen_destinations.add(dest)
                             _register_path(dest, seen_destinations, seen_parents)
                             seen_lower.add(dest.lower())
                             total_files += 1
+                            if record is not None:
+                                record.files.append(dest)
                         else:
                             missing_files.append(fe["name"])
                     finally:
@@ -1675,6 +1714,8 @@ def generate_emulator_pack(
                 seen_lower.add(dest.lower())
                 packed_at[dest.lower()] = local_path
                 total_files += 1
+                if record is not None:
+                    record.files.append(dest)
 
             # The trees after the files, as in a platform pack: a dump the
             # profile declares wins over the copy a tree bundles at the same
@@ -1705,7 +1746,9 @@ def generate_emulator_pack(
                 )
 
     # Remove empty ZIP (no files packed and no missing = nothing to ship)
-    if total_files + data_files == 0 and not (missing_files or rejected_files or instruction_files or unsourceable_files):
+    if record is None and total_files + data_files == 0 and not (
+        missing_files or rejected_files or instruction_files or unsourceable_files
+    ):
         os.unlink(zip_path)
 
     # Report. The members of data trees are counted apart: verify --emulator

scripts/packextras.py

@@ -209,7 +209,7 @@ def hidden_extras(
     emulators_dir: str,
     db: dict,
     profiles: dict,
-    shipped: dict[str, str],
+    shipped: dict[str, str | None],
     target_cores: set[str] | None = None,
     region_drops: set[str] | None = None,
     placed: set[str] | None = None,
@@ -229,7 +229,8 @@ def hidden_extras(
     while the report counted it in the pack. A twin carrying the same bytes
     serves the core on the filesystem that folds the names, so it is not a
     file left out. ``shipped`` maps the full destination of each platform
-    file the pack carries to its local copy. ``placed``, when given,
+    file the pack carries to its local copy, or to None for a slot it keeps
+    without one (user_provided, external). ``placed``, when given,
     receives the full destination of each held extra the pack carries.
     ``resolver`` is the builder's lookup. An extra it cannot resolve is
     skipped and takes no path, as the builder skips it.

scripts/pipeline.py

@@ -21,7 +21,7 @@ Steps, numbered as the run prints them:
   4b   generate_pack.py --manifest  install manifests
   4c   --manifest-targets           per-target core lists
   4d   bundled_only.py --strict     copies only an emulator install provides
-  5    consistency check            verify counts == pack counts
+  5    consistency check            verify == pack: counts, paths, every emulator pack
   6    --verify-packs               pack integrity, extraction and hashes
   7    generate_readme.py           README.md and CONTRIBUTING.md
   8    generate_site.py             MkDocs pages
@@ -41,13 +41,16 @@ Usage:
 from __future__ import annotations
 
 import argparse
+import atexit
 import contextlib
 import importlib.util
+import io
 import os
 import re
 import shutil
 import subprocess
 import sys
+import tempfile
 import time
 from pathlib import Path
 
@@ -309,6 +312,159 @@ def check_consistency(verify_output: str, pack_output: str) -> bool:
     return all_ok
 
 
+def parse_pack_paths(output: str) -> dict[str, str]:
+    """The archive each full pack was written to, per pack label."""
+    paths: dict[str, str] = {}
+    current_label = ""
+    for line in output.splitlines():
+        m = re.match(r"Generating (?:shared )?pack for (.+)\.\.\.", line)
+        if m:
+            current_label = _full_pack_label(m.group(1).strip())
+            continue
+        written = re.match(r"^\s+(\S.*?\.zip): \d+ files packed", line)
+        if current_label and written:
+            paths[current_label] = written.group(1)
+    return paths
+
+
+# Members the builder writes for the reader rather than for a core.
+_PACK_NOTES = re.compile(r"(^README\.txt$|^manifest\.json$|(^|/)(INSTRUCTIONS|RENAMED)_[^/]*\.txt$)")
+
+
+def _tree_members(platform: str, platforms_dir: str) -> dict[str, int]:
+    """Archive path -> size of every file the platform's data trees can add."""
+    import generate_pack  # noqa: PLC0415
+    from common import load_platform_config  # noqa: PLC0415
+    from packpaths import _flat  # noqa: PLC0415
+
+    config = load_platform_config(platform, platforms_dir)
+    base_dest = config.get("base_destination", "")
+    members: dict[str, int] = {}
+    with contextlib.redirect_stdout(io.StringIO()):
+        for src, full in generate_pack._data_directory_members(
+            config.get("systems", {}), load_data_dir_registry(platforms_dir),
+            platform, base_dest, False, set(), set(), set(),
+        ):
+            members[_flat(full, base_dest, True)] = os.path.getsize(src)
+    return members
+
+
+def _set_difference_lines(label: str, credited: set[str], written: set[str]) -> list[str]:
+    """The paths one side has and the other lacks, ten of each at most."""
+    lines = []
+    for side, paths in (("verify only", credited - written), ("pack only", written - credited)):
+        if paths:
+            shown = ", ".join(sorted(paths)[:10])
+            more = f" (+{len(paths) - 10})" if len(paths) > 10 else ""
+            lines.append(f"  {label}: {side} {len(paths)}: {shown}{more}")
+    return lines
+
+
+def check_pack_destinations(
+    destinations_file: str, pack_output: str, platforms_dir: str = "platforms",
+) -> bool:
+    """Compare the paths verify credits with the members of each archive.
+
+    Equal counts hid a file credited on one side and another written on the
+    other. The trees and the notes are the archive's own: a tree member is
+    set aside when its path and size are the cached file's and verify does
+    not credit the path.
+    """
+    import json  # noqa: PLC0415
+    import zipfile  # noqa: PLC0415
+
+    print("\n--- 5/8 consistency check: destinations ---")
+    try:
+        credited_by = json.loads(Path(destinations_file).read_text())
+    except (OSError, ValueError) as exc:
+        print(f"  no destinations from verify ({exc})")
+        return False
+    paths = parse_pack_paths(pack_output)
+    if not paths:
+        print("  no archive named in the pack output")
+        return False
+    all_ok = True
+    for label, zip_path in sorted(paths.items()):
+        names = [n for n in credited_by if _match_key(n) & _match_key(label)]
+        if not names:
+            print(f"  {label}: no verify destinations")
+            all_ok = False
+            continue
+        entry = credited_by[names[0]]
+        credited = set(entry["pack_destinations"])
+        trees = _tree_members(entry["platform"], platforms_dir)
+        with zipfile.ZipFile(zip_path) as zf:
+            written = {
+                info.filename for info in zf.infolist()
+                if not info.is_dir()
+                and not _PACK_NOTES.search(info.filename)
+                and not (
+                    trees.get(info.filename) == info.file_size
+                    and info.filename not in credited
+                )
+            }
+        lines = _set_difference_lines(label, credited, written)
+        if lines:
+            print("\n".join(lines))
+            all_ok = False
+        else:
+            print(f"  {label}: {len(credited)} paths, the same on both sides")
+    return all_ok
+
+
+def check_emulator_packs(
+    emulators_dir: str = "emulators", platforms_dir: str = "platforms",
+    db_path: str = "database.json",
+) -> bool:
+    """Build every emulator pack in memory and compare it with verify --emulator.
+
+    Each profile in each build it has (libretro, standalone): the paths the
+    pack fills with the profile's files against the ones verify credits. No
+    archive is written and no file read, so all of them take seconds.
+    """
+    import generate_pack  # noqa: PLC0415
+    import verify  # noqa: PLC0415
+    from common import (  # noqa: PLC0415
+        ProfileSelectionError,
+        build_zip_contents_index,
+        load_database,
+        load_emulator_profiles,
+        select_emulator_profiles,
+    )
+
+    print("\n--- 5/8 consistency check: emulator packs ---")
+    db = load_database(db_path)
+    zip_contents = build_zip_contents_index(db)
+    profiles = load_emulator_profiles(emulators_dir, skip_aliases=False)
+    all_ok = True
+    compared = 0
+    for name in sorted(profiles):
+        for standalone in (False, True):
+            try:
+                select_emulator_profiles([name], profiles, standalone)
+            except ProfileSelectionError:
+                continue
+            record = generate_pack.PackRecord()
+            with contextlib.redirect_stdout(io.StringIO()):
+                generate_pack.generate_emulator_pack(
+                    [name], emulators_dir, db, "bios", "", standalone, zip_contents,
+                    offline=True, platforms_dir=platforms_dir, record=record,
+                )
+                report = verify.verify_emulator(
+                    [name], emulators_dir, db, standalone, platforms_dir=platforms_dir,
+                )
+            compared += 1
+            label = f"{name}{' --standalone' if standalone else ''}"
+            lines = _set_difference_lines(
+                label, set(report["pack_destinations"]), set(record.files)
+            )
+            if lines:
+                print("\n".join(lines))
+                all_ok = False
+    print(f"  {compared} emulator packs compared, {'all equal' if all_ok else 'differences above'}")
+    return all_ok
+
+
 class _Skipped:
     """A step that did not run. Truthy, so it never fails the pipeline, and
     distinct, so the summary does not report it as work that was done."""
@@ -597,8 +753,19 @@ def main() -> None:
     # exclusive.
     hold_data_dirs(held, load_data_dir_registry("platforms"))
 
-    # Step 3: Verify
-    verify_cmd = [sys.executable, "scripts/verify.py", "--all", "--strict"]
+    # Step 3: Verify. The paths each pack holds by this report go to a file
+    # step 5 compares with the archives.
+    scratch = Path(__file__).resolve().parent.parent / "tmp"
+    scratch.mkdir(exist_ok=True)
+    fd, destinations_file = tempfile.mkstemp(
+        prefix="verify-destinations.", suffix=".json", dir=scratch
+    )
+    os.close(fd)
+    atexit.register(Path(destinations_file).unlink, missing_ok=True)
+    verify_cmd = [
+        sys.executable, "scripts/verify.py", "--all", "--strict",
+        "--destinations", destinations_file,
+    ]
     if args.include_archived:
         verify_cmd.append("--include-archived")
     if args.target:
@@ -749,6 +916,9 @@ def main() -> None:
         results["consistency"] = SKIPPED
     elif pack_output and verify_output:
         ok = check_consistency(verify_output, pack_output)
+        # Counts can agree while the paths differ, both ways at once.
+        ok = check_pack_destinations(destinations_file, pack_output) and ok
+        ok = check_emulator_packs() and ok
         results["consistency"] = ok
         all_ok = all_ok and ok
     else:

scripts/verify.py

@@ -30,6 +30,7 @@ import json
 import os
 import sys
 from collections.abc import Mapping
+from pathlib import Path
 
 import slots
 
@@ -859,7 +860,7 @@ def _mark_hidden(
     emulators_dir: str,
     db: dict,
     emu_profiles: dict | None,
-    shipped: dict[str, str],
+    shipped: dict[str, str | None],
     target_cores: set[str] | None,
     region_drops: set[str] | None = None,
     report: list[dict] | None = None,
@@ -891,6 +892,39 @@ def _mark_hidden(
     return placed
 
 
+def _builder_placement(
+    packed_order: list[tuple[str, str, str | None]], case_insensitive: bool,
+) -> tuple[dict[str, str], dict[str, str | None]]:
+    """The platform files the builder writes, and every path it keeps.
+
+    The builder walks the systems in name order and skips a path that a
+    written file blocks as a directory, or the reverse: RetroArch declares
+    SGB1.sfc and SGB1.sfc/program.rom, and only the first is in the pack.
+    Returns (path -> local copy written there, path -> copy or None for each
+    slot kept, a user_provided or external one included).
+    """
+    from packpaths import _has_path_conflict, _register_path
+
+    shipped: dict[str, str] = {}
+    occupied: dict[str, str | None] = {}
+    seen: set[str] = set()
+    folded: set[str] = set()
+    parents: set[str] = set()
+    for _sys_id, full, local in sorted(packed_order, key=lambda item: item[0]):
+        if (
+            full in occupied
+            or (case_insensitive and full.lower() in folded)
+            or _has_path_conflict(full, seen, parents)
+        ):
+            continue
+        _register_path(full, seen, parents)
+        folded.add(full.lower())
+        occupied[full] = local
+        if local is not None:
+            shipped[full] = local
+    return shipped, occupied
+
+
 def _dest_key(file_entry: dict, sys_id: str) -> str:
     """The path a declaration occupies, as the builder keys its pack."""
     dest = sanitize_pack_path(file_entry.get("destination", file_entry.get("name", "")))
@@ -1111,7 +1145,10 @@ def verify_platform(
         verify_systems, db, base_dest, zip_contents
     )
 
-    shipped: dict[str, str] = {}
+    # (system, full destination, local copy) of each declaration the builder
+    # writes, and None for one whose slot it keeps without a file from the
+    # collection (user_provided, external). Placed below in the builder's order.
+    packed_order: list[tuple[str, str, str | None]] = []
     shipped_keys: set[str] = set()
     results_at: dict[str, list[dict]] = {}
     for sys_id, system in verify_systems.items():
@@ -1135,6 +1172,12 @@ def verify_platform(
             if stored is not None:
                 result = stored
                 local_path = None
+                # The builder keeps the slot: instructions for a user_provided
+                # file, the download for an external one, whether it arrives
+                # or not. No core file goes there.
+                packed_order.append((
+                    sys_id, f"{base_dest}/{destination}" if base_dest else destination, None,
+                ))
             elif not reads_file_contents(mode):
                 result = verify_entry_existence(
                     file_entry,
@@ -1179,10 +1222,11 @@ def verify_platform(
                     file_entry, local_path, resolve_status, mode, config.get("hash_rule", "")
                 )[1] is None
             ):
-                shipped.setdefault(
+                packed_order.append((
+                    sys_id,
                     f"{base_dest}/{destination}" if base_dest else destination,
                     local_path,
-                )
+                ))
                 shipped_keys.add(_dest_key(file_entry, sys_id))
             if _twin_unmet(result, held_by):
                 continue
@@ -1212,6 +1256,10 @@ def verify_platform(
             ):
                 file_severity[dest] = sev
 
+    shipped, occupied = _builder_placement(
+        packed_order, bool(config.get("case_insensitive_fs", False))
+    )
+
     # Cross-reference undeclared files, called exactly as the builder calls
     # it: its declarations, not the enriched ones (a core file the platform
     # also declares under another name is packed at the core's own path, 21
@@ -1243,7 +1291,7 @@ def verify_platform(
 
     placed_by: dict[str, tuple[str, str, str]] = {}
     placed = _mark_hidden(
-        config, undeclared, emulators_dir, db, emu_profiles, shipped, target_cores,
+        config, undeclared, emulators_dir, db, emu_profiles, occupied, target_cores,
         region_drops, report_for_builder,
         ExtraResolver(
             db, DEFAULT_BIOS_DIR, zip_contents, data_dir_registry, validation_index,
@@ -1323,10 +1371,23 @@ def verify_platform(
     )
     total = len(file_status)
 
+    # The paths a pack built from this report holds, as written in the ZIP
+    # (base_destination stripped, as the builder flattens it). The pipeline
+    # compares this set with the archive: equal counts can hide one file
+    # credited here and another written there.
+    from packpaths import _flat
+
+    pack_destinations = sorted(
+        {_flat(dest, base_dest, True) for dest in shipped} | {
+            _flat(dest, base_dest, True) for dest in placed
+        }
+    )
+
     return {
         "platform": platform,
         "verification_mode": mode,
         "total_files": total,
+        "pack_destinations": pack_destinations,
         "severity_counts": counts,
         "status_counts": status_counts,
         "excluded_files": excluded,
@@ -2091,6 +2152,11 @@ def verify_emulator(
         "emulators": [n for n, _ in selected],
         "verification_mode": label,
         "total_files": total,
+        # The paths the emulator pack fills with the profile's files, for the
+        # pipeline's comparison with the pack built in memory.
+        "pack_destinations": sorted(
+            dest for dest, status in file_status.items() if status == Status.OK
+        ),
         "severity_counts": counts,
         "status_counts": status_counts,
         "details": details,
@@ -2271,6 +2337,7 @@ def _refuse_listing_narrowings(
         ("--json", args.json),
         ("--strict", args.strict),
         ("--verbose", args.verbose),
+        ("--destinations", args.destinations),
     ):
         if on:
             parser.error(f"{flag} is incompatible with {listing}")
@@ -2286,6 +2353,10 @@ def _refuse_mode_flags(args: argparse.Namespace, parser: argparse.ArgumentParser
          "--target requires --platform or --all"),
         (args.target and (args.emulator or args.system),
          "--target is incompatible with --emulator and --system"),
+        (args.destinations and not (args.platform or args.all),
+         "--destinations requires --platform or --all"),
+        (args.destinations and getattr(args, "region", None),
+         "--destinations describes the full pack and is incompatible with --region"),
     ):
         if refused:
             parser.error(message)
@@ -2363,6 +2434,11 @@ def _main(held: contextlib.ExitStack) -> None:
         help="Show emulator ground truth details",
     )
     parser.add_argument("--json", action="store_true", help="JSON output")
+    parser.add_argument(
+        "--destinations", metavar="FILE",
+        help="With --platform or --all, also write the paths each pack holds "
+        "according to this report to FILE (JSON), for comparison with the archive",
+    )
     parser.add_argument(
         "--strict",
         action="store_true",
@@ -2494,6 +2570,15 @@ def _main(held: contextlib.ExitStack) -> None:
         if len(group_results) > 1:
             print(_overall_summary(group_results))
 
+    if args.destinations:
+        held = {}
+        for (result, names), (group, _rep) in zip(group_results, groups):
+            for name, key in zip(names, group):
+                held[name] = {
+                    "platform": key, "pack_destinations": result["pack_destinations"],
+                }
+        Path(args.destinations).write_text(json.dumps(held, indent=1, sort_keys=True))
+
     if args.json:
         for r in all_results.values():
             r["details"] = _json_details(r["details"], args.verbose)