From 3129eaf10cb41347ece458731ab8ba8c33afd420 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 11 Nov 2026 11:03:41 +0100 Subject: [PATCH] feat: bind config launcher records on evidence A launcher record citing a config, option or feature file binds only when the platform's code installs the file (installs), its launch code was read for what replaces it (overrides), and the emulator's profile shows where the shipped build loads it and reads each key (config_reads, read at pin_build). Unproven records place no file and hand no directory. verify, cross_reference and the platform page list them apart, outside severity and --strict. --- schemas/emulator.schema.json | 66 +++++- schemas/platform.schema.json | 23 +++ scripts/common.py | 13 +- scripts/cross_reference.py | 47 ++++- scripts/generate_site.py | 32 ++- scripts/launchers.py | 319 ++++++++++++++++++++++++++++- scripts/profile_sync.py | 97 +++++++++ scripts/scraper/shell_launchers.py | 185 +++++++++++++++++ scripts/validate_schemas.py | 56 +++++ scripts/verify.py | 20 +- tests/test_launcher_gaps.py | 7 +- tests/test_launcher_layer.py | 8 +- tests/test_launcher_placement.py | 11 + wiki/adding-a-platform.md | 16 +- 14 files changed, 872 insertions(+), 28 deletions(-) diff --git a/schemas/emulator.schema.json b/schemas/emulator.schema.json index 7de174331..7bb563009 100644 --- a/schemas/emulator.schema.json +++ b/schemas/emulator.schema.json @@ -11,15 +11,16 @@ ], "allOf": [ { - "comment": "A build reference documents why a pin is held: it never stands without the pin_frozen that says so.", + "comment": "A build reference documents why a pin is held, or the build config_reads are read at: it never stands without the pin_frozen or the config_reads that say so.", "if": { "required": [ "pin_build" ] }, "then": { - "required": [ - "pin_frozen" + "anyOf": [ + {"required": ["pin_frozen"]}, + {"required": ["config_reads"]} ] } }, @@ -1078,14 +1079,36 @@ "standalone_directories": { "type": "array", "items": { - "type": "string" + "oneOf": [ + {"type": "string"}, + { + "type": "object", + "additionalProperties": false, + "required": ["handle", "source_ref"], + "properties": { + "handle": {"type": "string", "minLength": 1}, + "source_ref": {"type": "string", "minLength": 1, "description": "Where the build reads the handle and resolves its files against it"} + } + } + ] }, "description": "What the standalone build resolves standalone_path against, as its code names it: a directory ($XDG_DATA_HOME/flycast), a setting (Dreamcast.BiosPath), an environment variable or the function returning the directory. A platform launcher record that links or passes a folder of its BIOS tree under one of these puts the standalone files in that folder." }, "standalone_pools": { "type": "object", "additionalProperties": { - "type": "string" + "oneOf": [ + {"type": "string"}, + { + "type": "object", + "additionalProperties": false, + "required": ["folder", "source_ref"], + "properties": { + "folder": {"type": "string"}, + "source_ref": {"type": "string", "minLength": 1, "description": "Where the build reads the setting as a search list"} + } + } + ] }, "description": "Settings the standalone build reads as a list of directories it searches one after the other, each mapped to the subfolder of the layout (relative to standalone_directories, empty for the whole of it) its directories stand for: openMSX's filepool entries are searched like share/systemroms, XRoar's rompath entries hold the ROMs themselves. A file in any entry is read, so several platform records naming entries give the first one a pack can fill." }, @@ -1101,7 +1124,7 @@ "pin_build": { "type": "array", "minItems": 1, - "description": "The build file each platform that ships a pin_frozen build reads its revision from, so check_freshness can read it again where the platform's YAML now stands and say when the platform builds another revision.", + "description": "The build file each platform that ships a pin_frozen build, or the build a config_reads entry is read at, reads its revision from, so check_freshness can read it again where the platform's YAML now stands and say when the platform builds another revision.", "items": { "type": "object", "additionalProperties": false, @@ -1135,6 +1158,37 @@ } } }, + "config_reads": { + "type": "array", + "description": "The configs the build loads and the keys it opens a file from, as a platform launcher record citing a config needs them: link (c) is where the file is found and loaded, link (d) where each key is read and its value opened. An entry naming platforms is read at their pin_build revision, one naming none at source_commit.", + "items": { + "type": "object", + "additionalProperties": false, + "required": ["file", "source_ref", "keys"], + "dependentRequired": {"revision": ["platforms", "revision_from"]}, + "properties": { + "file": {"type": "string", "minLength": 1, "description": "The config as the code finds it: a name, a glob for any file a flag passes (*.uae), a path, the flag itself, \"command line\" or \"core options\""}, + "platforms": {"type": "array", "items": {"type": "string"}, "minItems": 1, "uniqueItems": true, "description": "Registry slugs of the platforms shipping the build these refs are read at, each with a pin_build entry unless revision names the build"}, + "revision": {"type": "string", "pattern": "^[0-9a-f]{40}$", "description": "The commit these refs are read at, for platforms installing a package no build file of theirs pins (RetroBat's stable archives, a Flathub app)"}, + "revision_from": {"type": "string", "minLength": 1, "description": "Where revision was read: the version string of the shipped binary, the Flathub manifest commit"}, + "source_ref": {"type": "string", "minLength": 1}, + "keys": { + "type": "array", + "minItems": 1, + "items": { + "type": "object", + "additionalProperties": false, + "required": ["key", "source_ref"], + "properties": { + "key": {"oneOf": [{"type": "string", "minLength": 1}, {"type": "array", "items": {"type": "string", "minLength": 1}, "minItems": 1}], "description": "Spelled as a launcher record's passes_to spells it: [Section] Key, or Key"}, + "source_ref": {"type": "string", "minLength": 1}, + "condition": {"type": "string", "description": "What else the read depends on"} + } + } + } + } + } + }, "pin_follows": { "type": "string", "description": "Why source_commit follows the branch tip although a single platform ships the profile: that platform builds the tip, cited by its build file. Without it or pin_frozen, a bump refuses a profile one platform ships." diff --git a/schemas/platform.schema.json b/schemas/platform.schema.json index c0a972192..1b3fb9479 100644 --- a/schemas/platform.schema.json +++ b/schemas/platform.schema.json @@ -341,6 +341,29 @@ "type": "string", "description": "Devices whose shipped config this record reads, comma-separated, set only where the configs of a package differ between devices" }, + "installs": { + "type": "object", + "additionalProperties": false, + "required": ["path", "source_ref"], + "properties": { + "path": {"type": "string", "minLength": 1, "description": "Where the config the record cites lands on the device, as the platform's code spells it"}, + "source_ref": {"type": "string", "minLength": 1, "description": "path:lines of the code that puts it there"} + }, + "description": "How the config file a record cites reaches the emulator: link (a) of the evidence a config record carries" + }, + "kept_if_present": { + "type": "boolean", + "description": "True when the install leaves a file already there in place (cp if absent, rsync --ignore-existing): the record holds on a fresh install" + }, + "overrides": { + "type": "array", + "items": {"type": "string"}, + "description": "path:lines of launch code that rewrites the record's keys or replaces the installed config, empty when the launch path writes neither: link (b)" + }, + "choices_from": { + "type": "string", + "description": "path:lines of the feature or option file (es_features, a core .opt) listing the values the option the record writes takes. It is never the record's evidence: source_ref cites the code writing the choice" + }, "source_ref": { "type": "string", "description": "path:lines in the platform's repository at the pinned revision" diff --git a/scripts/common.py b/scripts/common.py index cb3e294d1..9ed339cb7 100644 --- a/scripts/common.py +++ b/scripts/common.py @@ -3189,11 +3189,12 @@ def standalone_bindings_for( ``systems`` are those the entry is launched for (entry_launch_systems). """ - declared = profile.get("standalone_directories") or [] - pools = profile.get("standalone_pools") or {} + from launchers import directory_handles, pool_folders, standalone_bindings # noqa: PLC0415 + + declared = directory_handles(profile) + pools = pool_folders(profile) if not declared and not pools: return {} - from launchers import standalone_bindings # noqa: PLC0415 names = {emu_name, *(str(c) for c in profile.get("cores") or [])} layout = [ @@ -3298,9 +3299,9 @@ def platform_layouts( # The layout folders a search setting stands for (standalone_pools): # bound, the launcher passes the setting and the default is not read. - pool_folders = { - str(v).strip("/") for v in (profile.get("standalone_pools") or {}).values() - } - {""} + from launchers import pool_folders as _pool_folders # noqa: PLC0415 + + pool_folders = {v.strip("/") for v in _pool_folders(profile).values()} - {""} def passed_setting(entry: dict) -> bool: layout = str(standalone_layout_path(entry, profile) or "") diff --git a/scripts/cross_reference.py b/scripts/cross_reference.py index 2f993d8c8..580bacd39 100644 --- a/scripts/cross_reference.py +++ b/scripts/cross_reference.py @@ -56,9 +56,11 @@ from common import ( from hashing import parse_hash_list from launchers import ( LauncherLayer, + bound_launchers, catalogued_model_files, declared_destinations, held_record, + launcher_unproven, name_settings, set_by, setting_readers, @@ -1095,6 +1097,8 @@ def _settle_launcher_places( # The --json key of the launcher reads category, beside the emulator keys. LAUNCHER_READS_KEY = "_launcher_reads_not_held" +# The --json key of the launcher records a config file does not prove. +LAUNCHER_UNPROVEN_KEY = "_launcher_unproven" # A package record names no file: it records what a package builds. _NAMELESS_KINDS = frozenset({"package"}) @@ -1238,7 +1242,9 @@ def launcher_reads_not_held( rows: list[dict] = [] for platform, config in configs.items(): by_key: dict[str, dict] = {} - for record in config.get("launchers") or []: + # A record resting on a config file it cannot prove reads nothing + # yet: it is reported apart (launcher_unproven). + for record in bound_launchers(config, profiles): names = launcher_file_names(record) if not names: continue @@ -1331,6 +1337,25 @@ def launcher_reads_not_held( return rows +def print_launcher_unproven(rows: list[dict]) -> None: + """The launcher records resting on a config file the code does not prove. + + Neither a pack, a manifest nor a severity reads them: each is listed + with the links it lacks. + """ + if not rows: + return + platforms = sorted({r["platform"] for r in rows}) + noun = "platform" if len(platforms) == 1 else "platforms" + print(f"\nLauncher records unproven: {len(rows)} across {len(platforms)} {noun}") + for platform in platforms: + print(f"\n{platform}") + for row in (r for r in rows if r["platform"] == platform): + print(f" [{row['index']}] {row['emulator']} {row['kind']}: {row['passes_to']}") + for link in row["missing"]: + print(f" missing {link}") + + def print_launcher_reads(rows: list[dict]) -> None: """The launcher reads gap category, after the profile gaps. @@ -1649,8 +1674,25 @@ def main() -> None: db, all_profiles, registry, emulator or "", ) + unproven = [ + row for row in launcher_unproven( + { + name: load_platform_config(name, args.platforms_dir) + for name in (selected or list_registered_platforms(args.platforms_dir)) + }, + all_profiles, + ) + if not emulator or any( + emulator in LauncherLayer._profiles_named(part, all_profiles, set(all_profiles)) + for part in row["emulator"].split(", ") + ) + ] + if args.json: - print(json.dumps({**report, LAUNCHER_READS_KEY: launcher_reads}, indent=2)) + print(json.dumps( + {**report, LAUNCHER_READS_KEY: launcher_reads, LAUNCHER_UNPROVEN_KEY: unproven}, + indent=2, + )) else: if emulator: scope = emulator @@ -1660,6 +1702,7 @@ def main() -> None: scope = "all emulators" print_report(report, scope, show_clean=bool(args.emulator)) print_launcher_reads(launcher_reads) + print_launcher_unproven(unproven) if __name__ == "__main__": diff --git a/scripts/generate_site.py b/scripts/generate_site.py index 8272392a7..c67e31afd 100644 --- a/scripts/generate_site.py +++ b/scripts/generate_site.py @@ -3646,13 +3646,15 @@ def _served_line(f: dict, declared_sha1: str = "") -> str: def _launcher_refs(config: dict) -> dict[str, list[str]]: """Entry name, casefolded -> each launcher record that opens it, rendered.""" from launchers import _names as launcher_names + from launchers import bound_launchers from sitesources import launcher_ref_markdown, list_source # Every scraper reads its launchers in the repository and at the revision - # of the list it scrapes, so `source` pins each source_ref. + # of the list it scrapes, so `source` pins each source_ref. A record a + # config file does not prove opens nothing: it is listed apart. source = list_source(str(config.get("source") or "")) found: dict[str, list[str]] = {} - for record in config.get("launchers") or []: + for record in bound_launchers(config): if not isinstance(record, dict) or not record.get("source_ref"): continue text = (f"{_escape_tags(str(record.get('emulator', '')))} at " @@ -3664,6 +3666,31 @@ def _launcher_refs(config: dict) -> dict[str, list[str]]: return found +UNPROVEN_TITLE = "Launcher records a config does not prove" + + +def _launcher_unproven_section(config: dict) -> list[str]: + """The platform's launcher records resting on a config the code does not prove. + + A config file is proof only when the code installing it and the code + reading it are shown: these records place no file. + """ + from launchers import launcher_unproven + from sitesources import launcher_ref_markdown, list_source + + rows = launcher_unproven({str(config.get("_key") or ""): config}) + if not rows: + return [] + source = list_source(str(config.get("source") or "")) + body = [ + f"- {_escape_tags(row['emulator'])}, `{_escape_tags(row['passes_to'] or row['kind'])}` " + f"at {launcher_ref_markdown(source, row['source_ref'])}: " + f"{_escape_tags('; '.join(row['missing']))}" + for row in rows + ] + return ["", *markup.details("note", f"{UNPROVEN_TITLE} ({len(rows)})", body)] + + # The hash a row shows first, when the list does not say which it checks. _ROW_HASHES = ("sha1", "md5", "sha256", "crc32") # An untested entry whose reason is a hash the bytes do not match. @@ -4802,6 +4829,7 @@ def generate_platform_page( _render_platform_systems(by_system, config_files, emulator_files, (name, display), moved, config) ) + lines.extend(_launcher_unproven_section(config)) lines.extend(["", _date_line()]) return "\n".join(lines) + "\n" diff --git a/scripts/launchers.py b/scripts/launchers.py index 2884035b9..c790af076 100644 --- a/scripts/launchers.py +++ b/scripts/launchers.py @@ -254,11 +254,318 @@ def _names(record: dict) -> list[str]: return names -def launcher_places(config: dict) -> list[LauncherPlace]: +# How a record's passes_to names what carries the file when it names no +# config file: an option on the command line, or a libretro core option. +COMMAND_LINE = "command line" +CORE_OPTIONS = "core options" +_TAIL_WORDS = re.compile(r"\s+(?:unset|left empty)$") +_KEY_TOKEN = re.compile(r"^-{0,2}[\w.$()/\\]+$") + + +def config_target(record: dict) -> tuple[str, list[str]]: + """The config a record writes its file to, and the keys it names there. + + passes_to spells them as describe_keys does: "settings.ini [BIOS] + SearchDirectory, [BIOS] PathNTSCU, PathNTSCJ unset", "command line + -hard", "core option pcsx2_bios", "hatarib_tos = hatarib/". A + part that reads as prose ("only if the file exists") names no key. When + passes_to names no file ("romlist coco"), the file is the one the record + installs and the whole of passes_to is the key. + """ + # A parenthesis says how the file is passed, unless a config variable + # opens it: $(FILE_PATH) is amiberry's. + text = re.sub(r"(?" sets every key before the value. + names = [p.strip() for p in re.split(r"\s*=\s*", chunk)] + if len(names) > 1: + names = names[:-1] + for name in names: + if not _KEY_TOKEN.match(name): + continue + key = f"{section} {name}" if section else name + if key not in keys: + keys.append(key) + return head, keys + + +def _read_keys(read: dict) -> dict[str, str]: + """Each key a config_reads entry says the build reads, to its source_ref.""" + found: dict[str, str] = {} + for entry in read.get("keys") or []: + spelled = entry.get("key") + for key in spelled if isinstance(spelled, list) else [spelled]: + if key: + found[str(key)] = str(entry.get("source_ref") or "") + return found + + +def _handle_refs(profile: dict) -> dict[str, str]: + """The handles a profile names with the code that reads them.""" + found: dict[str, str] = {} + for item in profile.get("standalone_directories") or []: + if isinstance(item, dict) and item.get("source_ref"): + handle = str(item.get("handle") or "").partition(" = ")[0] + found[handle] = str(item["source_ref"]) + for handle, value in (profile.get("standalone_pools") or {}).items(): + if isinstance(value, dict) and value.get("source_ref"): + found[str(handle)] = str(value["source_ref"]) + return found + + +def directory_handles(profile: dict) -> list[str]: + """standalone_directories as handle strings, a {handle, source_ref} item + by its handle.""" + return [ + str(item.get("handle") or "") if isinstance(item, dict) else str(item) + for item in profile.get("standalone_directories") or [] + ] + + +def pool_folders(profile: dict) -> dict[str, str]: + """standalone_pools as handle to subfolder, a {folder, source_ref} value + by its folder.""" + return { + str(handle): str(value.get("folder") or "") if isinstance(value, dict) else str(value) + for handle, value in (profile.get("standalone_pools") or {}).items() + } + + +def _reads_apply(read: dict, profile: dict, platform: str) -> bool: + """Whether a config_reads entry describes the build ``platform`` ships. + + An entry naming platforms is read at their pin_build revision. One that + names none is read at the profile's pin, the build of every platform + with no pin_build of its own or one pinning the same commit. + """ + named = [str(p) for p in read.get("platforms") or []] + if named: + return platform in named + pin = str(profile.get("source_commit") or "") + builds = [b for b in profile.get("pin_build") or [] if b.get("platform") == platform] + return all(pin and str(b.get("value") or "").startswith(pin[:12]) for b in builds) + + +def _file_matches(read_file: str, file: str, record: dict) -> bool: + """Whether a config_reads file is the one a record writes. + + The entry names the file as the build finds it: a name, a glob ("*.uae", + any file the launcher passes), a path, or the flag the launcher passes + it with (amiberry's -f). + """ + from fnmatch import fnmatch # noqa: PLC0415 + + spelled = read_file.rsplit("/", 1)[-1].casefold() + if file and fnmatch(file.casefold(), spelled): + return True + installed = str((record.get("installs") or {}).get("path") or "").rsplit("/", 1)[-1] + if installed and fnmatch(installed.casefold(), spelled): + return True + return read_file.startswith("-") and read_file in str(record.get("passes_to") or "").split() + + +@dataclass(frozen=True) +class ConfigEvidence: + """Whether a launcher record that rests on a config holds, and why not. + + ``config`` holds the parts of its source_ref that cite a config, option + or feature file. A record citing none, and choosing among no listed + option values, rests on code alone and holds. + """ + + config: tuple[str, ...] + missing: tuple[str, ...] + readers: tuple[str, ...] = () + + @property + def proven(self) -> bool: + return not self.missing + + +def _named_profiles(record: dict, profiles: Mapping[str, dict], platform: str) -> list[str]: + """The profiles a record's emulator names, a platform's own build first.""" + found = [] + for part in (e.strip() for e in str(record.get("emulator", "")).split(",")): + named = [ + key for key, profile in profiles.items() + if profile.get("type") not in ("alias", "launcher") + and (key == part or part in {str(c) for c in profile.get("cores") or []}) + ] + own = [k for k in named if platform in (profiles[k].get("built_by") or [])] + found.extend(own or [k for k in named if not profiles[k].get("built_by")]) + return sorted(set(found)) + + +def config_evidence( + record: dict, profiles: Mapping[str, dict], platform: str = "" +) -> ConfigEvidence: + """What a record citing a config file proves, link by link. + + A config file is proof only when the code that installs it and the code + that reads it are shown: (a) the platform installs the file at a path + (``installs``), (b) its launch code was read for what replaces the file + or the key (``overrides``), (c) the emulator loads that file and (d) + reads each key and opens the value (``config_reads`` of its profile, at + the build the platform ships). A feature or option file only lists the + values a menu offers: it goes in ``choices_from``, beside the code that + writes the choice, and such a record needs (c) and (d) too. + """ + from scraper.shell_launchers import config_ref, is_feature_ref # noqa: PLC0415 + + cited = config_ref(str(record.get("source_ref") or "")) + choices = str(record.get("choices_from") or "") + if not cited and not choices: + return ConfigEvidence((), ()) + missing: list[str] = [] + features = [part for part in cited if is_feature_ref(part)] + if features: + missing.append("an option list cited as source, not as choices_from") + shipped = [part for part in cited if not is_feature_ref(part)] + if shipped: + installs = record.get("installs") or {} + if not (installs.get("path") and installs.get("source_ref")): + missing.append("installs: the code that puts the config in place") + if "overrides" not in record: + missing.append("overrides: the launch code read for what replaces it") + file, keys = config_target(record) + if not keys: + missing.append("a key the config hands the file to") + readers: list[str] = [] + uncovered = list(keys) + loaded = False + for key in _named_profiles(record, profiles, platform) or sorted(profiles): + profile = profiles[key] + handles = _handle_refs(profile) + for read in profile.get("config_reads") or []: + if not _reads_apply(read, profile, platform): + continue + if not _file_matches(str(read.get("file") or ""), file, record): + continue + loaded = True + known = _read_keys(read) + before = len(uncovered) + uncovered = [k for k in uncovered if not (known.get(k) or handles.get(k))] + if len(uncovered) < before and key not in readers: + readers.append(key) + if not loaded: + missing.append(f"config_reads: an emulator that loads {file or 'the config'}") + elif uncovered: + missing.append("config_reads: no read of " + ", ".join(uncovered)) + return ConfigEvidence(tuple(cited or [choices]), tuple(missing), tuple(readers)) + + +_DEFAULT_PROFILES: dict[str, dict] | None = None + + +def _profiles_or_repo(profiles: Mapping[str, dict] | None) -> Mapping[str, dict]: + """The profiles given, else this repository's, read once.""" + global _DEFAULT_PROFILES # noqa: PLW0603 + if profiles is not None: + return profiles + if _DEFAULT_PROFILES is None: + from pathlib import Path # noqa: PLC0415 + + from common import load_emulator_profiles # noqa: PLC0415 + + _DEFAULT_PROFILES = load_emulator_profiles( + str(Path(__file__).resolve().parent.parent / "emulators") + ) + return _DEFAULT_PROFILES + + +def _cites_config(record: dict) -> bool: + from scraper.shell_launchers import config_ref # noqa: PLC0415 + + return bool(record.get("choices_from")) or bool(config_ref(str(record.get("source_ref") or ""))) + + +def bound_launchers(config: dict, profiles: Mapping[str, dict] | None = None) -> list[dict]: + """The launcher records a platform's packs, manifests and reports consume. + + A record resting on a config file binds only when config_evidence holds + for it. The repository's profiles are read only when a record cites one. + """ + records = [r for r in config.get("launchers") or [] if isinstance(r, dict)] + if not any(_cites_config(r) for r in records): + return records + known = _profiles_or_repo(profiles) + platform = str(config.get("_key") or "") + return [r for r in records if config_evidence(r, known, platform).proven] + + +def launcher_unproven( + configs: Mapping[str, dict], profiles: Mapping[str, dict] | None = None +) -> list[dict]: + """The records resting on a config file whose evidence does not hold. + + One row per record, with the config parts it cites and each missing + link. Reported apart: no severity, count or exit code reads it. + """ + rows: list[dict] = [] + for platform, config in configs.items(): + records = [r for r in config.get("launchers") or [] if isinstance(r, dict)] + if not any(_cites_config(r) for r in records): + continue + known = _profiles_or_repo(profiles) + for index, record in enumerate(records): + evidence = config_evidence(record, known, str(config.get("_key") or platform)) + if evidence.proven: + continue + rows.append({ + "platform": platform, + "index": index, + "emulator": str(record.get("emulator", "")), + "kind": str(record.get("kind", "")), + "passes_to": str(record.get("passes_to") or ""), + "source_ref": str(record.get("source_ref", "")), + "config": list(evidence.config), + "missing": list(evidence.missing), + }) + return rows + + +def launcher_places( + config: dict, profiles: Mapping[str, dict] | None = None +) -> list[LauncherPlace]: """Every place a platform's launchers open a named file from.""" base = str(config.get("base_destination") or "bios") places: list[LauncherPlace] = [] - for record in config.get("launchers") or []: + for record in bound_launchers(config, profiles): kind = record.get("kind") if kind not in PLACE_KINDS: continue @@ -484,7 +791,9 @@ def _listed_directories(raw: str) -> list[str] | None: return None -def handed_directories(config: dict) -> list[HandedDirectory]: +def handed_directories( + config: dict, profiles: Mapping[str, dict] | None = None +) -> list[HandedDirectory]: """Every whole directory a platform links or passes to a standalone build. A record naming files (gpsp's gba_bios.bin) or one slot of a directory @@ -499,7 +808,7 @@ def handed_directories(config: dict) -> list[HandedDirectory]: natives = native_systems(config) declared = {d.lower() for d in declared_destinations(config)} found: list[HandedDirectory] = [] - for record in config.get("launchers") or []: + for record in bound_launchers(config, profiles): if record.get("build") != "standalone" or record.get("kind") not in HANDED_KINDS: continue if record.get("names") or record.get("slot"): @@ -988,7 +1297,7 @@ class LauncherLayer: db: dict, declared: Iterable[str], ) -> None: - self.places = launcher_places(config) + self.places = launcher_places(config, profiles) self.declared = set(declared) self.declared_folded = {d.lower() for d in self.declared} self.declared_names = {d.rsplit("/", 1)[-1] for d in self.declared} diff --git a/scripts/profile_sync.py b/scripts/profile_sync.py index b10c2531c..94c663b81 100644 --- a/scripts/profile_sync.py +++ b/scripts/profile_sync.py @@ -1047,6 +1047,11 @@ class ProfileReport: new_files_elided: dict[str, int] = field(default_factory=dict) new_hashes: list[dict] | None = None detection_skipped: str | None = None + # config_reads refs written for a build a platform ships, each read at + # that build (pin_build) rather than at source_commit: {platform, + # revision, ref, status, detail}. A status other than BUILD_OK is review + # work. + build_refs: list[dict] = field(default_factory=list) # Skips that say the profile cannot be checked as written: a repository # that no longer answers, no pin to read, a death the forge contradicts. @@ -1070,6 +1075,7 @@ class ProfileReport: + len(self.orphaned) + len(self.stale_prose or ()) + len(self.moved_repos) + + sum(1 for ref in self.build_refs if ref["status"] != "BUILD_OK") ) @@ -1603,6 +1609,81 @@ def moved_repositories( return moved +def shipped_build_holders(profile: dict) -> dict[int, dict]: + """The source_ref holders of config_reads entries naming platforms, by id. + + Such an entry describes the build those platforms ship, so its refs are + read at their pin_build revision, never at source_commit. + """ + found: dict[int, dict] = {} + for read in profile.get("config_reads") or []: + if not isinstance(read, dict) or not read.get("platforms"): + continue + found[id(read)] = read + for key in read.get("keys") or []: + if isinstance(key, dict): + found[id(key)] = read + return found + + +def build_revision(profile: dict, platform: str) -> str | None: + """The emulator revision a platform's pin_build entry names, None when + its value is no revision (a URL, a package name).""" + for build in profile.get("pin_build") or []: + if build.get("platform") != platform: + continue + value = str(build.get("value") or "").strip() + return value if value and re.fullmatch(r"[\w.+-]+", value) else None + return None + + +def _check_build_refs( + profile: dict, + deferred: list[tuple[Citation, dict]], + views: list[RepoView], + fetch: Callable[[upstream.Repo, str, str], list[str] | None], +) -> list[dict]: + """Each deferred config_reads ref read at the build of its platforms. + + The file must exist at that revision and every cited line lie inside + it. The entry declares no value to anchor on, so this is the check a + ref past the end of its file fails (BUILD_GONE), not a content match. + """ + results: list[dict] = [] + for citation, read in deferred: + for platform in read.get("platforms") or []: + revision = str(read.get("revision") or "") or build_revision(profile, str(platform)) + row = {"platform": str(platform), "revision": revision or "", + "ref": citation.ref, "field": citation.field} + if revision is None: + results.append({**row, "status": "BUILD_UNREAD", + "detail": "no pin_build revision for this platform"}) + continue + problems: list[str] = [] + for part in split_source_ref(citation.ref): + role, path = ref_role(part.path) + if " " in path: + # A project word in front (`ROCKNIX patches/x.patch:4`): + # a file of the platform's tree, read with the record. + continue + lines = next( + (found for view in views + if (not role or view.field == role) + and (found := fetch(view.repo, revision, path)) is not None), + None, + ) + if lines is None: + problems.append(f"{part.path} absent at {revision[:12]}") + elif (part.end or part.start or 0) > len(lines): + problems.append(f"{part.raw} past the end ({len(lines)} lines)") + results.append({ + **row, + "status": "BUILD_GONE" if problems else "BUILD_OK", + "detail": "; ".join(problems), + }) + return results + + def build_report( name: str, profile: dict, cache_dir: str, offline: bool = False, text: str | None = None, @@ -1621,7 +1702,13 @@ def build_report( ) modes = {mode for _, mode, _ in declared_repositories(profile) if mode} refs = [] + at_build = shipped_build_holders(profile) + deferred: list[tuple[Citation, dict]] = [] for citation in collect_citations(profile, text): + if citation.kind == "ref" and id(citation.holder) in at_build: + # Written for the build a platform ships: read at pin_build. + deferred.append((citation, at_build[id(citation.holder)])) + continue if citation.kind == "ref": entry = citation.entry display = str(entry.get("name") or "") or citation.field @@ -2204,6 +2291,8 @@ def build_report( ) return result + report.build_refs = _check_build_refs(profile, deferred, views, fetch_from) + staged = [] for entry_name, ref, tokens, hashes, ref_parts, citation in refs: flags = [self_checked(part, tokens) for part in ref_parts] @@ -2335,6 +2424,14 @@ def format_report(report: ProfileReport, changed_only: bool = False) -> str: lines.append(f" {len(report.entries)} refs: {summary}") for line in report.stale_prose or (): lines.append(f" prose under an older pin: {line}") + for ref in report.build_refs: + if changed_only and ref["status"] == "BUILD_OK": + continue + detail = f": {ref['detail']}" if ref["detail"] else "" + lines.append( + f" {ref['status']} at {ref['platform']} build " + f"{ref['revision'][:12] or '-'} {ref['ref']}{detail}" + ) lines.append(f" {report.needs_review()} to review") return "\n".join(lines) diff --git a/scripts/scraper/shell_launchers.py b/scripts/scraper/shell_launchers.py index 5aa9a56bf..d847b2666 100644 --- a/scripts/scraper/shell_launchers.py +++ b/scripts/scraper/shell_launchers.py @@ -63,6 +63,48 @@ def source_ref(parts: Iterable[tuple[str, Iterable[int]]]) -> str: ) +# The files a platform ships as configuration: an emulator reads them, the +# platform's code never runs them. Batocera's .yml lists and an EmuELEC .opt +# only offer the values a menu option takes. +_CONFIG_FILE = re.compile( + r"\.(?:ini|cfg|conf|rc|rc_\w+|opt|bml|toml|uae|template|yml|yaml)$", re.IGNORECASE +) +_FEATURE_FILE = re.compile(r"(?:^|/)es_features[^/]*$|\.opt$", re.IGNORECASE) + + +def split_ref(ref: str) -> list[str]: + """The parts of a source_ref, each "path:lines", a bare line span + continuing the path before it.""" + parts: list[str] = [] + for part in (p.strip() for p in ref.split(", ")): + if not part: + continue + if re.fullmatch(r"\d+(?:-\d+)?", part) and parts: + parts[-1] += f", {part}" + else: + parts.append(part) + return parts + + +def ref_path(part: str) -> str: + return part.rsplit(":", 1)[0] if re.search(r":\d", part) else part + + +def config_ref(ref: str) -> list[str]: + """The parts of a source_ref that cite a config, option or feature file. + + Such a part proves what the file says, never that an emulator reads it: + melonDS.ini keys are written on ROCKNIX for a build that reads them only + when another key is set. + """ + return [part for part in split_ref(ref) if _CONFIG_FILE.search(ref_path(part))] + + +def is_feature_ref(part: str) -> bool: + """Whether a source_ref part cites a file that lists a menu option's values.""" + return bool(_FEATURE_FILE.search(ref_path(part))) + + @dataclass class Word: """A shell word: as written, and with quotes removed.""" @@ -394,6 +436,149 @@ def _merge_links(links: list[Link]) -> list[Link]: return [link for link in merged if id(link) not in folded] +_COPY = re.compile(r"(?:^|[\s;&|(])(cp|rsync|install|mv)\s") +_GUARD = re.compile(r"\[\[?\s*!\s+-[fde]\s+(\S+)\s*\]") + + +@dataclass +class ConfigPlacement: + """How a start script puts a config where the emulator reads it. + + ``install`` are the lines copying it there, ``kept_if_present`` whether + every such copy runs only when the file (or its folder) is absent, and + ``overrides`` the sed lines rewriting one of the record's keys in it. + """ + + script: str + path: str + install: set[int] + kept_if_present: bool + overrides: set[int] + + +def _bare_key(key: str) -> str: + return key.rsplit("] ", 1)[-1].rsplit("/", 1)[-1].strip() + + +def _guarded(script: Script, number: int, target: str) -> bool: + """Whether a line runs only when target, or a folder above it, is absent.""" + + def covers(found: re.Match[str], at: int) -> bool: + guarded = script.expand(words(found.group(1))[0].text, at, set()).rstrip("/") + return target == guarded or target.startswith(guarded + "/") + + line = script.lines[number - 1] + if any(covers(m, number) for m in _GUARD.finditer(line)): + return True + depth = 0 + for at in range(number - 1, 0, -1): + text = script.lines[at - 1].strip() + if re.match(r"fi\b", text): + depth += 1 + elif re.match(r"if\b", text): + if depth: + depth -= 1 + continue + return any(covers(m, at) for m in _GUARD.finditer(text)) + return False + + +def config_placement( + script: Script, + target: str, + keys: Iterable[str] = (), + writers: Iterable[str] = (), + source_name: str = "", + required: bool = True, + sources: tuple[str, ...] = (), +) -> ConfigPlacement: + """The lines of a start script that install ``target`` and rewrite its keys. + + A copy installs the target when its destination is the file or a folder + above it. ``writers`` are other commands that write the file (ROCKNIX + generates mednafen.cfg with mednafen_gen_config.sh). A copy guarded by + `[ ! -f X ]` or `[ ! -d X ]` on the target or a folder above it, or made + with cp -n or rsync --ignore-existing, leaves a file already there. + """ + install: set[int] = set() + kept = True + overrides: set[int] = set() + bare = {_bare_key(k) for k in keys} - {""} + writer = re.compile("|".join(re.escape(w) for w in writers)) if writers else None + for number, line in script.code(): + if (copy := _COPY.search(line)) or (writer and writer.search(line)): + if copy: + args = [] + for word in words(line[copy.start(1):])[1:]: + # The command ends at a separator or a redirection. + if word.raw in {";", "&&", "||", "|"} or re.match(r"\d*>", word.raw): + break + args.append(word.text) + operands = [a for a in args if not a.startswith("-")] + if not operands: + continue + destination = script.expand(operands[-1], number, set()).rstrip("/") + source = script.expand(operands[0], number, set()).rstrip("/") + hits = target == destination or target.startswith(destination + "/") or ( + target.startswith(f"{destination}/{source.rsplit('/', 1)[-1]}") + ) + # A copy of another file into the same place is another + # record's: amiberry copies the A500 or the A1200 setup. + named = source.rsplit("/", 1)[-1] + if not hits or (source_name and "." in named and named != source_name): + continue + # A named file copied into a folder above the target is + # another file of that folder (ARMSX2's secrets.ini). + if ( + not source_name and "." in named and "*" not in named + and destination != target and named != target.rsplit("/", 1)[-1] + ): + continue + if sources and not source.startswith(sources): + continue + skip = "-n" in args or "--ignore-existing" in args + else: + skip = False + install.add(number) + if not (skip or _guarded(script, number, target)): + kept = False + elif re.search(r"\bsed\b.*\s-i\b|\bsed -i", line) and bare: + expanded = script.expand(line, number, set()) + if target in expanded or target.rsplit("/", 1)[-1] in expanded: + if any(re.search(rf"\b{re.escape(k)}\b", expanded) for k in bare): + overrides.add(number) + if not install and required: + raise script.missing(f"install of {target}") + return ConfigPlacement(script.path, target, install, kept, overrides) + + +def evidence_fields( + placements: Iterable[ConfigPlacement], + installed: Iterable[tuple[str, Iterable[int]]] = (), +) -> dict: + """The installs, kept_if_present and overrides fields of a config record. + + ``placements`` are the steps that put the file in place, in the order + they run (a first-boot copy, then the start script). ``installed`` are + the build steps that put it where they copy it from (a package.mk copy + into /usr/config). + """ + placements = list(placements) + steps = [p for p in placements if p.install] + if not steps: + raise LauncherPatternMissing(placements[0].path if placements else "", "install") + return { + "installs": { + "path": steps[-1].path, + "source_ref": source_ref([*installed, *((p.script, p.install) for p in steps)]), + }, + "kept_if_present": all(p.kept_if_present for p in steps), + "overrides": [ + source_ref([(p.script, {line})]) for p in placements for line in sorted(p.overrides) + ], + } + + @dataclass class Setting: section: str diff --git a/scripts/validate_schemas.py b/scripts/validate_schemas.py index 97b40707a..917dea729 100644 --- a/scripts/validate_schemas.py +++ b/scripts/validate_schemas.py @@ -81,7 +81,10 @@ def _validate_yaml_directory( continue out.extend(_errors(validator, data, str(path.relative_to(ROOT)))) out.extend(_unreachable_citations(path, data)) + if schema_name == "platform.schema.json": + out.extend(_config_records_without_evidence(path, data)) if schema_name == "emulator.schema.json": + out.extend(_config_reads_without_build(path, data)) out.extend(_checks_without_values(path, data)) out.extend(_save_dir_without_library_name(path, data)) out.extend(_systems_outside_profile(path, data)) @@ -102,6 +105,59 @@ _CHECK_FIELDS = { } +def _config_records_without_evidence(path: Path, document: object) -> list[str]: + """A launcher record citing a config without the links its extractor reads. + + A config file is proof only when the code installing it and the code + reading it are shown (launchers.config_evidence). The platform's half + is the extractor's to write: a record citing a shipped config carries + installs and overrides, and a feature or option file is never a + record's source, only its choices_from. + """ + if not isinstance(document, dict): + return [] + from scraper.shell_launchers import config_ref, is_feature_ref # noqa: PLC0415 + + out: list[str] = [] + for index, record in enumerate(document.get("launchers") or []): + if not isinstance(record, dict): + continue + cited = config_ref(str(record.get("source_ref") or "")) + where = f"{path.relative_to(ROOT)}: launchers[{index}] ({record.get('emulator')})" + features = [part for part in cited if is_feature_ref(part)] + if features: + out.append(f"{where} cites {features[0]} as source, choices_from holds it") + if [part for part in cited if not is_feature_ref(part)] and not ( + record.get("installs") and "overrides" in record + ): + out.append(f"{where} cites a config without installs and overrides") + return out + + +def _config_reads_without_build(path: Path, document: object) -> list[str]: + """A config_reads entry naming a platform the profile gives no pin_build. + + Such an entry is read at the build that platform ships, and pin_build is + where that build is named. + """ + if not isinstance(document, dict): + return [] + builds = {str(b.get("platform")) for b in document.get("pin_build") or []} + out: list[str] = [] + for index, read in enumerate(document.get("config_reads") or []): + if not isinstance(read, dict): + continue + if read.get("revision"): + continue + for platform in read.get("platforms") or []: + if str(platform) not in builds: + out.append( + f"{path.relative_to(ROOT)}: config_reads[{index}] ({read.get('file')}) " + f"names {platform}, which has no pin_build" + ) + return out + + def _systems_outside_profile(path: Path, document: object) -> list[str]: """An entry's system: or systems: naming a system its profile does not list. diff --git a/scripts/verify.py b/scripts/verify.py index 8c2d40e57..a3fe5fb02 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -100,7 +100,7 @@ from common import ( yaml = require_yaml() from exporter.candidates import BIOS, EntryCategories -from launchers import LauncherLayer, declared_destinations, held_record +from launchers import LauncherLayer, declared_destinations, held_record, launcher_unproven from cross_reference import launcher_reads_not_held from nativemode import ( SIZE_AND_ANY_HASH, @@ -1764,6 +1764,8 @@ def verify_platform( "launcher_reads_not_held": launcher_reads_not_held( {config.get("_key", platform): config}, db, profiles, data_dir_registry ), + # Records a config file does not prove: listed, never consumed. + "launcher_unproven": launcher_unproven({config.get("_key", platform): config}, profiles), "exclusion_notes": exclusions, "region_fallbacks": sorted(region_fallbacks), **({"left_out": left_out} if no_game_data else {}), @@ -2130,6 +2132,21 @@ def _print_undeclared_section(result: dict, verbose: bool) -> None: ) +def _print_launcher_unproven(result: dict, verbose: bool) -> None: + """Launcher records resting on a config file the code does not prove.""" + rows = result.get("launcher_unproven") or [] + if not rows: + return + listed = "" if verbose else " (--verbose lists them)" + print(f" Launcher records unproven: {len(rows)}{listed}") + if not verbose: + return + for row in rows: + print(f" [{row['index']}] {row['emulator']} {row['kind']}: {row['passes_to']}") + for link in row["missing"]: + print(f" missing {link}") + + def _print_launcher_reads(result: dict, verbose: bool) -> None: """Names the platform's launch code reads that the collection lacks.""" every = result.get("launcher_reads_not_held") or [] @@ -2220,6 +2237,7 @@ def print_platform_result( _print_detail_entries(result["details"], seen_details, verbose) _print_undeclared_section(result, verbose) _print_launcher_reads(result, verbose) + _print_launcher_unproven(result, verbose) # The same notes come back for every platform running these cores and # filled 985 of the 1114 lines of verify --all: counted, listed on ask. diff --git a/tests/test_launcher_gaps.py b/tests/test_launcher_gaps.py index e1bdac934..a2e8bdbc1 100644 --- a/tests/test_launcher_gaps.py +++ b/tests/test_launcher_gaps.py @@ -33,6 +33,7 @@ import verify # noqa: E402 from common import _emulator_profiles_cache, load_emulator_profiles, load_platform_config # noqa: E402 from cross_reference import ( # noqa: E402 LAUNCHER_READS_KEY, + LAUNCHER_UNPROVEN_KEY, launcher_file_names, launcher_reads_not_held, print_launcher_reads, @@ -278,7 +279,11 @@ class LauncherGapTest(unittest.TestCase): names = {r["name"] for r in report[LAUNCHER_READS_KEY]} self.assertEqual(names, {"absent.bin", "mixed.rom", "labelled.rom", "user.sf2"}) # The emulator rows keep their own shape and counts. - self.assertEqual(set(report) - {LAUNCHER_READS_KEY}, {"aliasemu", "missemu", "setemu"}) + self.assertEqual( + set(report) - {LAUNCHER_READS_KEY, LAUNCHER_UNPROVEN_KEY}, + {"aliasemu", "missemu", "setemu"}, + ) + self.assertEqual(report[LAUNCHER_UNPROVEN_KEY], []) def test_verify_counts_and_exit_code_are_unchanged(self) -> None: """A launcher name is reported, not judged: nothing else moves.""" diff --git a/tests/test_launcher_layer.py b/tests/test_launcher_layer.py index 0e6e8fae0..7f37fef6b 100644 --- a/tests/test_launcher_layer.py +++ b/tests/test_launcher_layer.py @@ -314,9 +314,9 @@ class LauncherLayerTest(unittest.TestCase): config = {"base_destination": "bios", "launchers": [{ "emulator": "e", "build": "libretro", "kind": "fixed_name", "names": ["a.rom", "b.rom"], "search_dir": "/storage/roms/bios", - "passes_to": "e.cfg ROM_A, ROM_B", "source_ref": "e.cfg:1", + "passes_to": "e.cfg ROM_A, ROM_B", "source_ref": "launch.sh:1", }]} - places = {p.paths[0]: p.passes_to for p in launcher_places(config)} + places = {p.paths[0]: p.passes_to for p in launcher_places(config, {})} self.assertEqual(places, {"a.rom": "e.cfg ROM_A", "b.rom": "e.cfg ROM_B"}) def test_a_core_option_names_the_dump_of_a_model(self) -> None: @@ -341,12 +341,16 @@ class LauncherLayerTest(unittest.TestCase): "files": [{"name": ".bin", "path": "ps2/bios/.bin", "required": True, "min_size": 32, "max_size": 64, "validation": ["size"], "config_key": "emu_bios"}], + "config_reads": [{"file": "core options", "source_ref": "core.c:1", + "keys": [{"key": "emu_bios", "source_ref": "core.c:2"}]}], } with open(os.path.join(self.emulators_dir, "optemu.yml"), "w") as fh: yaml.dump(profile, fh) self.config["launchers"].append({ "emulator": "retroarch", "build": "libretro", "kind": "core_option", "names": ["scph10000.bin"], "passes_to": "core option emu_bios", + "installs": {"path": "retroarch-core-options.cfg", "source_ref": "prepare.sh:24"}, + "overrides": [], "source_ref": "core-options.cfg:515", }) _emulator_profiles_cache.clear() diff --git a/tests/test_launcher_placement.py b/tests/test_launcher_placement.py index b8a24598c..71550bebb 100644 --- a/tests/test_launcher_placement.py +++ b/tests/test_launcher_placement.py @@ -962,6 +962,17 @@ class GzdoomOnRetroDECK(unittest.TestCase): self.assertFalse(any(e.get("standalone_path") for e in view["files"])) +def _reads_configs(profile: str) -> bool: + """Whether a repository profile carries config_reads.""" + path = REPO_ROOT / "emulators" / f"{profile}.yml" + return bool((yaml.safe_load(path.read_text(encoding="utf-8")) or {}).get("config_reads")) + + +# EmuELEC hands AdvanceMAME its folders through advmame.rc: the chain binds +# once the profile says where the build reads them (config_reads). +@unittest.skipUnless( + _reads_configs("advancemame"), "advancemame.yml carries no config_reads, dir_rom is unproven" +) class AdvanceMameOnEmuELEC(unittest.TestCase): """advmame.rc sets dir_rom /storage/roms/mame, /storage/roms/arcade for the arcade system (advmame.rc:11, advmame.sh:26-30).""" diff --git a/wiki/adding-a-platform.md b/wiki/adding-a-platform.md index 446d020bf..2e559804e 100644 --- a/wiki/adding-a-platform.md +++ b/wiki/adding-a-platform.md @@ -445,6 +445,10 @@ launchers: | `fails_scope` | `emulator` when the failure needs every record of the emulator to come up empty | | `depends_on_option` | Options or conditions that decide whether the record applies | | `device` | Devices whose shipped config the record reads, set only where a package ships configs that differ between devices | +| `installs` | For a record citing a config: `path` where the platform's code puts it and the `source_ref` of that code | +| `kept_if_present` | Whether that install leaves a file already there (a copy only if absent, rsync `--ignore-existing`) | +| `overrides` | Launch code rewriting the record's keys or replacing the config, empty when none does | +| `choices_from` | The feature or option file listing the values the option takes (es_features, a core `.opt`), never the record's evidence | | `source_ref` | `path:lines` in the repository and at the revision `source` pins, which the site links | Batocera's generators are read with `ast`, RetroBat's C# generators as text. @@ -453,18 +457,24 @@ package, so `scripts/scraper/shell_launchers.py` reads assignments, `for` loops, `ln -s` commands and `key = value` lines. A ROCKNIX package can ship one config per device: the configs are grouped by the value read, and `device` appears only when there is more than one group. melonDS writes its -seven BIOS paths on every device but AMD64, where the keys are left empty: +BIOS paths on every device but AMD64, where the keys are left empty: ```yaml - emulator: melonds build: standalone kind: fixed_name names: [] - passes_to: melonDS.ini BIOS9Path, BIOS7Path, FirmwarePath, DSiBIOS9Path, - DSiBIOS7Path, DSiFirmwarePath, DSiNANDPath left empty + passes_to: melonDS.ini BIOS9Path, BIOS7Path, FirmwarePath, DSiNANDPath left empty device: AMD64 ``` +A config file is proof only when the code installing it and the code reading +it are shown. A record citing a config carries `installs` and `overrides`, and +the emulator's profile says, in `config_reads`, where the build it ships loads +that file and reads each key. A record whose chain does not hold is listed as +unproven by `verify.py` and `cross_reference.py`, and no pack or manifest +places a file from it. + An extractor that no longer recognises the shape of the code raises: an empty list would read as a launcher that looks for nothing.