fix: swap a pack in only once it is complete

Packs were opened on their final name, so a rebuild truncated the
previous one and a build that failed half way left a readable ZIP
of part of the files. Each pack is now written beside its name and
replaces it only when the write finished.
This commit is contained in:
Abdessamad Derraz committed 2026-10-18 03:54:59 +02:00
1 parent 164bc9e207
commit 383960212d
2 files changed
+76 -3

No files matched your search

+26 -3
View File
@@ -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:
+50
View File
@@ -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.