diff --git a/scripts/common.py b/scripts/common.py index c50687e4..8a114964 100644 --- a/scripts/common.py +++ b/scripts/common.py @@ -31,10 +31,30 @@ except ImportError: +class DatabaseUnreadable(FileNotFoundError): + """The database a command was pointed at does not exist or is not JSON.""" + + def load_database(db_path: str) -> dict: - """Load database.json and return parsed dict.""" - with open(db_path) as f: - return json.load(f) + """Load database.json and return parsed dict. + + A missing or damaged file raises DatabaseUnreadable, which run_cli turns + into one line saying how to build it: `--db nosuch.json` ended at the + bottom of a stack trace. + """ + try: + with open(db_path) as f: + return json.load(f) + except FileNotFoundError: + raise DatabaseUnreadable( + f"{db_path} not found. Build it with: python scripts/generate_db.py " + "--force --bios-dir bios --output database.json" + ) from None + except json.JSONDecodeError as exc: + raise DatabaseUnreadable( + f"{db_path} is not valid JSON ({exc}). Rebuild it with: " + "python scripts/generate_db.py --force --bios-dir bios --output database.json" + ) from None @@ -2501,6 +2521,9 @@ def run_cli(main: Callable[[], int | None]) -> None: try: code = main() sys.stdout.flush() + except DatabaseUnreadable as exc: + print(f"ERROR: {exc}", file=sys.stderr) + sys.exit(1) except BrokenPipeError: os.dup2(os.open(os.devnull, os.O_WRONLY), sys.stdout.fileno()) sys.exit(141) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 4bac8260..60489646 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -2426,8 +2426,7 @@ def _run_verify_packs(args: argparse.Namespace) -> None: second implementation here answered a narrower one and reported OK while the core extras went unchecked. """ - with open(args.db) as f: - verify_db = json.load(f) + verify_db = load_database(args.db) if args.platform: platforms = [args.platform] diff --git a/scripts/generate_truth.py b/scripts/generate_truth.py index d4ad251a..236ddab6 100644 --- a/scripts/generate_truth.py +++ b/scripts/generate_truth.py @@ -78,8 +78,9 @@ def main(argv: list[str] | None = None) -> None: profiles = load_emulator_profiles(args.emulators_dir) # Load database (optional) + # The default may be absent; a --db that was named must exist. db: dict | None = None - if os.path.exists(args.db): + if args.db != DEFAULT_DB_FILE or os.path.exists(args.db): db = load_database(args.db) # Determine platforms diff --git a/scripts/slots.py b/scripts/slots.py index c40cbe68..64c296db 100644 --- a/scripts/slots.py +++ b/scripts/slots.py @@ -807,6 +807,7 @@ def main() -> int: from common import ( list_registered_platforms, + load_database, load_emulator_profiles, load_platform_config, require_platform, @@ -830,8 +831,7 @@ def main() -> int: ) args = parser.parse_args() - with open(args.db, encoding="utf-8") as handle: - db = json.load(handle) + db = load_database(args.db) profiles = load_emulator_profiles(args.emulators_dir) if args.platform: require_platform(args.platform, args.platforms_dir) diff --git a/scripts/validate_pr.py b/scripts/validate_pr.py index 2159e985..f2b915d8 100644 --- a/scripts/validate_pr.py +++ b/scripts/validate_pr.py @@ -326,13 +326,15 @@ def _files_to_validate( def _load_database_if_any(path: str) -> dict | None: - try: - return load_database(path) - except FileNotFoundError: - return None - except json.JSONDecodeError as e: - print(f"WARNING: corrupt database.json: {e}", file=sys.stderr) + """The database, optional only where the default is meant. + + A mistyped --db read as "no database" and every file passed its + duplicate and placement checks unchecked; a damaged one did the same + after a warning. + """ + if path == DEFAULT_DB and not os.path.exists(path): return None + return load_database(path) def main() -> None: diff --git a/scripts/verify.py b/scripts/verify.py index 5cf00018..40704888 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -56,6 +56,7 @@ from common import ( list_emulator_profiles, list_system_ids, load_data_dir_registry, + load_database, load_emulator_profiles, load_platform_config, md5sum, @@ -2178,8 +2179,7 @@ def _main(held: contextlib.ExitStack) -> None: hold_data_dirs(held, load_data_dir_registry(args.platforms_dir)) - with open(args.db) as f: - db = json.load(f) + db = load_database(args.db) # Emulator mode if args.emulator: diff --git a/tests/test_system_selection.py b/tests/test_system_selection.py index 6c3a46db..57adb608 100644 --- a/tests/test_system_selection.py +++ b/tests/test_system_selection.py @@ -171,6 +171,43 @@ class CrossReferenceAnswersForTheEmulatorAsked(unittest.TestCase): self.assertIn("not run by misterfpga", proc.stderr) +class AMissingDatabaseIsNamed(unittest.TestCase): + """`slots.py --db nosuch.json` and `verify.py --all --db nosuch.json` + ended on a FileNotFoundError at the bottom of a stack trace; + generate_truth and validate_pr carried on without one.""" + + EXTRA = { + "auto_fetch.py": ["--all", "--dry-run"], + "generate_truth.py": ["--all", "--output-dir", "{out}"], + "generate_site.py": ["--docs-dir", "{out}"], + "generate_pack.py": ["--platform", "retroarch", "--offline", "--output-dir", "{out}"], + "validate_pr.py": ["bios/Sony/PlayStation/scph5501.bin"], + "verify.py": ["--all"], + } + + def _taking_db(self) -> list[str]: + return sorted( + path.name for path in (REPO_ROOT / "scripts").glob("*.py") + if re.search(r'add_argument\(\s*"--db"', path.read_text(encoding="utf-8")) + ) + + def test_every_script_names_the_missing_file(self): + scratch = tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") + self.addCleanup(scratch.cleanup) + out = str(Path(scratch.name) / "out") + for script in self._taking_db(): + extra = [arg.format(out=out) for arg in self.EXTRA.get(script, [])] + with self.subTest(script=script): + proc = subprocess.run( + [sys.executable, f"scripts/{script}", "--db", "nosuch.json", *extra], + capture_output=True, text=True, cwd=str(REPO_ROOT), timeout=600, + check=False, + ) + self.assertNotEqual(proc.returncode, 0, proc.stdout[-500:]) + self.assertNotIn("Traceback", proc.stderr, proc.stderr[-1500:]) + self.assertIn("nosuch.json", proc.stderr) + + class StrictVerifyFailsOnACriticalMiss(unittest.TestCase): """verify exited 0 with a required file missing on an md5 platform, so the pipeline printed "verify OK" over a pack the frontend rejects."""