mirror of
https://github.com/Abdess/retroarch_system.git
synced 2026-11-06 01:53:56 -06:00
fix: compare system packs in pipeline step 5
Step 5 built emulator packs one profile at a time, where two profiles never share a path. It now also builds each system pack that gathers several profiles and compares it with verify. The first run found five standalone MSX and ColecoVision packs where verify skipped the data tree of a profile the system filter left without files, and credited msx-emu's copies the pack never wrote. verify now places those trees too: 541 system packs, all equal.
This commit is contained in:
1 parent
b20d65d129
commit
c1f0e8c2ca
5 files changed
+137
-24
No files matched your search
+43
-3
@@ -22,7 +22,7 @@ Steps, numbered as the run prints them:
|
|||||||
4c --manifest-targets per-target core lists
|
4c --manifest-targets per-target core lists
|
||||||
4d bundled_only.py --strict copies only an emulator install provides
|
4d bundled_only.py --strict copies only an emulator install provides
|
||||||
4e generate_games_index.py install/games/: what the games-on-card mode reads
|
4e generate_games_index.py install/games/: what the games-on-card mode reads
|
||||||
5 consistency check verify == pack: counts, paths, every emulator pack
|
5 consistency check verify == pack: counts, paths, emulator and system packs
|
||||||
6 --verify-packs pack integrity, extraction and hashes
|
6 --verify-packs pack integrity, extraction and hashes
|
||||||
7 generate_readme.py README.md and CONTRIBUTING.md
|
7 generate_readme.py README.md and CONTRIBUTING.md
|
||||||
generate_installer_doc.py run samples and tables of wiki/installer.md
|
generate_installer_doc.py run samples and tables of wiki/installer.md
|
||||||
@@ -518,7 +518,7 @@ def check_emulator_packs(
|
|||||||
emulators_dir: str = "emulators", platforms_dir: str = "platforms",
|
emulators_dir: str = "emulators", platforms_dir: str = "platforms",
|
||||||
db_path: str = "database.json",
|
db_path: str = "database.json",
|
||||||
) -> bool:
|
) -> bool:
|
||||||
"""Build every emulator pack in memory and compare it with verify --emulator.
|
"""Build every emulator and system pack in memory and compare it with verify.
|
||||||
|
|
||||||
Each profile in each build it has (libretro, standalone): the paths the
|
Each profile in each build it has (libretro, standalone): the paths the
|
||||||
pack fills with the profile's files against the ones verify credits. No
|
pack fills with the profile's files against the ones verify credits. No
|
||||||
@@ -528,9 +528,11 @@ def check_emulator_packs(
|
|||||||
import verify # noqa: PLC0415
|
import verify # noqa: PLC0415
|
||||||
from common import ( # noqa: PLC0415
|
from common import ( # noqa: PLC0415
|
||||||
ProfileSelectionError,
|
ProfileSelectionError,
|
||||||
|
_norm_system_id,
|
||||||
build_zip_contents_index,
|
build_zip_contents_index,
|
||||||
load_database,
|
load_database,
|
||||||
load_emulator_profiles,
|
load_emulator_profiles,
|
||||||
|
profiles_for_systems,
|
||||||
select_emulator_profiles,
|
select_emulator_profiles,
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -566,7 +568,45 @@ def check_emulator_packs(
|
|||||||
print("\n".join(lines))
|
print("\n".join(lines))
|
||||||
all_ok = False
|
all_ok = False
|
||||||
print(f" {compared} emulator packs compared, {'all equal' if all_ok else 'differences above'}")
|
print(f" {compared} emulator packs compared, {'all equal' if all_ok else 'differences above'}")
|
||||||
return all_ok
|
# A system pack builds every profile of the system together: a placement
|
||||||
|
# rule that only goes wrong when two profiles share a path (the bluemsx
|
||||||
|
# tree beside msx-emu's files) never meets in a one-profile pack.
|
||||||
|
systems = sorted({
|
||||||
|
_norm_system_id(str(sid))
|
||||||
|
for profile in profiles.values()
|
||||||
|
if profile.get("type") not in ("launcher", "alias", "test")
|
||||||
|
for sid in profile.get("systems") or []
|
||||||
|
})
|
||||||
|
systems_ok = True
|
||||||
|
compared = 0
|
||||||
|
for sid in systems:
|
||||||
|
for standalone in (False, True):
|
||||||
|
matching = profiles_for_systems(profiles, [sid], standalone)
|
||||||
|
if len(matching) < 2:
|
||||||
|
continue
|
||||||
|
record = generate_pack.PackRecord()
|
||||||
|
with contextlib.redirect_stdout(io.StringIO()), \
|
||||||
|
contextlib.redirect_stderr(io.StringIO()):
|
||||||
|
generate_pack.generate_emulator_pack(
|
||||||
|
matching, emulators_dir, db, "bios", "", standalone, zip_contents,
|
||||||
|
offline=True, platforms_dir=platforms_dir, record=record,
|
||||||
|
system_ids=[sid],
|
||||||
|
)
|
||||||
|
report = verify.verify_emulator(
|
||||||
|
matching, emulators_dir, db, standalone, platforms_dir=platforms_dir,
|
||||||
|
system_ids=[sid],
|
||||||
|
)
|
||||||
|
compared += 1
|
||||||
|
label = f"--system {sid}{' --standalone' if standalone else ''}"
|
||||||
|
lines = _set_difference_lines(
|
||||||
|
label, set(report["pack_destinations"]), set(record.files)
|
||||||
|
)
|
||||||
|
if lines:
|
||||||
|
print("\n".join(lines))
|
||||||
|
systems_ok = False
|
||||||
|
print(f" {compared} system packs compared, "
|
||||||
|
f"{'all equal' if systems_ok else 'differences above'}")
|
||||||
|
return all_ok and systems_ok
|
||||||
|
|
||||||
|
|
||||||
class _Skipped:
|
class _Skipped:
|
||||||
|
|||||||
+26
-18
@@ -2116,6 +2116,30 @@ def verify_emulator(
|
|||||||
return
|
return
|
||||||
placed[dest.lower()] = (path, owner, dest)
|
placed[dest.lower()] = (path, owner, dest)
|
||||||
_register_path(dest, placed_files, placed_parents)
|
_register_path(dest, placed_files, placed_parents)
|
||||||
|
|
||||||
|
def occupy_trees(emu_name: str, profile: dict) -> None:
|
||||||
|
"""The profile's data trees follow its files in the pack, which
|
||||||
|
writes them even when the system filter leaves it no file:
|
||||||
|
bluemsx's Machines/ tree holds msx-emu's coleco.rom path in a
|
||||||
|
--system colecovision pack."""
|
||||||
|
for dd in deliverable(profile.get("data_directories", []), standalone):
|
||||||
|
cache = (data_registry or {}).get(dd.get("ref", ""), {}).get("local_cache", "")
|
||||||
|
if not cache or not os.path.isdir(cache):
|
||||||
|
continue
|
||||||
|
dd_dest = dd.get("destination", "")
|
||||||
|
structure = profile.get("pack_structure")
|
||||||
|
prefix = (
|
||||||
|
structure.get("standalone" if standalone else "libretro", "")
|
||||||
|
if structure else ""
|
||||||
|
)
|
||||||
|
if prefix:
|
||||||
|
dd_dest = f"{prefix}/{dd_dest}" if dd_dest else prefix
|
||||||
|
for root, _dirs, names in os.walk(cache):
|
||||||
|
for fname in names:
|
||||||
|
src = os.path.join(root, fname)
|
||||||
|
rel = os.path.relpath(src, cache).replace(os.sep, "/")
|
||||||
|
occupy(f"{dd_dest}/{rel}" if dd_dest else rel, src, emu_name)
|
||||||
|
|
||||||
for emu_name, profile in sorted(selected):
|
for emu_name, profile in sorted(selected):
|
||||||
files = expand_agnostic_entries(
|
files = expand_agnostic_entries(
|
||||||
expand_directory_entries(
|
expand_directory_entries(
|
||||||
@@ -2162,6 +2186,7 @@ def verify_emulator(
|
|||||||
"ground_truth": [],
|
"ground_truth": [],
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
|
occupy_trees(emu_name, profile)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
# Verify archives as units (e.g., neogeo.zip, aes.zip)
|
# Verify archives as units (e.g., neogeo.zip, aes.zip)
|
||||||
@@ -2424,24 +2449,7 @@ def verify_emulator(
|
|||||||
) > _SEVERITY_ORDER.get(prev_sev, 0):
|
) > _SEVERITY_ORDER.get(prev_sev, 0):
|
||||||
file_severity[dest] = sev
|
file_severity[dest] = sev
|
||||||
|
|
||||||
# The profile's data trees follow its files in the pack.
|
occupy_trees(emu_name, profile)
|
||||||
for dd in deliverable(profile.get("data_directories", []), standalone):
|
|
||||||
cache = (data_registry or {}).get(dd.get("ref", ""), {}).get("local_cache", "")
|
|
||||||
if not cache or not os.path.isdir(cache):
|
|
||||||
continue
|
|
||||||
dd_dest = dd.get("destination", "")
|
|
||||||
structure = profile.get("pack_structure")
|
|
||||||
prefix = (
|
|
||||||
structure.get("standalone" if standalone else "libretro", "")
|
|
||||||
if structure else ""
|
|
||||||
)
|
|
||||||
if prefix:
|
|
||||||
dd_dest = f"{prefix}/{dd_dest}" if dd_dest else prefix
|
|
||||||
for root, _dirs, names in os.walk(cache):
|
|
||||||
for fname in names:
|
|
||||||
src = os.path.join(root, fname)
|
|
||||||
rel = os.path.relpath(src, cache).replace(os.sep, "/")
|
|
||||||
occupy(f"{dd_dest}/{rel}" if dd_dest else rel, src, emu_name)
|
|
||||||
|
|
||||||
counts = {
|
counts = {
|
||||||
Severity.OK: 0,
|
Severity.OK: 0,
|
||||||
|
|||||||
@@ -189,6 +189,9 @@ class EveryEmulatorPackMatchesItsReport(unittest.TestCase):
|
|||||||
finally:
|
finally:
|
||||||
os.chdir(previous)
|
os.chdir(previous)
|
||||||
self.assertTrue(ok, out.getvalue())
|
self.assertTrue(ok, out.getvalue())
|
||||||
|
# Packs of several profiles too: a placement rule that only goes
|
||||||
|
# wrong when two profiles share a path never meets in one profile.
|
||||||
|
self.assertIn("system packs compared, all equal", out.getvalue())
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
|
|||||||
@@ -424,6 +424,67 @@ class ASystemPackHoldsThatSystem(unittest.TestCase):
|
|||||||
self.assertEqual(report["total_files"], 2)
|
self.assertEqual(report["total_files"], 2)
|
||||||
|
|
||||||
|
|
||||||
|
class ATreeHoldsAPathForAnotherProfile(unittest.TestCase):
|
||||||
|
"""--system colecovision --standalone: bluemsx keeps no file of the
|
||||||
|
system but its Machines/ tree is packed, and holds msx-emu's coleco.rom
|
||||||
|
path. verify skipped the tree of a profile left without files and
|
||||||
|
credited msx-emu's copy the pack never wrote."""
|
||||||
|
|
||||||
|
def test_report_and_pack_agree(self):
|
||||||
|
import hashlib
|
||||||
|
import io
|
||||||
|
import shutil
|
||||||
|
from contextlib import redirect_stdout
|
||||||
|
|
||||||
|
import yaml
|
||||||
|
from verify import verify_emulator
|
||||||
|
|
||||||
|
common._emulator_profiles_cache.clear()
|
||||||
|
(REPO_ROOT / "tmp").mkdir(exist_ok=True)
|
||||||
|
root = Path(tempfile.mkdtemp(dir=REPO_ROOT / "tmp"))
|
||||||
|
self.addCleanup(shutil.rmtree, root, True)
|
||||||
|
previous = os.getcwd()
|
||||||
|
os.chdir(root)
|
||||||
|
self.addCleanup(os.chdir, previous)
|
||||||
|
data = b"coleco" * 4
|
||||||
|
sha1 = hashlib.sha1(data).hexdigest()
|
||||||
|
rel = "bios/Coleco/ColecoVision/coleco.rom"
|
||||||
|
Path(rel).parent.mkdir(parents=True)
|
||||||
|
Path(rel).write_bytes(data)
|
||||||
|
tree = Path("data/tree/Machines/COL")
|
||||||
|
tree.mkdir(parents=True)
|
||||||
|
(tree / "coleco.rom").write_bytes(data)
|
||||||
|
db = {"files": {sha1: {"path": rel, "name": "coleco.rom", "size": len(data),
|
||||||
|
"md5": hashlib.md5(data).hexdigest(), "crc32": ""}},
|
||||||
|
"indexes": {"by_md5": {}, "by_name": {"coleco.rom": [sha1]},
|
||||||
|
"by_crc32": {}, "by_path_suffix": {}}}
|
||||||
|
Path("platforms").mkdir()
|
||||||
|
Path("platforms/_data_dirs.yml").write_text(yaml.safe_dump(
|
||||||
|
{"data_directories": {"tree": {"local_cache": "data/tree"}}}))
|
||||||
|
Path("emulators").mkdir()
|
||||||
|
Path("emulators/atree.yml").write_text(yaml.safe_dump({
|
||||||
|
"emulator": "ATree", "type": "libretro", "cores": ["atree"],
|
||||||
|
"systems": ["colecovision", "msx"],
|
||||||
|
"data_directories": [{"ref": "tree"}],
|
||||||
|
"files": [{"name": "msx.rom", "system": "msx"}],
|
||||||
|
}))
|
||||||
|
Path("emulators/msxemu.yml").write_text(yaml.safe_dump({
|
||||||
|
"emulator": "MsxEmu", "type": "libretro", "cores": ["msxemu"],
|
||||||
|
"systems": ["colecovision"],
|
||||||
|
"files": [{"name": "coleco.rom", "path": "Machines/COL/coleco.rom"}],
|
||||||
|
}))
|
||||||
|
names = ["atree", "msxemu"]
|
||||||
|
report = verify_emulator(names, "emulators", db, platforms_dir="platforms",
|
||||||
|
system_ids=["colecovision"])
|
||||||
|
record = builder.PackRecord()
|
||||||
|
with redirect_stdout(io.StringIO()):
|
||||||
|
builder.generate_emulator_pack(
|
||||||
|
names, "emulators", db, "bios", "", offline=True,
|
||||||
|
platforms_dir="platforms", record=record, system_ids=["colecovision"],
|
||||||
|
)
|
||||||
|
self.assertEqual(sorted(report["pack_destinations"]), sorted(record.files))
|
||||||
|
|
||||||
|
|
||||||
class ListingsKeepTheirColumns(unittest.TestCase):
|
class ListingsKeepTheirColumns(unittest.TestCase):
|
||||||
"""--list-systems and --list-emulators started their last column at a
|
"""--list-systems and --list-emulators started their last column at a
|
||||||
position that moved with the file count and the profile type."""
|
position that moved with the file count and the profile type."""
|
||||||
|
|||||||
+4
-3
@@ -52,7 +52,7 @@ Pipeline steps:
|
|||||||
| 4c | Generate target manifests | `--skip-packs` |
|
| 4c | Generate target manifests | `--skip-packs` |
|
||||||
| 4d | Collected copies only an emulator install provides (`bundled_only.py --strict`, gates the run) | - |
|
| 4d | Collected copies only an emulator install provides (`bundled_only.py --strict`, gates the run) | - |
|
||||||
| 4e | Games index: what the installer reads for the games on a card (`generate_games_index.py`, fails under its coverage floor) | - (reads the committed install lists) |
|
| 4e | Games index: what the installer reads for the games on a card (`generate_games_index.py`, fails under its coverage floor) | - (reads the committed install lists) |
|
||||||
| 5 | Consistency check: counts, paths per archive, every emulator pack | `--skip-packs`, or no full pack built (`--source platform` or `truth`) |
|
| 5 | Consistency check: counts, paths per archive, every emulator and system pack | `--skip-packs`, or no full pack built (`--source platform` or `truth`) |
|
||||||
| 6 | Pack integrity (extract + hash) | `--skip-packs`, or no full pack built |
|
| 6 | Pack integrity (extract + hash) | `--skip-packs`, or no full pack built |
|
||||||
| 7 | Generate README and CONTRIBUTING, then the installer page's tables and run samples (`generate_installer_doc.py`) | `--skip-docs` |
|
| 7 | Generate README and CONTRIBUTING, then the installer page's tables and run samples (`generate_installer_doc.py`) | `--skip-docs` |
|
||||||
| 8 | Generate site | `--skip-docs` |
|
| 8 | Generate site | `--skip-docs` |
|
||||||
@@ -91,8 +91,9 @@ alone, run `python scripts/check_buildbot_system.py`.
|
|||||||
- **verify.py and generate_pack.py read the same functions.** A count or a
|
- **verify.py and generate_pack.py read the same functions.** A count or a
|
||||||
path that differs between the report and the pack is a defect, which is
|
path that differs between the report and the pack is a defect, which is
|
||||||
what step 5 checks: the counts, the paths each archive holds against the
|
what step 5 checks: the counts, the paths each archive holds against the
|
||||||
ones verify credits (`--destinations`), and every emulator pack, each
|
ones verify credits (`--destinations`), every emulator pack, each
|
||||||
profile in each build, built in memory against `verify --emulator`.
|
profile in each build, built in memory against `verify --emulator`, and
|
||||||
|
every system pack that gathers several profiles against `verify --system`.
|
||||||
Equal counts once hid four RetroArch paths credited and never written.
|
Equal counts once hid four RetroArch paths credited and never written.
|
||||||
- **Generators write through `write_if_changed()`.** A new timestamp alone
|
- **Generators write through `write_if_changed()`.** A new timestamp alone
|
||||||
rewrites nothing, so a diff on a generated file always means content.
|
rewrites nothing, so a diff on a generated file always means content.
|
||||||
|
|||||||
Reference in new issue
Block a user