fix: read all RetroPie helps, propose only its own

Eight scriptmodules write "bios" in lower case (xm7, np2pi, quasi88,
minivmac, px68k...) and none was examined. A module reported "left as
is" for a name no profile documents was written all the same: the skip
had no continue. The cap of six names came from that case-sensitive
reading, where xm7 writes thirteen: a sentence now grows no longer than
the longest RetroPie writes. lr- was stripped whatever the build, so the
standalone minivmac package matched the libretro profile, which reads
another file. A file of a system RetroPie lacks is proposed to no
package: lr-geargrafx adds pcengine only (lr-geargrafx.sh:41-42) and
was given the LaserActive's pac-n1.bin and pac-n10.bin.

scripts/exporter/retropie_exporter.py

@@ -37,7 +37,6 @@ EMULATORS_DIR = Path(__file__).resolve().parents[2] / "emulators"
 # The longest list RetroPie writes is six names (lr-atari800). Past that the
 # sentence stops being something a reader uses, so the names are reported
 # instead of appended.
-MAX_NAMES = 6
 
 _MODULE_ID = re.compile(r'rp_module_id="([^"]+)"')
 _MODULE_HELP = re.compile(r'rp_module_help="((?:[^"\\]|\\.)*)"')
@@ -48,8 +47,8 @@ _ABBREVIATION = re.compile(r"[a-z]\.[a-z]", re.I)
 # a.bin and b.bin to $biosdir", "The Sega CD requires the BIOS files a.bin,
 # b.bin copied to $biosdir"), so the clause is found by the word BIOS rather
 # than by a sentence template, and the names in it are the list to extend.
-_BIOS_CLAUSE = re.compile(r"BIOS\b.*?(?=\\n|$)", re.DOTALL)
-_BIOS_NOUN = re.compile(r"BIOS (files?)\b")
+_BIOS_CLAUSE = re.compile(r"BIOS\b.*?(?=\\n|$)", re.DOTALL | re.I)
+_BIOS_NOUN = re.compile(r"(BIOS) (files?)\b", re.I)
 _SEPARATOR = re.compile(r"\s*(?:,\s*(?:and\s+)?|and\s+)")
 _EXAMPLES = re.compile(r"\b(?:e\.?g\b|for example|for instance|such as|like)\W*$", re.I)
 
@@ -99,17 +98,28 @@ class Exporter(BaseExporter):
                 if handle is None:
                     continue
                 text = handle.read().decode("utf-8", errors="replace")
-                if "BIOS" in text:
+                # xm7, np2pi, quasi88 and five more write "bios".
+                if "bios" in text.lower():
                     found[relative] = text
         return found
 
-    def _core_index(self) -> dict[str, str]:
-        """Module id (without its lr- prefix) to the profile it stands for."""
-        index: dict[str, str] = {}
+    def _core_index(self) -> dict[str, dict[str, str]]:
+        """Module id (without its lr- prefix) to the profile it stands for,
+        per build.
+
+        An lr- package is a libretro core and a bare one a standalone
+        emulator: the standalone minivmac package matched the libretro
+        profile, which reads MacII.ROM where Mini vMac 37 reads vMac.ROM.
+        """
+        index: dict[str, dict[str, str]] = {"libretro": {}, "standalone": {}}
         for key, profile in load_emulator_profiles(str(EMULATORS_DIR)).items():
-            index[key.replace("-", "_").lower()] = key
-            for name in profile.get("cores", []) or []:
-                index[str(name).replace("-", "_").lower()] = key
+            kind = str(profile.get("type", ""))
+            for build in ("libretro", "standalone"):
+                if build not in kind:
+                    continue
+                index[build][key.replace("-", "_").lower()] = key
+                for name in profile.get("cores", []) or []:
+                    index[build][str(name).replace("-", "_").lower()] = key
         return index
 
     @staticmethod
@@ -269,7 +279,7 @@ class Exporter(BaseExporter):
                 continue
             module = _MODULE_ID.search(text)
             help_text = _MODULE_HELP.search(text)
-            if not module or not help_text or "BIOS" not in help_text.group(1):
+            if not module or not help_text or "bios" not in help_text.group(1).lower():
                 continue
             found[relative] = (module.group(1), help_text.group(1))
         return found
@@ -289,6 +299,9 @@ class Exporter(BaseExporter):
             )
 
         index = self._core_index()
+        # A sentence grows no longer than the longest RetroPie writes itself:
+        # past that it stops reading as one, and the names are reported.
+        self._longest = max(len(self._listed(help_text)) for _, help_text in modules.values())
         by_core = self._files_by_core(systems)
         # Aliases count as names we know: RetroPie writes dc_flash.bin where
         # flycast's profile files it as an alias of another primary.
@@ -306,7 +319,8 @@ class Exporter(BaseExporter):
             self._skipped.setdefault(reason, []).append(module)
 
         for relative, (module_id, help_text) in sorted(modules.items()):
