From 57ccca640a353bdbfe4550d8d9792a16652d5fd3 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 6 Nov 2026 08:13:32 +0100 Subject: [PATCH] refactor: importers write through write_if_changed game_dat_importer and arcade_parent_importer each kept an exact-text copy of write_if_changed, so a redump refresh rewrote every catalog for its fetched date alone. Both now use artifacts.write_if_changed, which treats fetched as a timestamp. A No-Intro rebuild still rewrites its catalogs, since asset_updated_at moves with it, so check_freshness keeps reading a fetched date no older than the pack. --- scripts/artifacts.py | 1 + scripts/scraper/arcade_parent_importer.py | 13 +++---------- scripts/scraper/game_dat_importer.py | 14 ++++---------- tests/test_game_catalogs.py | 11 +++++++++++ 4 files changed, 19 insertions(+), 20 deletions(-) diff --git a/scripts/artifacts.py b/scripts/artifacts.py index 66b752837..ef6818657 100644 --- a/scripts/artifacts.py +++ b/scripts/artifacts.py @@ -19,6 +19,7 @@ import re _TIMESTAMP_PATTERNS = [ re.compile(r'"generated_at":\s*"[^"]*"'), # database.json re.compile(r'"imported_at":\s*"[^"]*"'), # provenance snapshots + re.compile(r'"fetched":\s*"[^"]*"'), # games/catalogs/ re.compile(r"\*Auto-generated on [^*]*\*"), # README.md re.compile(r"\*Generated on [^*]*\*"), # docs site pages # The decorated pages carry the same stamp again as a rendered element, diff --git a/scripts/scraper/arcade_parent_importer.py b/scripts/scraper/arcade_parent_importer.py index ee1eca55a..a884b3188 100644 --- a/scripts/scraper/arcade_parent_importer.py +++ b/scripts/scraper/arcade_parent_importer.py @@ -49,7 +49,7 @@ from collections.abc import Callable, Iterator from dataclasses import dataclass, field from pathlib import Path -from ..artifacts import write_text_atomic +from ..artifacts import write_if_changed from ..common import load_emulator_profiles, run_cli from .romset_dat_importer import ( LISTXML_CHUNK, @@ -1114,14 +1114,6 @@ def dump(doc: dict) -> str: return json.dumps(doc, sort_keys=True, separators=(",", ":"), ensure_ascii=False) + "\n" -def write_if_changed(path: Path, text: str) -> bool: - if path.is_file() and path.read_text(encoding="utf-8") == text: - return False - path.parent.mkdir(parents=True, exist_ok=True) - write_text_atomic(str(path), text) - return True - - def main() -> int: parser = argparse.ArgumentParser(allow_abbrev=False, description=__doc__.splitlines()[0]) which = parser.add_mutually_exclusive_group(required=True) @@ -1197,7 +1189,8 @@ def main() -> int: continue if not args.dry_run: target = args.output_dir / f"{profile}.json" - if write_if_changed(target, dump(doc)): + target.parent.mkdir(parents=True, exist_ok=True) + if write_if_changed(str(target), dump(doc)): print(f" wrote {target}") return 1 if failed else 0 diff --git a/scripts/scraper/game_dat_importer.py b/scripts/scraper/game_dat_importer.py index e15a57fda..0968008bf 100644 --- a/scripts/scraper/game_dat_importer.py +++ b/scripts/scraper/game_dat_importer.py @@ -38,7 +38,7 @@ _SCRIPTS_DIR = str(Path(__file__).resolve().parents[1]) if _SCRIPTS_DIR not in sys.path: sys.path.insert(0, _SCRIPTS_DIR) -from artifacts import copy_stream_atomic, write_text_atomic # noqa: E402 +from artifacts import copy_stream_atomic, write_if_changed, write_text_atomic # noqa: E402 from common import parse_untrusted_xml # noqa: E402 from region import REGIONS, WORLD # noqa: E402 from region_audit import NOINTRO # noqa: E402 @@ -275,14 +275,6 @@ def dump_catalog(doc: dict) -> str: return f'{text},"games":[\n{games}\n]}}\n' -def write_if_changed(path: Path, text: str) -> bool: - if path.is_file() and path.read_text(encoding="utf-8") == text: - return False - path.parent.mkdir(parents=True, exist_ok=True) - write_text_atomic(str(path), text) - return True - - # Fetching --------------------------------------------------------------- @@ -503,7 +495,9 @@ def main() -> int: failures.append(f"{doc['source']}/{doc['system']}: {cov['games']} games, " f"floor {floor_of(doc)}") if not args.dry_run: - write_if_changed(args.output_dir / doc["source"] / f"{doc['system']}.json", text) + target = args.output_dir / doc["source"] / f"{doc['system']}.json" + target.parent.mkdir(parents=True, exist_ok=True) + write_if_changed(str(target), text) print(f"{len(docs)} catalogs, {raw_total} bytes raw, {gz_total} bytes gzip") for failure in failures: print(f"FAIL {failure}", file=sys.stderr) diff --git a/tests/test_game_catalogs.py b/tests/test_game_catalogs.py index 61ca33b63..306d78844 100644 --- a/tests/test_game_catalogs.py +++ b/tests/test_game_catalogs.py @@ -102,6 +102,17 @@ class FixtureImport(unittest.TestCase): twin = second / path.relative_to(first) self.assertEqual(path.read_bytes(), twin.read_bytes(), path.name) + def test_a_new_fetch_date_alone_rewrites_nothing(self) -> None: + """A refresh rewrote every catalog for the day it ran, and the diff + read before commit no longer showed which DAT had moved.""" + self.assertEqual(run_import("redump", self.out).returncode, 0) + path = self.out / "redump" / "sega-megacd.json" + doc = json.loads(path.read_text()) + doc["fetched"] = "2000-01-01" + path.write_text(importer.dump_catalog(doc)) + self.assertEqual(run_import("redump", self.out).returncode, 0) + self.assertEqual(json.loads(path.read_text())["fetched"], "2000-01-01") + def test_floor_refuses_a_short_dat(self) -> None: result = run_import("no-intro", self.out, "--floor", "--dry-run") self.assertEqual(result.returncode, 1)