mirror of
https://github.com/Abdess/retroarch_system.git
synced 2026-10-29 15:17:20 -05:00
fix: refuse a platform scrape that lists no file
scraper_cli wrote a platform YAML with no system when the parse came back empty, and exited 0. It now exits 1 before writing, unless the scraper declares lists_files = False (RetroPie, whose files are inherited); RetroPie in turn raises when its archive declares no package.
This commit is contained in:
1 parent
981468874c
commit
6318eaa28b
3 files changed
+82
No files matched your search
@@ -164,6 +164,11 @@ def fetch_bytes(
|
|||||||
class BaseScraper(ABC):
|
class BaseScraper(ABC):
|
||||||
"""Abstract base class for platform BIOS requirement scrapers."""
|
"""Abstract base class for platform BIOS requirement scrapers."""
|
||||||
|
|
||||||
|
# False for a platform whose file list is inherited and whose scraper
|
||||||
|
# reads only its core list (RetroPie). Otherwise an empty read is an
|
||||||
|
# upstream that changed shape, never a platform with no file.
|
||||||
|
lists_files: bool = True
|
||||||
|
|
||||||
def __init__(self, url: str = "") -> None:
|
def __init__(self, url: str = "") -> None:
|
||||||
self.url = url
|
self.url = url
|
||||||
self._raw_data: str | None = None
|
self._raw_data: str | None = None
|
||||||
@@ -366,6 +371,12 @@ def scraper_cli(
|
|||||||
reqs = scraper.fetch_requirements()
|
reqs = scraper.fetch_requirements()
|
||||||
except (OSError, ValueError, RuntimeError) as e:
|
except (OSError, ValueError, RuntimeError) as e:
|
||||||
_exit_on_scrape_error(e)
|
_exit_on_scrape_error(e)
|
||||||
|
if scraper.lists_files and not reqs:
|
||||||
|
# Written, an empty read became a platform with no system that
|
||||||
|
# verify and the pack count 0/0 and call consistent.
|
||||||
|
_exit_on_scrape_error(RuntimeError(
|
||||||
|
f"{scraper_class.__module__}: upstream returned no BIOS entry"
|
||||||
|
))
|
||||||
|
|
||||||
if args.dry_run:
|
if args.dry_run:
|
||||||
by_system: dict[str, list] = {}
|
by_system: dict[str, list] = {}
|
||||||
@@ -425,6 +436,12 @@ def scraper_cli(
|
|||||||
if req.zipped_file:
|
if req.zipped_file:
|
||||||
entry["zipped_file"] = req.zipped_file
|
entry["zipped_file"] = req.zipped_file
|
||||||
config["systems"][sys_id]["files"].append(entry)
|
config["systems"][sys_id]["files"].append(entry)
|
||||||
|
if scraper.lists_files and not any(
|
||||||
|
(system or {}).get("files") for system in (config.get("systems") or {}).values()
|
||||||
|
):
|
||||||
|
_exit_on_scrape_error(RuntimeError(
|
||||||
|
f"{scraper_class.__module__}: the generated platform lists no file"
|
||||||
|
))
|
||||||
# Merge into existing YAML: preserve fields the scraper doesn't generate
|
# Merge into existing YAML: preserve fields the scraper doesn't generate
|
||||||
# (data_directories, case_insensitive_fs, manually added metadata).
|
# (data_directories, case_insensitive_fs, manually added metadata).
|
||||||
# The scraper replaces systems + files, everything else is preserved.
|
# The scraper replaces systems + files, everything else is preserved.
|
||||||
|
|||||||
@@ -59,6 +59,8 @@ _PACKAGE_DIRS = _SCRIPT_SECTIONS
|
|||||||
class Scraper(BaseScraper):
|
class Scraper(BaseScraper):
|
||||||
"""Scraper for the RetroPie package list."""
|
"""Scraper for the RetroPie package list."""
|
||||||
|
|
||||||
|
lists_files = False
|
||||||
|
|
||||||
def __init__(self, url: str | None = None) -> None:
|
def __init__(self, url: str | None = None) -> None:
|
||||||
# The package list is read from master: the commit read is recorded.
|
# The package list is read from master: the commit read is recorded.
|
||||||
if url is None:
|
if url is None:
|
||||||
@@ -122,6 +124,8 @@ class Scraper(BaseScraper):
|
|||||||
else:
|
else:
|
||||||
found.add(match.group(1))
|
found.add(match.group(1))
|
||||||
|
|
||||||
|
if not found:
|
||||||
|
raise ValueError(f"{self.url}: no rp_module_id under {SCRIPTMODULES}/")
|
||||||
self._modules = sorted(found)
|
self._modules = sorted(found)
|
||||||
return self._modules
|
return self._modules
|
||||||
|
|
||||||
|
|||||||
@@ -941,5 +941,66 @@ class HashScraperRefusesAnEmptyRead(unittest.TestCase):
|
|||||||
self.assertTrue(f._is_cache_fresh())
|
self.assertTrue(f._is_cache_fresh())
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
class EmptyScrapeWritesNothing(unittest.TestCase):
|
||||||
|
"""An upstream that changed shape parses to nothing: the run fails and
|
||||||
|
the committed platform file stays as it was."""
|
||||||
|
|
||||||
|
def _run(self, scraper_class, output: Path) -> int:
|
||||||
|
from scraper.base_scraper import scraper_cli # noqa: PLC0415
|
||||||
|
|
||||||
|
argv = ["scraper", "-o", str(output)]
|
||||||
|
with mock.patch.object(sys, "argv", argv), \
|
||||||
|
mock.patch("sys.stderr", new_callable=io.StringIO):
|
||||||
|
try:
|
||||||
|
scraper_cli(scraper_class)
|
||||||
|
except SystemExit as exc:
|
||||||
|
return int(exc.code or 0)
|
||||||
|
return 0
|
||||||
|
|
||||||
|
def test_no_requirement_exits_non_zero(self):
|
||||||
|
with tempfile.TemporaryDirectory() as tmp:
|
||||||
|
output = Path(tmp) / "platform.yml"
|
||||||
|
output.write_text("systems:\n a:\n files:\n - name: x\n")
|
||||||
|
self.assertEqual(self._run(_StubScraper, output), 1)
|
||||||
|
self.assertIn("name: x", output.read_text())
|
||||||
|
|
||||||
|
def test_a_generated_file_without_files_exits_non_zero(self):
|
||||||
|
class NoSystems(_StubScraper):
|
||||||
|
def __init__(self):
|
||||||
|
super().__init__([BiosRequirement(name="a.bin", system="s")])
|
||||||
|
|
||||||
|
def generate_platform_yaml(self) -> dict:
|
||||||
|
return {"platform": "x", "systems": {}}
|
||||||
|
|
||||||
|
with tempfile.TemporaryDirectory() as tmp:
|
||||||
|
output = Path(tmp) / "platform.yml"
|
||||||
|
self.assertEqual(self._run(NoSystems, output), 1)
|
||||||
|
self.assertFalse(output.exists())
|
||||||
|
|
||||||
|
def test_a_platform_that_inherits_its_files_may_list_none(self):
|
||||||
|
class CoresOnly(_StubScraper):
|
||||||
|
lists_files = False
|
||||||
|
|
||||||
|
def generate_platform_yaml(self) -> dict:
|
||||||
|
return {"inherits": "retroarch", "cores": ["mgba"]}
|
||||||
|
|
||||||
|
with tempfile.TemporaryDirectory() as tmp:
|
||||||
|
output = Path(tmp) / "platform.yml"
|
||||||
|
with mock.patch("sys.stdout", new_callable=io.StringIO):
|
||||||
|
self.assertEqual(self._run(CoresOnly, output), 0)
|
||||||
|
self.assertIn("mgba", output.read_text())
|
||||||
|
|
||||||
|
def test_retropie_without_a_package_raises(self):
|
||||||
|
archive = RetroPiePackageList._archive(
|
||||||
|
{"scriptmodules/supplementary/autostart.sh": 'rp_module_id="autostart"\n'}
|
||||||
|
)
|
||||||
|
from scraper.retropie_scraper import Scraper # noqa: PLC0415
|
||||||
|
|
||||||
|
scraper = Scraper(url="https://codeload.github.com/RetroPie/RetroPie-Setup/tar.gz/" + "0" * 40)
|
||||||
|
with self.assertRaises(ValueError):
|
||||||
|
scraper.module_ids(archive)
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.main()
|
unittest.main()
|
||||||
Reference in new issue
Block a user