fix: name every narrowed artifact from one list

This commit is contained in:
Abdessamad Derraz committed 2026-09-07 06:27:35 +02:00
1 parent 29fcfdef34
commit 23c4dc2aae
5 files changed
+144 -13

No files matched your search

+58 -9
View File
@@ -1545,9 +1545,16 @@ def generate_system_pack(
offline=offline,
)
if result:
# Rename to system-based name
rgn_tag = f"_{region_mod.region_tag(regions)}" if regions else ""
new_name = f"{sys_display}{rgn_tag}_BIOS_Pack.zip"
# Rename to system-based name. Every dimension goes through the one
# list: keeping only the region tag made --system X --required-only
# overwrite the pack built without it.
tags = "".join(
tag
for tag, _label in _narrowings(
"full", regions, None, False, required_only, standalone=standalone
)
)
new_name = f"{sys_display}{tags}_BIOS_Pack.zip"
new_path = os.path.join(output_dir, new_name)
if new_path != result:
os.rename(result, new_path)
@@ -1616,8 +1623,12 @@ def generate_split_packs(
source, regions, target_name, one_per_slot, required_only
)
)
# 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}_Split"
output_dir,
f"{platform_display.replace(' ', '_')}{split_tags}{group_tag}_Split",
)
os.makedirs(split_dir, exist_ok=True)
@@ -1768,7 +1779,17 @@ def generate_md5_pack(
plat_file_index[alias.lower()] = fe
context_name = plat_display if platform_name else (emu_display or "Custom")
zip_name = f"{context_name.replace(' ', '_')}_Custom_BIOS_Pack.zip"
# --standalone changes the destination layout, so a run with it must not
# take the name of a run without it.
custom_tags = "".join(
tag
for tag, _label in _narrowings(
"full", None, None, False, False, standalone=standalone
)
)
zip_name = (
f"{context_name.replace(' ', '_')}_Custom{custom_tags}_BIOS_Pack.zip"
)
zip_path = os.path.join(output_dir, zip_name)
os.makedirs(output_dir, exist_ok=True)
@@ -3149,6 +3170,37 @@ def _target_cores_for(
return cache.get(platform_name)
def _content_narrowing_tags() -> tuple[str, ...]:
"""Tags naming a pack that holds fewer files than the platform declares.
Read from the name builder rather than retyped, so the two cannot drift.
Region and target are absent on purpose: both are handed to the check
itself, which narrows its expectation instead of skipping it.
"""
dimensions = (
{"source": "platform"},
{"source": "truth"},
{"one_per_slot": True},
{"required_only": True},
)
tags: list[str] = []
for dimension in dimensions:
tags.extend(
tag
for tag, _label in _narrowings(
dimension.get("source", "full"),
None,
None,
dimension.get("one_per_slot", False),
dimension.get("required_only", False),
)
)
return tuple(tags)
_CONTENT_NARROWING_TAGS = _content_narrowing_tags()
def _narrows_contents(pack_name: str) -> bool:
"""True when a pack holds fewer files than the platform declares.
@@ -3157,10 +3209,7 @@ def _narrows_contents(pack_name: str) -> bool:
Region is not listed: the region filter is passed to the check itself, and
a hardware target is passed the same way.
"""
return any(
tag in pack_name
for tag in ("_Platform_", "_Truth_", "_Required", "_OnePerSlot")
)
return any(f"{tag}_" in pack_name for tag in _CONTENT_NARROWING_TAGS)
def verify_and_finalize_packs(
+10 -2
View File
@@ -10,6 +10,7 @@ from __future__ import annotations
import argparse
import os
import re
import sys
sys.path.insert(0, os.path.dirname(__file__))
@@ -87,7 +88,14 @@ def main(argv: list[str] | None = None) -> None:
else:
platforms = [args.platform]
os.makedirs(args.output_dir, exist_ok=True)
# A target-narrowed model is a different artifact: written under the
# platform's own name it overwrote the full one, and diff_truth then
# compared a narrowed model against the whole scrape.
output_dir = args.output_dir
if args.target:
slug = re.sub(r"[^a-z0-9]+", "-", args.target.strip().lower()).strip("-")
output_dir = os.path.join(output_dir, slug or "target")
os.makedirs(output_dir, exist_ok=True)
for name in platforms:
# Resolve target cores
@@ -120,7 +128,7 @@ def main(argv: list[str] | None = None) -> None:
target_cores=target_cores,
)
out_path = os.path.join(args.output_dir, f"{name}.yml")
out_path = os.path.join(output_dir, f"{name}.yml")
with open(out_path, "w") as f:
yaml.dump(
result,
+10 -1
View File
@@ -347,6 +347,15 @@ def main():
print("\n--- 2b check buildbot system: SKIPPED (--offline) ---")
# Step 2c: Generate truth YAMLs
# A targeted run writes its model in a subdirectory of its own, and the
# diff has to read the same one or it compares a narrowed model against
# the whole scrape.
truth_dir = Path(args.output_dir) / "truth"
if args.target:
truth_dir = truth_dir / re.sub(
r"[^a-z0-9]+", "-", args.target.strip().lower()
).strip("-")
if args.with_truth or args.with_export:
truth_cmd = [
sys.executable,
@@ -366,7 +375,7 @@ def main():
# Step 2d: Diff truth vs scraped
if args.with_truth or args.with_export:
diff_cmd = [sys.executable, "scripts/diff_truth.py", "--all"]
diff_cmd.extend(["--truth-dir", str(Path(args.output_dir) / "truth")])
diff_cmd.extend(["--truth-dir", str(truth_dir)])
ok, _ = run(diff_cmd, "2d diff truth")
results["diff_truth"] = ok
all_ok = all_ok and ok
+33 -1
View File
@@ -5917,9 +5917,41 @@ struct BurnDriver BurnDrvneogeo = {
'source_tag = {"platform": "_Platform"',
'req_suffix = "_required"',
'slot_tag = "_OnePerSlot"',
# The --system rename kept the region tag alone, the custom pack
# took no tag at all, and the skip list retyped four of them.
'_Custom_BIOS_Pack.zip"',
'{rgn_tag}_BIOS_Pack.zip"',
'("_Platform_", "_Truth_"',
):
self.assertNotIn(hand_rolled, source, hand_rolled)
self.assertGreaterEqual(source.count("_narrowings("), 5)
self.assertGreaterEqual(source.count("_narrowings("), 8)
def test_a_narrowed_system_pack_does_not_take_the_full_name(self):
"""Every dimension has to show in the filename or two builds collide.
--system X --required-only produced the same name as --system X and
overwrote it, because the rename kept only the region tag.
"""
import subprocess
repo = os.path.join(os.path.dirname(__file__), "..")
names = {}
for label, extra in (("full", []), ("required", ["--required-only"])):
out = os.path.join(self.root, f"sysname_{label}")
proc = subprocess.run(
[sys.executable, "scripts/generate_pack.py", "--system",
"atari-lynx", "--offline", *extra, "--output-dir", out],
capture_output=True, text=True, cwd=repo, timeout=300,
)
self.assertEqual(proc.returncode, 0, proc.stdout + proc.stderr)
names[label] = sorted(
f for f in os.listdir(out) if f.endswith("_BIOS_Pack.zip")
)
self.assertTrue(names["full"] and names["required"])
self.assertNotEqual(
names["full"], names["required"],
"a required-only system pack took the name of the full one",
)
def test_a_full_pack_is_not_announced_as_narrowed(self):
from generate_pack import _build_readme, _narrowings
+33
View File
@@ -9,6 +9,7 @@ discrepancy that does not exist. Content decides, as everywhere else here.
from __future__ import annotations
import pathlib
import sys
import unittest
from pathlib import Path
@@ -23,6 +24,38 @@ def _entry(name: str, **hashes) -> dict:
return {"name": name, **hashes}
class ATargetedModelIsItsOwnArtifact(unittest.TestCase):
"""A narrowed truth model must not take the full model's place.
generate_truth wrote dist/truth/<platform>.yml whatever --target said, so
a targeted run overwrote the full model and diff_truth then compared a
narrowed model against the whole scrape.
"""
def test_a_target_writes_beside_the_full_model(self):
import subprocess
import tempfile
repo = pathlib.Path(__file__).resolve().parent.parent
with tempfile.TemporaryDirectory(dir=str(repo / "tmp")) as directory:
for extra in ([], ["--target", "browser"]):
proc = subprocess.run(
[sys.executable, "scripts/generate_truth.py", "--platform",
"romm", *extra, "--output-dir", directory],
capture_output=True, text=True, cwd=str(repo), timeout=400,
)
self.assertEqual(proc.returncode, 0, proc.stdout + proc.stderr)
produced = sorted(
str(p.relative_to(directory))
for p in pathlib.Path(directory).rglob("*.yml")
)
self.assertEqual(
produced,
["browser/romm.yml", "romm.yml"],
"the targeted model did not land beside the full one",
)
class RenameMatching(unittest.TestCase):
def test_a_shared_sha1_pairs_the_two_names(self):
truth = [_entry("bios_CD_U.bin", sha1="a" * 40)]