From 61b37408f181bb01cadf275efe222c150de7a26c Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Sun, 18 Oct 2026 08:44:41 +0200 Subject: [PATCH] feat: name the folder a PR's BIOS belongs in --- scripts/validate_pr.py | 60 ++++++++++++++++++++++++++++++++------- tests/test_validate_pr.py | 41 +++++++++++++++++++++++++- 2 files changed, 90 insertions(+), 11 deletions(-) diff --git a/scripts/validate_pr.py b/scripts/validate_pr.py index b801d533..df324ab8 100644 --- a/scripts/validate_pr.py +++ b/scripts/validate_pr.py @@ -109,9 +109,18 @@ class ValidationResult: return "\n".join(lines) -def load_platform_hashes(platforms_dir: str) -> dict: - """Load all known hashes from platform configs.""" - known = {"sha1": set(), "md5": set(), "names": set()} +def load_platform_hashes(platforms_dir: str, db: dict | None = None) -> dict: + """Load all known hashes from platform configs. + + With the database, `folders` maps each declared hash to the + bios/// folders the collection files its system + under, read from the other files of that system it holds. + """ + known = {"sha1": set(), "md5": set(), "names": set(), "folders": {}} + systems_of: dict[str, set[str]] = {} + folders_of: dict[str, dict[str, int]] = {} + files = (db or {}).get("files", {}) + by_md5 = (db or {}).get("indexes", {}).get("by_md5", {}) if not os.path.isdir(platforms_dir) or yaml is None: return known @@ -127,16 +136,37 @@ def load_platform_hashes(platforms_dir: str) -> dict: print(f"WARNING: {name}: {exc}", file=sys.stderr) continue - for system in config.get("systems", {}).values(): + for system_id, system in config.get("systems", {}).items(): for file_entry in system.get("files", []): sha1 = file_entry.get("sha1") - for value in sha1 if isinstance(sha1, list) else [sha1]: - if value: - known["sha1"].add(str(value).lower()) - known["md5"].update(parse_md5_list(file_entry.get("md5"))) + sha1s = [str(v).lower() for v in (sha1 if isinstance(sha1, list) else [sha1]) if v] + md5s = parse_md5_list(file_entry.get("md5")) + known["sha1"].update(sha1s) + known["md5"].update(md5s) if "name" in file_entry: known["names"].add(file_entry["name"]) + for value in (*sha1s, *md5s): + systems_of.setdefault(value, set()).add(system_id) + held = next((s for s in sha1s if s in files), None) or next( + (by_md5[m] for m in md5s if m in by_md5), None + ) + parts = str(files.get(held, {}).get("path", "")).split("/") + if len(parts) >= 4 and parts[0] == "bios" and not parts[2].startswith("."): + folder = "/".join(parts[:3]) + "/" + counts = folders_of.setdefault(system_id, {}) + counts[folder] = counts.get(folder, 0) + 1 + # A system whose files sit under several manufacturers (arcade sets, + # computers sharing ROM chips) names no single home: only a system + # filed under one manufacturer says where a newcomer belongs. + for value, systems in systems_of.items(): + folders: set[str] = set() + for system_id in systems: + held_in = folders_of.get(system_id, {}) + if len({folder.split("/")[1] for folder in held_in}) == 1: + folders.update(held_in) + if folders: + known["folders"][value] = folders return known @@ -257,7 +287,17 @@ def validate_file( normalized = os.path.normpath(filepath) if normalized.startswith("bios" + os.sep): parts = normalized.split(os.sep) - if len(parts) >= 4: + expected = (platform_hashes.get("folders", {}).get(result.sha1) + or platform_hashes.get("folders", {}).get(result.md5) or set()) + here = f"bios/{parts[1]}/{parts[2]}/" if len(parts) >= 4 else "" + makers = {folder.split("/")[1] for folder in expected} + if expected and (len(parts) < 4 or parts[1] not in makers): + # Depth alone passed a PS1 BIOS filed under bios/Nintendo/Game Boy/. + result.add_warning( + "Placed under " + (here or "the bios/ root") + "; the other files of " + "its system are in " + " or ".join(f"`{f}`" for f in sorted(expected)) + ) + elif len(parts) >= 4: result.add_check(True, f"Correct placement: bios/{parts[1]}/{parts[2]}/") else: result.add_warning("File should be in bios/Manufacturer/Console/ structure") @@ -372,7 +412,7 @@ def main() -> None: return db = _load_database_if_any(args.db) - platform_hashes = load_platform_hashes(args.platforms_dir) + platform_hashes = load_platform_hashes(args.platforms_dir, db) profile_hashes = load_profile_hashes(args.emulators_dir) results = [] diff --git a/tests/test_validate_pr.py b/tests/test_validate_pr.py index 65f4d3f2..99ce0d50 100644 --- a/tests/test_validate_pr.py +++ b/tests/test_validate_pr.py @@ -195,6 +195,45 @@ class HashRecognition(_Fixture): self.assertIn("Already in the database at this path", markdown) +class PlacementBySystem(_Fixture): + """Depth alone passed a PS1 BIOS filed under bios/Nintendo/Game Boy/: + a declared hash names its system, and the files of that system already + held name its folder.""" + + def _known(self, sha1: str) -> dict: + return {"sha1": {sha1}, "md5": set(), "names": set(), + "folders": {sha1: {"bios/Sony/PlayStation/"}}} + + def test_a_file_under_another_maker_is_told_where_its_system_lives(self): + rel = self._bios_file("bios/Nintendo/Game Boy/scph.bin") + probe = validate_pr.validate_file(rel, None, EMPTY_HASHES) + markdown = validate_pr.validate_file(rel, None, self._known(probe.sha1)).to_markdown() + self.assertIn("bios/Sony/PlayStation/", markdown) + self.assertNotIn("Correct placement", markdown) + + def test_the_folder_of_its_system_passes(self): + rel = self._bios_file("bios/Sony/PlayStation/scph.bin") + probe = validate_pr.validate_file(rel, None, EMPTY_HASHES) + markdown = validate_pr.validate_file(rel, None, self._known(probe.sha1)).to_markdown() + self.assertIn("Correct placement: bios/Sony/PlayStation/", markdown) + + def test_systems_spread_over_makers_name_no_home(self): + platforms = self.tmp / "platforms" + platforms.mkdir() + (platforms / "_registry.yml").write_text( + "platforms:\n p:\n config: p.yml\n status: active\n", encoding="utf-8") + (platforms / "p.yml").write_text( + "platform: P\nsystems:\n arcade:\n files:\n" + " - {name: a.zip, sha1: aaaa}\n - {name: b.zip, sha1: bbbb}\n", + encoding="utf-8", + ) + db = {"files": {"aaaa": {"path": "bios/Arcade/MAME/a.zip"}, + "bbbb": {"path": "bios/Apple/Apple II/b.zip"}}, + "indexes": {"by_md5": {}}} + known = validate_pr.load_platform_hashes(str(platforms), db) + self.assertEqual(known["folders"], {}) + + class Placement(_Fixture): def test_manufacturer_and_console_directories_are_accepted(self): rel = self._bios_file("bios/Sega/Saturn/sega_101.bin") @@ -235,7 +274,7 @@ class Reporting(_Fixture): class PlatformHashLoading(_Fixture): def test_missing_directory_yields_empty_sets(self): known = validate_pr.load_platform_hashes(str(self.tmp / "absent")) - self.assertEqual(known, {"sha1": set(), "md5": set(), "names": set()}) + self.assertEqual(known, {"sha1": set(), "md5": set(), "names": set(), "folders": {}}) def test_declared_hashes_and_names_are_collected(self): platforms = self.tmp / "platforms"