fix: scrape the mame release that labels the data

scripts/scraper/_hash_merge.py

@@ -18,6 +18,7 @@ from common import yaml_load
 
 
 _MAME_RELEASE_RE = re.compile(r"^0\.\d+")
+_MAMEDEV = "https://github.com/mamedev/mame"
 
 
 def merge_mame_profile(
@@ -45,6 +46,11 @@ def merge_mame_profile(
     # never built from.
     if _MAME_RELEASE_RE.match(str(profile.get("core_version") or "")):
         profile["core_version"] = hashes.get("version", profile.get("core_version"))
+        # The refs about to be written are line numbers of that release's
+        # mamedev/mame tree: the pin moves with them, or the profile cites
+        # lines its declared revision does not hold.
+        if profile.get("upstream") == _MAMEDEV and hashes.get("commit"):
+            profile["upstream_commit"] = hashes["commit"]
 
     files = profile.get("files", [])
     bios_zip, non_bios = _split_files(files, lambda f: f.get("category") == "bios_zip")
@@ -380,6 +386,7 @@ def _backup_and_write(path: str, data: dict) -> None:
 
     original = p.read_text(encoding="utf-8")
     patched = _patch_core_version(original, data.get("core_version", ""))
+    patched = _patch_scalar(patched, "upstream_commit", data.get("upstream_commit", ""))
     patched = _patch_bios_entries(patched, data.get("files", []))
     patched = _append_new_entries(patched, data.get("files", []), original)
 
@@ -388,13 +395,16 @@ def _backup_and_write(path: str, data: dict) -> None:
 
 def _patch_core_version(text: str, version: str) -> str:
     """Replace core_version value in-place."""
-    if not version:
-        return text
-    import re
+    return _patch_scalar(text, "core_version", version)
 
+
+def _patch_scalar(text: str, key: str, value: str) -> str:
+    """Replace a top-level scalar in-place, quoted; absent value, no change."""
+    if not value:
+        return text
     return re.sub(
-        r"^(core_version:\s*).*$",
-        rf'\g<1>"{version}"',
+        rf"^({re.escape(key)}:\s*).*$",
+        rf'\g<1>"{value}"',
         text,
         count=1,
         flags=re.MULTILINE,

scripts/scraper/mame_hash_scraper.py

@@ -86,17 +86,25 @@ def _run_git(
     )
 
 
-def _sparse_clone() -> None:
+def _sparse_clone(tag: str) -> None:
+    """Clone the release the data will be labelled with, not master.
+
+    The version comes from the latest release; cloning master gave line
+    numbers and hashes of a later revision under that release's label, and
+    the profiles cited lines no declared revision holds.
+    """
     if _CLONE_DIR.exists():
         shutil.rmtree(_CLONE_DIR)
     _CLONE_DIR.parent.mkdir(parents=True, exist_ok=True)
 
-    log.info("sparse cloning mamedev/mame into %s", _CLONE_DIR)
+    log.info("sparse cloning mamedev/mame %s into %s", tag, _CLONE_DIR)
     _run_git(
         [
             "clone",
             "--depth",
             "1",
+            "--branch",
+            tag,
             "--filter=blob:none",
             "--sparse",
             _REPO_URL,
@@ -110,7 +118,12 @@ def _sparse_clone() -> None:
 
 
 def _get_version() -> str:
-    """The latest MAME release, from the GitHub API.
+    """The latest MAME release version, such as 0.289."""
+    return _parse_version_tag(_get_release_tag())
+
+
+def _get_release_tag() -> str:
+    """The latest MAME release tag, from the GitHub API.
 
     version.cpp is generated at build time, not in the repo. A failed lookup
     raises: "unknown" was cached for a day and written as core_version.
@@ -131,7 +144,7 @@ def _get_version() -> str:
         raise RuntimeError(f"cannot read the MAME release from {url}: {exc}") from exc
     if not tag:
         raise RuntimeError(f"no tag_name in {url}")
-    return _parse_version_tag(tag)
+    return tag
 
 
 def _parse_version_tag(tag: str) -> str:
@@ -251,9 +264,10 @@ def _fetch_hashes(force: bool) -> dict[str, Any]:
         return cache  # type: ignore[return-value]
 
     try:
-        _sparse_clone()
+        tag = _get_release_tag()
+        _sparse_clone(tag)
         bios_sets = parse_mame_source_tree(str(_CLONE_DIR))
-        version = _get_version()
+        version = _parse_version_tag(tag)
         commit = _get_commit()
 
         data: dict[str, Any] = {

tests/test_hash_merge.py

@@ -311,6 +311,64 @@ class TestMameMerge(unittest.TestCase):
             self.assertEqual(entry["source_ref"], "src/mame/neogeo/neogeo.cpp:2432")
 
 
+class TheMamePinMovesWithItsRefs(unittest.TestCase):
+    """The refs the merge writes are line numbers of the scraped release:
+    leaving upstream_commit on an older revision made the profile cite lines
+    its declared revision does not hold (pgm.cpp:5548 is a comment there)."""
+
+    def _merged_text(self, **profile_overrides) -> str:
+        with tempfile.TemporaryDirectory() as td:
+            p = Path(td)
+            profile_path = _write_yaml(
+                p / "mame.yml", _make_mame_profile(**profile_overrides)
+            )
+            hashes_path = _write_json(
+                p / "hashes.json", _make_mame_hashes(commit="f" * 40)
+            )
+            merge_mame_profile(profile_path, hashes_path, write=True)
+            return Path(profile_path).read_text(encoding="utf-8")
+
+    def test_the_upstream_pin_follows_the_release(self):
+        text = self._merged_text(
+            upstream="https://github.com/mamedev/mame", upstream_commit="a" * 40
+        )
+        written = yaml.safe_load(text)
+        self.assertEqual(written["upstream_commit"], "f" * 40)
+        self.assertEqual(written["core_version"], "0.286")
+
+    def test_a_fork_keeps_its_own_pin(self):
+        text = self._merged_text(
+            upstream="https://github.com/example/fork", upstream_commit="a" * 40
+        )
+        self.assertEqual(yaml.safe_load(text)["upstream_commit"], "a" * 40)
+
+
+class TheScraperClonesTheReleaseItLabels(unittest.TestCase):
+    """Cloning master and labelling it with the latest release tag gave
+    master's line numbers and hashes under the release's name."""
+
+    def test_clone_and_version_come_from_one_tag(self):
+        from unittest import mock
+
+        from scripts.scraper import mame_hash_scraper as scraper
+
+        calls: list[list[str]] = []
+        with mock.patch.object(scraper, "_get_release_tag", return_value="mame0289"), \
+                mock.patch.object(scraper, "_run_git",
+                                  side_effect=lambda args, cwd=None: calls.append(args)), \
+                mock.patch.object(scraper, "parse_mame_source_tree", return_value={}), \
+                mock.patch.object(scraper, "_get_commit", return_value="c" * 40), \
+                mock.patch.object(scraper, "_write_cache"), \
+                mock.patch.object(scraper, "_cleanup"), \
+                mock.patch.object(scraper, "_load_cache", return_value={}), \
+                mock.patch.object(scraper, "_is_stale", return_value=True):
+            data = scraper._fetch_hashes(force=True)
+        clone = next(args for args in calls if args[0] == "clone")
+        self.assertEqual(clone[clone.index("--branch") + 1], "mame0289")
+        self.assertEqual(data["version"], "0.289")
+        self.assertEqual(data["commit"], "c" * 40)
+
+
 class TestFbneoMerge(unittest.TestCase):
     """Tests for merge_fbneo_profile."""