-            core = index.get(module_id.removeprefix("lr-").replace("-", "_").lower())
+            build = "libretro" if module_id.startswith("lr-") else "standalone"
+            core = index[build].get(module_id.removeprefix("lr-").replace("-", "_").lower())
             files = by_core.get(core or "", [])
             if not files:
                 skip("which no profiled core matches", module_id)
@@ -319,13 +333,17 @@ class Exporter(BaseExporter):
                     f"whose help text names a file no profile documents yet ({', '.join(unknown)})",
                     module_id,
                 )
+                continue
 
             # A file already listed under one of its other names is not
             # missing: proposing the primary would name the same bytes twice.
+            # A system RetroPie does not have is no package's: lr-geargrafx
+            # adds pcengine only, not the LaserActive its core also reads.
             candidates = [
                 fe
                 for fe in files
-                if self._required_for(fe, core)
+                if not fe.foreign_system
+                and self._required_for(fe, core)
                 and not (
                     {fe.name.lower()}
                     | {str(a).lower() for a in (fe.native("aliases", []) or [])}
@@ -350,13 +368,13 @@ class Exporter(BaseExporter):
                 skip("whose help text lists no files to add to", module_id)
                 continue
 
-            if len(listed) + len(missing) > MAX_NAMES:
+            if len(listed) + len(missing) > self._longest:
                 skip("whose help text would grow longer than any it has today", module_id)
                 continue
 
             new_help = self._extended(help_text, enumeration, missing)
             if len(listed) + len(missing) > 1:
-                new_help = _BIOS_NOUN.sub("BIOS files", new_help, count=1)
+                new_help = _BIOS_NOUN.sub(r"\1 files", new_help, count=1)
             produced[relative] = originals[relative].replace(
                 f'rp_module_help="{help_text}"',
                 f'rp_module_help="{new_help}"',
@@ -394,10 +412,10 @@ class Exporter(BaseExporter):
             if not help_text:
                 issues.append(f"{relative}: the help string is not closed")
                 continue
-            if "BIOS" not in help_text.group(1):
+            if "bios" not in help_text.group(1).lower():
                 issues.append(f"{relative}: the BIOS sentence is gone")
             names = self._listed(help_text.group(1))
-            if len(names) > MAX_NAMES:
+            if len(names) > getattr(self, "_longest", len(names)):
                 issues.append(
                     f"{relative}: {len(names)} names, past what the help enumerates"
                 )

tests/test_export_counts.py

@@ -18,7 +18,7 @@ from pathlib import Path
 REPO_ROOT = Path(__file__).resolve().parents[1]
 sys.path.insert(0, str(REPO_ROOT / "scripts"))
 
-from exporter.baseline import NativeFile, NativeSystem, build_native_model  # noqa: E402
+from exporter.baseline import NativeFile, NativeSystem, Report, build_native_model  # noqa: E402
 from exporter.bizhawk_exporter import Exporter as BizHawk  # noqa: E402
 from exporter.misterfpga_exporter import Exporter as Mister  # noqa: E402
 from exporter.romm_exporter import Exporter as Romm  # noqa: E402
@@ -610,6 +610,96 @@ class RetroPieProposals(unittest.TestCase):
         self.assertFalse(RetroPie._required_for(installed, "gngeopi"))
         self.assertTrue(RetroPie._required_for(brought, "gngeopi"))
 
+    @staticmethod
+    def _module(module_id: str, help_text: str) -> dict[str, str]:
+        script = f'rp_module_id="{module_id}"\nrp_module_help="{help_text}"\n'
+        return {f"scriptmodules/emulators/{module_id}.sh": script}
+
+    @staticmethod
+    def _wanting(core: str, *names: str) -> dict:
+        files = [NativeFile(n, n, "sys", truth={
+            "required": True, "_required_by": [core], "_cores": [core],
+            "_brought_by": [f"{core}:standalone"]}) for n in names]
+        return {"sys": NativeSystem("sys", files=files)}
+
+    def test_a_lowercase_bios_sentence_is_read(self):
+        """Eight scriptmodules write "bios" in lower case (xm7, np2pi,
+        quasi88, minivmac, px68k...) and none of them was examined."""
+        module = self._module("xm7", "Copy bios files a.rom and b.rom to $biosdir/xm7")
+        self.assertEqual(len(RetroPie().modules(module)), 1)
+
+    def test_a_module_left_as_is_is_not_written(self):
+        """minivmac was reported "left as is" for a name no profile documents
+        and written all the same: the skip had no continue after it."""
+        from unittest import mock
+
+        exporter = RetroPie()
+        module = self._module("vmac", "Copy the required BIOS file vMac.ROM to $biosdir")
+        with mock.patch.object(RetroPie, "_core_index",
+                               return_value={"standalone": {"vmac": "vmac"}, "libretro": {}}):
+            produced = exporter.render(self._wanting("vmac", "MacII.ROM"), Report(), module)
+        self.assertEqual(produced, {})
+
+    def test_a_standalone_package_does_not_borrow_a_libretro_profile(self):
+        """lr- was stripped whatever the build: RetroPie's standalone
+        minivmac package matched the libretro profile, which reads
+        MacII.ROM where Mini vMac 37 reads vMac.ROM."""
+        from unittest import mock
+
+        module = self._module("minivmac", "Copy the required BIOS file vMac.ROM to $biosdir")
+        index = {"libretro": {"minivmac": "minivmac"}, "standalone": {}}
+        with mock.patch.object(RetroPie, "_core_index", return_value=index):
+            exporter = RetroPie()
+            produced = exporter.render(self._wanting("minivmac", "MacII.ROM"), Report(), module)
+        self.assertEqual(produced, {})
+        self.assertIn("minivmac", sum(exporter._skipped.values(), []))
+
+    def test_a_list_is_capped_by_retropies_longest_not_a_constant(self):
+        """The cap of six came from the longest list read, case-sensitively:
+        xm7 writes thirteen names."""
+        from unittest import mock
+
+        names = [f"n{i}.rom" for i in range(8)]
+        modules = {
+            **self._module("long", "Copy bios files " + ", ".join(names[:7]) + " and "
+                           + names[7] + " to $biosdir"),
+            **self._module("short", "Copy the required BIOS files a.rom and b.rom to $biosdir"),
+        }
+        index = {"standalone": {"short": "short"}, "libretro": {}}
+        with mock.patch.object(RetroPie, "_core_index", return_value=index):
+            exporter = RetroPie()
+            produced = exporter.render(
+                self._wanting("short", "a.rom", "b.rom", "c.rom", "d.rom", "e.rom",
+                              "f.rom", "g.rom"), Report(), modules)
+        self.assertEqual(len(produced), 1)
+        self.assertEqual(exporter.validate({}, produced), [])
+
+    def test_a_file_of_a_system_retropie_lacks_is_not_proposed(self):
+        """lr-geargrafx adds pcengine only (addSystem, lr-geargrafx.sh:42),
+        and its sentence was extended with the LaserActive's pac-n1.bin and
+        pac-n10.bin, a system RetroPie does not have."""
+        from unittest import mock
+
+        files = [NativeFile(n, n, sys_id, truth={
+            "required": True, "_required_by": ["geargrafx"], "_cores": ["geargrafx"],
+            "_brought_by": ["geargrafx:libretro"]}, foreign_system=foreign)
+            for n, sys_id, foreign in (("gexpress.pce", "pce", False),
+                                       ("pac-n1.bin", "laseractive", True))]
+        listed = NativeFile("syscard3.pce", "syscard3.pce", "pce", platform={"name": "syscard3.pce"})
+        systems = {"pce": NativeSystem("pce", files=[listed, files[0]]),
+                   "laseractive": NativeSystem("laseractive", files=[files[1]])}
+        module = {
+            **self._module("lr-geargrafx", "Copy the required BIOS file syscard3.pce to $biosdir"),
+            **self._module("xm7", "Copy bios files a.rom, b.rom and c.rom to $biosdir"),
+        }
+        with mock.patch.object(RetroPie, "_core_index",
+                               return_value={"libretro": {"geargrafx": "geargrafx"},
+                                             "standalone": {}}):
+            produced = RetroPie().render(systems, Report(), module)
+        (text,) = produced.values()
+        self.assertIn("gexpress.pce", text)
+        self.assertNotIn("pac-n1.bin", text)
+
     def test_a_list_of_alternatives_is_not_extended(self):
         alternatives = "Copy the required BIOS file a.rom or b.rom to $biosdir"
         enumeration = "Copy the required BIOS files a.rom and b.rom to $biosdir"

tests/test_exporters.py

@@ -656,9 +656,13 @@ class SearchOrder(unittest.TestCase):
         script = ('rp_module_id="lr-picodrive"\nrp_module_help="The Sega CD requires the '
                   'BIOS files us_scd1_9210.bin, eu_mcd1_9210.bin copied to $biosdir"\n')
         exporter = discover_exporters()["retropie"]()
+        # The cap is the longest list RetroPie writes: xm7 names thirteen.
+        longest = ('rp_module_id="xm7"\nrp_module_help="Copy bios files '
+                   + ", ".join(f"n{i}.rom" for i in range(12)) + ' and n12.rom to $biosdir"\n')
         produced = exporter.render(
             {"segacd": NativeSystem("segacd", files=files)}, None,
-            {"scriptmodules/libretrocores/lr-picodrive.sh": script},
+            {"scriptmodules/libretrocores/lr-picodrive.sh": script,
+             "scriptmodules/emulators/xm7.sh": longest},
         )["scriptmodules/libretrocores/lr-picodrive.sh"]
         self.assertIn("us_scd1_9210.bin, eu_mcd1_9210.bin and jp_mcd2_921222.bin copied",
                       produced)