diff --git a/scripts/export_native.py b/scripts/export_native.py index 291a3cfe..dea289a2 100644 --- a/scripts/export_native.py +++ b/scripts/export_native.py @@ -513,7 +513,8 @@ def export_platform( # it, so their own file loses a line. Worth saying out loud. lost += 1 - summary = exporter.outcome(systems, produced) + own_summary = exporter.outcome(systems, produced) + summary = own_summary if summary is None: summary = ( f"{report.files_kept} kept, {landed} added, " @@ -531,7 +532,7 @@ def export_platform( if refused: summary += f", {_count(refused, 'addition')} left out" if lost: - summary += f", {_count(lost, 'entry', 'entries')} of theirs the format drops" + summary += f", {_count(lost, 'platform entry', 'platform entries')} the format cannot keep" withheld = exporter.withheld() if withheld: summary += f", {_count(len(withheld), 'correction')} withheld" @@ -567,6 +568,12 @@ def export_platform( "originals": wanted_sources(exporter, systems, scraped), "files": sorted(produced), "summary": summary, + "counts": None if own_summary is not None else { + "kept": report.files_kept, + "added": landed, + "not_added": refused, + "not_kept": lost, + }, "hashes_corrected": applied, "hashes_filled": filled, "sizes_corrected": resized, @@ -650,7 +657,7 @@ def main() -> None: group.add_argument("--platform", type=platform_arg, help="a single platform") parser.add_argument( "--output-dir", default="exports", - help="directory the corrected platform files go to (default: %(default)s)", + help="directory the updated platform files go to (default: %(default)s)", ) parser.add_argument( "--truth-dir", default="dist/truth", diff --git a/scripts/exporter/retropie_exporter.py b/scripts/exporter/retropie_exporter.py index ae8d7d94..edf2309c 100644 --- a/scripts/exporter/retropie_exporter.py +++ b/scripts/exporter/retropie_exporter.py @@ -303,14 +303,14 @@ class Exporter(BaseExporter): core = index.get(module_id.removeprefix("lr-").replace("-", "_").lower()) files = by_core.get(core or "", []) if not files: - skip("no core of ours packages it", module_id) + skip("which no profiled core matches", module_id) continue listed = self._listed(help_text) unknown = sorted(listed - known) if unknown: skip( - f"names a file we have never seen ({', '.join(unknown)})", + f"whose help text names a file no profile documents yet ({', '.join(unknown)})", module_id, ) @@ -341,11 +341,11 @@ class Exporter(BaseExporter): if enumeration is None: # No enumeration to extend, and a sentence we would have to # write ourselves is a documentation change, not a correction. - skip("names no file to extend", module_id) + skip("whose help text lists no files to add to", module_id) continue if len(listed) + len(missing) > MAX_NAMES: - skip("more names than the help enumerates", module_id) + skip("whose help text would grow longer than any it has today", module_id) continue new_help = self._extended(help_text, enumeration, missing) @@ -366,12 +366,12 @@ class Exporter(BaseExporter): ) -> str: """Packages are the unit here, not file entries.""" skipped: dict[str, list[str]] = getattr(self, "_skipped", {}) - parts = [f"{len(produced)} packages corrected"] + parts = [f"{len(produced)} packages updated"] for reason, modules in sorted(skipped.items()): shown = ", ".join(sorted(modules)[:4]) if len(modules) > 4: shown += f" and {len(modules) - 4} more" - parts.append(f"{shown}: {reason}") + parts.append(f"Left as is: {shown}, {reason}") return ". ".join(parts) def validate( diff --git a/scripts/generate_site.py b/scripts/generate_site.py index c3f20048..219d61c9 100644 --- a/scripts/generate_site.py +++ b/scripts/generate_site.py @@ -1251,7 +1251,7 @@ def _page_description(markdown: str, relative: Path, title: str) -> str: "data.md": "Versioned RetroBIOS JSON API, CSV exports, SQLite snapshot, schemas, checksums and data interpretation guidance.", "cross-reference.md": "Cross-reference from retrogaming platforms to emulator cores, systems, upstream projects and profiled firmware files.", "gaps.md": "Current RetroBIOS verification gaps, platform-to-emulator divergences, missing files and documented source limitations.", - "exports.md": "Each platform's own BIOS list (System.dat, batocera-systems, es_bios.xml and the rest) rewritten with the hashes and files the emulator source code proves.", + "exports.md": "Each platform's own BIOS list (System.dat, batocera-systems, es_bios.xml and the rest) with the hashes and files the emulator source code shows, offered to its maintainers.", "provenance.md": "Hash-based comparison of RetroBIOS metadata with No-Intro, Redump and TOSEC catalog snapshots.", "which-pack.md": "One-line automatic RetroBIOS installation and platform-specific pack destinations, with verification and release caveats.", } @@ -1899,7 +1899,7 @@ def generate_platform_page( note = " (a branch: the revision read is not recorded)" if moving else "" lines.append(f"| Transcribed from | <{transcribed}>{note} |") if name in _EXPORTED: - lines.append(f"| Corrected file | [Download it and see what changed](../exports.md#{name}) |") + lines.append(f"| Updated file | [Download it and see the changes](../exports.md#{name}) |") contrib_list = (registry or {}).get(name, {}).get("contributed_by", []) if contrib_list: for cb in contrib_list: @@ -4683,7 +4683,7 @@ def generate_mkdocs_nav( {"Cross-reference": "cross-reference.md"}, {"Gap analysis": "gaps.md"}, {"Dump provenance": "provenance.md"}, - {"Corrected platform files": "exports.md"}, + {"Updated platform files": "exports.md"}, {"Data & API": "data.md"}, {"Wiki": wiki_nav}, {"Contributing": "contributing.md"}, @@ -4924,7 +4924,7 @@ def _generate(args: argparse.Namespace) -> None: ), ) - print("Generating corrected platform files page...") + print("Generating updated platform files page...") write_if_changed( str(docs / "exports.md"), generate_exports_page( diff --git a/scripts/siteexports.py b/scripts/siteexports.py index 85b88a52..7f47287a 100644 --- a/scripts/siteexports.py +++ b/scripts/siteexports.py @@ -2,7 +2,7 @@ export_native.py writes the platform's own file, corrected, under exports// and what it changed beside it as exports/.json. -This page reads those reports: the reader finds their platform, the file its +This page reads those reports: the reader finds a platform, the file its maintainers keep, the corrected copy, and every change named. """ @@ -18,15 +18,42 @@ REPO_RAW = "https://raw.githubusercontent.com/Abdess/retrobios/main" # Report key, then how a line of it reads on the page. CHANGE_KINDS = ( - ("hashes_corrected", "Hashes corrected"), - ("hashes_filled", "Hashes filled where the platform had none"), - ("sizes_corrected", "Sizes corrected"), - ("sizes_removed", "Sizes removed"), - ("hashes_cleared", "Hashes cleared"), - ("withheld", "Corrections withheld"), + ("hashes_corrected", "Hashes updated to the emulator source"), + ("hashes_filled", "Hashes added where the original lists none"), + ("sizes_corrected", "Sizes updated to the emulator source"), + ("sizes_removed", "Sizes no longer stated"), + ("hashes_cleared", "Hashes no longer listed"), + ("withheld", "Changes not applied"), ) +def _plural(count: int, noun: str, plural: str = "") -> str: + return f"{count:,} {noun if count == 1 else plural or noun + 's'}" + + +def change_text(report: dict) -> str: + """What the updated file changes, from the report's numbers. + + The command line prints its own terse summary. The page states each + kind of change once, in plain terms, and leaves out what did not + happen. RetroPie's unit is a package, so its own sentence is kept. + """ + counts = report.get("counts") + if not counts: + return report["summary"] + parts = [f"{_plural(counts['kept'], 'entry', 'entries')} kept"] + extra = [ + (counts["added"], "file", "files", "added"), + (len(report.get("hashes_corrected") or []), "hash", "hashes", "updated"), + (report.get("requirements_corrected") or 0, "required flag", "required flags", + "aligned with the code"), + (len(report.get("hashes_filled") or []), "hash", "hashes", "added"), + (len(report.get("sizes_corrected") or []), "size", "sizes", "updated"), + ] + parts += [f"{_plural(n, one, many)} {verb}" for n, one, many, verb in extra if n] + return ", ".join(parts) + + def load_export_reports(directory: str = EXPORTS_DIR) -> list[dict]: """Every platform report the export wrote, by platform name.""" root = Path(directory) @@ -61,38 +88,44 @@ def _original_link(report: dict) -> str: def generate_exports_page(reports: list[dict], names: dict[str, str]) -> str: """The page, names mapping each platform key to the name it displays.""" lines = [ - "# Corrected platform files", + "# Updated platform files", "", "Each platform keeps its own list of the BIOS files it checks: " "libretro's `System.dat`, Batocera's `batocera-systems`, Recalbox's " - "`es_bios.xml` and so on. Those lists are written by hand and drift " - "from what the emulators actually read. Here is each platform's own " - "file again, with what the emulator source code proves applied. A " - "wrong hash is corrected, a file the platform's emulators need and " - "the list lacks is added, and nothing the platform declares is ever " - "removed. Code the file carries (a checker script, a C# database, a " - "shell library) is patched in place and keeps working.", + "`es_bios.xml` and so on. Keeping such a list in step with every " + "emulator release is a lot of work. This project reads the emulators' " + "source code file by file, and offers each platform its own file with " + "those findings applied: a hash is updated where the emulator source " + "states another, files the platform's emulators read are added, and " + "nothing the platform declares is removed. Code the file carries (a " + "checker script, a C# database, a shell library) is patched in place " + "and keeps working.", "", - "Each file is taken at the revision our data was transcribed from, " - "so a maintainer can diff it against theirs and take the changes " - "line by line.", + "Each file starts from the revision of the original that this project " + "last read, so it compares cleanly with it, and every change can be " + "taken or left on its own. Each value traces back to the emulator code " + "cited on its profile page. If a change does not fit a platform, an " + "issue on the [repository](https://github.com/Abdess/retrobios/issues) " + "is welcome: this project's profile is revised, and every platform's " + "file follows.", "", - "A file the emulators read and the list lacks is added only when the " - "format can carry it. Most platforms check an md5, so a file whose " - "source states no md5 is left out, and so is one filed under a system " - "the platform's checker never reads. Those are counted as left out.", + "A file is added only when the format can carry it. A platform that " + "checks an md5 needs one, so a file whose emulator source states no " + "md5 is not added, and neither is a file under a system the " + "platform's checker does not read. Those are counted on each " + "platform's section.", "", ] if not reports: lines.extend([ - "No corrected file has been published yet. They are written by " + "No updated file has been published yet. They are written by " "`python scripts/export_native.py --all --fetch`.", "", ]) return "\n".join(lines) lines.extend([ - "| Platform | Their file | Corrected file | What changed |", + "| Platform | Original file | Updated file | Changes |", "|----------|------------|----------------|--------------|", ]) for report in reports: @@ -100,17 +133,32 @@ def generate_exports_page(reports: list[dict], names: dict[str, str]) -> str: lines.append( f"| [{_display(platform, names)}](#{platform}) | " f"{_original_link(report)} | {_corrected_link(report)} | " - f"{report['summary']} |" + f"{change_text(report)} |" ) lines.append("") for report in reports: platform = report["platform"] lines.extend([f"## {_display(platform, names)} {{ #{platform} }}", ""]) - lines.extend([report["summary"][:1].upper() + report["summary"][1:] + ".", ""]) + text = change_text(report) + lines.extend([text[:1].upper() + text[1:] + ".", ""]) + counts = report.get("counts") or {} + if counts.get("not_added"): + lines.extend([ + f"{_plural(counts['not_added'], 'other file')} the emulators read " + f"{'is' if counts['not_added'] == 1 else 'are'} not added, for the " + "reasons above.", + "", + ]) + if counts.get("not_kept"): + lines.extend([ + f"{_plural(counts['not_kept'], 'entry', 'entries')} of the original " + "cannot be written in this format and should be checked by hand.", + "", + ]) if report.get("invalid"): lines.extend([ - '!!! warning "The corrected file fails the platform\'s own checks"', + '!!! warning "The updated file does not pass the platform\'s own checks"', "", *[f" - {issue}" for issue in report["invalid"]], "", @@ -121,26 +169,26 @@ def generate_exports_page(reports: list[dict], names: dict[str, str]) -> str: raw = [f"{REPO_RAW}/{EXPORTS_DIR}/{platform}/{name}" for name in files] if len(files) == 1: lines.extend([ - f"[Download the corrected file]({raw[0]}){{ .md-button .md-button--primary }} " + f"[Download the updated file]({raw[0]}){{ .md-button .md-button--primary }} " f"[See it on GitHub]({REPO_BLOB}/{EXPORTS_DIR}/{platform}/{files[0]}){{ .md-button }}", "", ]) else: lines.extend([ - f"[Browse the {len(files)} corrected files]" + f"[Browse the {len(files)} updated files]" f"({REPO_TREE}/{EXPORTS_DIR}/{platform}){{ .md-button .md-button--primary }}", "", ]) if len(originals) > 1: lines.extend([ - f'??? note "The {len(originals)} files this export patches"', + f'??? note "The {len(originals)} original files it is based on"', "", *[f" - [`{name}`]({url})" for name, url in sorted(originals.items())], "", ]) elif originals: (name, url), = originals.items() - lines.extend([f"Patched from [`{name}`]({url}).", ""]) + lines.extend([f"Based on [`{name}`]({url}).", ""]) for key, title in CHANGE_KINDS: items = report.get(key) or [] diff --git a/tests/test_export_report.py b/tests/test_export_report.py index 68886834..8277756c 100644 --- a/tests/test_export_report.py +++ b/tests/test_export_report.py @@ -83,10 +83,10 @@ class CorrectedFilesPage(unittest.TestCase): def test_several_files_link_to_their_directory(self): self.assertIn("tree/main/exports/retropie", self.page) - self.assertIn('??? note "The 2 files this export patches"', self.page) + self.assertIn('??? note "The 2 original files it is based on"', self.page) def test_every_change_is_listed(self): - self.assertIn('??? note "Hashes corrected (12)"', self.page) + self.assertIn('??? note "Hashes updated to the emulator source (12)"', self.page) for i in range(12): self.assertIn(f" - `sys/file{i}.bin md5`", self.page) @@ -99,7 +99,7 @@ class CorrectedFilesPage(unittest.TestCase): def test_site_has_the_page_in_its_nav(self): source = (ROOT / "scripts" / "generate_site.py").read_text(encoding="utf-8") - self.assertIn('{"Corrected platform files": "exports.md"}', source) + self.assertIn('{"Updated platform files": "exports.md"}', source) workflow = (ROOT / ".github" / "workflows" / "deploy-site.yml").read_text(encoding="utf-8") self.assertIn('"exports/**"', workflow) @@ -118,10 +118,35 @@ class SummariesReadAsSentences(unittest.TestCase): from exporter.retropie_exporter import Exporter as RetroPieExporter exporter = RetroPieExporter() - exporter._skipped = {"names no file to extend": ["fs-uae", "amiberry"]} + exporter._skipped = {"whose help text lists no files to add to": ["fs-uae", "amiberry"]} text = exporter.outcome({}, {"a.sh": ""}) self.assertNotIn(";", text) - self.assertIn("amiberry, fs-uae: names no file to extend", text) + self.assertIn("Left as is: amiberry, fs-uae, whose help text lists no files to add to", text) + + +class ThePageAddressesMaintainersAsPeers(unittest.TestCase): + """The page offers the platforms' own maintainers a file to take or + leave. "Their file", "corrected" and "of theirs" read as a verdict on + their work, which the page is not.""" + + def test_the_change_sentence_is_built_from_the_numbers(self): + from siteexports import change_text + + report = _report("batocera", ["batocera-systems"], + hashes_corrected=["a md5", "b md5"], requirements_corrected=3) + report["counts"] = {"kept": 692, "added": 253, "not_added": 10, "not_kept": 0} + self.assertEqual( + change_text(report), + "692 entries kept, 253 files added, 2 hashes updated, " + "3 required flags aligned with the code", + ) + + def test_no_wording_that_judges_the_original(self): + report = _report("batocera", ["batocera-systems"], hashes_corrected=["a md5"]) + report["counts"] = {"kept": 1, "added": 0, "not_added": 4, "not_kept": 1} + page = generate_exports_page([report], {}).lower() + for word in ("their file", "theirs", "corrected", "wrong", "drift", "left out"): + self.assertNotIn(word, page) if __name__ == "__main__": diff --git a/wiki/advanced-usage.md b/wiki/advanced-usage.md index 02b755f2..27b467ee 100644 --- a/wiki/advanced-usage.md +++ b/wiki/advanced-usage.md @@ -344,7 +344,7 @@ python scripts/export_native.py --all --fetch # every platform, i The corrected files are committed under `exports//`, and `exports/.json` names every change the run made. The site's -[corrected platform files](../exports.md) page is built from those reports: +[updated platform files](../exports.md) page is built from those reports: one row per platform, a download link, and each corrected hash, filled hash and withheld correction listed in full.