From d6fc66274fd31ebeb413a4df288f9b764ead2550 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:57:32 +0200 Subject: [PATCH 01/10] fix: let check_freshness read the profile report check_freshness ran profile_sync with --json and the detection flags, which profile_sync refuses, so --profiles always reported an unreadable report. The version check now lives on the report and travels in the JSON and markdown outputs; check_freshness asks for --json --check-version only. --- scripts/check_freshness.py | 2 -- scripts/profile_sync.py | 64 +++++++++++++++++++++++------------ tests/test_check_freshness.py | 18 ++++++++++ tests/test_profile_sync.py | 15 +++++++- 4 files changed, 74 insertions(+), 25 deletions(-) diff --git a/scripts/check_freshness.py b/scripts/check_freshness.py index ca555380..604fcfcb 100644 --- a/scripts/check_freshness.py +++ b/scripts/check_freshness.py @@ -1253,8 +1253,6 @@ def check_profiles(run: bool, offline: bool) -> list[Finding]: "--all", "--json", "--check-version", - "--detect-new-files", - "--watch-hashes", ] if offline: cmd.append("--offline") diff --git a/scripts/profile_sync.py b/scripts/profile_sync.py index 1b890213..2d670119 100644 --- a/scripts/profile_sync.py +++ b/scripts/profile_sync.py @@ -950,6 +950,10 @@ class ProfileReport: # review, or the triage read "0 to review" on a profile whose pin could # not move (blastem, xenia-edge, lindbergh-loader). stale_prose: list[str] | None = None + # --check-version on a forge: core_version against the latest tag and + # release, kept on the report so every output carries it. + version: VersionReport | None = None + version_warning: str | None = None # 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. @@ -1958,6 +1962,15 @@ def format_report(report: ProfileReport, changed_only: bool = False) -> str: if report.page_version and report.page_version == report.no_genuine_copy else "") ) + if report.version: + version = report.version + lines.append( + f" version: declared {version.declared}, " + f"latest tag {version.latest_tag}, " + f"latest release {version.latest_release} ({version.release_date})" + ) + if report.version_warning: + lines.append(f" {report.version_warning}") if report.skipped: lines.append(f" skipped: {report.skipped}") return "\n".join(lines) @@ -3632,23 +3645,6 @@ def _cited_paths(report: ProfileReport) -> list[str]: ) -def _print_version( - args: argparse.Namespace, profile: dict, report: ProfileReport, - repo: upstream.Repo, -) -> None: - version = check_version(profile, repo, args.cache_dir, args.offline) - if version is None: - return - print( - f" version: declared {version.declared}, " - f"latest tag {version.latest_tag}, " - f"latest release {version.latest_release} ({version.release_date})" - ) - warning = version_warning(version, report.pin) - if warning: - print(f" {warning}") - - def _print_detection( args: argparse.Namespace, profile: dict, report: ProfileReport, repo: upstream.Repo, @@ -3682,6 +3678,27 @@ def _print_detection( print(f" new {kind}: {path} {value}") +def _attach_version( + args: argparse.Namespace, profile: dict, report: ProfileReport +) -> None: + """Record core_version against the forge's latest tag and release. + + Kept on the report rather than printed, so --json carries it as the text + report does. A forge that refuses is named on the profile's own line. + """ + repo = select_repo(profile) if report.head else None + if repo is None: + return + try: + report.version = check_version(profile, repo, args.cache_dir, args.offline) + except upstream.RateLimitError: + raise + except upstream.UpstreamError as exc: + print(f"{report.name}: version check skipped: {exc}", file=sys.stderr) + return + report.version_warning = version_warning(report.version, report.pin) + + def _print_extras( args: argparse.Namespace, profile: dict, report: ProfileReport ) -> None: @@ -3693,8 +3710,7 @@ def _print_extras( stops everything. """ wants = ( - args.check_version - or args.detect_new_files + args.detect_new_files or args.watch_hashes or args.full_diff or args.tree_diff @@ -3716,8 +3732,6 @@ def _print_extras_from( args: argparse.Namespace, profile: dict, report: ProfileReport, repo: upstream.Repo, ) -> None: - if args.check_version: - _print_version(args, profile, report, repo) if args.detect_new_files or args.watch_hashes: _print_detection(args, profile, report, repo) if args.full_diff: @@ -3864,9 +3878,13 @@ def _refuse_flag_combinations(args: argparse.Namespace) -> None: ("--triage", args.triage), ) if on ] + # The version check lives on the report, so the outputs built from the + # report (text, --json, --markdown) carry it. A URL list and a ranking + # do not. + lists_only = args.fetch_plan or args.triage extras = [ flag for flag, on in ( - ("--check-version", args.check_version), + ("--check-version", args.check_version and lists_only), ("--detect-new-files", args.detect_new_files), ("--watch-hashes", args.watch_hashes), ("--full-diff", args.full_diff), @@ -3963,6 +3981,8 @@ def main() -> None: report.page_version = page_version(profile, args.cache_dir, args.offline) report.no_genuine_copy = str( profile["version_source"].get("no_genuine_copy") or "") or None + if args.check_version: + _attach_version(args, profile, report) reports.append(report) if writes: try: diff --git a/tests/test_check_freshness.py b/tests/test_check_freshness.py index a0c36df1..664fc636 100644 --- a/tests/test_check_freshness.py +++ b/tests/test_check_freshness.py @@ -388,6 +388,24 @@ class ProfileSummaryTests(unittest.TestCase): self.assertNotIn("a", by_subject) self.assertNotIn("b", by_subject) + def test_profile_sync_accepts_the_command_it_is_given(self): + """--json with the detection flags was refused, so --profiles read an + empty stdout and reported "unreadable report" on every run.""" + import profile_sync # noqa: PLC0415 + + captured = [] + + def run(cmd, **kwargs): + captured.append(cmd) + return mock.Mock(returncode=0, stdout="[]", stderr="") + + with mock.patch.object(cf.subprocess, "run", side_effect=run): + cf.check_profiles(True, offline=True) + (cmd,) = captured + args = profile_sync.build_parser().parse_args(cmd[2:]) + profile_sync._refuse_flag_combinations(args) + self.assertTrue(args.as_json) + def test_a_non_list_report_is_an_error_not_a_crash(self): findings = cf.summarize_profile_sync({"oops": 1}) self.assertEqual(findings[0].status, cf.ERROR) diff --git a/tests/test_profile_sync.py b/tests/test_profile_sync.py index aa47269c..754fd28c 100644 --- a/tests/test_profile_sync.py +++ b/tests/test_profile_sync.py @@ -4022,7 +4022,20 @@ class TestOutputModesApplyTheirFlags(unittest.TestCase): self.assertIn("prose under an older pin", out) def test_extras_are_refused_with_json(self): - code, _, err = self._main("--json", "--check-version") + code, _, err = self._main("--json", "--detect-new-files") + self.assertEqual(code, 1) + self.assertIn("--detect-new-files", err) + + def test_json_carries_the_version_check(self): + """check_freshness asks for --json --check-version: refused, its + profile row read an empty report as unreadable on every run.""" + code, out, err = self._main("--json", "--check-version") + self.assertEqual(code, 0, err) + rows = json.loads(out) + self.assertTrue(all("version" in row and "page_version" in row for row in rows)) + + def test_rankings_refuse_the_version_check(self): + code, _, err = self._main("--triage", "--check-version") self.assertEqual(code, 1) self.assertIn("--check-version", err) From b784a73d5c4377456b820cab43a812bc5ffb6957 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:59:12 +0200 Subject: [PATCH 02/10] fix: name a repository the forge renamed GitHub, GitLab and Gitea redirect a renamed repository, and every call follows the redirect: AloysHF/NicaiEmu answers as AloysHF/CoolbarEmu and the profile read as current. profile_sync now reads the name the forge answers under and counts a move as review work. --- scripts/profile_sync.py | 31 +++++++++++++++++++++++++++++-- scripts/upstream.py | 19 +++++++++++++++++++ tests/test_profile_sync.py | 27 +++++++++++++++++++++++++++ tests/test_upstream.py | 24 ++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 2 deletions(-) diff --git a/scripts/profile_sync.py b/scripts/profile_sync.py index 2d670119..4d6c929f 100644 --- a/scripts/profile_sync.py +++ b/scripts/profile_sync.py @@ -939,6 +939,10 @@ class ProfileReport: # Repositories whose pin is no ancestor of the branch the profile # tracks: history was rewritten, or the pin lies on another branch. orphaned: list[str] = field(default_factory=list) + # Repositories the forge answers under another owner/name: renamed or + # transferred. The redirect is followed in silence, so the move is named + # and counted as review work, since a renamed project is re-profiled. + moved_repos: list[str] = field(default_factory=list) # A profile off any forge names the page announcing its releases # (version_source): the version read there, beside the one declared. declared_version: str | None = None @@ -976,6 +980,7 @@ class ProfileReport: sum(counts.get(status, 0) for status in REVIEW_STATUSES) + len(self.orphaned) + len(self.stale_prose or ()) + + len(self.moved_repos) ) @@ -1332,6 +1337,25 @@ def _path_roles(refs: list) -> dict[str, set[str]]: return roles +def moved_repositories( + views: list[RepoView], cache_dir: str, offline: bool +) -> list[str]: + """Declared repositories the forge now answers under another name.""" + moved = [] + seen: set[tuple[str, str]] = set() + for view in views: + key = (view.repo.host, view.repo.slug) + if key in seen: + continue + seen.add(key) + current = upstream.current_slug(view.repo, cache_dir, offline) + if current and current.lower() != view.repo.slug.lower(): + moved.append( + f"{view.repo.slug} answers as {current} (renamed or transferred)" + ) + return moved + + def build_report( name: str, profile: dict, cache_dir: str, offline: bool = False, text: str | None = None, @@ -1451,6 +1475,7 @@ def build_report( report.upstream_pin = view.pin report.upstream_head = view.head report.upstream_origin = view.origin + report.moved_repos = moved_repositories(views, cache_dir, offline) # A profile can hold its pin on purpose, documenting a build that is that # revision. Comparing it to HEAD says nothing and recaling would repoint # its refs at code the build never had, so it is judged like a frozen tag: @@ -1954,6 +1979,8 @@ def format_report(report: ProfileReport, changed_only: bool = False) -> str: lines.append(f" not read (unsupported host): {url}") for line in report.orphaned: lines.append(f" orphaned pin: {line}") + for line in report.moved_repos: + lines.append(f" moved repository: {line}") if report.declared_version or report.page_version: lines.append( f" version page: declared {report.declared_version or '-'}, " @@ -3509,9 +3536,9 @@ def _check_quota(count: int, offline: bool) -> None: """Refuse a run the anonymous quota cannot carry to completion.""" if offline or github_token(): return - if count * 4 > ANON_QUOTA: + if count * 5 > ANON_QUOTA: print( - f"{count} profiles need roughly {count * 4} API calls, the anonymous " + f"{count} profiles need roughly {count * 5} API calls, the anonymous " f"quota is {ANON_QUOTA} per hour. Log in with `gh auth login`, set " "GITHUB_TOKEN, or use --offline.", file=sys.stderr, diff --git a/scripts/upstream.py b/scripts/upstream.py index 4272cf55..50ddd9d3 100644 --- a/scripts/upstream.py +++ b/scripts/upstream.py @@ -455,6 +455,25 @@ def _first_sha(payload: object) -> str | None: return None +def current_slug(repo: Repo, cache_dir: str, offline: bool = False) -> str | None: + """The owner/name the forge answers for a repository today. + + A renamed or transferred repository keeps answering under its old name: + GitHub redirects (AloysHF/NicaiEmu answers as AloysHF/CoolbarEmu), GitLab + and Gitea do the same. Every later call follows the redirect, so only the + name in the payload tells the project moved. + """ + if repo.family == "gitlab": + url, key = f"{repo.api_base}/projects/{_project(repo)}", "path_with_namespace" + else: + url, key = f"{repo.api_base}/repos/{repo.slug}", "full_name" + payload = _api(url, cache_dir, offline, MOVING_TTL) + if not isinstance(payload, dict): + return None + value = payload.get(key) + return str(value) if value else None + + def resolve_head( repo: Repo, cache_dir: str, offline: bool = False, branch: str | None = None ) -> str | None: diff --git a/tests/test_profile_sync.py b/tests/test_profile_sync.py index 754fd28c..9b723d35 100644 --- a/tests/test_profile_sync.py +++ b/tests/test_profile_sync.py @@ -1163,6 +1163,13 @@ class TestBuildReport(unittest.TestCase): profile_sync.upstream._http_json, profile_sync.upstream._http_text, profile_sync.upstream.list_gitlinks, + profile_sync.upstream.current_slug, + ) + self.current_slugs: dict[str, str] = {} + profile_sync.upstream.current_slug = ( + lambda repo, cache_dir, offline=False: self.current_slugs.get( + repo.slug, repo.slug + ) ) self.tags: list[str] = [] self.gitlinks: dict[str, dict[str, str]] = {} @@ -1218,6 +1225,7 @@ class TestBuildReport(unittest.TestCase): profile_sync.upstream._http_json, profile_sync.upstream._http_text, profile_sync.upstream.list_gitlinks, + profile_sync.upstream.current_slug, ) = self._orig self.tmp.cleanup() @@ -1232,6 +1240,25 @@ class TestBuildReport(unittest.TestCase): ], } + def test_a_renamed_repository_is_review_work(self): + """AloysHF/NicaiEmu answers as AloysHF/CoolbarEmu: the redirect was + followed in silence and the profile read as up to date.""" + self.files[("pinsha", "a.c")] = ["x", "y"] + self.files[("headsha", "a.c")] = ["x", "y"] + self.current_slugs["o/n"] = "o/renamed" + report = build_report("test", self._profile(["a.c:2"]), self.dir) + self.assertEqual(len(report.moved_repos), 1) + self.assertIn("o/renamed", report.moved_repos[0]) + self.assertGreaterEqual(report.needs_review(), 1) + self.assertIn("moved repository", profile_sync.format_report(report)) + + def test_a_repository_under_its_own_name_is_not_moved(self): + self.files[("pinsha", "a.c")] = ["x", "y"] + self.files[("headsha", "a.c")] = ["x", "y"] + self.current_slugs["o/n"] = "O/N" + report = build_report("test", self._profile(["a.c:2"]), self.dir) + self.assertEqual(report.moved_repos, []) + def test_a_refusing_forge_reads_as_unread_not_gone(self): """git.eden-emu.dev answers 403 to scripts: the muted repository's citations read "absent at HEAD" for files that exist.""" diff --git a/tests/test_upstream.py b/tests/test_upstream.py index 282cd57b..c88ece29 100644 --- a/tests/test_upstream.py +++ b/tests/test_upstream.py @@ -805,5 +805,29 @@ class TestGiteaCommitAt(unittest.TestCase): ) + +class TestCurrentSlug(unittest.TestCase): + """The name a forge answers under, read from the repository payload.""" + + def test_github_and_gitlab_payloads(self): + answers = { + "https://api.github.com/repos/AloysHF/NicaiEmu": { + "full_name": "AloysHF/CoolbarEmu" + }, + "https://gitlab.com/api/v4/projects/g%2Fsub%2Fp": { + "path_with_namespace": "g/sub/p" + }, + } + with tempfile.TemporaryDirectory() as tmp, \ + mock.patch.object(upstream, "_http_json", side_effect=answers.get): + github = parse_repo("https://github.com/AloysHF/NicaiEmu") + gitlab = parse_repo("https://gitlab.com/g/sub/p") + self.assertEqual(upstream.current_slug(github, tmp), "AloysHF/CoolbarEmu") + self.assertEqual(upstream.current_slug(gitlab, tmp), "g/sub/p") + self.assertIsNone( + upstream.current_slug(parse_repo("https://github.com/a/b"), tmp) + ) + + if __name__ == "__main__": unittest.main() From 1b3763e58fe00da4085ad5747f248e42338f6131 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:59:51 +0200 Subject: [PATCH 03/10] feat: read the voidpoint.io gitlab eduke32 and fury declare https://voidpoint.io/terminx/eduke32 and were skipped as an unsupported host. The instance serves the GitLab v4 API (/api/v4/projects/terminx%2Feduke32 answers 200) and /-/raw/ paths. --- scripts/upstream.py | 6 ++++++ tests/test_upstream.py | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/scripts/upstream.py b/scripts/upstream.py index 50ddd9d3..e5eec95f 100644 --- a/scripts/upstream.py +++ b/scripts/upstream.py @@ -46,6 +46,12 @@ _HOSTS: dict[str, tuple[str, str, str]] = { "https://git.libretro.com/api/v4", "https://git.libretro.com", ), + # EDuke32 and its forks (eduke32, fury) are developed on this GitLab. + "voidpoint.io": ( + "gitlab", + "https://voidpoint.io/api/v4", + "https://voidpoint.io", + ), "codeberg.org": ( "forgejo", "https://codeberg.org/api/v1", diff --git a/tests/test_upstream.py b/tests/test_upstream.py index c88ece29..c4f7302a 100644 --- a/tests/test_upstream.py +++ b/tests/test_upstream.py @@ -66,6 +66,16 @@ class TestParseRepo(unittest.TestCase): "/-/raw/abc123/source/Engine.cpp", ) + def test_voidpoint_is_a_gitlab(self): + """eduke32 and fury were skipped as an unsupported host.""" + repo = parse_repo("https://voidpoint.io/terminx/eduke32") + self.assertEqual(repo.family, "gitlab") + self.assertEqual(repo.api_base, "https://voidpoint.io/api/v4") + self.assertEqual( + raw_url(repo, "abc123", "source/build/src/build.cpp"), + "https://voidpoint.io/terminx/eduke32/-/raw/abc123/source/build/src/build.cpp", + ) + def test_libretro_gitlab_is_a_gitlab(self): repo = parse_repo("https://git.libretro.com/libretro/wqxemu.git") self.assertEqual(repo.family, "gitlab") From 57ac252482bfe7ccff8c2c10fefa0fd90a57f385 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 06:04:00 +0200 Subject: [PATCH 04/10] feat: check the profiles a branch adds or edits profile_sync gains --changed (added or edited profiles, uncommitted ones included) and --added, both against the merge base of --base (origin/main by default). CONTRIBUTING runs --changed --strict instead of one --emulator call per new profile, and the CI job runs --added in place of its own diff loop. --- .github/workflows/validate.yml | 9 +---- CONTRIBUTING.md | 2 +- scripts/generate_readme.py | 2 +- scripts/profile_sync.py | 57 ++++++++++++++++++++++++++ tests/test_audit_regressions.py | 2 +- tests/test_profile_sync.py | 72 ++++++++++++++++++++++++++++++++- wiki/release-process.md | 7 ++-- wiki/tools.md | 7 +++- 8 files changed, 141 insertions(+), 17 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index c10821e1..80572b13 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -186,14 +186,7 @@ jobs: else base=$BEFORE fi - status=0 - for file in $(git diff --name-only --diff-filter=A "$base" HEAD -- 'emulators/*.yml'); do - name=$(basename "$file" .yml) - echo "::group::$name" - python scripts/profile_sync.py --emulator "$name" --strict || status=1 - echo "::endgroup::" - done - exit $status + python scripts/profile_sync.py --added --base "$base" --strict run-tests: runs-on: ubuntu-latest diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index df37fb8c..fa5b7e12 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -49,7 +49,7 @@ The checks CI runs on a pull request (Python 3.12 or newer): pip install pyyaml "jsonschema[format-nongpl]" markdown python scripts/validate_pr.py --changed # the BIOS files the branch adds python scripts/validate_schemas.py --source-only # profiles and platform files -python scripts/profile_sync.py --emulator NAME --strict # each profile the branch adds +python scripts/profile_sync.py --changed --strict # each profile the branch adds or edits python scripts/refresh_data_dirs.py # data directories the pack-building tests read python -m unittest discover -s tests -t . ``` diff --git a/scripts/generate_readme.py b/scripts/generate_readme.py index f1254e51..d901fc18 100644 --- a/scripts/generate_readme.py +++ b/scripts/generate_readme.py @@ -757,7 +757,7 @@ The checks CI runs on a pull request (Python 3.12 or newer): pip install pyyaml "jsonschema[format-nongpl]" markdown python scripts/validate_pr.py --changed # the BIOS files the branch adds python scripts/validate_schemas.py --source-only # profiles and platform files -python scripts/profile_sync.py --emulator NAME --strict # each profile the branch adds +python scripts/profile_sync.py --changed --strict # each profile the branch adds or edits python scripts/refresh_data_dirs.py # data directories the pack-building tests read python -m unittest discover -s tests -t . ``` diff --git a/scripts/profile_sync.py b/scripts/profile_sync.py index 4d6c929f..603f4303 100644 --- a/scripts/profile_sync.py +++ b/scripts/profile_sync.py @@ -3367,6 +3367,20 @@ def build_parser() -> argparse.ArgumentParser: group = parser.add_mutually_exclusive_group(required=True) group.add_argument("--emulator", help="a single profile") group.add_argument("--all", action="store_true", help="every profile") + group.add_argument( + "--changed", action="store_true", + help="the profiles the branch adds or edits against --base, " + "uncommitted ones included", + ) + group.add_argument( + "--added", action="store_true", + help="the profiles the branch adds against --base, as CI checks them", + ) + parser.add_argument( + "--base", default="origin/main", + help="revision --changed and --added compare with, through its merge " + "base with HEAD (default: %(default)s)", + ) parser.add_argument( "--emulators-dir", default="emulators", help="emulator profile directory (default: %(default)s)", @@ -3481,8 +3495,51 @@ def build_parser() -> argparse.ArgumentParser: return parser +def _git_lines(args: list[str]) -> list[str]: + result = subprocess.run( + ["git", *args], capture_output=True, text=True, check=False + ) + if result.returncode: + detail = result.stderr.strip() or f"exit {result.returncode}" + raise SystemExit(f"ERROR: git {' '.join(args)}: {detail}") + return [line for line in result.stdout.splitlines() if line] + + +def branch_profiles(emulators_dir: str, base: str, added_only: bool) -> list[str]: + """Names of the profiles the branch adds (or edits) against ``base``. + + The diff starts at the merge base, so commits main gained since the + branch point are not counted, and ends at the working tree, so a + profile not yet committed is checked too. A rename reads as an addition. + """ + merge_base = _git_lines(["merge-base", base, "HEAD"])[0] + paths = _git_lines([ + "diff", "--name-only", "--no-renames", + "--diff-filter=A" if added_only else "--diff-filter=AM", + merge_base, "--", emulators_dir, + ]) + paths += _git_lines([ + "ls-files", "--others", "--exclude-standard", "--full-name", + "--", emulators_dir, + ]) + top = Path(_git_lines(["rev-parse", "--show-toplevel"])[0]) + root = Path(emulators_dir).resolve() + return sorted({ + full.stem + for path in paths + if (full := (top / path).resolve()).suffix == ".yml" and full.parent == root + }) + + def select_profiles(profiles: dict, args: argparse.Namespace) -> dict: """Profiles matching the selection flags.""" + if args.changed or args.added: + names = branch_profiles(args.emulators_dir, args.base, args.added) + selected = {name: profiles[name] for name in names if name in profiles} + if not selected: + verb = "adds" if args.added else "adds or edits" + print(f"the branch {verb} no profile against {args.base}", file=sys.stderr) + return selected if args.emulator: if args.emulator not in profiles: # Suggested from what the listing it names shows: counted over the diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 0c392b3a..19cfac28 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -276,7 +276,7 @@ class WorkflowRegressions(unittest.TestCase): self.assertTrue(runs, "no job confronts profiles with their upstream") for name, body in runs: self.assertIn("--strict", body) - self.assertIn("--diff-filter=A", body) + self.assertIn("--added", body) self.assertIsNone(workflow["jobs"][name].get("if"), f"{name} must run on push too") def test_both_events_watch_the_same_paths(self): diff --git a/tests/test_profile_sync.py b/tests/test_profile_sync.py index 9b723d35..ffe4d85c 100644 --- a/tests/test_profile_sync.py +++ b/tests/test_profile_sync.py @@ -4261,8 +4261,9 @@ class TestSelectionNamesWhatMatchedNothing(unittest.TestCase): } def _args(self, **values): - base = {"emulator": None, "system": None, "type_": None, - "stale_before": None, "limit": None} + base = {"emulator": None, "changed": False, "added": False, + "system": None, "type_": None, "stale_before": None, + "limit": None} return argparse.Namespace(**{**base, **values}) def test_a_misspelt_system_stops_and_suggests(self): @@ -4610,5 +4611,72 @@ class TheTriageCountsWhatBlocksThePin(unittest.TestCase): self.assertEqual(len(err.getvalue().splitlines()), 1, err.getvalue()) +class TestBranchProfiles(unittest.TestCase): + """--changed and --added name the profiles a branch brings, not by hand.""" + + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + self.root = Path(self._tmp.name) + self._cwd = os.getcwd() + self.addCleanup(os.chdir, self._cwd) + os.chdir(self.root) + env = {**os.environ, "GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@t", + "GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@t"} + self.env = env + self._git("init", "-q", "-b", "main") + (self.root / "emulators").mkdir() + (self.root / "emulators" / "kept.yml").write_text("a: 1\n") + (self.root / "emulators" / "edited.yml").write_text("a: 1\n") + self._git("add", "emulators") + self._git("commit", "-q", "--no-verify", "-m", "base") + self._git("checkout", "-q", "-b", "topic") + (self.root / "emulators" / "edited.yml").write_text("a: 2\n") + (self.root / "emulators" / "new.yml").write_text("a: 1\n") + self._git("add", "emulators") + self._git("commit", "-q", "--no-verify", "-m", "topic") + (self.root / "emulators" / "draft.yml").write_text("a: 1\n") + (self.root / "emulators" / "notes.txt").write_text("x\n") + + def _git(self, *args): + subprocess.run(["git", *args], cwd=self.root, env=self.env, check=True) + + def test_changed_takes_added_edited_and_uncommitted(self): + self.assertEqual( + profile_sync.branch_profiles("emulators", "main", added_only=False), + ["draft", "edited", "new"], + ) + + def test_added_leaves_edited_profiles_out(self): + self.assertEqual( + profile_sync.branch_profiles("emulators", "main", added_only=True), + ["draft", "new"], + ) + + def test_the_flag_selects_those_profiles(self): + args = profile_sync.build_parser().parse_args(["--changed", "--base", "main"]) + profiles = {"kept": {}, "edited": {}, "new": {}, "draft": {}} + self.assertEqual( + list(profile_sync.select_profiles(profiles, args)), + ["draft", "edited", "new"], + ) + + +class TestBranchCheckIsWired(unittest.TestCase): + """CONTRIBUTING and CI name the branch's profiles through the flag.""" + + def test_contributing_and_ci_use_the_flags(self): + root = Path(__file__).resolve().parents[1] + sys.path.insert(0, str(root / "scripts")) + import generate_readme # noqa: PLC0415 + + text = generate_readme.generate_contributing() + self.assertIn("profile_sync.py --changed --strict", text) + self.assertNotIn("--emulator NAME --strict", text) + workflow = (root / ".github" / "workflows" / "validate.yml").read_text() + self.assertIn("profile_sync.py --added --base", workflow) + self.assertNotIn('--emulator "$name"', workflow) + + if __name__ == "__main__": unittest.main() diff --git a/wiki/release-process.md b/wiki/release-process.md index 5151da20..864fb59c 100644 --- a/wiki/release-process.md +++ b/wiki/release-process.md @@ -103,9 +103,10 @@ every emulator profile against `schemas/emulator.schema.json`. Both schemas set `additionalProperties: false`, so a typo in a field name fails the job instead of being silently ignored. -**check-new-profiles.** Runs `profile_sync.py --emulator NAME --strict` on -each profile the change adds (the files `git diff --diff-filter=A` names -against the pull request base, or against the commit a push moved main from). +**check-new-profiles.** Runs `profile_sync.py --added --base BASE --strict`, +which checks each profile the change adds (the files `git diff --diff-filter=A` +names against the pull request base, or against the commit a push moved main +from). A profile written from a clone can cite lines that do not hold the value it declares while schemas and tests pass it. This job confronts it with its upstream. An edited profile is not held to it. diff --git a/wiki/tools.md b/wiki/tools.md index e29832d9..ef7244dc 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -401,13 +401,18 @@ and `--limit 20`. Output takes `--markdown` (a report written under `--report-dir`, `reports/` by default), and `--full-diff` takes `--ref PATH` to show one cited file and `--context N` lines around each hunk. `--strict` exits 1 when a profile has refs to review, and CI runs it on every profile a change -adds. `--offline` answers from `--cache-dir` alone. +adds. `--offline` answers from `--cache-dir` alone. `--changed` selects the +profiles the branch adds or edits, uncommitted ones included, and `--added` +the ones it adds, both against the merge base of `--base` (`origin/main` by +default) and HEAD. ```bash python scripts/profile_sync.py --all --system sega-saturn --changed-only python scripts/profile_sync.py --all --stale-before 2026-06-01 --limit 20 --triage python scripts/profile_sync.py --emulator vice --full-diff --ref vice/src/c64/c64rom.h --context 6 python scripts/profile_sync.py --emulator vice --strict +python scripts/profile_sync.py --changed --strict +python scripts/profile_sync.py --added --base upstream/main --strict ``` Per part of a ref: `ANCHORED` (same content, same lines), `SHIFTED` (same From 66937d21d7bc0a0749abbd1ab63a4e99bd5facbd Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 06:05:13 +0200 Subject: [PATCH 05/10] docs: install the site packages for contributors The contributor install line left out mkdocs-material and pymdown-extensions, so pipeline step 8b (render site) was skipped. The ranges are the ones deploy-site.yml installs, and a test reads them from the workflow. --- CONTRIBUTING.md | 3 ++- scripts/generate_readme.py | 3 ++- tests/test_contributing_text.py | 12 ++++++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fa5b7e12..4860f55e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -46,7 +46,8 @@ Full walkthrough: [profiling guide](https://abdess.github.io/retrobios/wiki/prof The checks CI runs on a pull request (Python 3.12 or newer): ```bash -pip install pyyaml "jsonschema[format-nongpl]" markdown +pip install pyyaml "jsonschema[format-nongpl]" markdown \ + "mkdocs-material>=9.7.5,<10" "pymdown-extensions>=10.14" python scripts/validate_pr.py --changed # the BIOS files the branch adds python scripts/validate_schemas.py --source-only # profiles and platform files python scripts/profile_sync.py --changed --strict # each profile the branch adds or edits diff --git a/scripts/generate_readme.py b/scripts/generate_readme.py index d901fc18..e0650551 100644 --- a/scripts/generate_readme.py +++ b/scripts/generate_readme.py @@ -754,7 +754,8 @@ Full walkthrough: [profiling guide]({link("wiki/profiling")}). The checks CI runs on a pull request (Python 3.12 or newer): ```bash -pip install pyyaml "jsonschema[format-nongpl]" markdown +pip install pyyaml "jsonschema[format-nongpl]" markdown \\ + "mkdocs-material>=9.7.5,<10" "pymdown-extensions>=10.14" python scripts/validate_pr.py --changed # the BIOS files the branch adds python scripts/validate_schemas.py --source-only # profiles and platform files python scripts/profile_sync.py --changed --strict # each profile the branch adds or edits diff --git a/tests/test_contributing_text.py b/tests/test_contributing_text.py index dda3c7a0..1f72f67d 100644 --- a/tests/test_contributing_text.py +++ b/tests/test_contributing_text.py @@ -70,6 +70,18 @@ class BeforeOpeningAPullRequest(unittest.TestCase): workflow = (REPO_ROOT / ".github" / "workflows" / "validate.yml").read_text(encoding="utf-8") self.assertIn("validate_schemas.py --source-only", workflow) + def test_it_installs_the_packages_the_site_render_needs(self): + """Without mkdocs-material and pymdown-extensions the pipeline's + render step (8b) is skipped for every contributor.""" + deploy = (REPO_ROOT / ".github" / "workflows" / "deploy-site.yml").read_text( + encoding="utf-8" + ) + specs = re.findall(r'"((?:mkdocs-material|pymdown-extensions)[^"]*)"', deploy) + self.assertEqual(len(specs), 2) + text = generate_readme.generate_contributing() + for spec in specs: + self.assertIn(f'"{spec}"', text) + if __name__ == "__main__": unittest.main() From b8baf92f1deed8a3d74f1c4b8f15c18510cfb3a3 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 06:08:27 +0200 Subject: [PATCH 06/10] fix: give atomic writes the umask mode mkstemp creates its scratch 0600 and os.replace kept that mode, so database.json, the manifests, the packs, the recipe snapshots and the large-file cache were readable by their owner only. set_plain_mode gives the scratch 0o666 less the umask, as open() would, before each rename. A test walks every function that renames a mkstemp file. --- scripts/artifacts.py | 21 +++++++++ scripts/common.py | 2 + scripts/generate_db.py | 2 + scripts/generate_pack.py | 4 +- scripts/largefiles.py | 2 + scripts/refresh_data_dirs.py | 3 +- scripts/upstream.py | 3 ++ scripts/ziptools.py | 2 + tests/test_atomic_artifacts.py | 85 ++++++++++++++++++++++++++++++++++ 9 files changed, 122 insertions(+), 2 deletions(-) diff --git a/scripts/artifacts.py b/scripts/artifacts.py index b233bb2e..e0442d53 100644 --- a/scripts/artifacts.py +++ b/scripts/artifacts.py @@ -50,6 +50,25 @@ def write_if_changed( return True +def plain_file_mode() -> int: + """The mode open() gives a new file: 0o666 less the process umask.""" + # Reading the umask sets it, so it is put back at once. + umask = os.umask(0) + os.umask(umask) + return 0o666 & ~umask + + +def set_plain_mode(path: str | os.PathLike) -> None: + """Give a scratch file the mode a plain open() would have. + + mkstemp creates its file 0600 so no other user reads it while it is + written. Renamed into place, that mode stayed: database.json, the + manifests, the packs and the recipe snapshots came out unreadable to + every other account on the machine and to a web server serving them. + """ + os.chmod(path, plain_file_mode()) + + def write_text_atomic(path: str, content: str) -> None: """Write a whole file or nothing. @@ -65,6 +84,7 @@ def write_text_atomic(path: str, content: str) -> None: try: with os.fdopen(handle, "w", encoding="utf-8") as f: f.write(content) + set_plain_mode(scratch) os.replace(scratch, path) except BaseException: with contextlib.suppress(OSError): @@ -81,6 +101,7 @@ def write_bytes_atomic(path: str, content: bytes) -> None: try: with os.fdopen(handle, "wb") as f: f.write(content) + set_plain_mode(scratch) os.replace(scratch, path) except BaseException: with contextlib.suppress(OSError): diff --git a/scripts/common.py b/scripts/common.py index 79404098..35b53465 100644 --- a/scripts/common.py +++ b/scripts/common.py @@ -2805,6 +2805,8 @@ from artifacts import ( # noqa: E402,F401 write_if_changed, write_text_atomic, copy_file_atomic, + plain_file_mode, + set_plain_mode, ArtifactLockBusy, artifact_lock, hold_artifact_lock, diff --git a/scripts/generate_db.py b/scripts/generate_db.py index 398a3b71..855a6fd6 100644 --- a/scripts/generate_db.py +++ b/scripts/generate_db.py @@ -27,6 +27,7 @@ from common import ( list_registered_platforms, load_provenance_snapshots, require_yaml, + set_plain_mode, source_yaml_paths, variant_base_name, write_if_changed, @@ -309,6 +310,7 @@ def _restore(cached: str, path: str) -> None: os.close(fd) try: shutil.copy2(cached, scratch) + set_plain_mode(scratch) os.replace(scratch, path) finally: if os.path.exists(scratch): diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index e1cb1060..2fb7f728 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -14,7 +14,7 @@ from __future__ import annotations from collections.abc import Callable, Iterator -from artifacts import _build_timestamp +from artifacts import _build_timestamp, set_plain_mode import argparse import contextlib import functools @@ -221,6 +221,7 @@ def _writing_pack(zip_path: str) -> Iterator[zipfile.ZipFile]: try: with zipfile.ZipFile(scratch, "w", zipfile.ZIP_DEFLATED) as zf: yield zf + set_plain_mode(scratch) os.replace(scratch, zip_path) except BaseException: Path(scratch).unlink(missing_ok=True) @@ -4573,6 +4574,7 @@ def inject_manifest(zip_path: str, manifest: dict) -> None: continue dst.writestr(item, src.read(item.filename)) _write_generated_member(dst, "manifest.json", manifest_json) + set_plain_mode(scratch) os.replace(scratch, zip_path) except BaseException: Path(scratch).unlink(missing_ok=True) diff --git a/scripts/largefiles.py b/scripts/largefiles.py index 5089f44e..2c54fd43 100644 --- a/scripts/largefiles.py +++ b/scripts/largefiles.py @@ -19,6 +19,7 @@ from collections import Counter from collections.abc import Iterable from pathlib import Path +from artifacts import set_plain_mode from hashing import compute_hashes @@ -219,6 +220,7 @@ def _fetch_asset( compute_hashes(tmp_path), expected_sha1, expected_md5 ): return None + set_plain_mode(tmp_path) os.replace(tmp_path, cached) return cached finally: diff --git a/scripts/refresh_data_dirs.py b/scripts/refresh_data_dirs.py index d39e855d..f1f0c4b5 100644 --- a/scripts/refresh_data_dirs.py +++ b/scripts/refresh_data_dirs.py @@ -25,7 +25,7 @@ import urllib.request import zipfile from collections.abc import Iterator from pathlib import Path -from artifacts import file_lock as _file_lock +from artifacts import file_lock as _file_lock, set_plain_mode from common import github_token, platform_arg, require_platform, yaml_load try: @@ -133,6 +133,7 @@ def _save_versions( with os.fdopen(handle, "w") as f: json.dump(versions, f, indent=2, sort_keys=True) f.write("\n") + set_plain_mode(scratch) os.replace(scratch, path) except BaseException: with contextlib.suppress(OSError): diff --git a/scripts/upstream.py b/scripts/upstream.py index e5eec95f..025b1de2 100644 --- a/scripts/upstream.py +++ b/scripts/upstream.py @@ -21,6 +21,8 @@ from dataclasses import dataclass from datetime import datetime from pathlib import Path +from artifacts import set_plain_mode + USER_AGENT = "retrobios-profile-sync/1.0" # Where fetched upstream files live, shared by every reader of this cache. CACHE_DIR = ".cache/upstream" @@ -343,6 +345,7 @@ def write_cache(target: Path, text: str) -> None: try: with os.fdopen(fd, "w", encoding="utf-8") as fh: fh.write(text) + set_plain_mode(scratch) os.replace(scratch, target) except OSError: Path(scratch).unlink(missing_ok=True) diff --git a/scripts/ziptools.py b/scripts/ziptools.py index 7e8ab3f1..cb82d41b 100644 --- a/scripts/ziptools.py +++ b/scripts/ziptools.py @@ -14,6 +14,7 @@ import sys import tempfile import zipfile +from artifacts import set_plain_mode from hashing import md5sum @@ -141,6 +142,7 @@ def _write_member_cache(path: str, data: dict[str, list[list]]) -> None: try: with os.fdopen(fd, "w", encoding="utf-8") as fh: json.dump(data, fh, separators=(",", ":")) + set_plain_mode(scratch) os.replace(scratch, path) except OSError as exc: print(f"warning: could not write {path}: {exc}", file=sys.stderr) diff --git a/tests/test_atomic_artifacts.py b/tests/test_atomic_artifacts.py index 497e6650..5da43354 100644 --- a/tests/test_atomic_artifacts.py +++ b/tests/test_atomic_artifacts.py @@ -129,5 +129,90 @@ class TheNativeIndexKeepsEveryPlatform(unittest.TestCase): self.assertEqual(recorded.get("batocera/file"), "https://a") self.assertEqual((root / "batocera" / "file").read_bytes(), b"payload") +class ScratchFilesGetThePlainMode(unittest.TestCase): + """mkstemp makes its file 0600 and the rename kept it: database.json, + the manifests and the packs came out readable by their owner only.""" + + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.addCleanup(self._tmp.cleanup) + self.dir = Path(self._tmp.name) + previous = os.umask(0o022) + self.addCleanup(os.umask, previous) + + def _mode(self, path: Path) -> int: + return path.stat().st_mode & 0o777 + + def test_text_and_bytes_writers(self): + artifacts.write_text_atomic(str(self.dir / "a.json"), "{}") + artifacts.write_bytes_atomic(str(self.dir / "b.bin"), b"x") + self.assertEqual(self._mode(self.dir / "a.json"), 0o644) + self.assertEqual(self._mode(self.dir / "b.bin"), 0o644) + + def test_the_umask_decides(self): + os.umask(0o002) + artifacts.write_text_atomic(str(self.dir / "a.json"), "{}") + self.assertEqual(self._mode(self.dir / "a.json"), 0o664) + + def test_packs_and_their_manifest(self): + import zipfile # noqa: PLC0415 + + import generate_pack # noqa: PLC0415 + + pack = self.dir / "Pack.zip" + with generate_pack._writing_pack(str(pack)) as zf: + zf.writestr("a.bin", b"x") + self.assertEqual(self._mode(pack), 0o644) + os.chmod(pack, 0o600) + generate_pack.inject_manifest(str(pack), {"files": []}) + self.assertEqual(self._mode(pack), 0o644) + with zipfile.ZipFile(pack) as zf: + self.assertIn("manifest.json", zf.namelist()) + + def test_every_renamed_scratch_gets_the_mode(self): + """A function that renames a mkstemp file into place sets its mode. + + copy_file_atomic is the one exception: copy2 gives the scratch the + source file's own mode. + """ + import ast # noqa: PLC0415 + + def name_of(node: ast.AST) -> str | None: + return node.id if isinstance(node, ast.Name) else None + + def callee(call: ast.Call) -> str: + func = call.func + return func.attr if isinstance(func, ast.Attribute) else name_of(func) or "" + + unset = [] + for path in sorted((REPO_ROOT / "scripts").rglob("*.py")): + tree = ast.parse(path.read_text(encoding="utf-8")) + for func in ast.walk(tree): + if not isinstance(func, (ast.FunctionDef, ast.AsyncFunctionDef)): + continue + if func.name == "copy_file_atomic": + continue + scratches: set[str] = set() + for node in ast.walk(func): + if (isinstance(node, ast.Assign) and isinstance(node.value, ast.Call) + and callee(node.value) == "mkstemp" + and isinstance(node.targets[0], ast.Tuple)): + scratch = name_of(node.targets[0].elts[1]) + if scratch: + scratches.add(scratch) + calls = [n for n in ast.walk(func) if isinstance(n, ast.Call)] + moded = { + name_of(c.args[0]) for c in calls + if callee(c) in ("set_plain_mode", "chmod") and c.args + } + for call in calls: + if callee(call) != "replace" or not call.args: + continue + first = name_of(call.args[0]) + if first in scratches and first not in moded: + unset.append(f"{path.name}:{call.lineno} {func.name}") + self.assertEqual(unset, []) + + if __name__ == "__main__": unittest.main() From f19a8f99d362d724f1737569a867639e5828bd3a Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 06:12:08 +0200 Subject: [PATCH 07/10] fix: read composite zip md5s apart in diff_truth Recalbox hashes a .zip by its members. diff_truth set that composite against the profile's container md5 and reported neocdz.zip and neocd.zip as mismatches. _hash_mismatch now drops it through comparable(), the rule pair_entries and the export already apply. --- scripts/truth.py | 7 +++++-- tests/test_truth_diff.py | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/scripts/truth.py b/scripts/truth.py index 718387a6..fd409653 100644 --- a/scripts/truth.py +++ b/scripts/truth.py @@ -735,10 +735,13 @@ def _hash_mismatch(t_entry: dict, s_entry: dict) -> dict | None: A value shared on any field settles it: the truth holds every revision a core accepts, and a profile may give one revision's sha1 without its md5. - A hash the code does not check contradicts nothing (checked_hashes). + A hash the code does not check contradicts nothing (checked_hashes), and + a platform md5 that is a composite of a zip's members is no container + md5 (comparable), the rule pair_entries applies. """ - from exporter.baseline import checked_hashes + from exporter.baseline import checked_hashes, comparable + s_entry = comparable(s_entry) if _hash_set(t_entry) & _hash_set(s_entry): return None t_entry = checked_hashes(t_entry) or {} diff --git a/tests/test_truth_diff.py b/tests/test_truth_diff.py index beaefbbe..75d559c5 100644 --- a/tests/test_truth_diff.py +++ b/tests/test_truth_diff.py @@ -227,6 +227,25 @@ class PlatformMultiHash(unittest.TestCase): self.assertEqual(pair_entries(truth, list(scraped.values())), {0: 0}) +class ACompositeZipMd5IsNoContainerMd5(unittest.TestCase): + """Recalbox hashes a .zip by its members. Its neocdz.zip md5 was set + against geolith's container md5 and reported as a mismatch the export, + which reads the same mark, never corrects.""" + + TRUTH = {"files": [{"name": "neocdz.zip", "md5": "b31a246b69493c35194fe0bac120dc37"}]} + + def _scraped(self, **mark): + return {"files": [{"name": "neocdz.zip", "md5": "f39572af7584cb5b3f70ae8cc848aba2", + **mark}]} + + def test_a_composite_md5_contradicts_nothing(self): + self.assertEqual(_diff_system(self.TRUTH, self._scraped(zip_digest="composite")), {}) + + def test_a_container_md5_still_does(self): + result = _diff_system(self.TRUTH, self._scraped()) + self.assertEqual([m["name"] for m in result["hash_mismatch"]], ["neocdz.zip"]) + + class OnlySystemDirectoryEntries(unittest.TestCase): """The truth kept files read from the save, content or working directory: diff_truth listed Cemu's mlc01 certificates, panda3ds's seeddb.bin and From 566d36275f9eadfdbb7525bcce51fee1993c9579 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 06:18:32 +0200 Subject: [PATCH 08/10] fix: count alias readers in a file's requirers Ymir reads sega_101.bin or saturn_bios.bin, so the sega_101.bin entry carries saturn_bios.bin as an alias. kronos, yabause and yabasanshiro open saturn_bios.bin and fall back to sega_101.bin: they were in that entry's _cores but not its _required_by, while their saturn_bios.bin entry had them requiring it. A reader that requires one of an entry's aliases now requires the entry. No reader is added, and same-name entries kept apart by path are left alone. No export changes. --- scripts/truth.py | 33 +++++++++++++++++++++++++++++ tests/test_truth_diff.py | 45 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/scripts/truth.py b/scripts/truth.py index fd409653..13116bb5 100644 --- a/scripts/truth.py +++ b/scripts/truth.py @@ -200,6 +200,36 @@ def _same_file(files: list[dict], file_entry: dict, emu_name: str) -> dict | Non ) +def _require_by_slot(files: list[dict]) -> None: + """A core that requires a name an entry answers for requires the entry. + + Merged by name, one core's alternatives land in several entries. Ymir + reads sega_101.bin, mpr-17933.bin or saturn_bios.bin, so the sega_101.bin + entry carries those aliases. kronos, yabause and yabasanshiro open + saturn_bios.bin first and sega_101.bin as a fallback: they sat in that + entry's _cores, out of its _required_by, while their saturn_bios.bin entry + had them requiring it, and a platform line paired through the alias read + them as optional. A core already among the entry's readers that requires + one of its aliases, as another entry of the system, is one of its + requirers. No reader is added. + """ + required_by_name: dict[str, set[str]] = {} + for entry in files: + required_by_name.setdefault(entry["name"].lower(), set()).update( + entry.get("_required_by", ()) + ) + for entry in files: + # Only the aliases: entries sharing the entry's own name are other + # files that name already kept apart (pak0.pak under each game). + names = {str(alias).lower() for alias in entry.get("aliases") or ()} + names.discard(entry["name"].lower()) + requirers = set().union(*(required_by_name.get(n, set()) for n in names)) + added = (requirers & entry.get("_cores", set())) - entry.get("_required_by", set()) + if added: + entry["_required_by"] = entry.get("_required_by", set()) | added + entry["required"] = True + + def _brought(file_entry: dict, emu_name: str) -> set[str]: """The core and build that need the file brought, empty when the emulator installs it. An entry read without layer (a test, a caller of @@ -666,6 +696,9 @@ def generate_platform_truth( ) sys_cov["unprofiled"].add(emu_name) + for sys_data in systems.values(): + _require_by_slot(sys_data.get("files", [])) + # Drop files with no exploitable data AFTER all cores have contributed. # A file declared by one core without hash/size/path may be enriched by # another core that has the same entry with data, so the filter must run diff --git a/tests/test_truth_diff.py b/tests/test_truth_diff.py index 75d559c5..fa48edf6 100644 --- a/tests/test_truth_diff.py +++ b/tests/test_truth_diff.py @@ -246,6 +246,51 @@ class ACompositeZipMd5IsNoContainerMd5(unittest.TestCase): self.assertEqual([m["name"] for m in result["hash_mismatch"]], ["neocdz.zip"]) +class OneSlotUnderSeveralNames(unittest.TestCase): + """kronos opens saturn_bios.bin, then sega_101.bin as a fallback. Ymir + reads either name, so the sega_101.bin entry answers for saturn_bios.bin + too. kronos sat in that entry's _cores and out of its _required_by while + its saturn_bios.bin entry had it requiring the file.""" + + PROFILES = { + "kronos": { + "emulator": "Kronos", "type": "libretro", "systems": ["sega-saturn"], + "files": [ + {"name": "saturn_bios.bin", "path": "kronos/saturn_bios.bin", + "required": True, "size": 524288}, + {"name": "sega_101.bin", "required": False, "size": 524288}, + ], + }, + "ymir": { + "emulator": "Ymir", "type": "libretro", "systems": ["sega-saturn"], + "files": [ + {"name": "sega_101.bin", "required": True, "size": 524288, + "aliases": ["saturn_bios.bin"]}, + ], + }, + "other": { + "emulator": "Other", "type": "libretro", "systems": ["sega-saturn"], + "files": [{"name": "saturn_bios.bin", "required": True, "size": 524288}], + }, + } + + def _files(self) -> dict[str, dict]: + cores = ["kronos", "ymir", "other"] + model = generate_platform_truth("p", {"cores": cores}, {}, self.PROFILES) + return {f["name"]: f for f in model["systems"]["sega-saturn"]["files"]} + + def test_a_reader_requiring_an_alias_requires_the_entry(self): + sega = self._files()["sega_101.bin"] + self.assertEqual(sega["_cores"], ["kronos", "ymir"]) + self.assertEqual(sega["_required_by"], ["kronos", "ymir"]) + + def test_no_reader_is_added(self): + """other requires saturn_bios.bin but never opens sega_101.bin.""" + sega = self._files()["sega_101.bin"] + self.assertNotIn("other", sega["_required_by"]) + self.assertTrue(set(sega["_required_by"]) <= set(sega["_cores"])) + + class OnlySystemDirectoryEntries(unittest.TestCase): """The truth kept files read from the save, content or working directory: diff_truth listed Cemu's mlc01 certificates, panda3ds's seeddb.bin and From 3613c0927b4aa5aeb0b43cab30f21f21a207c273 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 06:18:59 +0200 Subject: [PATCH 09/10] docs: open the download page on its content The which-pack page announced itself ("This page picks the right pack for a setup") before naming a pack. The sentence goes, the pointer to Getting started stays. --- scripts/generate_site.py | 3 +-- tests/test_doc_surfaces.py | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/scripts/generate_site.py b/scripts/generate_site.py index 0caabb92..40b0a809 100644 --- a/scripts/generate_site.py +++ b/scripts/generate_site.py @@ -4671,8 +4671,7 @@ Without them, the emulator either refuses to start the game or runs it with reduced accuracy. RetroBIOS maps those requirements to source-traced emulator profiles and verifies local content against the evidence each platform exposes. -This page picks the right pack for a setup. For BIOS directory paths per -platform, verification, and the CLI, see +BIOS directory paths per platform, verification and the CLI are in [Getting started](wiki/getting-started.md). ## Quick install diff --git a/tests/test_doc_surfaces.py b/tests/test_doc_surfaces.py index 4a793a88..6d825714 100644 --- a/tests/test_doc_surfaces.py +++ b/tests/test_doc_surfaces.py @@ -272,5 +272,20 @@ class ContributorChecksMatchCi(unittest.TestCase): ) +class PagesOpenOnTheirContent(unittest.TestCase): + """The download page said "This page picks the right pack for a setup" + before saying anything about a pack.""" + + def test_no_paragraph_announces_its_page(self): + import generate_site # noqa: PLC0415 + + text = generate_site.generate_which_pack() + announcing = [ + line for line in text.splitlines() + if re.match(r"(This|The) (page|section) ", line) + ] + self.assertEqual(announcing, []) + + if __name__ == "__main__": unittest.main() From 76e44a74b4717d80018edce4d973037a849bf68b Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 06:20:50 +0200 Subject: [PATCH 10/10] refactor: drop comments that restate the code Thirty one-line labels in verify.py, generate_site.py and generate_truth.py named the statement under them ("# Load registry", "# Copy branding assets"). Comments that give a reason stay, as do the module section headers. A test reads the three files for new ones. --- scripts/generate_site.py | 13 ------------- scripts/generate_truth.py | 4 ---- scripts/verify.py | 15 --------------- tests/test_audit_regressions.py | 28 ++++++++++++++++++++++++++++ 4 files changed, 28 insertions(+), 32 deletions(-) diff --git a/scripts/generate_site.py b/scripts/generate_site.py index 40b0a809..b8649fe0 100644 --- a/scripts/generate_site.py +++ b/scripts/generate_site.py @@ -379,7 +379,6 @@ def generate_home( ] ) - # Emulator classification breakdown lines.extend( [ "## Emulator profiles", @@ -413,7 +412,6 @@ def generate_home( ] ) - # Quick links lines.extend( [ "---", @@ -1950,7 +1948,6 @@ def generate_platform_page( if fname: config_files.setdefault((sys_id, fname), []).append(fe) - # Group details by system by_system: dict[str, list] = {} for d in cov["details"]: sys_id = d.get("system", "unknown") @@ -2647,7 +2644,6 @@ def generate_system_page( icon_md = system_icon_markdown(manufacturer, console_name) lines.append(f"## {icon_md}{console_name}") lines.append("") - # Separate main files from variants main_files = [f for f in files if "/.variants/" not in f["path"]] variant_files = [f for f in files if "/.variants/" in f["path"]] @@ -2682,7 +2678,6 @@ def generate_emulators_index(profiles: dict) -> str: } aliases = {k: v for k, v in profiles.items() if v.get("type") == "alias"} - # Group by classification by_class: dict[str, list[tuple[str, dict]]] = {} for name in sorted(unique.keys()): p = unique[name] @@ -4315,7 +4310,6 @@ def _render_xref_by_platform( sys_str += f" +{len(systems) - 3}" file_count = len(files) - # Count mode divergences libretro_only = sum(1 for f in files if f.get("mode") == "libretro") standalone_only = sum(1 for f in files if f.get("mode") == "standalone") file_str = str(file_count) @@ -4364,7 +4358,6 @@ def _render_xref_by_upstream( ] ) - # Group profiles by upstream by_upstream: dict[str, list[str]] = {} for emu_name, p in sorted(unique.items()): # A null upstream is the project's own source: grouping it under @@ -4374,7 +4367,6 @@ def _render_xref_by_upstream( if up_str: by_upstream.setdefault(up_str, []).append(emu_name) - # Build platform membership per core platform_membership: dict[str, set[str]] = {} for pname, cov in coverages.items(): for key in resolve_platform_cores(cov["config"], unique): @@ -4857,7 +4849,6 @@ def generate_mkdocs_nav( k: v for k, v in profiles.items() if v.get("type") not in ("alias", "test") } - # Group emulators by classification for nav by_class: dict[str, list[tuple[str, str]]] = {} for name in sorted(unique_profiles.keys()): p = unique_profiles[name] @@ -4865,7 +4856,6 @@ def generate_mkdocs_nav( display = p.get("emulator", name) by_class.setdefault(cls, []).append((display, f"emulators/{name}.md")) - # Classification display names cls_labels = { "pure_libretro": "Pure libretro", "official_port": "Official ports", @@ -4983,7 +4973,6 @@ def _generate(args: argparse.Namespace) -> None: for d in GENERATED_DIRS: (docs / d).mkdir(parents=True, exist_ok=True) - # Copy stylesheet if source exists css_src = Path("docs_assets") / "extra.css" css_dest = docs / "stylesheets" / "extra.css" if css_src.exists(): @@ -5000,7 +4989,6 @@ def _generate(args: argparse.Namespace) -> None: # the sitemap instead (overrides/main.html). (docs / "robots.txt").unlink(missing_ok=True) - # Copy branding assets images_dest = docs / "assets" / "images" images_dest.mkdir(parents=True, exist_ok=True) assets_src = Path(".github") / "assets" @@ -5047,7 +5035,6 @@ def _generate(args: argparse.Namespace) -> None: f" {len(profiles)} profiles ({unique_count} unique, {len(profiles) - unique_count} aliases)" ) - # Build cross-reference indexes consumers = _build_file_consumers(db, coverages, profiles) emulator_files = _build_emulator_file_index(profiles) diff --git a/scripts/generate_truth.py b/scripts/generate_truth.py index 2c529d47..96ecda21 100644 --- a/scripts/generate_truth.py +++ b/scripts/generate_truth.py @@ -72,12 +72,10 @@ def parse_args(argv: list[str] | None = None) -> argparse.Namespace: def main(argv: list[str] | None = None) -> None: args = parse_args(argv) - # Load registry registry_path = os.path.join(args.platforms_dir, "_registry.yml") with open(registry_path) as f: registry = (yaml_load(f) or {}).get("platforms", {}) - # Load emulator profiles profiles = load_emulator_profiles(args.emulators_dir) # Load database (optional) @@ -86,7 +84,6 @@ def main(argv: list[str] | None = None) -> None: if args.db != DEFAULT_DB_FILE or os.path.exists(args.db): db = load_database(args.db) - # Determine platforms if args.all: platforms = list_registered_platforms( args.platforms_dir, @@ -108,7 +105,6 @@ def main(argv: list[str] | None = None) -> None: output_dir, [name], args.target, args.platforms_dir ) os.makedirs(output_dir, exist_ok=True) - # Resolve target cores target_cores: set[str] | None = None if args.target: try: diff --git a/scripts/verify.py b/scripts/verify.py index 2bb2581d..08d23c50 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -144,7 +144,6 @@ class Status: MISSING = "missing" -# Severity for per-file required/optional distinction class Severity: CRITICAL = "critical" # required file missing or bad hash (Recalbox RED) WARNING = "warning" # optional missing or hash mismatch (Recalbox YELLOW) @@ -378,8 +377,6 @@ def compute_severity( return Severity.OK -# ZIP content index - # Cross-reference: undeclared files used by cores @@ -416,7 +413,6 @@ def _name_in_index( basename = name.rsplit("/", 1)[-1] if "/" in name else name if basename != name and basename in by_name: return True - # Case-insensitive by_name lookup if by_name_lower: key = name.lower() if key in by_name_lower: @@ -508,7 +504,6 @@ def find_undeclared_files( config.get("verification_mode") ) - # Collect data_directory refs declared_dd: set[str] = set() for sys_id, system in config.get("systems", {}).items(): for dd in system.get("data_directories", []): @@ -601,7 +596,6 @@ def find_undeclared_files( archive = f.get("archive") seen_files.add(seen_key) - # Archived files are grouped by archive if archive: archive_key = ( archive, @@ -722,7 +716,6 @@ def find_undeclared_files( undeclared = [u for u in undeclared if id(u) not in settled] undeclared.extend(added) - # Append grouped archive entries for entry in sorted(archive_entries.values(), key=lambda e: e["name"]): undeclared.append(entry) @@ -762,7 +755,6 @@ def find_exclusion_notes( emu_display = profile.get("emulator", emu_name) - # Launcher excluded entirely if profile.get("type") == "launcher": notes.append( { @@ -1088,7 +1080,6 @@ def verify_platform( validation_index = _build_validation_index(platform_profiles) validation_owners = destination_owners(platform_profiles) - # Filter systems by target if not target_cores: plat_cores = None verify_systems = filter_systems_by_target( @@ -1098,9 +1089,7 @@ def verify_platform( platform_cores=plat_cores, ) - # Per-entry results details = [] - # Per-destination aggregation file_status: dict[str, str] = {} file_required: dict[str, bool] = {} file_severity: dict[str, str] = {} @@ -1340,7 +1329,6 @@ def verify_platform( config, emulators_dir, emu_profiles, target_cores=target_cores ) - # Ground truth coverage gt_filenames = set(validation_index) profiled_names: set[str] = set() for profile in profiles.values(): @@ -1630,7 +1618,6 @@ def print_platform_result( ok_count = c[Severity.OK] problems = total - ok_count - # Summary line if not reads_file_contents(mode): if problems: missing = c.get(Severity.WARNING, 0) + c.get(Severity.CRITICAL, 0) @@ -2289,7 +2276,6 @@ def print_emulator_result(result: dict, verbose: bool = False) -> None: f" Note: data directory '{ref}' required but not included (use refresh_data_dirs.py)" ) - # Ground truth coverage footer gt_cov = result.get("ground_truth_coverage") if gt_cov and gt_cov["total"] > 0: pct = round(gt_cov["with_validation"] * 100 / gt_cov["total"]) @@ -2449,7 +2435,6 @@ def _main(held: contextlib.ExitStack) -> None: _run_listing(args, parser) return - # Mutual exclusion modes = sum(1 for x in (args.platform, args.all, args.emulator, args.system) if x) if modes == 0: parser.error("Specify --platform, --all, --emulator, or --system") diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 19cfac28..4a5a9deb 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -6125,5 +6125,33 @@ class SkipPacksHelpSaysWhatRuns(unittest.TestCase): self.assertEqual(skipped, {"4", "4b", "4c", "6"}) +class CommentsSayWhy(unittest.TestCase): + """A one-line comment that names the next statement ("# Load registry", + "# Build cross-reference indexes") repeats the code. A comment that + stays says why (for, only, so, because).""" + + LABEL = re.compile( + r"^\s*# (Load|Build|Collect|Determine|Resolve|Group|Copy|Count|Append|" + r"Filter|Separate|Compute|Create|Get|Check|Generate|Write|Read|Parse|Sort)" + r"\b[^.:;,()]{0,60}$" + ) + REASON = re.compile(r"\b(for|only|so|because|if|when|unless|before|after)\b") + + def test_no_label_comment_restates_the_next_line(self): + found = [] + for name in ("verify.py", "generate_site.py", "generate_truth.py"): + lines = (ROOT / "scripts" / name).read_text(encoding="utf-8").splitlines() + for index, line in enumerate(lines): + if not self.LABEL.match(line) or self.REASON.search(line): + continue + around = (lines[index - 1].strip(), lines[index + 1].strip()) + if any(neighbour.startswith("#") for neighbour in around): + continue + if not line.startswith(" "): + continue + found.append(f"{name}:{index + 1}: {line.strip()}") + self.assertEqual(found, []) + + if __name__ == "__main__": unittest.main()