diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index f57c1032..a789f9db 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -598,9 +598,14 @@ def generate_pack( target_name: str | None = None, one_per_slot: bool = False, offline: bool | None = None, + pack_name: str | None = None, ) -> str | None: """Generate a ZIP pack for a platform. + ``pack_name`` replaces the derived file name: a split part is named after + its group, and a name built from every system of a hundred-system group + is longer than a file name can be. + Returns the path to the generated ZIP, or None on failure. """ config = load_platform_config(platform_name, platforms_dir) @@ -620,7 +625,7 @@ def generate_pack( ) narrow_tags = "".join(tag for tag, _label in narrowings) stem = _platform_pack_stem([platform_name], platform_name, platforms_dir) - zip_name = f"{stem}{narrow_tags}_BIOS_Pack{_system_tag(system_filter)}.zip" + zip_name = pack_name or f"{stem}{narrow_tags}_BIOS_Pack{_system_tag(system_filter)}.zip" zip_path = os.path.join(output_dir, zip_name) os.makedirs(output_dir, exist_ok=True) @@ -1683,10 +1688,13 @@ def generate_split_packs( # Two groupings write different files; without the tag they accumulate in # one directory under one SHA256SUMS.txt. group_tag = "" if group_by == "system" else f"_By{group_by.title()}" - split_dir = os.path.join( - output_dir, - f"{platform_display.replace(' ', '_')}{split_tags}{group_tag}_Split", - ) + # The parts carry the version, so the directory does too: a rescrape that + # moves the version would otherwise leave the old parts beside the new + # under one SHA256SUMS.txt. + version = config.get("version", config.get("dat_version", "")) + ver_tag = f"_{version.replace(' ', '')}" if version else "" + part_prefix = f"{platform_display.replace(' ', '_')}{ver_tag}{split_tags}" + split_dir = os.path.join(output_dir, f"{part_prefix}{group_tag}_Split") os.makedirs(split_dir, exist_ok=True) systems = config.get("systems", {}) @@ -1710,14 +1718,6 @@ def generate_split_packs( ) else: all_extras = [] - version = config.get("version", config.get("dat_version", "")) - ver_tag = f"_{version.replace(' ', '')}" if version else "" - narrow_tags = "".join( - tag - for tag, _label in _narrowings( - source, regions, target_name, one_per_slot, required_only - ) - ) results = [] for group_name, group_system_ids in sorted(groups.items()): group_extras = _extras_for_systems(all_extras, group_system_ids) @@ -1740,14 +1740,9 @@ def generate_split_packs( target_name=target_name, one_per_slot=one_per_slot, offline=offline, + pack_name=f"{part_prefix}_{_name_part(group_name, '_')}_BIOS_Pack.zip", ) if zip_path: - safe_group = _name_part(group_name, "_") - new_name = f"{platform_display.replace(' ', '_')}{ver_tag}{narrow_tags}_{safe_group}_BIOS_Pack.zip" - new_path = os.path.join(split_dir, new_name) - if new_path != zip_path: - os.rename(zip_path, new_path) - zip_path = new_path results.append(zip_path) # Extras whose system the platform does not declare, or that name none, @@ -1768,16 +1763,10 @@ def generate_split_packs( target_cores=target_cores, required_only=required_only, precomputed_extras=undistributed, extras_only=True, source=source, regions=regions, target_name=target_name, one_per_slot=one_per_slot, - offline=offline, + offline=offline, pack_name=f"{part_prefix}_Other_Cores_BIOS_Pack.zip", ) if zip_path: - new_path = os.path.join( - split_dir, - f"{platform_display.replace(' ', '_')}{ver_tag}{narrow_tags}" - "_Other_Cores_BIOS_Pack.zip", - ) - os.replace(zip_path, new_path) - results.append(new_path) + results.append(zip_path) return results diff --git a/tests/test_split_names.py b/tests/test_split_names.py new file mode 100644 index 00000000..ecab4b93 --- /dev/null +++ b/tests/test_split_names.py @@ -0,0 +1,110 @@ +"""A --split part is named after its group and its platform version. + +The intermediate name of each part joined every system id of the group: the +"Other" group of RetroBat (100 systems) gave a 786-byte file name, the write +failed with ENAMETOOLONG and the platform's split was never produced. The +directory holding the parts did not carry the version the parts carry, so a +rescrape that moved the version left both generations under one +SHA256SUMS.txt. +""" + +from __future__ import annotations + +import os +import sys +import tempfile +import unittest +from pathlib import Path + +import yaml + +REPO_ROOT = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(REPO_ROOT / "scripts")) + +import common +from common import build_zip_contents_index, compute_hashes +from generate_pack import generate_split_packs + + +class SplitPartNames(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + root = Path(self.tmp.name) + self.platforms = root / "platforms" + self.emulators = root / "emulators" + self.out = root / "dist" + bios = root / "bios" / "Test" + for path in (self.platforms, self.emulators, bios): + path.mkdir(parents=True) + blob = bios / "shared.bin" + blob.write_bytes(b"shared") + h = compute_hashes(str(blob)) + self.db = { + "files": { + h["sha1"]: { + "name": "shared.bin", "md5": h["md5"], "sha1": h["sha1"], + "sha256": h["sha256"], "path": str(blob), "paths": [str(blob)], + } + }, + "indexes": { + "by_md5": {h["md5"]: h["sha1"]}, + "by_name": {"shared.bin": [h["sha1"]]}, + "by_crc32": {}, + "by_path_suffix": {}, + }, + } + (self.platforms / "_registry.yml").write_text( + yaml.dump({"platforms": {"longplat": {"status": "active"}}}) + ) + self.sha1 = h["sha1"] + + def tearDown(self): + self.tmp.cleanup() + + def _platform(self, version: str, systems: int) -> None: + config = { + "platform": "LongTest", + "version": version, + "verification_mode": "existence", + "systems": { + f"unbranded-system-number-{n:03d}": { + "files": [{"name": "shared.bin", "sha1": self.sha1}] + } + for n in range(systems) + }, + } + (self.platforms / "longplat.yml").write_text(yaml.dump(config)) + common._platform_config_cache.clear() + + def _split(self, group_by: str) -> list[str]: + return generate_split_packs( + "longplat", str(self.platforms), self.db, self.tmp.name, str(self.out), + group_by=group_by, emulators_dir=str(self.emulators), + zip_contents=build_zip_contents_index(self.db), emu_profiles={}, + ) + + def test_a_large_group_is_written(self): + self._platform("1.0", 40) + parts = self._split("manufacturer") + self.assertEqual( + [os.path.basename(p) for p in parts], ["LongTest_1.0_Other_BIOS_Pack.zip"] + ) + self.assertTrue(os.path.isfile(parts[0])) + + def test_two_versions_do_not_share_a_directory(self): + self._platform("1.0", 2) + first = {os.path.dirname(p) for p in self._split("system")} + self._platform("1.1", 2) + second = {os.path.dirname(p) for p in self._split("system")} + self.assertEqual(len(first), 1) + self.assertEqual(len(second), 1) + self.assertNotEqual(first, second) + for directory in second: + self.assertTrue( + all("_1.1_" in name for name in os.listdir(directory)), + os.listdir(directory), + ) + + +if __name__ == "__main__": + unittest.main()