mirror of
https://github.com/Abdess/retroarch_system.git
synced 2026-10-10 13:33:24 -05:00
fix: narrow pack conformance by hardware target
This commit is contained in:
1 parent
a61c800462
commit
2a4dd59bb2
4 files changed
+87
-86
No files matched your search
+25
-59
@@ -2149,6 +2149,9 @@ def _run_verify_packs(args):
|
|||||||
emu_profiles=verify_profiles,
|
emu_profiles=verify_profiles,
|
||||||
regions=verify_regions,
|
regions=verify_regions,
|
||||||
data_registry=verify_data_registry,
|
data_registry=verify_data_registry,
|
||||||
|
target_cores=_target_cores_for(
|
||||||
|
platform_name, args.target, args.platforms_dir
|
||||||
|
),
|
||||||
)
|
)
|
||||||
ok, errors = result[0], result[3]
|
ok, errors = result[0], result[3]
|
||||||
bl_checked, bl_present = result[4], result[5]
|
bl_checked, bl_present = result[4], result[5]
|
||||||
@@ -2283,6 +2286,7 @@ def _run_platform_packs(
|
|||||||
skip_conformance=skip_conf,
|
skip_conformance=skip_conf,
|
||||||
data_registry=data_registry,
|
data_registry=data_registry,
|
||||||
regions=getattr(args, "regions", None),
|
regions=getattr(args, "regions", None),
|
||||||
|
target_name=args.target,
|
||||||
)
|
)
|
||||||
if args.split:
|
if args.split:
|
||||||
for entry in os.listdir(args.output_dir):
|
for entry in os.listdir(args.output_dir):
|
||||||
@@ -2422,12 +2426,10 @@ def main():
|
|||||||
# Combined with --all-variants, generation runs first then verify
|
# Combined with --all-variants, generation runs first then verify
|
||||||
if args.verify_packs and not args.all_variants:
|
if args.verify_packs and not args.all_variants:
|
||||||
# This mode checks packs already on disk against the platform's own
|
# This mode checks packs already on disk against the platform's own
|
||||||
# list. It reads the region priority above and nothing else, so a
|
# list, narrowed by the region priority above and by --target. A
|
||||||
# narrowing flag it cannot honour is refused rather than dropped: a
|
# narrowing flag it cannot honour is refused rather than dropped: a
|
||||||
# dropped flag answers about an artifact the caller did not name, and
|
# dropped flag answers about an artifact the caller did not name.
|
||||||
# an unknown target name reads as accepted.
|
|
||||||
for flag, given in (
|
for flag, given in (
|
||||||
("--target", args.target),
|
|
||||||
("--one-per-slot", args.one_per_slot),
|
("--one-per-slot", args.one_per_slot),
|
||||||
("--required-only", args.required_only),
|
("--required-only", args.required_only),
|
||||||
("--source", args.source != "full"),
|
("--source", args.source != "full"),
|
||||||
@@ -3087,71 +3089,33 @@ def inject_manifest(zip_path: str, manifest: dict) -> None:
|
|||||||
|
|
||||||
|
|
||||||
|
|
||||||
_TARGET_TAGS_CACHE: set[str] | None = None
|
def _target_cores_for(
|
||||||
|
platform_name: str, target_name: str | None, platforms_dir: str
|
||||||
|
) -> set[str] | None:
|
||||||
|
"""Cores a target leaves available, or None when no target was asked for.
|
||||||
|
|
||||||
|
None means no narrowing, which is what an unfiltered pack expects.
|
||||||
def _known_target_tags(platforms_dir: str = "platforms") -> set[str]:
|
|
||||||
"""Every filename tag a hardware target can put in a pack name.
|
|
||||||
|
|
||||||
A target tag is derived from the target's own name, so unlike the other
|
|
||||||
narrowings it cannot be a literal list.
|
|
||||||
"""
|
"""
|
||||||
global _TARGET_TAGS_CACHE
|
if not target_name:
|
||||||
if _TARGET_TAGS_CACHE is not None:
|
return None
|
||||||
return _TARGET_TAGS_CACHE
|
cache, _kept = build_target_cores_cache(
|
||||||
tags: set[str] = set()
|
[platform_name], target_name, platforms_dir
|
||||||
targets_dir = os.path.join(platforms_dir, "targets")
|
)
|
||||||
if os.path.isdir(targets_dir):
|
return cache.get(platform_name)
|
||||||
for entry in sorted(os.listdir(targets_dir)):
|
|
||||||
if not entry.endswith(".yml") or entry.startswith("_"):
|
|
||||||
continue
|
|
||||||
try:
|
|
||||||
with open(os.path.join(targets_dir, entry)) as fh:
|
|
||||||
data = yaml_load(fh) or {}
|
|
||||||
except OSError as exc:
|
|
||||||
print(f"warning: cannot read {entry}: {exc}", file=sys.stderr)
|
|
||||||
continue
|
|
||||||
for name in (data.get("targets") or {}):
|
|
||||||
tags.add(f"_{_target_tag(str(name))}")
|
|
||||||
# A pack is named after what the caller typed, and an alias is a name a
|
|
||||||
# caller may type: --target switch names the pack _Switch while the target
|
|
||||||
# file only knows nintendo-switch.
|
|
||||||
overrides_path = os.path.join(targets_dir, "_overrides.yml")
|
|
||||||
if os.path.exists(overrides_path):
|
|
||||||
try:
|
|
||||||
with open(overrides_path) as fh:
|
|
||||||
overrides = yaml_load(fh) or {}
|
|
||||||
except OSError as exc:
|
|
||||||
print(f"warning: cannot read _overrides.yml: {exc}", file=sys.stderr)
|
|
||||||
overrides = {}
|
|
||||||
for platform in overrides.values():
|
|
||||||
if not isinstance(platform, dict):
|
|
||||||
continue
|
|
||||||
for target in (platform.get("targets") or {}).values():
|
|
||||||
if not isinstance(target, dict):
|
|
||||||
continue
|
|
||||||
for alias in target.get("aliases") or []:
|
|
||||||
tags.add(f"_{_target_tag(str(alias))}")
|
|
||||||
_TARGET_TAGS_CACHE = tags
|
|
||||||
return tags
|
|
||||||
|
|
||||||
|
|
||||||
def _narrows_contents(pack_name: str) -> bool:
|
def _narrows_contents(pack_name: str) -> bool:
|
||||||
"""True when a pack holds fewer files than the platform declares.
|
"""True when a pack holds fewer files than the platform declares.
|
||||||
|
|
||||||
A source-restricted, required-only or target-filtered build is narrower by
|
A source-restricted or required-only build is narrower by design, so the
|
||||||
design, so the full platform expectation does not apply to it and
|
full platform expectation does not apply to it and conformance is skipped.
|
||||||
conformance is skipped. Without the target tag here, every targeted pack
|
Region is not listed: the region filter is passed to the check itself, and
|
||||||
was checked against the platform's whole system list and reported the
|
a hardware target is passed the same way.
|
||||||
systems the target itself had removed as missing.
|
|
||||||
Region is not listed: the region filter is passed to the check itself.
|
|
||||||
"""
|
"""
|
||||||
if any(
|
return any(
|
||||||
tag in pack_name
|
tag in pack_name
|
||||||
for tag in ("_Platform_", "_Truth_", "_Required", "_OnePerSlot")
|
for tag in ("_Platform_", "_Truth_", "_Required", "_OnePerSlot")
|
||||||
):
|
)
|
||||||
return True
|
|
||||||
return any(f"{tag}_" in pack_name for tag in _known_target_tags())
|
|
||||||
|
|
||||||
|
|
||||||
def verify_and_finalize_packs(
|
def verify_and_finalize_packs(
|
||||||
@@ -3161,6 +3125,7 @@ def verify_and_finalize_packs(
|
|||||||
skip_conformance: bool = False,
|
skip_conformance: bool = False,
|
||||||
data_registry: dict | None = None,
|
data_registry: dict | None = None,
|
||||||
regions: list[str] | None = None,
|
regions: list[str] | None = None,
|
||||||
|
target_name: str | None = None,
|
||||||
) -> bool:
|
) -> bool:
|
||||||
"""Verify all packs, inject manifests, generate SHA256SUMS.
|
"""Verify all packs, inject manifests, generate SHA256SUMS.
|
||||||
|
|
||||||
@@ -3227,6 +3192,7 @@ def verify_and_finalize_packs(
|
|||||||
db=db,
|
db=db,
|
||||||
regions=regions,
|
regions=regions,
|
||||||
data_registry=data_registry,
|
data_registry=data_registry,
|
||||||
|
target_cores=_target_cores_for(pname, target_name, platforms_dir),
|
||||||
)
|
)
|
||||||
status = "OK" if p_ok else "FAILED"
|
status = "OK" if p_ok else "FAILED"
|
||||||
exclusion_note = (
|
exclusion_note = (
|
||||||
|
|||||||
+18
-3
@@ -19,6 +19,7 @@ from ziptools import check_inside_zip
|
|||||||
from nativemode import digest_algorithm
|
from nativemode import digest_algorithm
|
||||||
from nativemode import hash_mismatch_excludes_file
|
from nativemode import hash_mismatch_excludes_file
|
||||||
import hashlib
|
import hashlib
|
||||||
|
from common import filter_systems_by_target
|
||||||
from common import load_emulator_profiles
|
from common import load_emulator_profiles
|
||||||
from common import load_platform_config
|
from common import load_platform_config
|
||||||
import os
|
import os
|
||||||
@@ -26,6 +27,7 @@ from packextras import platform_region_groups
|
|||||||
from nativemode import reads_file_contents
|
from nativemode import reads_file_contents
|
||||||
import region as region_mod
|
import region as region_mod
|
||||||
from packresolve import resolve_file
|
from packresolve import resolve_file
|
||||||
|
from common import resolve_platform_cores
|
||||||
from common import sanitize_pack_path
|
from common import sanitize_pack_path
|
||||||
import zipfile
|
import zipfile
|
||||||
def verify_pack(
|
def verify_pack(
|
||||||
@@ -449,11 +451,14 @@ def verify_pack_against_platform(
|
|||||||
emu_profiles: dict | None = None,
|
emu_profiles: dict | None = None,
|
||||||
regions: list[str] | None = None,
|
regions: list[str] | None = None,
|
||||||
data_registry: dict | None = None,
|
data_registry: dict | None = None,
|
||||||
|
target_cores: set[str] | None = None,
|
||||||
) -> tuple[bool, int, int, list[str], int, int, int, int, int]:
|
) -> tuple[bool, int, int, list[str], int, int, int, int, int]:
|
||||||
"""Verify a pack ZIP against its platform config and core requirements.
|
"""Verify a pack ZIP against its platform config and core requirements.
|
||||||
|
|
||||||
A region priority list narrows the expectation to what the builder would
|
A region priority list narrows the expectation to what the builder would
|
||||||
have packed, using the same selection function.
|
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.
|
||||||
|
|
||||||
Checks:
|
Checks:
|
||||||
1. Every baseline file declared by the platform exists in the ZIP
|
1. Every baseline file declared by the platform exists in the ZIP
|
||||||
@@ -477,12 +482,21 @@ def verify_pack_against_platform(
|
|||||||
if emu_profiles is None:
|
if emu_profiles is None:
|
||||||
emu_profiles = load_emulator_profiles(emulators_dir)
|
emu_profiles = load_emulator_profiles(emulators_dir)
|
||||||
|
|
||||||
|
expected_systems = config.get("systems", {})
|
||||||
|
if target_cores is not None:
|
||||||
|
expected_systems = filter_systems_by_target(
|
||||||
|
expected_systems,
|
||||||
|
emu_profiles,
|
||||||
|
target_cores,
|
||||||
|
resolve_platform_cores(config, emu_profiles),
|
||||||
|
)
|
||||||
|
|
||||||
region_drops: set[str] = set()
|
region_drops: set[str] = set()
|
||||||
if regions:
|
if regions:
|
||||||
region_index = region_mod.build_region_index(emu_profiles)
|
region_index = region_mod.build_region_index(emu_profiles)
|
||||||
region_groups, _extra_dests = platform_region_groups(
|
region_groups, _extra_dests = platform_region_groups(
|
||||||
config,
|
config,
|
||||||
config.get("systems", {}),
|
expected_systems,
|
||||||
emulators_dir,
|
emulators_dir,
|
||||||
db,
|
db,
|
||||||
base_dest,
|
base_dest,
|
||||||
@@ -520,7 +534,7 @@ def verify_pack_against_platform(
|
|||||||
for i in range(1, len(parts)):
|
for i in range(1, len(parts)):
|
||||||
zip_parents.add("/".join(parts[:i]))
|
zip_parents.add("/".join(parts[:i]))
|
||||||
baseline_groups: dict[str, list[dict]] = {}
|
baseline_groups: dict[str, list[dict]] = {}
|
||||||
for _sys_id, system in config.get("systems", {}).items():
|
for _sys_id, system in expected_systems.items():
|
||||||
for fe in system.get("files", []):
|
for fe in system.get("files", []):
|
||||||
dest = sanitize_pack_path(fe.get("destination", fe.get("name", "")))
|
dest = sanitize_pack_path(fe.get("destination", fe.get("name", "")))
|
||||||
if not dest:
|
if not dest:
|
||||||
@@ -600,6 +614,7 @@ def verify_pack_against_platform(
|
|||||||
set(),
|
set(),
|
||||||
base_dest,
|
base_dest,
|
||||||
emu_profiles,
|
emu_profiles,
|
||||||
|
target_cores=target_cores,
|
||||||
)
|
)
|
||||||
seen_conformance: set[str] = set(zip_set)
|
seen_conformance: set[str] = set(zip_set)
|
||||||
seen_parents: set[str] = set()
|
seen_parents: set[str] = set()
|
||||||
|
|||||||
@@ -532,6 +532,11 @@ def main():
|
|||||||
]
|
]
|
||||||
if args.include_archived:
|
if args.include_archived:
|
||||||
integrity_cmd.append("--include-archived")
|
integrity_cmd.append("--include-archived")
|
||||||
|
# Step 4 built with the target, so step 6 has to check against the
|
||||||
|
# same expectation. Without it a targeted run reported every system
|
||||||
|
# the target removed as missing from a pack that was exactly right.
|
||||||
|
if args.target:
|
||||||
|
integrity_cmd.extend(["--target", args.target])
|
||||||
ok, _ = run(integrity_cmd, "6/8 pack integrity")
|
ok, _ = run(integrity_cmd, "6/8 pack integrity")
|
||||||
results["pack_integrity"] = ok
|
results["pack_integrity"] = ok
|
||||||
all_ok = all_ok and ok
|
all_ok = all_ok and ok
|
||||||
|
|||||||
+39
-24
@@ -5814,13 +5814,10 @@ struct BurnDriver BurnDrvneogeo = {
|
|||||||
"refuse"),
|
"refuse"),
|
||||||
(["--platform", "retroarch", "--from-md5", "d8f1"], ["--one-per-slot"],
|
(["--platform", "retroarch", "--from-md5", "d8f1"], ["--one-per-slot"],
|
||||||
"refuse"),
|
"refuse"),
|
||||||
# --verify-packs returns before the argument checks run, so it used
|
# --verify-packs returns before the argument checks run, so it
|
||||||
# to accept all four of these -- an unknown target name included --
|
# used to accept these and answer about the pack sitting in the
|
||||||
# and answer about the pack sitting in the output directory.
|
# output directory. --target is honoured instead of refused: the
|
||||||
(["--platform", "retroarch", "--verify-packs"], ["--target", "switch"],
|
# check narrows by it.
|
||||||
"refuse"),
|
|
||||||
(["--platform", "retroarch", "--verify-packs"], ["--target", "no-such-xyz"],
|
|
||||||
"refuse"),
|
|
||||||
(["--platform", "retroarch", "--verify-packs"], ["--one-per-slot"],
|
(["--platform", "retroarch", "--verify-packs"], ["--one-per-slot"],
|
||||||
"refuse"),
|
"refuse"),
|
||||||
(["--platform", "retroarch", "--verify-packs"], ["--required-only"],
|
(["--platform", "retroarch", "--verify-packs"], ["--required-only"],
|
||||||
@@ -5842,27 +5839,45 @@ struct BurnDriver BurnDrvneogeo = {
|
|||||||
)
|
)
|
||||||
self.assertIn("error:", combined)
|
self.assertIn("error:", combined)
|
||||||
|
|
||||||
def test_a_targeted_pack_skips_full_platform_conformance(self):
|
def test_conformance_narrows_by_target_like_it_does_by_region(self):
|
||||||
"""A target-filtered pack is narrower, so the full expectation is off.
|
"""A targeted pack is checked against the targeted expectation.
|
||||||
|
|
||||||
The tag was missing from the skip list, so every targeted pack was
|
--target narrowed the build and nothing else: the conformance stage
|
||||||
checked against the platform's whole system list and reported the
|
held every targeted pack to the platform's whole system list and
|
||||||
systems the target itself had removed as missing: a correct build
|
core set, so a correct build exited non-zero on hundreds of files it
|
||||||
exited non-zero on hundreds of files it was never asked to carry.
|
was never asked to carry. Region was already passed to the check;
|
||||||
|
target has to travel the same way, which is why the pack is not in
|
||||||
|
the skip list.
|
||||||
"""
|
"""
|
||||||
from generate_pack import _narrowings, _narrows_contents
|
import inspect
|
||||||
|
|
||||||
|
import packverify
|
||||||
|
from generate_pack import _narrows_contents, _narrowings
|
||||||
|
|
||||||
|
source = inspect.getsource(packverify.verify_pack_against_platform)
|
||||||
|
self.assertIn(
|
||||||
|
"target_cores",
|
||||||
|
inspect.signature(packverify.verify_pack_against_platform).parameters,
|
||||||
|
"the pack verifier takes no target, so it cannot narrow by one",
|
||||||
|
)
|
||||||
|
self.assertIn(
|
||||||
|
"filter_systems_by_target(",
|
||||||
|
source,
|
||||||
|
"the baseline expectation is not narrowed by the target",
|
||||||
|
)
|
||||||
|
self.assertIn(
|
||||||
|
"target_cores=target_cores",
|
||||||
|
source,
|
||||||
|
"the core-extras expectation is not narrowed by the target",
|
||||||
|
)
|
||||||
|
|
||||||
|
# And the pack must not be skipped instead: skipping checks nothing.
|
||||||
for target in ("nintendo-switch", "switch", "rpi4"):
|
for target in ("nintendo-switch", "switch", "rpi4"):
|
||||||
with self.subTest(target=target):
|
tag = _narrowings("full", None, target, False, False)[0][0]
|
||||||
applied = _narrowings("full", None, target, False, False)
|
self.assertFalse(
|
||||||
self.assertEqual(len(applied), 1, target)
|
_narrows_contents(f"Platform_1.0{tag}_BIOS_Pack.zip"),
|
||||||
tag = applied[0][0]
|
f"a pack built for {target} would skip conformance entirely",
|
||||||
self.assertTrue(
|
)
|
||||||
_narrows_contents(f"Platform_1.0{tag}_BIOS_Pack.zip"),
|
|
||||||
f"a pack built for {target} would be held to the full list",
|
|
||||||
)
|
|
||||||
|
|
||||||
self.assertFalse(_narrows_contents("Platform_1.0_BIOS_Pack.zip"))
|
|
||||||
|
|
||||||
def test_standalone_mode_names_itself(self):
|
def test_standalone_mode_names_itself(self):
|
||||||
"""--emulator --standalone ships a different file set; without a tag it
|
"""--emulator --standalone ships a different file set; without a tag it
|
||||||
|
|||||||
Reference in new issue
Block a user