fix: name split parts directly, version their dir

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
 

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()