mirror of
https://github.com/Abdess/retroarch_system.git
synced 2026-10-24 20:59:03 -05:00
feat: name doubled destinations in exports
A platform list that declares one path for two files fails one of the two checks on every install. The export report lists each declaration with its hash and the emulators whose profile reads that file.
This commit is contained in:
1 parent
eb7b265758
commit
2ee777824a
6 files changed
+341
-2
No files matched your search
@@ -36,6 +36,7 @@ from artifacts import file_lock, write_bytes_atomic, write_if_changed, write_tex
|
||||
import yaml
|
||||
|
||||
from common import (
|
||||
build_zip_contents_index,
|
||||
platform_arg,
|
||||
list_registered_platforms,
|
||||
load_data_dir_registry,
|
||||
@@ -50,6 +51,7 @@ from common import (
|
||||
from exporter import discover_exporters
|
||||
from exporter.baseline import HASH_FIELDS, _hash_values, _single_size, build_native_model
|
||||
from exporter.candidates import removal_candidates
|
||||
from exporter.doubled import doubled_destinations
|
||||
from truth import generate_platform_truth
|
||||
|
||||
DEFAULT_CACHE = ".cache/upstream-native"
|
||||
@@ -436,6 +438,24 @@ def refresh_cache(
|
||||
return True, [f"{len(wanted)} file(s) fetched"]
|
||||
|
||||
|
||||
@functools.cache
|
||||
def _collection(db_path: str) -> tuple[dict, dict]:
|
||||
"""The database and its archive index, read once for every platform."""
|
||||
db = load_database(db_path)
|
||||
return db, build_zip_contents_index(db)
|
||||
|
||||
|
||||
def doubled_in(scraped: dict | None, platforms_dir: str) -> list[dict]:
|
||||
"""The destinations the platform's own list declares for two files."""
|
||||
if not scraped:
|
||||
return []
|
||||
db, zip_contents = _collection(TRUTH_DB)
|
||||
return doubled_destinations(
|
||||
scraped, load_emulator_profiles("emulators"), db, zip_contents,
|
||||
load_data_dir_registry(platforms_dir),
|
||||
)
|
||||
|
||||
|
||||
def _count(number: int, noun: str, plural: str = "") -> str:
|
||||
""""1 hash", "2 hashes": a count with its noun in the right number."""
|
||||
plural = plural or noun + ("es" if noun.endswith(("sh", "ch", "s", "x")) else "s")
|
||||
@@ -764,6 +784,8 @@ def export_platform(
|
||||
systems, scraped, load_emulator_profiles("emulators"),
|
||||
load_data_dir_registry(platforms_dir),
|
||||
)
|
||||
# Reported, never rewritten: both declarations stay as the platform wrote them.
|
||||
doubled = doubled_in(scraped, platforms_dir)
|
||||
messages.append(summary)
|
||||
if unwritten:
|
||||
messages.append("not in the file: " + ", ".join(unwritten))
|
||||
@@ -774,6 +796,11 @@ def export_platform(
|
||||
+ (f" ({open_question} undetermined)" if open_question else "")
|
||||
+ ", listed in the report"
|
||||
)
|
||||
if doubled:
|
||||
messages.append(
|
||||
f"{_count(len(doubled), 'destination')} declared twice for two files"
|
||||
", listed in the report"
|
||||
)
|
||||
messages.extend(f"withheld: {reason}" for reason in withheld)
|
||||
for heading, labels in (
|
||||
("hash updated", applied),
|
||||
@@ -807,6 +834,7 @@ def export_platform(
|
||||
"withheld": list(withheld),
|
||||
"found_by_content": report.labels,
|
||||
"removal_candidates": candidates,
|
||||
"doubled": doubled,
|
||||
"invalid": issues,
|
||||
})
|
||||
return True, messages
|
||||
|
||||
@@ -0,0 +1,137 @@
|
||||
"""Destinations a platform's own list declares twice, for two files.
|
||||
|
||||
One path holds one file, so on an install one of the two checks always
|
||||
fails. RetroDECK declares bios/disk.rom for the PC-88 core and for XRoar,
|
||||
with two md5 that name two different ROMs. slots.find_collisions finds
|
||||
these. The export keeps both entries and names, beside each declaration,
|
||||
the emulators whose profile reads the file it declares.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
from common import (
|
||||
contents_members,
|
||||
holds_required_members,
|
||||
platform_layouts,
|
||||
resolve_platform_cores,
|
||||
standalone_set,
|
||||
)
|
||||
from exporter.baseline import HASH_FIELDS, _hash_values
|
||||
from slots import _accepted_by_all, _lists_hashes, find_collisions
|
||||
from validation import opens_by_name, outside_gap_scope
|
||||
|
||||
|
||||
def _names(entry: dict) -> set[str]:
|
||||
"""The file names a profile entry is opened under."""
|
||||
names = {entry.get("name"), *(entry.get("aliases") or [])}
|
||||
for key in ("path", "standalone_path"):
|
||||
if entry.get(key):
|
||||
names.add(str(entry[key]).rsplit("/", 1)[-1])
|
||||
return {str(n).casefold() for n in names if n}
|
||||
|
||||
|
||||
def _size_fits(entry: dict, local: str) -> bool:
|
||||
"""Whether the file's size is one the entry states, or none is stated."""
|
||||
try:
|
||||
size = os.path.getsize(local)
|
||||
except OSError:
|
||||
return False
|
||||
declared = entry.get("size")
|
||||
sizes = declared if isinstance(declared, list) else [declared]
|
||||
sizes = [s for s in sizes if isinstance(s, int)]
|
||||
if sizes and size not in sizes:
|
||||
return False
|
||||
if isinstance(entry.get("min_size"), int) and size < entry["min_size"]:
|
||||
return False
|
||||
return not (isinstance(entry.get("max_size"), int) and size > entry["max_size"])
|
||||
|
||||
|
||||
def _reads(entry: dict, by_name: bool, local: str, db: dict) -> bool:
|
||||
"""Whether a profile entry describes the file at local.
|
||||
|
||||
An archive is read by its members, a dump by its hashes. A name the code
|
||||
opens with no hash to check is read by any copy of a size it accepts. A
|
||||
name that is only a label (a core identifying files by content) reads
|
||||
nothing by name.
|
||||
"""
|
||||
if entry.get("contents"):
|
||||
return holds_required_members(local, contents_members(entry))
|
||||
if _lists_hashes(entry):
|
||||
return _accepted_by_all(local, [entry], db)
|
||||
return by_name and _size_fits(entry, local)
|
||||
|
||||
|
||||
def _hashes(entry: dict) -> dict[str, list[str]]:
|
||||
found = {f: _hash_values(entry, f) for f in HASH_FIELDS}
|
||||
return {f: values for f, values in found.items() if values}
|
||||
|
||||
|
||||
def doubled_destinations(
|
||||
scraped: dict | None,
|
||||
profiles: dict[str, dict],
|
||||
db: dict,
|
||||
zip_contents: dict | None = None,
|
||||
data_registry: dict | None = None,
|
||||
) -> list[dict]:
|
||||
"""Each destination the platform declares twice for two files.
|
||||
|
||||
Every declaration is listed with its system, component, hashes and the
|
||||
file it resolves to, and the emulators the platform runs whose profile
|
||||
reads that file there, with the profile's source_ref. Nothing is
|
||||
removed from the platform's file.
|
||||
"""
|
||||
if not scraped:
|
||||
return []
|
||||
collisions = find_collisions(scraped, db, zip_contents, data_registry)
|
||||
if not collisions:
|
||||
return []
|
||||
standalone = standalone_set(scraped)
|
||||
named: dict[str, list[tuple[str, dict, dict, bool]]] = {}
|
||||
for key in sorted(resolve_platform_cores(scraped, profiles)):
|
||||
profile = profiles[key]
|
||||
if profile.get("type") in ("launcher", "alias"):
|
||||
continue
|
||||
for layout, is_standalone in platform_layouts(key, profile, standalone):
|
||||
for entry in layout.get("files") or []:
|
||||
if not isinstance(entry, dict) or outside_gap_scope(entry, is_standalone):
|
||||
continue
|
||||
by_name = opens_by_name(entry, layout, is_standalone)
|
||||
for name in _names(entry):
|
||||
named.setdefault(name, []).append((key, layout, entry, by_name))
|
||||
|
||||
doubled = []
|
||||
for collision in collisions:
|
||||
base = collision.destination.rsplit("/", 1)[-1].casefold()
|
||||
declarations = []
|
||||
for entry, local in collision.declarations:
|
||||
readers: list[dict] = []
|
||||
for key, layout, profile_entry, by_name in named.get(base, []):
|
||||
if not local or not _reads(profile_entry, by_name, local, db):
|
||||
continue
|
||||
reader = {
|
||||
"emulator": layout.get("emulator") or key,
|
||||
"profile": key,
|
||||
"entry": profile_entry.get("name", ""),
|
||||
"source_ref": profile_entry.get("source_ref", ""),
|
||||
}
|
||||
if reader not in readers:
|
||||
readers.append(reader)
|
||||
declarations.append({
|
||||
"system": entry.get("native_system") or "",
|
||||
"component": entry.get("component") or "",
|
||||
"name": entry.get("name", ""),
|
||||
"hashes": _hashes(entry),
|
||||
"file": local,
|
||||
"readers": readers,
|
||||
})
|
||||
doubled.append({
|
||||
"destination": collision.destination,
|
||||
"declarations": declarations,
|
||||
})
|
||||
return doubled
|
||||
@@ -156,6 +156,49 @@ def candidate_lines(candidates: list[dict]) -> list[str]:
|
||||
return lines
|
||||
|
||||
|
||||
def _declaration(declaration: dict) -> str:
|
||||
"""One declaration of a doubled destination: who declares it, its hash,
|
||||
and the emulators that read the file it names."""
|
||||
where = f"`{declaration['system']}`"
|
||||
if declaration.get("component"):
|
||||
where += f" (`{declaration['component']}`)"
|
||||
parts = [where] + [
|
||||
f"{field} {_value(values[0] if len(values) == 1 else values)}"
|
||||
for field, values in (declaration.get("hashes") or {}).items()
|
||||
]
|
||||
readers = declaration.get("readers") or []
|
||||
read = (
|
||||
"read by " + ", ".join(
|
||||
r["emulator"] + (f" (`{r['source_ref']}`)" if r.get("source_ref") else "")
|
||||
for r in readers
|
||||
)
|
||||
if readers else "no emulator this platform runs is known to read this file"
|
||||
)
|
||||
return " ".join(parts) + f": {read}"
|
||||
|
||||
|
||||
def doubled_lines(doubled: list[dict]) -> list[str]:
|
||||
"""The destinations the original declares twice, for two files.
|
||||
|
||||
One path holds one file, so one of the two checks fails on every
|
||||
install. Both entries stay in the updated file.
|
||||
"""
|
||||
if not doubled:
|
||||
return []
|
||||
count = _plural(len(doubled), "destination")
|
||||
lines = [
|
||||
f"{count} of the original {'is' if len(doubled) == 1 else 'are'} declared "
|
||||
"twice, for two different files. One path holds one file, so one of the "
|
||||
"two checks fails on every install. Both entries stay in the updated file.",
|
||||
"",
|
||||
]
|
||||
for item in doubled:
|
||||
lines.append(f"- `{item['destination']}`")
|
||||
lines.extend(f" - {_declaration(d)}" for d in item["declarations"])
|
||||
lines.append("")
|
||||
return lines
|
||||
|
||||
|
||||
def load_export_reports(directory: str = EXPORTS_DIR) -> list[dict]:
|
||||
"""Every platform report the export wrote, by platform name."""
|
||||
root = Path(directory)
|
||||
@@ -266,6 +309,7 @@ def generate_exports_page(reports: list[dict], names: dict[str, str]) -> str:
|
||||
"",
|
||||
])
|
||||
lines.extend(candidate_lines(report.get("removal_candidates") or []))
|
||||
lines.extend(doubled_lines(report.get("doubled") or []))
|
||||
if report.get("invalid"):
|
||||
lines.extend(markup.aside(
|
||||
"warning", "The updated file does not pass the platform's own checks",
|
||||
|
||||
+7
-1
@@ -675,6 +675,8 @@ class Collision:
|
||||
|
||||
destination: str
|
||||
resolved: list[str]
|
||||
# Each declaration with the file it resolved to, in the platform's order.
|
||||
declarations: list[tuple[dict, str | None]] = field(default_factory=list)
|
||||
|
||||
|
||||
def _collision_json(collision: Collision) -> dict:
|
||||
@@ -714,6 +716,7 @@ def find_collisions(
|
||||
if len(entries) < 2:
|
||||
continue
|
||||
resolved = []
|
||||
declarations = []
|
||||
for entry in entries:
|
||||
local, _ = resolve_local_file(
|
||||
entry,
|
||||
@@ -722,6 +725,7 @@ def find_collisions(
|
||||
dest_hint=entry.get("destination", ""),
|
||||
data_dir_registry=data_dir_registry,
|
||||
)
|
||||
declarations.append((entry, local))
|
||||
if local and local not in resolved:
|
||||
resolved.append(local)
|
||||
if len(resolved) > 1 and not _same_file_family(resolved):
|
||||
@@ -730,7 +734,9 @@ def find_collisions(
|
||||
# also names the first one's file.
|
||||
if any(_accepted_by_all(path, entries, db) for path in resolved):
|
||||
continue
|
||||
collisions.append(Collision(destination=key, resolved=resolved))
|
||||
collisions.append(
|
||||
Collision(destination=key, resolved=resolved, declarations=declarations)
|
||||
)
|
||||
return collisions
|
||||
|
||||
|
||||
|
||||
+117
-1
@@ -202,17 +202,23 @@ class TheRunReportStatesWhatTheFileCarries(unittest.TestCase):
|
||||
{"name": "a.bin", "destination": "a.bin", "native_system": "psx",
|
||||
"md5": "a" * 32},
|
||||
]}}}
|
||||
doubled = {"destination": "a.bin", "declarations": []}
|
||||
with tempfile.TemporaryDirectory(dir=ROOT / "tmp") as scratch, \
|
||||
mock.patch.object(export_native, "load_inputs",
|
||||
return_value=(truth, scraped)), \
|
||||
mock.patch.object(export_native, "unpinned", return_value=[]), \
|
||||
mock.patch.object(export_native, "removal_candidates", return_value=[]):
|
||||
mock.patch.object(export_native, "removal_candidates", return_value=[]), \
|
||||
mock.patch.object(export_native, "doubled_in", return_value=[doubled]):
|
||||
ok, messages = export_native.export_platform(
|
||||
"plat", Plain, Path(scratch), Path(scratch) / "out",
|
||||
str(ROOT / "platforms"), Path(scratch) / "cache", False,
|
||||
)
|
||||
self.assertTrue(ok, messages)
|
||||
report = json.loads((Path(scratch) / "out" / "plat.json").read_text())
|
||||
self.assertEqual(report["doubled"], [doubled])
|
||||
self.assertIn(
|
||||
"1 destination declared twice for two files, listed in the report", messages
|
||||
)
|
||||
self.assertEqual(
|
||||
report["summary"],
|
||||
"1 kept, 1 file added, 1 hash updated to the emulator source",
|
||||
@@ -542,5 +548,115 @@ class RemovalCandidatesNeedAReadingNoCoreAnswers(unittest.TestCase):
|
||||
self.assertIn('"removal_candidates": candidates,', body)
|
||||
|
||||
|
||||
class ADestinationDeclaredTwiceIsNamed(unittest.TestCase):
|
||||
"""RetroDECK aims a PC-88 disk ROM and a CoCo disk ROM at one
|
||||
bios/disk.rom, each with its own md5. One path holds one file, so one
|
||||
check fails on every install. The export keeps both entries and names
|
||||
the contradiction, with the emulators that read each file."""
|
||||
|
||||
PC88 = b"\x01" * 2048
|
||||
COCO = b"\x02" * 8194
|
||||
|
||||
def setUp(self):
|
||||
import hashlib # noqa: PLC0415
|
||||
|
||||
self._tmp = tempfile.TemporaryDirectory(dir=ROOT / "tmp")
|
||||
root = Path(self._tmp.name)
|
||||
files, by_md5, by_name = {}, {}, {}
|
||||
self.md5 = {}
|
||||
for label, data, rel in (
|
||||
("pc88", self.PC88, "bios/NEC/PC-88/N88SUB.ROM"),
|
||||
("coco", self.COCO, "bios/Tandy/CoCo/disk.rom"),
|
||||
):
|
||||
path = root / rel
|
||||
path.parent.mkdir(parents=True)
|
||||
path.write_bytes(data)
|
||||
sha1 = hashlib.sha1(data).hexdigest()
|
||||
md5 = hashlib.md5(data).hexdigest()
|
||||
self.md5[label] = md5
|
||||
files[sha1] = {"path": str(path), "name": path.name, "md5": md5,
|
||||
"sha1": sha1, "crc32": "", "size": len(data)}
|
||||
by_md5[md5] = sha1
|
||||
by_name.setdefault(path.name, []).append(sha1)
|
||||
self.db = {"files": files, "indexes": {
|
||||
"by_md5": by_md5, "by_name": by_name, "by_crc32": {}, "by_path_suffix": {},
|
||||
}}
|
||||
self.scraped = {"cores": ["pcemu", "cocoemu"], "systems": {
|
||||
"nec-pc-88": {"files": [
|
||||
{"name": "disk.rom", "destination": "bios/disk.rom",
|
||||
"md5": self.md5["pc88"], "native_system": "pc88",
|
||||
"component": "retroarch"},
|
||||
]},
|
||||
"coco": {"files": [
|
||||
{"name": "disk.rom", "destination": "bios/disk.rom",
|
||||
"md5": self.md5["coco"], "native_system": "coco",
|
||||
"component": "xroar"},
|
||||
]},
|
||||
}}
|
||||
self.profiles = {
|
||||
"pcemu": {"emulator": "PC Emu", "type": "libretro", "systems": ["nec-pc-88"],
|
||||
"files": [{"name": "n88sub.rom", "aliases": ["disk.rom"],
|
||||
"size": 2048, "source_ref": "libretro.c:120"}]},
|
||||
"cocoemu": {"emulator": "CoCo Emu", "type": "libretro", "systems": ["coco"],
|
||||
"files": [{"name": "disk11.rom", "source_ref": "xroar.c:714"}]},
|
||||
}
|
||||
|
||||
def tearDown(self):
|
||||
self._tmp.cleanup()
|
||||
|
||||
def _doubled(self) -> list[dict]:
|
||||
from exporter.doubled import doubled_destinations # noqa: PLC0415
|
||||
|
||||
return doubled_destinations(self.scraped, self.profiles, self.db)
|
||||
|
||||
def test_each_declaration_is_named_with_its_reader(self):
|
||||
doubled = self._doubled()
|
||||
self.assertEqual([d["destination"] for d in doubled], ["bios/disk.rom"])
|
||||
pc88, coco = doubled[0]["declarations"]
|
||||
self.assertEqual((pc88["system"], pc88["component"]), ("pc88", "retroarch"))
|
||||
self.assertEqual(pc88["hashes"], {"md5": [self.md5["pc88"]]})
|
||||
self.assertTrue(pc88["file"].endswith("bios/NEC/PC-88/N88SUB.ROM"))
|
||||
self.assertEqual(pc88["readers"], [{
|
||||
"emulator": "PC Emu", "profile": "pcemu", "entry": "n88sub.rom",
|
||||
"source_ref": "libretro.c:120",
|
||||
}])
|
||||
# A size the profile states rules out the CoCo ROM, and XRoar's own
|
||||
# profile never opens disk.rom.
|
||||
self.assertEqual((coco["system"], coco["component"]), ("coco", "xroar"))
|
||||
self.assertEqual(coco["hashes"], {"md5": [self.md5["coco"]]})
|
||||
self.assertEqual(coco["readers"], [])
|
||||
|
||||
def test_the_platform_entries_are_left_as_written(self):
|
||||
import copy # noqa: PLC0415
|
||||
|
||||
before = copy.deepcopy(self.scraped)
|
||||
self._doubled()
|
||||
self.assertEqual(self.scraped, before)
|
||||
|
||||
def test_a_destination_both_hashes_accept_is_not_doubled(self):
|
||||
self.scraped["systems"]["coco"]["files"][0]["md5"] = (
|
||||
f"{self.md5['coco']},{self.md5['pc88']}"
|
||||
)
|
||||
self.assertEqual(self._doubled(), [])
|
||||
|
||||
def test_the_run_counts_it_and_the_report_carries_it(self):
|
||||
source = (ROOT / "scripts" / "export_native.py").read_text(encoding="utf-8")
|
||||
body = source[source.index("def export_platform("):source.index("def write_report(")]
|
||||
self.assertIn('"doubled": doubled,', body)
|
||||
self.assertIn("declared twice for two files", body)
|
||||
|
||||
def test_the_page_names_each_declaration(self):
|
||||
page = generate_exports_page(
|
||||
[_report("retrodeck", ["a.json"], doubled=self._doubled())], {})
|
||||
self.assertIn("1 destination of the original is declared twice", page)
|
||||
self.assertIn("- `bios/disk.rom`", page)
|
||||
self.assertIn(
|
||||
f" - `pc88` (`retroarch`) md5 `{self.md5['pc88']}`: read by PC Emu "
|
||||
"(`libretro.c:120`)", page)
|
||||
self.assertIn(
|
||||
f" - `coco` (`xroar`) md5 `{self.md5['coco']}`: no emulator this "
|
||||
"platform runs is known to read this file", page)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -350,6 +350,14 @@ profile (`common.unresolved_platform_cores`) or when the entry is a ROM set
|
||||
and a core of the system opens sets its profile does not list. BizHawk and
|
||||
MiSTer resolve no core, so nothing is examined and nothing is listed.
|
||||
|
||||
A destination the platform's own list declares twice, for two different
|
||||
files, is written under `doubled` (`slots.find_collisions` finds it). One
|
||||
path holds one file, so one of the two checks fails on every install.
|
||||
Each declaration carries its system, component, hashes and the file it
|
||||
resolves to, with the emulators the platform runs whose profile reads
|
||||
that file: a member list it holds, a hash it matches, or a name and a
|
||||
size it fits. Both entries stay in the updated file.
|
||||
|
||||
MAME and FBNeo entries are `.zip` ROM sets: their meaningful hashes belong
|
||||
to the files inside the archive, so the exported DAT lists those entries
|
||||
without a container sha1. Anyone submitting the DAT upstream should mention
|
||||
|
||||
Reference in new issue
Block a user