mirror of
https://github.com/Abdess/retroarch_system.git
synced 2026-11-06 18:13:48 -06:00
refactor: ask the mode module instead of retyping it
Four sites still compared the verification mode to a literal after the module owning that policy existed. One of them mattered: an unrecognised mode fell through to MD5 verification while compute_severity was scoring it as existence, so a typo in a platform YAML produced a report whose checks and severities described different platforms. The mode is normalized once per run and the consumers ask for what they need. A test reads the sources and fails on a literal comparison, so the next consumer cannot quietly grow a fifth copy.
This commit is contained in:
1 parent
b11c8b0638
commit
5e168b86c8
4 files changed
+74
-12
No files matched your search
@@ -56,7 +56,11 @@ from common import (
|
|||||||
import region as region_mod
|
import region as region_mod
|
||||||
import slot as slot_mod
|
import slot as slot_mod
|
||||||
from deterministic_zip import _FIXED_DATE_TIME, rebuild_zip_deterministic
|
from deterministic_zip import _FIXED_DATE_TIME, rebuild_zip_deterministic
|
||||||
from nativemode import hash_mismatch_excludes_file
|
from nativemode import (
|
||||||
|
digest_algorithm,
|
||||||
|
hash_mismatch_excludes_file,
|
||||||
|
reads_file_contents,
|
||||||
|
)
|
||||||
from validation import (
|
from validation import (
|
||||||
_build_validation_index,
|
_build_validation_index,
|
||||||
check_file_validation,
|
check_file_validation,
|
||||||
@@ -1745,7 +1749,9 @@ def generate_pack(
|
|||||||
file_reasons[dedup_key] = "not found"
|
file_reasons[dedup_key] = "not found"
|
||||||
continue
|
continue
|
||||||
|
|
||||||
if status == "hash_mismatch" and verification_mode != "existence":
|
if status == "hash_mismatch" and hash_mismatch_excludes_file(
|
||||||
|
verification_mode
|
||||||
|
):
|
||||||
zf_name = file_entry.get("zipped_file")
|
zf_name = file_entry.get("zipped_file")
|
||||||
if zf_name and local_path:
|
if zf_name and local_path:
|
||||||
inner_md5_raw = file_entry.get("md5", "")
|
inner_md5_raw = file_entry.get("md5", "")
|
||||||
@@ -3779,7 +3785,8 @@ def generate_manifest(
|
|||||||
# hash the local dump contradicts is not a reason to withhold
|
# hash the local dump contradicts is not a reason to withhold
|
||||||
# the file. Hash platforms would reject it, so they omit it.
|
# the file. Hash platforms would reject it, so they omit it.
|
||||||
if status in ("not_found", "external") or (
|
if status in ("not_found", "external") or (
|
||||||
status == "hash_mismatch" and verification_mode != "existence"
|
status == "hash_mismatch"
|
||||||
|
and hash_mismatch_excludes_file(verification_mode)
|
||||||
):
|
):
|
||||||
record_omission(full_dest, file_entry, sys_id, status, None)
|
record_omission(full_dest, file_entry, sys_id, status, None)
|
||||||
continue
|
continue
|
||||||
@@ -4529,19 +4536,20 @@ def verify_pack_against_platform(
|
|||||||
# error when the repo holds a file matching a declaration: without
|
# error when the repo holds a file matching a declaration: without
|
||||||
# one, the pack ships its best effort and the gap is a data issue
|
# one, the pack ships its best effort and the gap is a data issue
|
||||||
# reported by verify.py, not a generation bug.
|
# reported by verify.py, not a generation bug.
|
||||||
if verification_mode in ("md5", "sha1"):
|
digest = digest_algorithm(verification_mode)
|
||||||
|
if reads_file_contents(verification_mode) and digest:
|
||||||
for member, decl_entries in decl_by_member.items():
|
for member, decl_entries in decl_by_member.items():
|
||||||
checkable = [
|
checkable = [
|
||||||
fe
|
fe
|
||||||
for fe in decl_entries
|
for fe in decl_entries
|
||||||
if str(fe.get(verification_mode) or "").strip()
|
if str(fe.get(digest) or "").strip()
|
||||||
]
|
]
|
||||||
if not checkable:
|
if not checkable:
|
||||||
continue
|
continue
|
||||||
member_errors = []
|
member_errors = []
|
||||||
satisfied = False
|
satisfied = False
|
||||||
for fe in checkable:
|
for fe in checkable:
|
||||||
err = _check_member_hash(zf, member, fe, verification_mode)
|
err = _check_member_hash(zf, member, fe, digest)
|
||||||
if err is None:
|
if err is None:
|
||||||
satisfied = True
|
satisfied = True
|
||||||
break
|
break
|
||||||
|
|||||||
@@ -42,6 +42,7 @@ from common import (
|
|||||||
write_if_changed,
|
write_if_changed,
|
||||||
yaml_load,
|
yaml_load,
|
||||||
)
|
)
|
||||||
|
from nativemode import reads_file_contents
|
||||||
|
|
||||||
yaml = require_yaml()
|
yaml = require_yaml()
|
||||||
from generate_readme import compute_coverage, manifest_totals
|
from generate_readme import compute_coverage, manifest_totals
|
||||||
@@ -1603,7 +1604,7 @@ def generate_platform_page(
|
|||||||
|
|
||||||
pct_val = cov["present"] / cov["total"] * 100 if cov["total"] else 0
|
pct_val = cov["present"] / cov["total"] * 100 if cov["total"] else 0
|
||||||
mode_badge = (
|
mode_badge = (
|
||||||
"rb-badge-success" if mode in ("md5", "sha1") else "rb-badge-info"
|
"rb-badge-success" if reads_file_contents(mode) else "rb-badge-info"
|
||||||
)
|
)
|
||||||
|
|
||||||
lines = [
|
lines = [
|
||||||
|
|||||||
+12
-5
@@ -53,7 +53,12 @@ from common import (
|
|||||||
)
|
)
|
||||||
|
|
||||||
yaml = require_yaml()
|
yaml = require_yaml()
|
||||||
from nativemode import hash_mismatch_excludes_file, reads_file_contents
|
from nativemode import (
|
||||||
|
digest_algorithm,
|
||||||
|
hash_mismatch_excludes_file,
|
||||||
|
normalize as normalize_mode,
|
||||||
|
reads_file_contents,
|
||||||
|
)
|
||||||
from validation import (
|
from validation import (
|
||||||
_build_validation_index,
|
_build_validation_index,
|
||||||
_parse_validation,
|
_parse_validation,
|
||||||
@@ -764,7 +769,9 @@ def verify_platform(
|
|||||||
A region priority list narrows the report to the files a pack built with the
|
A region priority list narrows the report to the files a pack built with the
|
||||||
same list would carry, using the same selection function as the builder.
|
same list would carry, using the same selection function as the builder.
|
||||||
"""
|
"""
|
||||||
mode = config.get("verification_mode", "existence")
|
# Normalized once: a typo in a platform YAML must not verify with one
|
||||||
|
# mode and be scored with another.
|
||||||
|
mode = normalize_mode(config.get("verification_mode"))
|
||||||
platform = config.get("platform", "unknown")
|
platform = config.get("platform", "unknown")
|
||||||
|
|
||||||
has_zipped = any(
|
has_zipped = any(
|
||||||
@@ -842,14 +849,14 @@ def verify_platform(
|
|||||||
zip_contents,
|
zip_contents,
|
||||||
data_dir_registry=data_dir_registry,
|
data_dir_registry=data_dir_registry,
|
||||||
)
|
)
|
||||||
if mode == "existence":
|
if not reads_file_contents(mode):
|
||||||
result = verify_entry_existence(
|
result = verify_entry_existence(
|
||||||
file_entry,
|
file_entry,
|
||||||
local_path,
|
local_path,
|
||||||
validation_index,
|
validation_index,
|
||||||
db,
|
db,
|
||||||
)
|
)
|
||||||
elif mode == "sha1":
|
elif digest_algorithm(mode) == "sha1":
|
||||||
result = verify_entry_sha1(file_entry, local_path)
|
result = verify_entry_sha1(file_entry, local_path)
|
||||||
else:
|
else:
|
||||||
result = verify_entry_md5(file_entry, local_path, resolve_status)
|
result = verify_entry_md5(file_entry, local_path, resolve_status)
|
||||||
@@ -1158,7 +1165,7 @@ def print_platform_result(
|
|||||||
problems = total - ok_count
|
problems = total - ok_count
|
||||||
|
|
||||||
# Summary line
|
# Summary line
|
||||||
if mode == "existence":
|
if not reads_file_contents(mode):
|
||||||
if problems:
|
if problems:
|
||||||
missing = c.get(Severity.WARNING, 0) + c.get(Severity.CRITICAL, 0)
|
missing = c.get(Severity.WARNING, 0) + c.get(Severity.CRITICAL, 0)
|
||||||
optional_missing = c.get(Severity.INFO, 0)
|
optional_missing = c.get(Severity.INFO, 0)
|
||||||
|
|||||||
@@ -116,6 +116,52 @@ class BothConsumersAgree(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class ThePolicyIsSpeltInOnePlace(unittest.TestCase):
|
||||||
|
"""Consumers ask the module; they do not re-spell the rule.
|
||||||
|
|
||||||
|
Four sites still compared the mode to a literal after this module existed,
|
||||||
|
and one of them dispatched an unknown mode to MD5 verification while
|
||||||
|
compute_severity was scoring it as existence.
|
||||||
|
"""
|
||||||
|
|
||||||
|
LITERALS = ('!= "existence"', '== "existence"', 'in ("md5", "sha1")')
|
||||||
|
|
||||||
|
def test_no_consumer_compares_the_mode_to_a_literal(self):
|
||||||
|
scripts = Path(__file__).resolve().parent.parent / "scripts"
|
||||||
|
offenders = []
|
||||||
|
for path in sorted(scripts.glob("*.py")):
|
||||||
|
if path.name == "nativemode.py":
|
||||||
|
continue
|
||||||
|
for number, line in enumerate(path.read_text().splitlines(), 1):
|
||||||
|
if any(literal in line for literal in self.LITERALS):
|
||||||
|
offenders.append(f"{path.name}:{number}: {line.strip()}")
|
||||||
|
self.assertEqual(
|
||||||
|
offenders, [],
|
||||||
|
"route these through nativemode: " + "; ".join(offenders),
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_an_unknown_mode_verifies_the_way_it_is_scored(self):
|
||||||
|
"""A typo in a platform YAML must not verify one way and score another."""
|
||||||
|
import verify
|
||||||
|
|
||||||
|
self.assertFalse(nativemode.reads_file_contents("sha256"))
|
||||||
|
self.assertEqual(
|
||||||
|
verify.compute_severity(verify.Status.MISSING, True, "sha256"),
|
||||||
|
verify.Severity.WARNING,
|
||||||
|
)
|
||||||
|
config = {
|
||||||
|
"platform": "Typo",
|
||||||
|
"verification_mode": "shaa1",
|
||||||
|
"systems": {},
|
||||||
|
"cores": [],
|
||||||
|
}
|
||||||
|
result = verify.verify_platform(
|
||||||
|
config, {"files": {}, "indexes": {}}, emu_profiles={},
|
||||||
|
supplemental_names=set(),
|
||||||
|
)
|
||||||
|
self.assertEqual(result["verification_mode"], nativemode.DEFAULT_MODE)
|
||||||
|
|
||||||
|
|
||||||
class GapAnalysisAgreesWithTheBuilder(unittest.TestCase):
|
class GapAnalysisAgreesWithTheBuilder(unittest.TestCase):
|
||||||
""""Available" must mean the pack will carry it.
|
""""Available" must mean the pack will carry it.
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user