mirror of
https://github.com/Abdess/retroarch_system.git
synced 2026-10-23 20:29:06 -05:00
fix: check narrowed packs against their own list
After --all-variants, --verify-packs printed "hashes checked during the build" for the platform-only, truth and required-only packs and compared none of them, so one missing a file its narrowed list requires passed. Each pack is now held to the platform list narrowed the way it was built: source and required_only, read from its name, reach the baseline, the region grouping and the replay of the core extras.
This commit is contained in:
1 parent
db0e10b687
commit
7ca6adca56
5 files changed
+175
-27
No files matched your search
@@ -2632,16 +2632,15 @@ def _run_verify_packs(args: argparse.Namespace) -> None:
|
||||
# A group pack and a pack built for the platform alone can both be
|
||||
# present, and each one is a pack of this platform.
|
||||
for pack_name in found:
|
||||
if _narrows_contents(pack_name):
|
||||
# Only reached after an --all-variants build, whose stage 1
|
||||
# re-hashed every member against the collection above. The
|
||||
# platform's list describes the full pack, so a narrowed one
|
||||
# is not compared to it.
|
||||
variant = _pack_variant(pack_name)
|
||||
if variant is None:
|
||||
# One BIOS per slot: the slot choice is not replayed here.
|
||||
print(
|
||||
f" {platform_name} [{pack_name}]: hashes checked during the build; "
|
||||
"narrowed by design, so not compared to the platform's full list"
|
||||
"one per slot by design, so not compared to the platform's full list"
|
||||
)
|
||||
continue
|
||||
source, required_only = variant
|
||||
zip_path = os.path.join(args.output_dir, pack_name)
|
||||
result = verify_pack_against_platform(
|
||||
zip_path,
|
||||
@@ -2653,6 +2652,8 @@ def _run_verify_packs(args: argparse.Namespace) -> None:
|
||||
regions=verify_regions,
|
||||
data_registry=verify_data_registry,
|
||||
target_cores=tc,
|
||||
source=source,
|
||||
required_only=required_only,
|
||||
)
|
||||
(ok, _checked, _present, errors, bl_checked, bl_present,
|
||||
core_checked, core_present, bl_excluded) = result
|
||||
@@ -4732,6 +4733,30 @@ def _narrows_contents(pack_name: str) -> bool:
|
||||
return any(f"{tag}_" in pack_name for tag in _CONTENT_NARROWING_TAGS)
|
||||
|
||||
|
||||
def _pack_variant(pack_name: str) -> tuple[str, bool] | None:
|
||||
"""The (source, required_only) a pack was built as, read from its name.
|
||||
|
||||
None for a one-per-slot pack, whose slot choice the check does not
|
||||
replay. The tags come from the name builder, as for _narrows_contents.
|
||||
"""
|
||||
def tagged(**dimension: object) -> bool:
|
||||
tags = [
|
||||
tag for tag, _label in _narrowings(
|
||||
str(dimension.get("source", "full")), None, None,
|
||||
bool(dimension.get("one_per_slot", False)),
|
||||
bool(dimension.get("required_only", False)),
|
||||
)
|
||||
]
|
||||
return all(f"{tag}_" in pack_name for tag in tags)
|
||||
|
||||
if tagged(one_per_slot=True):
|
||||
return None
|
||||
source = next(
|
||||
(s for s in ("platform", "truth") if tagged(source=s)), "full"
|
||||
)
|
||||
return source, tagged(required_only=True)
|
||||
|
||||
|
||||
def _pack_archives(output_dir: str) -> list[str]:
|
||||
"""The ZIPs of a directory that are packs.
|
||||
|
||||
|
||||
@@ -226,6 +226,8 @@ def hidden_extras(
|
||||
resolver: ExtraResolver | None = None,
|
||||
placed_kind: dict[str, str] | None = None,
|
||||
placed_local: dict[str, str] | None = None,
|
||||
include_all: bool = False,
|
||||
required_only: bool = False,
|
||||
) -> dict[tuple[str, str, str], str]:
|
||||
"""Report entries the builder holds and still leaves out, with the reason.
|
||||
|
||||
@@ -247,7 +249,9 @@ def hidden_extras(
|
||||
report entry names. ``placed_local`` receives the local copy the builder
|
||||
writes at each of those destinations, for the pack integrity check.
|
||||
``resolver`` is the builder's lookup. An extra it cannot resolve is
|
||||
skipped and takes no path, as the builder skips it.
|
||||
skipped and takes no path, as the builder skips it. ``include_all`` and
|
||||
``required_only`` are the builder's for a --source truth or a
|
||||
--required-only pack: an optional extra is skipped before it takes a path.
|
||||
"""
|
||||
from packpaths import _register_path, placement_block
|
||||
|
||||
@@ -267,8 +271,10 @@ def hidden_extras(
|
||||
hidden: dict[tuple[str, str, str], str] = {}
|
||||
for fe in _collect_emulator_extras(
|
||||
config, emulators_dir, db, set(shipped), base_dest, profiles,
|
||||
target_cores=target_cores, undeclared=report,
|
||||
target_cores=target_cores, include_all=include_all, undeclared=report,
|
||||
):
|
||||
if required_only and fe.get("required") is False:
|
||||
continue
|
||||
dest = sanitize_pack_path(fe.get("destination", fe.get("name", "")))
|
||||
if not dest or (region_drops and dest in region_drops):
|
||||
continue
|
||||
|
||||
+18
-3
@@ -535,6 +535,8 @@ def _builder_extras(
|
||||
target_cores: set[str] | None,
|
||||
region_drops: set[str],
|
||||
data_registry: dict | None,
|
||||
include_all: bool = False,
|
||||
required_only: bool = False,
|
||||
) -> dict[str, str]:
|
||||
"""{full destination: local copy} of each core extra the builder writes.
|
||||
|
||||
@@ -584,6 +586,8 @@ def _builder_extras(
|
||||
destination_owners(platform_profiles), True, profiles=emu_profiles,
|
||||
),
|
||||
placed_local=placed_local,
|
||||
include_all=include_all,
|
||||
required_only=required_only,
|
||||
)
|
||||
return placed_local
|
||||
|
||||
@@ -607,13 +611,18 @@ def verify_pack_against_platform(
|
||||
regions: list[str] | None = None,
|
||||
data_registry: dict | None = None,
|
||||
target_cores: set[str] | None = None,
|
||||
source: str = "full",
|
||||
required_only: bool = False,
|
||||
) -> tuple[bool, int, int, list[str], int, int, int, int, int]:
|
||||
"""Verify a pack ZIP against its platform config and core requirements.
|
||||
|
||||
A region priority list narrows the expectation to what the builder would
|
||||
have packed, using the same selection function. A hardware target narrows
|
||||
it the same way and for the same reason: the systems it removes are not
|
||||
missing from the pack, they were never asked for.
|
||||
missing from the pack, they were never asked for. ``source`` and
|
||||
``required_only`` are the variant the pack was built as: a platform-only
|
||||
pack is held to the platform's list alone, a truth pack to the cores'
|
||||
files alone, and a required-only pack to the required entries of both.
|
||||
|
||||
Checks:
|
||||
1. Every baseline file declared by the platform exists in the ZIP
|
||||
@@ -659,6 +668,9 @@ def verify_pack_against_platform(
|
||||
base_dest,
|
||||
emu_profiles,
|
||||
target_cores=target_cores,
|
||||
include_extras=(source != "platform"),
|
||||
include_all=(source == "truth"),
|
||||
required_only=required_only,
|
||||
held_out=held,
|
||||
)
|
||||
region_drops = region_mod.resolve_region_drops(
|
||||
@@ -700,8 +712,10 @@ def verify_pack_against_platform(
|
||||
for i in range(1, len(parts)):
|
||||
zip_parents.add("/".join(parts[:i]))
|
||||
baseline_groups: dict[str, list[dict]] = {}
|
||||
for _sys_id, system in expected_systems.items():
|
||||
for _sys_id, system in (expected_systems if source != "truth" else {}).items():
|
||||
for fe in system.get("files", []):
|
||||
if required_only and fe.get("required") is False:
|
||||
continue
|
||||
dest = sanitize_pack_path(fe.get("destination", fe.get("name", "")))
|
||||
if not dest:
|
||||
continue
|
||||
@@ -787,11 +801,12 @@ def verify_pack_against_platform(
|
||||
# holds the platform's other dump.
|
||||
core_checked = 0
|
||||
core_present = 0
|
||||
if db is not None:
|
||||
if db is not None and source != "platform":
|
||||
placed_local = _builder_extras(
|
||||
config, emulators_dir, db, emu_profiles, baseline_groups,
|
||||
base_dest, is_flat, zip_set, zip_lower, target_cores,
|
||||
region_drops, data_registry,
|
||||
include_all=(source == "truth"), required_only=required_only,
|
||||
)
|
||||
for full, local in sorted(placed_local.items()):
|
||||
core_checked += 1
|
||||
|
||||
@@ -87,5 +87,106 @@ class CoreFilesAsTheBuilderPlacesThem(unittest.TestCase):
|
||||
)
|
||||
|
||||
|
||||
class NarrowedPacksAgainstTheirOwnList(unittest.TestCase):
|
||||
"""--verify-packs after --all-variants said "hashes checked during the
|
||||
build" for the five narrowed packs and compared none of them: a narrowed
|
||||
pack missing a file its narrowed list requires passed. Each variant is
|
||||
now held to the platform list narrowed the way it was built."""
|
||||
|
||||
def setUp(self):
|
||||
import common
|
||||
import generate_db
|
||||
import generate_pack
|
||||
|
||||
self.gp = generate_pack
|
||||
tmp = tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp")
|
||||
self.addCleanup(tmp.cleanup)
|
||||
previous = os.getcwd()
|
||||
os.chdir(tmp.name)
|
||||
self.addCleanup(os.chdir, previous)
|
||||
files: dict = {}
|
||||
for rel in ("bios/p/req.bin", "bios/p/opt.bin", "bios/c/core.bin", "bios/c/extra.bin"):
|
||||
payload = rel.encode()
|
||||
Path(rel).parent.mkdir(parents=True, exist_ok=True)
|
||||
Path(rel).write_bytes(payload)
|
||||
sha1 = hashlib.sha1(payload).hexdigest()
|
||||
files[sha1] = {"path": rel, "name": Path(rel).name, "size": len(payload),
|
||||
"sha1": sha1, "md5": hashlib.md5(payload).hexdigest(),
|
||||
"sha256": hashlib.sha256(payload).hexdigest(),
|
||||
"crc32": "00000001"}
|
||||
self.db = {"files": files, "indexes": generate_db.build_indexes(files, {})}
|
||||
Path("platforms").mkdir()
|
||||
config = {"platform": "P", "verification_mode": "existence", "cores": ["c"],
|
||||
"systems": {"s": {"files": [
|
||||
{"name": "req.bin", "destination": "req.bin", "required": True},
|
||||
{"name": "opt.bin", "destination": "opt.bin", "required": False},
|
||||
]}}}
|
||||
Path("platforms/p.yml").write_text(yaml.safe_dump(config), encoding="utf-8")
|
||||
Path("platforms/_registry.yml").write_text(
|
||||
yaml.safe_dump({"platforms": {"p": {"status": "active"}}}), encoding="utf-8"
|
||||
)
|
||||
Path("emulators").mkdir()
|
||||
Path("emulators/c.yml").write_text(yaml.safe_dump({
|
||||
"emulator": "c", "type": "libretro", "systems": ["s"], "files": [
|
||||
{"name": "core.bin", "path": "core.bin", "required": True},
|
||||
{"name": "extra.bin", "path": "extra.bin", "required": False},
|
||||
]}), encoding="utf-8")
|
||||
common._platform_config_cache.clear()
|
||||
common._emulator_profiles_cache.clear()
|
||||
self.addCleanup(common._platform_config_cache.clear)
|
||||
self.addCleanup(common._emulator_profiles_cache.clear)
|
||||
self.profiles = common.load_emulator_profiles("emulators")
|
||||
|
||||
def _build(self, source: str, required: bool) -> tuple[str, set[str]]:
|
||||
zip_path = self.gp.generate_pack(
|
||||
"p", "platforms", self.db, "bios", "out", emulators_dir="emulators",
|
||||
emu_profiles=self.profiles, source=source, required_only=required,
|
||||
offline=True,
|
||||
)
|
||||
with zipfile.ZipFile(zip_path) as zf:
|
||||
return zip_path, set(zf.namelist()) - {"README.txt"}
|
||||
|
||||
def _check(self, zip_path: str, source: str, required: bool) -> tuple:
|
||||
return packverify.verify_pack_against_platform(
|
||||
zip_path, "p", "platforms", self.db, emulators_dir="emulators",
|
||||
emu_profiles=self.profiles, source=source, required_only=required,
|
||||
)
|
||||
|
||||
def test_every_variant_passes_as_built(self):
|
||||
expected = {
|
||||
("full", False): {"req.bin", "opt.bin", "core.bin", "extra.bin"},
|
||||
("full", True): {"req.bin", "core.bin"},
|
||||
("platform", False): {"req.bin", "opt.bin"},
|
||||
("platform", True): {"req.bin"},
|
||||
("truth", False): {"core.bin", "extra.bin"},
|
||||
("truth", True): {"core.bin"},
|
||||
}
|
||||
for (source, required), members in expected.items():
|
||||
with self.subTest(source=source, required=required):
|
||||
zip_path, held = self._build(source, required)
|
||||
self.assertEqual(held, members)
|
||||
ok, _c, _p, errors, *_rest = self._check(zip_path, source, required)
|
||||
self.assertTrue(ok, errors)
|
||||
self.assertEqual(
|
||||
self.gp._pack_variant(os.path.basename(zip_path)), (source, required)
|
||||
)
|
||||
|
||||
def test_a_narrowed_pack_missing_a_required_file_fails(self):
|
||||
for source, required, lost in (
|
||||
("full", True, "core.bin"), ("platform", True, "req.bin"),
|
||||
("truth", False, "extra.bin"), ("truth", True, "core.bin"),
|
||||
):
|
||||
with self.subTest(source=source, required=required):
|
||||
zip_path, held = self._build(source, required)
|
||||
with zipfile.ZipFile(zip_path) as zf:
|
||||
kept = {n: zf.read(n) for n in held if n != lost}
|
||||
with zipfile.ZipFile(zip_path, "w") as zf:
|
||||
for name, data in kept.items():
|
||||
zf.writestr(name, data)
|
||||
ok, _c, _p, errors, *_rest = self._check(zip_path, source, required)
|
||||
self.assertFalse(ok)
|
||||
self.assertTrue(any(lost in e for e in errors), errors)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -63,8 +63,13 @@ class VerifyPackChoice(unittest.TestCase):
|
||||
|
||||
def record(zip_path, platform_name, *a, **kw):
|
||||
checked.append((platform_name, os.path.basename(zip_path)))
|
||||
self.variants.append(
|
||||
(kw.get("source", "full"), kw.get("required_only", False))
|
||||
)
|
||||
return _ok_result()
|
||||
|
||||
self.variants: list[tuple[str, bool]] = []
|
||||
|
||||
with mock.patch.object(self.gp, "verify_pack_against_platform", record):
|
||||
self.gp._run_verify_packs(args)
|
||||
return checked
|
||||
@@ -94,26 +99,22 @@ class VerifyPackChoice(unittest.TestCase):
|
||||
self._run(self._args())
|
||||
self.assertEqual(ctx.exception.code, 1)
|
||||
|
||||
def test_all_variants_names_every_variant_it_built(self):
|
||||
"""After --all-variants the check opened the full pack only: the five
|
||||
narrowed packs the same run wrote passed without a line."""
|
||||
import contextlib
|
||||
import io
|
||||
|
||||
names = []
|
||||
def test_all_variants_checks_every_variant_as_it_was_built(self):
|
||||
"""After --all-variants the check opened the full pack only, and then
|
||||
said "hashes checked during the build" for the five narrowed packs:
|
||||
one missing a file its narrowed list requires passed. Each is now
|
||||
held to the platform's list narrowed the same way."""
|
||||
names = {}
|
||||
for source, required in self.gp.ALL_VARIANTS:
|
||||
tags = "".join(
|
||||
tag for tag, _ in self.gp._narrowings(source, None, None, False, required)
|
||||
)
|
||||
names.append(f"{self.stem}{tags}_BIOS_Pack.zip")
|
||||
self._touch(names[-1])
|
||||
out = io.StringIO()
|
||||
with contextlib.redirect_stdout(out):
|
||||
checked = self._run(self._args(all_variants=True))
|
||||
self.assertEqual(checked, [("retroarch", f"{self.stem}_BIOS_Pack.zip")])
|
||||
printed = out.getvalue()
|
||||
for name in names[1:]:
|
||||
self.assertIn(f"[{name}]: hashes checked during the build", printed)
|
||||
names[f"{self.stem}{tags}_BIOS_Pack.zip"] = (source, required)
|
||||
self._touch(f"{self.stem}{tags}_BIOS_Pack.zip")
|
||||
checked = self._run(self._args(all_variants=True))
|
||||
self.assertEqual(
|
||||
dict(zip((name for _p, name in checked), self.variants)), names
|
||||
)
|
||||
|
||||
def test_the_run_ends_on_a_summary(self):
|
||||
"""A failing pack was named only above its own error lines, and the
|
||||
|
||||
Reference in new issue
Block a user