fix: write the truth model and clone map atomically

scripts/dedup.py

@@ -28,7 +28,7 @@ from collections import defaultdict
 from pathlib import Path
 
 sys.path.insert(0, os.path.dirname(__file__))
-from common import compute_hashes
+from common import compute_hashes, write_text_atomic
 
 DEFAULT_BIOS_DIR = "bios"
 
@@ -168,6 +168,25 @@ def deduplicate(bios_dir: str, dry_run: bool = False) -> dict:
 
     Returns dict of {sha1: {"canonical": path, "removed": [paths], "aliases": [names]}}
     """
+    # Beside the scanned tree, where get_mame_clone_map() reads it for
+    # the real layout. A path relative to the working directory let a
+    # scan of a fixture tree rewrite the repository's own map.
+    clone_path = os.path.join(
+        os.path.dirname(os.path.abspath(bios_dir)), "_mame_clones.json"
+    )
+    # Read before anything is removed: the map is cumulative, and a run that
+    # stopped on an unreadable one after deleting clones would lose them.
+    try:
+        with open(clone_path) as f:
+            previous = json.load(f)
+    except FileNotFoundError:
+        previous = {}
+    except json.JSONDecodeError as e:
+        raise SystemExit(
+            f"{clone_path} is not valid JSON ({e}); restore it from git "
+            "before deduplicating again"
+        ) from e
+
     sha1_groups = scan_duplicates(bios_dir)
     results = {}
     total_removed = 0
@@ -253,23 +272,12 @@ def deduplicate(bios_dir: str, dry_run: bool = False) -> dict:
 
     # Write MAME clone mapping
     if mame_clones:
-        # Beside the scanned tree, where get_mame_clone_map() reads it for
-        # the real layout. A path relative to the working directory let a
-        # scan of a fixture tree rewrite the repository's own map.
-        clone_path = os.path.join(
-            os.path.dirname(os.path.abspath(bios_dir)), "_mame_clones.json"
-        )
         # A group is only visible while both copies are on disk, and this run
         # has just deleted the clone. Writing only what was seen this time
         # therefore erases every mapping an earlier run recorded, and the
         # canonical zip stops answering to the names it stands in for. The
         # older entries stay valid: their canonical file is still here.
         if not dry_run:
-            try:
-                with open(clone_path) as f:
-                    previous = json.load(f)
-            except (FileNotFoundError, json.JSONDecodeError):
-                previous = {}
             if isinstance(previous, dict):
                 merged = dict(previous)
                 merged.update(mame_clones)
@@ -281,8 +289,7 @@ def deduplicate(bios_dir: str, dry_run: bool = False) -> dict:
                 f"{sum(len(v['clones']) for v in mame_clones.values())} clones"
             )
         else:
-            with open(clone_path, "w") as f:
-                json.dump(mame_clones, f, indent=2, sort_keys=True)
+            write_text_atomic(clone_path, json.dumps(mame_clones, indent=2, sort_keys=True))
             print(f"\nWrote MAME clone map: {clone_path}")
             print(
                 f"  {len(mame_clones)} canonical ZIPs with "

scripts/generate_truth.py

@@ -21,6 +21,7 @@ from common import (
     load_platform_config,
     load_target_config,
     require_yaml,
+    write_text_atomic,
     yaml_load,
 )
 from truth import generate_platform_truth
@@ -129,14 +130,17 @@ def main(argv: list[str] | None = None) -> None:
         )
 
         out_path = os.path.join(output_dir, f"{name}.yml")
-        with open(out_path, "w") as f:
+        # Whole or not at all: export_native and diff_truth read this file,
+        # and a run stopped midway left a truncated model behind.
+        write_text_atomic(
+            out_path,
             yaml.dump(
                 result,
-                f,
                 default_flow_style=False,
                 sort_keys=False,
                 allow_unicode=True,
-            )
+            ),
+        )
 
         n_systems = len(result.get("systems", {}))
         n_files = sum(

tests/test_audit_regressions.py

@@ -2471,6 +2471,39 @@ class TheZipIndexCacheKnowsItsInputs(unittest.TestCase):
             ziptools._zip_contents_cache = None
 
 
+class GeneratedModelsAreWrittenWhole(unittest.TestCase):
+    """The truth model and the cumulative MAME clone map were written by
+    truncate-then-write; a corrupt clone map also read as empty and was
+    then overwritten, losing what earlier runs recorded."""
+
+    def test_both_writers_replace_their_file_atomically(self):
+        for module in ("generate_truth.py", "dedup.py"):
+            source = (ROOT / "scripts" / module).read_text(encoding="utf-8")
+            with self.subTest(module=module):
+                self.assertIn("write_text_atomic(", source)
+                self.assertNotIn('open(out_path, "w")', source)
+                self.assertNotIn('open(clone_path, "w")', source)
+
+    def test_an_unreadable_clone_map_stops_the_run(self):
+        from scripts import dedup
+
+        with tempfile.TemporaryDirectory(dir=TMP_ROOT) as directory:
+            root = Path(directory)
+            bios = root / "bios"
+            # A device clone: two archive names, the same ROMs, in MAME's tree.
+            (bios / "Arcade" / "MAME").mkdir(parents=True)
+            for name in ("set.zip", "setclone.zip"):
+                with zipfile.ZipFile(bios / "Arcade" / "MAME" / name, "w") as archive:
+                    archive.writestr("rom.bin", b"rom")
+            data = (bios / "Arcade" / "MAME" / "set.zip").read_bytes()
+            (bios / "Arcade" / "MAME" / "setclone.zip").write_bytes(data)
+            (root / "_mame_clones.json").write_text("<<<<<<< HEAD")
+            with self.assertRaises(SystemExit):
+                dedup.deduplicate(str(bios), dry_run=False)
+            # Stopped before removing anything.
+            self.assertTrue((bios / "Arcade" / "MAME" / "setclone.zip").exists())
+
+
 class TheDatabaseCacheIsWrittenWhole(unittest.TestCase):
     """A crash while serializing left a truncated db_cache.json behind, and
     the next run rehashed the whole collection from scratch."""