From f285c32d67136eb9c729f970b9156d2835d11dbe Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Thu, 5 Nov 2026 11:48:31 +0100 Subject: [PATCH] feat: validate every file of a folder --- CONTRIBUTING.md | 3 ++- scripts/generate_readme.py | 3 ++- scripts/validate_pr.py | 29 +++++++++++++++++++++++++++-- tests/test_validate_pr.py | 11 +++++++++++ wiki/tools.md | 2 +- 5 files changed, 43 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2fa3a8ece..a9741f8df 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -24,7 +24,8 @@ the most useful contribution. From a clone, the file goes to `bios/Manufacturer/Console/filename`, and `python scripts/validate_pr.py path/to/file` names that folder for a file still -outside `bios/`. A file over the browser's 25 MB limit goes in that way. +outside `bios/`. Given a folder, such as an old card's BIOS folder, it checks +every file under it. A file over the browser's 25 MB limit goes in that way. ## Add a platform diff --git a/scripts/generate_readme.py b/scripts/generate_readme.py index aba4e9c9f..2f2facb91 100644 --- a/scripts/generate_readme.py +++ b/scripts/generate_readme.py @@ -931,7 +931,8 @@ the most useful contribution. From a clone, the file goes to `bios/Manufacturer/Console/filename`, and `python scripts/validate_pr.py path/to/file` names that folder for a file still -outside `bios/`. A file over the browser's 25 MB limit goes in that way. +outside `bios/`. Given a folder, such as an old card's BIOS folder, it checks +every file under it. A file over the browser's 25 MB limit goes in that way. ## Add a platform diff --git a/scripts/validate_pr.py b/scripts/validate_pr.py index 127a38558..ae92bfac1 100644 --- a/scripts/validate_pr.py +++ b/scripts/validate_pr.py @@ -23,6 +23,7 @@ import os import re import subprocess import sys +from pathlib import Path sys.path.insert(0, os.path.dirname(__file__)) from common import ( @@ -301,8 +302,12 @@ def validate_file( "changed under bios/") return result + if os.path.isdir(filepath): + result.add_check(False, "Folder with no file in it") + return result + if not os.path.isfile(filepath): - result.add_check(False, "Not a regular file") + result.add_check(False, "Not a regular file: a device, a pipe or a socket") return result result.size = os.path.getsize(filepath) @@ -561,6 +566,26 @@ def get_changed_files() -> list[str]: ] +def expand_folders(paths: list[str]) -> list[str]: + """Each folder named replaced by the files under it, subfolders included. + + A card's BIOS folder was refused as "Not a regular file", and the + contributor had to list its files by hand. A link inside the folder + stays listed, so it is refused like a named one, and an empty folder + stays as itself to be reported. + """ + out: list[str] = [] + for path in paths: + if not os.path.isdir(path) or os.path.islink(path): + out.append(path) + continue + found = sorted( + str(f) for f in Path(path).rglob("*") if f.is_symlink() or f.is_file() + ) + out.extend(found or [path]) + return out + + def _files_to_validate( args: argparse.Namespace, parser: argparse.ArgumentParser ) -> list: @@ -572,7 +597,7 @@ def _files_to_validate( if not args.changed: if not args.files: parser.error("No files specified. Use --changed or provide file paths.") - return args.files + return expand_folders(args.files) try: files = get_changed_files() except (RuntimeError, OSError) as exc: diff --git a/tests/test_validate_pr.py b/tests/test_validate_pr.py index 9a14257f7..90ae9dcf0 100644 --- a/tests/test_validate_pr.py +++ b/tests/test_validate_pr.py @@ -54,6 +54,17 @@ class FileShapeChecks(_Fixture): self.assertNotIn(label, markdown) self.assertIn("--changed", markdown) + def test_a_folder_is_read_file_by_file(self): + self._bios_file("card/bios/a.bin") + self._bios_file("card/bios/sub/b.bin") + (self.tmp / "empty").mkdir() + self.assertEqual( + validate_pr.expand_folders(["card", "empty", "loose.bin"]), + [str(Path("card/bios/a.bin")), str(Path("card/bios/sub/b.bin")), "empty", "loose.bin"], + ) + result = validate_pr.validate_file("empty", None, EMPTY_HASHES) + self.assertIn("Folder with no file in it", result.to_markdown()) + def test_empty_file_fails(self): rel = self._bios_file("bios/Sony/PS/empty.bin", b"") result = validate_pr.validate_file(rel, None, EMPTY_HASHES) diff --git a/wiki/tools.md b/wiki/tools.md index 1aa6aef87..e4a19bc10 100644 --- a/wiki/tools.md +++ b/wiki/tools.md @@ -1349,7 +1349,7 @@ Further options, by script: | `romset_recipes.py` | `--dat-recipes PATH`, a snapshot or a directory of them (default `recipes/`) | | `split_pack.py` | `--max-size 1900M`, the ceiling per part (K, M, G, default 2G) | | `torrentzip.py` | `--check ZIP` reports conformance, `--rebuild ZIP -o OUT` normalizes an archive | -| `validate_pr.py` | `--changed` takes the BIOS files the branch changes, `--markdown`, `--json` | +| `validate_pr.py` | `--changed` takes the BIOS files the branch changes, a folder named checks every file under it, `--markdown`, `--json` | | `validate_site.py` | `--site-dir` (rendered site, default `website/dist`), `--base` (path the site is served under), `--max-errors N` | | `scraper/coreinfo_scraper.py` | `--compare-db database.json` lists the `.info` firmware the database lacks | | `scraper/mame_hash_scraper.py` | `--release mameNNNN --profile FILE --source-commit SHA --core NAME --built-by PLATFORM` writes the profile of a MAME build a platform compiles. `--devices --listxml FILE` documents in `mame.yml` every device set with dumped ROMs that the release's `-listxml` names (`isdevice="yes"`), with `--dry-run` to preview |