diff --git a/scripts/artifacts.py b/scripts/artifacts.py index 21124928..08ff5baa 100644 --- a/scripts/artifacts.py +++ b/scripts/artifacts.py @@ -60,7 +60,7 @@ def write_text_atomic(path: str, content: str) -> None: dir=directory, prefix=f".{os.path.basename(path)}.", suffix=".tmp" ) try: - with os.fdopen(handle, "w") as f: + with os.fdopen(handle, "w", encoding="utf-8") as f: f.write(content) os.replace(scratch, path) except BaseException: diff --git a/scripts/scraper/_hash_merge.py b/scripts/scraper/_hash_merge.py index 6952f21b..c78742d2 100644 --- a/scripts/scraper/_hash_merge.py +++ b/scripts/scraper/_hash_merge.py @@ -14,7 +14,7 @@ from pathlib import Path from typing import Any -from common import yaml_load +from common import write_text_atomic, yaml_load _MAME_RELEASE_RE = re.compile(r"^0\.\d+") @@ -390,7 +390,7 @@ def _backup_and_write(path: str, data: dict) -> None: patched = _patch_bios_entries(patched, data.get("files", [])) patched = _append_new_entries(patched, data.get("files", []), original) - p.write_text(patched, encoding="utf-8") + write_text_atomic(str(p), patched) def _patch_core_version(text: str, version: str) -> str: @@ -606,4 +606,4 @@ def _backup_and_write_fbneo(path: str, data: dict, hashes: dict) -> None: lines.append("") patched = patched.rstrip("\n") + "\n\n" + "\n".join(lines) - p.write_text(patched, encoding="utf-8") + write_text_atomic(str(p), patched) diff --git a/scripts/scraper/base_scraper.py b/scripts/scraper/base_scraper.py index 44d1c37f..c5c84a95 100644 --- a/scripts/scraper/base_scraper.py +++ b/scripts/scraper/base_scraper.py @@ -12,7 +12,7 @@ import yaml from abc import ABC, abstractmethod from dataclasses import dataclass, field from pathlib import Path -from common import yaml_load +from common import write_text_atomic, yaml_load @dataclass @@ -373,11 +373,12 @@ def scraper_cli( for field in ("data_directories",): if field in old_sys and field not in sys_data: sys_data[field] = old_sys[field] - with open(output_path, "w") as f: - yaml.dump( - config, f, Dumper=_PlatformDumper, - default_flow_style=False, allow_unicode=True, sort_keys=False, - ) + # Written whole or not at all: a reader in the middle of a streamed + # dump parsed a valid platform with a fraction of its systems. + write_text_atomic(output_path, yaml.dump( + config, Dumper=_PlatformDumper, + default_flow_style=False, allow_unicode=True, sort_keys=False, + )) print(f"Written {len(reqs)} entries to {args.output}") return diff --git a/scripts/scraper/fbneo_hash_scraper.py b/scripts/scraper/fbneo_hash_scraper.py index 5d38f11a..d52721db 100644 --- a/scripts/scraper/fbneo_hash_scraper.py +++ b/scripts/scraper/fbneo_hash_scraper.py @@ -20,7 +20,7 @@ from typing import Any import yaml -from common import yaml_load +from common import write_text_atomic, yaml_load from scripts.scraper._hash_merge import compute_diff, merge_fbneo_profile from scripts.scraper.fbneo_parser import parse_fbneo_source_tree @@ -187,9 +187,8 @@ def fetch_and_cache(force: bool = False) -> dict[str, Any]: } CACHE_PATH.parent.mkdir(parents=True, exist_ok=True) - CACHE_PATH.write_text( - json.dumps(cache, indent=2, ensure_ascii=False) + "\n", - encoding="utf-8", + write_text_atomic( + str(CACHE_PATH), json.dumps(cache, indent=2, ensure_ascii=False) + "\n" ) log.info("wrote %d BIOS sets to %s", len(bios_sets), CACHE_PATH) diff --git a/scripts/scraper/mame_hash_scraper.py b/scripts/scraper/mame_hash_scraper.py index 4f4534e2..ae51e6ec 100644 --- a/scripts/scraper/mame_hash_scraper.py +++ b/scripts/scraper/mame_hash_scraper.py @@ -22,7 +22,7 @@ from typing import Any import yaml -from common import yaml_load +from common import write_text_atomic, yaml_load from ._hash_merge import compute_diff, merge_mame_profile from .mame_parser import parse_mame_source_tree @@ -66,8 +66,9 @@ def _is_stale(cache: dict[str, Any] | None) -> bool: def _write_cache(data: dict[str, Any]) -> None: _CACHE_PATH.parent.mkdir(parents=True, exist_ok=True) - with open(_CACHE_PATH, "w", encoding="utf-8") as f: - json.dump(data, f, indent=2, ensure_ascii=False) + write_text_atomic( + str(_CACHE_PATH), json.dumps(data, indent=2, ensure_ascii=False) + ) log.info("cache written to %s", _CACHE_PATH) diff --git a/scripts/scraper/targets/__init__.py b/scripts/scraper/targets/__init__.py index 3049ca0b..b193eda4 100644 --- a/scripts/scraper/targets/__init__.py +++ b/scripts/scraper/targets/__init__.py @@ -28,8 +28,11 @@ class BaseTargetScraper(ABC): import yaml except ImportError: raise ImportError("PyYAML required: pip install pyyaml") - with open(output_path, "w") as f: - yaml.dump(data, f, default_flow_style=False, sort_keys=False) + from common import write_text_atomic + + write_text_atomic( + output_path, yaml.dump(data, default_flow_style=False, sort_keys=False) + ) _scrapers: dict[str, type] = {} diff --git a/tests/test_atomic_artifacts.py b/tests/test_atomic_artifacts.py index b6649ed4..5e7748a7 100644 --- a/tests/test_atomic_artifacts.py +++ b/tests/test_atomic_artifacts.py @@ -65,5 +65,33 @@ class EveryWriterGoesThroughIt(unittest.TestCase): self.assertNotIn("shutil.copy2(source, path)", (scripts / "restore_large_files.py").read_text()) + def test_scrapers_write_whole_files(self): + """A streamed yaml.dump into platforms/.yml let a concurrent reader + parse a valid platform holding a fraction of its systems.""" + import re + + streamed = re.compile( + r"open\([^)]*['\"](?:w|wb|a)['\"]|\.write_text\(|\.write_bytes\(" + ) + offenders = [ + f"{path.relative_to(REPO_ROOT)}:{number}" + for path in sorted((REPO_ROOT / "scripts" / "scraper").rglob("*.py")) + for number, line in enumerate( + path.read_text(encoding="utf-8").splitlines(), 1 + ) + if streamed.search(line) + ] + self.assertEqual(offenders, []) + + def test_the_text_is_utf8_whatever_the_locale(self): + import tempfile + + with tempfile.TemporaryDirectory() as directory: + target = Path(directory) / "out.yml" + artifacts.write_text_atomic(str(target), "name: \u00e9mulateur \u2014 \u30d5\n") + self.assertEqual( + target.read_bytes().decode("utf-8"), "name: \u00e9mulateur \u2014 \u30d5\n" + ) + if __name__ == "__main__": unittest.main()