feat: record each export change before and after

The page named the entries a change touched and never the values, so
a maintainer could not check a correction. The report now carries
changes, one object per change with kind, entry, field, old and new,
read from the platform entry and from the value the render writes
(written() per format: record value for System.dat and RomM, one md5
where a single one is checked). The existing label lists are derived
from it. The page writes "md5 X becomes Y" and names each required
flag with its new value. A test parses the original and updated
System.dat and es_bios.xml and checks the list equals the diff.
This commit is contained in:
Abdessamad Derraz committed 2026-10-20 20:00:54 +02:00
1 parent 5cba005d67
commit 545e3f3ba1
9 files changed
+382 -28

No files matched your search

+111 -26
View File
@@ -429,26 +429,89 @@ def refresh_cache(
return True, [f"{len(wanted)} file(s) fetched"]
@dataclass
class LandedChanges:
"""What an export changed in the platform's file, field by field."""
applied: list[str] = field(default_factory=list)
filled: list[str] = field(default_factory=list)
resized: list[str] = field(default_factory=list)
unsized: list[str] = field(default_factory=list)
cleared: list[str] = field(default_factory=list)
requirements: int = 0
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")
return f"{number} {noun if number == 1 else plural}"
@dataclass
class LandedChanges:
"""What an export changed in the platform's file, field by field.
`records` holds each change with its value before and after, so a
maintainer can check it against the diff. The lists of labels are read
from it, for the summary and the page.
"""
records: list[dict] = field(default_factory=list)
def _labels(self, kind: str) -> list[str]:
return [
f"{r['system']}/{r['name']} {r['field']}"
for r in self.records if r["kind"] == kind
]
@property
def applied(self) -> list[str]:
return self._labels("hash_corrected")
@property
def filled(self) -> list[str]:
return self._labels("hash_filled")
@property
def cleared(self) -> list[str]:
return self._labels("hash_cleared")
@property
def resized(self) -> list[str]:
return [label.removesuffix(" size") for label in self._labels("size_corrected")]
@property
def unsized(self) -> list[str]:
return [label.removesuffix(" size") for label in self._labels("size_removed")]
@property
def requirements(self) -> int:
return sum(r["kind"] == "requirement_corrected" for r in self.records)
def _change(
kind: str, entry: object, field_name: str | None, old: object, new: object
) -> dict:
return {
"kind": kind,
"system": entry.native_system,
"name": entry.name,
"destination": entry.destination,
"field": field_name,
"old": old,
"new": new,
}
def _platform_value(entry: object, field_name: str) -> str:
return ",".join(_hash_values(entry.platform or {}, field_name))
def _added_fields(exporter: object, entry: object) -> dict:
"""The fields an addition is written with, as the render chooses them."""
carried = exporter.carries()
written: dict[str, object] = {
name: value
for name in HASH_FIELDS
if name in carried and (value := exporter.written(entry, name))
}
if "size" in carried and entry.size() is not None:
written["size"] = entry.size()
if "required" in carried:
written["required"] = entry.required
return written
def landed_changes(exporter: object, systems: dict) -> LandedChanges:
"""The changes the written file carries.
"""The changes the written file carries, with their values.
Only what the format states: a correction to a field the file has no
place for, or one its render keeps out, is not a change the maintainer
@@ -456,26 +519,39 @@ def landed_changes(exporter: object, systems: dict) -> LandedChanges:
did not do. A hash written where the platform left none changes the
check from existence to content, so it is counted too. The truth taking
over an entry also rewrites its size and drops the platform's hashes it
does not declare: both are in the diff.
does not declare: both are in the diff. An addition carries the fields
it is written with as its new value.
"""
changes = LandedChanges()
records = changes.records
for system in systems.values():
for entry in system.files:
label = f"{entry.native_system}/{entry.name}"
if entry.platform is None:
if entry.name and exporter.lands(entry):
records.append(_change(
"added", entry, None, None, _added_fields(exporter, entry)
))
continue
for field_name in entry.corrections:
if not exporter.states(entry, field_name):
continue
if field_name == "required":
changes.requirements += 1
records.append(_change(
"requirement_corrected", entry, "required",
bool(entry.platform.get("required")), entry.required,
))
else:
changes.applied.append(f"{label} {field_name}")
changes.filled.extend(
f"{label} {field_name}"
records.append(_change(
"hash_corrected", entry, field_name,
_platform_value(entry, field_name),
exporter.written(entry, field_name),
))
records.extend(
_change("hash_filled", entry, field_name, None,
exporter.written(entry, field_name))
for field_name in entry.filled
if exporter.fills(entry, field_name)
)
if entry.platform is None:
continue
theirs = entry.platform.get("size")
ours = entry.size()
if (
@@ -484,7 +560,9 @@ def landed_changes(exporter: object, systems: dict) -> LandedChanges:
and _single_size(theirs) not in (None, ours)
and exporter.states(entry, "size")
):
changes.resized.append(label)
records.append(_change(
"size_corrected", entry, "size", _single_size(theirs), ours
))
elif (
theirs
and ours is None
@@ -493,9 +571,12 @@ def landed_changes(exporter: object, systems: dict) -> LandedChanges:
):
# The truth took the entry over without a size of its own:
# System.dat's neocdz.zip line went out without size 953078.
changes.unsized.append(label)
changes.cleared.extend(
f"{label} {field_name}"
records.append(_change(
"size_removed", entry, "size", _single_size(theirs), None
))
records.extend(
_change("hash_cleared", entry, field_name,
_platform_value(entry, field_name), None)
for field_name in HASH_FIELDS
if _hash_values(entry.platform, field_name)
and not entry.hashes(field_name)
@@ -503,7 +584,10 @@ def landed_changes(exporter: object, systems: dict) -> LandedChanges:
)
# A value the render rewrote though the model reads it as the
# platform's own (RomM's sha1 with a leading space).
changes.applied.extend(getattr(exporter, "normalized", []))
records.extend(
_change("hash_corrected", entry, field_name, old, new)
for entry, field_name, old, new in getattr(exporter, "normalized_values", [])
)
return changes
@@ -661,6 +745,7 @@ def export_platform(
"not_added": refused,
"not_kept": lost,
},
"changes": changes.records,
"hashes_corrected": applied,
"hashes_filled": filled,
"sizes_corrected": resized,
+11
View File
@@ -180,6 +180,17 @@ class BaseExporter(ABC):
return ""
return fe.hash(field_name)
def written(self, fe: NativeFile, field_name: str) -> str:
"""The value the render writes for one hash field, comma-joined.
The report gives each change its value before and after, so it reads
the value the way the render chooses it: one md5 where the format
checks one, every accepted value where it lists them.
"""
if field_name == self.SINGLE_HASH_FIELD:
return self.single_value(fe, field_name)
return ",".join(fe.hashes(field_name))
def states(self, fe: NativeFile, field_name: str) -> bool:
"""Whether the written file carries this entry's corrected field.
+4
View File
@@ -64,6 +64,10 @@ class Exporter(BaseExporter):
return False
return super().states(fe, field_name)
def written(self, fe: NativeFile, field_name: str) -> str:
"""An entry is checked against one md5, the first accepted."""
return fe.hash(field_name)
def _by_path(self, systems: dict[str, NativeSystem]) -> dict[str, object]:
indexed: dict[str, object] = {}
for system in systems.values():
+6
View File
@@ -90,6 +90,12 @@ class Exporter(BaseExporter):
return False
return super().writable(fe, require)
def written(self, fe: NativeFile, field_name: str) -> str:
"""md5 is a list of every accepted dump, sha256 a single value."""
if field_name == "sha256":
return fe.hash(field_name)
return super().written(fe, field_name)
def states(self, fe: NativeFile, field_name: str) -> bool:
if id(fe) in getattr(self, "_appended", ()):
return False
+6
View File
@@ -113,6 +113,7 @@ class Exporter(BaseExporter):
}
self._first_addition = {}
self.normalized: list[str] = []
self.normalized_values: list[tuple[NativeFile, str, str, str]] = []
for system in sorted(systems.values(), key=lambda s: s.native_id):
for fe in sorted(system.files, key=lambda f: f.name):
key = (fe.native_system, fe.name)
@@ -159,6 +160,10 @@ class Exporter(BaseExporter):
does not keep."""
return bool(_hash_values(fe.platform or {}, name)) and not fe.hashes(name)
def written(self, fe: NativeFile, field_name: str) -> str:
"""A record holds one value per field, the one record_hash names."""
return fe.record_hash(field_name)
def fills(self, fe: NativeFile, field_name: str) -> bool:
return self.states(fe, field_name) and self.record_fills(fe, field_name)
@@ -179,6 +184,7 @@ class Exporter(BaseExporter):
and name not in fe.corrections
):
self.normalized.append(f"{fe.native_system}/{fe.name} {name}")
self.normalized_values.append((fe, name, str(theirs), value))
entry[field] = value
def validate(
+4
View File
@@ -154,6 +154,10 @@ class Exporter(BaseExporter):
)
return lines
def written(self, fe: NativeFile, field_name: str) -> str:
"""A record holds one value per field, the one record_hash names."""
return fe.record_hash(field_name)
def fills(self, fe: NativeFile, field_name: str) -> bool:
return self.states(fe, field_name) and self.record_fills(fe, field_name)
+57 -2
View File
@@ -27,6 +27,60 @@ CHANGE_KINDS = (
)
# Report key, then the kind each line of `changes` carries.
CHANGE_RECORDS = {
"hashes_corrected": "hash_corrected",
"hashes_filled": "hash_filled",
"sizes_corrected": "size_corrected",
"sizes_removed": "size_removed",
"hashes_cleared": "hash_cleared",
}
def _value(value: object) -> str:
return f"`{value}`"
def describe_change(change: dict) -> str:
"""One change in words, with the value before and after it."""
label = f"`{change['system']}/{change['name']}` {change['field']}"
kind, old, new = change["kind"], change["old"], change["new"]
if kind in ("hash_corrected", "size_corrected"):
return f"{label} {_value(old)} becomes {_value(new)}"
if kind == "hash_filled":
return f"{label} {_value(new)} added"
if kind == "requirement_corrected":
return f"`{change['system']}/{change['name']}` becomes {'required' if new else 'optional'}"
return f"{label} {_value(old)} no longer listed"
def change_items(report: dict, key: str) -> list[str]:
"""The lines a kind of change shows. A report written before values
were recorded names the entries only."""
kind = CHANGE_RECORDS.get(key)
changes = report.get("changes")
if kind and changes is not None:
return [describe_change(c) for c in changes if c["kind"] == kind]
items = report.get(key) or []
return [str(i) if key == "withheld" else f"`{i}`" for i in items]
def requirement_lines(report: dict) -> list[str]:
"""Each required flag aligned with the code, and what it is now."""
items = [
describe_change(c) for c in report.get("changes") or []
if c["kind"] == "requirement_corrected"
]
if not items:
return []
return [
f'??? note "Required flags aligned with the code ({len(items)})"',
"",
*[f" - {item}" for item in items],
"",
]
def _plural(count: int, noun: str, plural: str = "") -> str:
return f"{count:,} {noun if count == 1 else plural or noun + 's'}"
@@ -241,14 +295,15 @@ def generate_exports_page(reports: list[dict], names: dict[str, str]) -> str:
(name, url), = originals.items()
lines.extend([f"Based on [`{name}`]({url}).", ""])
lines.extend(requirement_lines(report))
for key, title in CHANGE_KINDS:
items = report.get(key) or []
items = change_items(report, key)
if not items:
continue
lines.extend([
f'??? note "{title} ({len(items)})"',
"",
*[f" - {item}" if key == "withheld" else f" - `{item}`" for item in items],
*[f" - {item}" for item in items],
"",
])
return "\n".join(lines)
+177
View File
@@ -8,6 +8,7 @@ file from and no list to check the diff against.
from __future__ import annotations
import json
import re
import sys
import tempfile
import unittest
@@ -157,6 +158,182 @@ class ThePageAddressesMaintainersAsPeers(unittest.TestCase):
self.assertNotIn(word, page)
UPSTREAM_CACHE = ROOT / ".cache" / "upstream-native"
_DAT_ROM = re.compile(r"^\trom \( name (\"[^\"]+\"|\S+) (.*) \)$")
_DAT_FIELDS = {"size": "size", "crc": "crc32", "md5": "md5", "sha1": "sha1"}
_HASH_KINDS = {
"hash_corrected", "hash_filled", "hash_cleared", "size_corrected", "size_removed",
}
def _norm(value: object) -> str | None:
"""A value as the frontend reads it: a list split on commas, each value
stripped, so the layout of a wrapped attribute is not a change."""
if value in (None, ""):
return None
return ",".join(v.strip() for v in str(value).casefold().split(","))
def _parse_dat(text: str) -> dict[tuple[str, str], dict[str, str]]:
"""Each rom line of a System.dat, keyed by its section and rom name."""
roms: dict[tuple[str, str], dict[str, str]] = {}
section = ""
for line in text.splitlines():
if line.startswith('\tcomment "'):
section = line.split('"')[1]
continue
match = _DAT_ROM.match(line)
if not match:
continue
tokens = match.group(2).split()
fields = {
_DAT_FIELDS[tokens[i]]: tokens[i + 1]
for i in range(0, len(tokens) - 1, 2) if tokens[i] in _DAT_FIELDS
}
roms.setdefault((section, match.group(1).strip('"')), fields)
return roms
def _parse_es_bios(text: str) -> dict[tuple[str, str], dict[str, str]]:
"""Each bios element of an es_bios.xml, keyed by system and path."""
from common import parse_untrusted_xml
root = parse_untrusted_xml(text)
elements: dict[tuple[str, str], dict[str, str]] = {}
for system in root.iter("system"):
for bios in system.iter("bios"):
elements.setdefault((system.get("platform", ""), bios.get("path", "")), {
"md5": bios.get("md5", ""),
"required": "false" if bios.get("mandatory") == "false" else "true",
})
return elements
class ReportedChangesAreTheDiff(unittest.TestCase):
"""The report gives each change its value before and after. Read back
from the original and the updated file, the two must agree entry by
entry, or a maintainer checks the diff against a list that differs."""
def _run(self, platform: str, filename: str) -> tuple[list[dict], str, str, dict]:
from common import load_platform_config, native_layer
from exporter import discover_exporters
from exporter.baseline import build_native_model
original_path = UPSTREAM_CACHE / platform / filename
if not original_path.is_file():
self.skipTest(f"no cached original for {platform}")
config = load_platform_config(platform, str(ROOT / "platforms"))
truth = export_native.current_truth(platform, config, str(ROOT / "platforms"))
scraped = native_layer(config)
systems, report = build_native_model(truth, scraped)
exporter = discover_exporters()[platform]()
original = original_path.read_text(encoding="utf-8")
originals = {
p.name: p.read_text(encoding="utf-8")
for p in (UPSTREAM_CACHE / platform).iterdir() if p.is_file()
}
produced = exporter.render(systems, report, originals, scraped)[filename]
changes = export_native.landed_changes(exporter, systems).records
return changes, original, produced, {"exporter": exporter, "systems": systems}
@staticmethod
def _diff(before: dict, after: dict) -> tuple[set, set]:
changed = {
(key, name, _norm(before[key].get(name)), _norm(after[key].get(name)))
for key in before.keys() & after.keys()
for name in before[key].keys() | after[key].keys()
if _norm(before[key].get(name)) != _norm(after[key].get(name))
}
return changed, set(after) - set(before)
def test_system_dat(self):
changes, original, produced, ctx = self._run("retroarch", "System.dat")
exporter = ctx["exporter"]
rom_name = {
(fe.native_system, fe.name, fe.destination): exporter._rom_name(fe)
for system in ctx["systems"].values() for fe in system.files
}
changed, added = self._diff(_parse_dat(original), _parse_dat(produced))
reported = {
((c["system"], rom_name[(c["system"], c["name"], c["destination"])]),
c["field"], _norm(c["old"]), _norm(c["new"]))
for c in changes if c["kind"] in _HASH_KINDS
}
self.assertTrue(changed, "nothing changed, the comparison proves nothing")
self.assertEqual(reported, changed)
reported_added = {
(c["system"], rom_name[(c["system"], c["name"], c["destination"])])
for c in changes if c["kind"] == "added"
}
self.assertEqual(reported_added, added)
def test_recalbox(self):
changes, original, produced, ctx = self._run("recalbox", "es_bios.xml")
exporter = ctx["exporter"]
path_of = {
(fe.native_system, fe.name, fe.destination): exporter._path(fe)
for system in ctx["systems"].values() for fe in system.files
}
changed, added = self._diff(_parse_es_bios(original), _parse_es_bios(produced))
reported = {
((c["system"], path_of[(c["system"], c["name"], c["destination"])]),
c["field"], _norm(c["old"]), _norm(c["new"]))
for c in changes
if c["kind"] in _HASH_KINDS or c["kind"] == "requirement_corrected"
}
self.assertTrue(changed, "nothing changed, the comparison proves nothing")
self.assertEqual(reported, changed)
reported_added = {
(c["system"], path_of[(c["system"], c["name"], c["destination"])])
for c in changes if c["kind"] == "added"
}
self.assertEqual(reported_added, added)
class ThePageShowsEachValueBeforeAndAfter(unittest.TestCase):
"""A list of names gave a maintainer nothing to check a correction
against: each line now says what the value was and what it becomes."""
def setUp(self):
def change(kind: str, field: str | None, old: object, new: object) -> dict:
return {"kind": kind, "system": "psx", "name": "scph5501.bin",
"destination": "scph5501.bin", "field": field,
"old": old, "new": new}
self.report = _report(
"recalbox", ["es_bios.xml"],
hashes_corrected=["psx/scph5501.bin md5"], requirements_corrected=1,
changes=[
change("hash_corrected", "md5", "5626" + "0" * 28, "c523" + "1" * 28),
change("requirement_corrected", "required", True, False),
change("hash_filled", "sha1", None, "f" * 40),
],
)
self.page = generate_exports_page([self.report], {})
def test_a_corrected_hash_shows_both_values(self):
self.assertIn(
f" - `psx/scph5501.bin` md5 `5626{'0' * 28}` becomes `c523{'1' * 28}`",
self.page,
)
def test_a_requirement_names_the_entry_and_its_value(self):
self.assertIn('??? note "Required flags aligned with the code (1)"', self.page)
self.assertIn(" - `psx/scph5501.bin` becomes optional", self.page)
def test_a_filled_hash_shows_its_value(self):
self.assertIn(f" - `psx/scph5501.bin` sha1 `{'f' * 40}` added", self.page)
def test_no_arrow_glyph(self):
for glyph in (chr(0x2192), "->", "=>"):
self.assertNotIn(glyph, self.page)
def test_the_report_carries_every_change(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('"changes": changes.records,', body)
class RemovalCandidatesNeedAReadingNoCoreAnswers(unittest.TestCase):
"""An entry the truth does not pair can still be read: by an alias, by its
content, under a directory a core walks. Of 2776 such entries, 1189
+6
View File
@@ -271,6 +271,12 @@ state is reported rather than written: EmuDeck's arrays are shared
between emulators its file does not name, so a hash is corrected in place
and never added.
`exports/<platform>.json` lists every change under `changes`, each with
its kind, entry, field and the value before and after (an addition carries
the fields it is written with). The exports page shows both values, and a
test reads the original and the updated System.dat and es_bios.xml back to
check that the list is the diff.
No entry the platform declares is removed. One the truth does not pair is
looked for in the profiles of every core the platform runs: by name or
alias, by the basename or tail of a `path:`, by any hash (archive members