From 80084390339ff527edb02c1b7a77eb7c04639125 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 21 Oct 2026 05:06:36 +0200 Subject: [PATCH] fix: keep manifests and target lists apart Both are written as .json. --manifest-targets aimed at install/ replaced install/retroarch.json with a target table, and --manifest aimed at install/targets/ did the reverse, both with exit 0. Each mode now stops when the output directory holds the other kind and names the directory to use. --- scripts/generate_pack.py | 49 +++++++++++++++++++++++++++++++++ tests/test_audit_regressions.py | 36 ++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index ff3cc9ced..c4b06d0f0 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -2181,6 +2181,53 @@ def generate_md5_pack( return zip_path +def _json_artifact_kind(path: Path) -> str | None: + """"manifest" for an install manifest, "targets" for a per-target core + list, None for anything else. The two share their file names: install.py + fetches install/.json and install/targets/.json.""" + try: + data = json.loads(path.read_text(encoding="utf-8")) + except (OSError, UnicodeDecodeError, json.JSONDecodeError): + return None + if not isinstance(data, dict) or not data: + return None + if "manifest_version" in data: + return "manifest" + if all(v is None or isinstance(v, list) for v in data.values()): + return "targets" + return None + + +def _refuse_mixed_output(output_dir: str, writing: str) -> None: + """Stop before one kind of installer JSON overwrites the other. + + Both are named .json, so --manifest-targets aimed at install/ + replaced install/retroarch.json, the file every installer downloads, with + a target table, and the reverse replaced a target list by a manifest. + """ + directory = Path(output_dir) + if not directory.is_dir(): + return + other = "targets" if writing == "manifest" else "manifest" + held = sorted( + p.name for p in directory.glob("*.json") if _json_artifact_kind(p) == other + ) + if not held: + return + what = { + "manifest": "install manifests (install/ in the repository)", + "targets": "per-target core lists (install/targets/ in the repository)", + } + flag = "--manifest" if writing == "manifest" else "--manifest-targets" + print( + f"ERROR: {output_dir} holds {what[other]}: {', '.join(held[:3])}" + f"{', ...' if len(held) > 3 else ''}. {flag} writes {what[writing]} under the " + f"same file names; pass the --output-dir that holds those instead.", + file=sys.stderr, + ) + sys.exit(1) + + def generate_target_manifests(targets_dir: str, output_dir: str) -> None: """Convert target YAMLs to installer JSON manifests.""" os.makedirs(output_dir, exist_ok=True) @@ -3100,6 +3147,7 @@ def _main(held: contextlib.ExitStack) -> None: _run_verify_packs(args) return if args.manifest_targets: + _refuse_mixed_output(args.output_dir, "targets") with _pack_output_lock(args.output_dir): generate_target_manifests( os.path.join(args.platforms_dir, "targets"), args.output_dir @@ -3276,6 +3324,7 @@ def _main(held: contextlib.ExitStack) -> None: ) if args.manifest: + _refuse_mixed_output(args.output_dir, "manifest") with _pack_output_lock(args.output_dir): _run_manifest_mode( args, groups, db, zip_contents, emu_profiles, target_cores_cache diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 387506981..c098693b1 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -5848,5 +5848,41 @@ class RequiredOnlyReachesDataDirectories(unittest.TestCase): ]) +class InstallerJsonKindsNeverOverwriteEachOther(unittest.TestCase): + """Install manifests and target core lists are both .json. + --manifest-targets aimed at install/ replaced install/retroarch.json, the + file every installer downloads, with a target table, exit 0.""" + + def _run(self, *flags: str, held: str) -> tuple: + with tempfile.TemporaryDirectory(dir=TMP_ROOT) as tmp: + target = Path(tmp, "retroarch.json") + before = (ROOT / held).read_text(encoding="utf-8") + target.write_text(before, encoding="utf-8") + proc = subprocess.run( + [sys.executable, "scripts/generate_pack.py", *flags, + "--offline", "--output-dir", tmp], + cwd=ROOT, capture_output=True, text=True, timeout=600, + ) + return proc, before, target.read_text(encoding="utf-8") + + def test_target_lists_refuse_a_manifest_directory(self): + proc, before, after = self._run( + "--manifest-targets", held="install/retroarch.json" + ) + self.assertEqual(proc.returncode, 1, proc.stdout + proc.stderr) + self.assertIn("holds install manifests", proc.stderr) + self.assertIn("--output-dir", proc.stderr) + self.assertEqual(after, before) + + def test_manifests_refuse_a_target_list_directory(self): + proc, before, after = self._run( + "--manifest", "--platform", "retroarch", + held="install/targets/retroarch.json", + ) + self.assertEqual(proc.returncode, 1, proc.stdout + proc.stderr) + self.assertIn("holds per-target core lists", proc.stderr) + self.assertEqual(after, before) + + if __name__ == "__main__": unittest.main()