diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index b611b652e..31984c0b6 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -182,6 +182,29 @@ def _narrowings( return applied +@contextlib.contextmanager +def _writing_pack(zip_path: str) -> Iterator[zipfile.ZipFile]: + """A pack written beside its final name, swapped in once complete. + + Opened on the final name, a rebuild truncated the previous pack at once, + and a build that raised half way still wrote a central directory: a + readable ZIP holding part of the files, which the run then checked and + summed as the pack. + """ + directory = os.path.dirname(zip_path) or "." + fd, scratch = tempfile.mkstemp( + prefix=f".{os.path.basename(zip_path)}.", suffix=".partial", dir=directory + ) + os.close(fd) + try: + with zipfile.ZipFile(scratch, "w", zipfile.ZIP_DEFLATED) as zf: + yield zf + os.replace(scratch, zip_path) + except BaseException: + Path(scratch).unlink(missing_ok=True) + raise + + def _target_tag(target_name: str) -> str: """Filename tag for a hardware target. @@ -735,7 +758,7 @@ def generate_pack( target_cores, source, regions, one_per_slot, required_only, ) - with zipfile.ZipFile(zip_path, "w", zipfile.ZIP_DEFLATED) as zf: + with _writing_pack(zip_path) as zf: if source != "truth": for sys_id, system in sorted(pack_systems.items()): for file_entry in system.get("files", []): @@ -1385,7 +1408,7 @@ def generate_emulator_pack( platforms_dir or os.path.join(os.path.dirname(__file__), "..", "platforms") ) - with zipfile.ZipFile(zip_path, "w", zipfile.ZIP_DEFLATED) as zf: + with _writing_pack(zip_path) as zf: for emu_name, profile in sorted(selected): pack_structure = profile.get("pack_structure") files = expand_directory_entries( @@ -2014,7 +2037,7 @@ def generate_md5_pack( not_in_db: list[str] = [] seen: set[str] = set() - with zipfile.ZipFile(zip_path, "w", zipfile.ZIP_DEFLATED) as zf: + with _writing_pack(zip_path) as zf: for hash_type, hash_val in hashes: sha1 = None if hash_type == "sha1" and hash_val in files_db: diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 5349b28c5..c97a6ea7d 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -407,6 +407,56 @@ class OnePlatformPerFlag(unittest.TestCase): self.assertIn("batocera", str(caught.exception)) +class APackIsSwappedInWhole(unittest.TestCase): + """Packs were opened on their final name: a rebuild truncated the good + pack at once, and a build that raised half way left a readable ZIP of + part of the files, which the run then verified and summed.""" + + def _dir(self) -> Path: + import shutil + import tempfile + + (ROOT / "tmp").mkdir(exist_ok=True) + directory = Path(tempfile.mkdtemp(dir=ROOT / "tmp")) + self.addCleanup(shutil.rmtree, directory, True) + return directory + + def test_a_failed_build_keeps_the_previous_pack(self): + import zipfile + + from generate_pack import _writing_pack + + directory = self._dir() + pack = directory / "Pack.zip" + with zipfile.ZipFile(pack, "w") as zf: + zf.writestr("old.bin", b"complete") + with self.assertRaises(OSError): + with _writing_pack(str(pack)) as zf: + zf.writestr("new.bin", b"half") + raise OSError("disk full") + with zipfile.ZipFile(pack) as zf: + self.assertEqual(zf.namelist(), ["old.bin"]) + self.assertEqual(sorted(p.name for p in directory.iterdir()), ["Pack.zip"]) + + def test_a_finished_build_replaces_it(self): + import zipfile + + from generate_pack import _writing_pack + + directory = self._dir() + pack = directory / "Pack.zip" + pack.write_bytes(b"stale") + with _writing_pack(str(pack)) as zf: + zf.writestr("new.bin", b"whole") + with zipfile.ZipFile(pack) as zf: + self.assertEqual(zf.namelist(), ["new.bin"]) + self.assertEqual(sorted(p.name for p in directory.iterdir()), ["Pack.zip"]) + + def test_no_pack_is_opened_on_its_final_name(self): + source = (ROOT / "scripts" / "generate_pack.py").read_text() + self.assertNotIn('ZipFile(zip_path, "w"', source) + + class FaqRegressions(unittest.TestCase): """The FAQ states facts the code owns; these tie it back to the source.