From 17cecbf186bc3f360dda59e64680c1c604a31781 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Sun, 4 Oct 2026 16:36:00 +0200 Subject: [PATCH] feat: read libretro gitlab and mark unchecked refs --- scripts/profile_sync.py | 73 +++++++++++++++++++----- scripts/upstream.py | 7 +++ tests/test_profile_sync.py | 110 +++++++++++++++++++++++++++++++++++-- tests/test_upstream.py | 6 ++ 4 files changed, 177 insertions(+), 19 deletions(-) diff --git a/scripts/profile_sync.py b/scripts/profile_sync.py index a0fc2df0..43960de2 100644 --- a/scripts/profile_sync.py +++ b/scripts/profile_sync.py @@ -35,8 +35,8 @@ ANON_QUOTA = 60 TRIAGE_PATH_SAMPLE = 5 STATUS_ORDER = ( - "ANCHORED", "EXTERNAL", "BINARY", "SHIFTED", "RENAMED", "MOVED", "AMBIGUOUS", - "CHANGED", "GONE", + "ANCHORED", "UNCHECKED", "EXTERNAL", "BINARY", "SHIFTED", "RENAMED", "MOVED", + "AMBIGUOUS", "CHANGED", "GONE", ) REVIEW_STATUSES = ("CHANGED", "GONE", "AMBIGUOUS") REBASE_STATUSES = ("SHIFTED", "RENAMED", "MOVED") @@ -973,6 +973,10 @@ class ProfileReport: entries: list[EntryReport] = None skipped: str | None = None counts: dict[str, int] = None + # Declared repositories whose forge the tool cannot read. They are named + # rather than dropped: a `source` on an unknown host used to fall back to + # `upstream` in silence, and a divergence between the two went unseen. + unread: list[str] = None def needs_review(self) -> int: counts = self.counts or {} @@ -1109,7 +1113,14 @@ def verify_at_pin(part: RefPart, pin_lines, tokens, hash_tokens=()) -> PartResul part, "GONE", None, None, None, [], "beyond the end of the file" ) if not tokens: - return PartResult(part, "ANCHORED", None, None, None, []) + # Nothing to look for: an entry with neither a hash nor a name (an + # analysis block, a `filename:` field) cannot be checked against its + # own revision. Calling it anchored made the check vacuously true, and + # a pin advanced without a recale hid a ref that had become false. + return PartResult( + part, "UNCHECKED", None, None, None, [], + "entry declares no value to look for", + ) # An archive ref cites the line declaring the set; its members follow, one # per line, so the window reaches forward as far as the entry has members. reach = SELF_CHECK_CONTEXT + len(tokens) @@ -1256,6 +1267,11 @@ def build_report( cited_dirs.add(directory) directory = posixpath.dirname(directory) + report.unread = [ + url + for _, _, url in declared_repositories(profile) + if upstream.parse_repo(url) is None + ] if select_repo(profile) is None: declared = str(profile.get("source") or profile.get("upstream") or "") report.skipped = f"unsupported host: {declared or 'none declared'}" @@ -1651,6 +1667,8 @@ def format_report(report: ProfileReport, changed_only: bool = False) -> str: ) elif report.pin and report.pin == report.head: lines.append(" pin is HEAD: checked for self-consistency") + for url in report.unread or []: + lines.append(f" not read (unsupported host): {url}") if report.skipped: lines.append(f" skipped: {report.skipped}") return "\n".join(lines) @@ -2629,8 +2647,17 @@ def build_parser() -> argparse.ArgumentParser: parser.add_argument("--tree-diff", action="store_true") parser.add_argument("--triage", action="store_true") parser.add_argument("--backfill-commits", action="store_true") - parser.add_argument("--rebase-refs", action="store_true") - parser.add_argument("--bump-commit", action="store_true") + parser.add_argument( + "--rebase-refs", + action="store_true", + help="rewrite shifted refs for HEAD and move source_commit with them, " + "all or nothing", + ) + parser.add_argument( + "--bump-commit", + action="store_true", + help="move source_commit to HEAD when every ref already anchors there", + ) parser.add_argument( "--realign-prose", action="store_true", @@ -2698,13 +2725,14 @@ def _apply_writes(args, name: str, profile: dict, report: ProfileReport) -> None print(f"{name}: source_commit {report.pin[:7]}") if not (args.rebase_refs or args.bump_commit): return - both = args.rebase_refs and args.bump_commit with tempfile.TemporaryDirectory() as scratch: # Always work on a copy. A dry run reports what it left there; a real - # run promotes it. Asking for both writes makes the pass atomic: the - # pin has to follow the refs or neither moves, because a profile whose - # refs describe one revision and whose pin names another is exactly - # what the all-or-nothing rule exists to prevent. + # run promotes it. A recale rewrites the refs for HEAD, so the pin + # has to follow or neither moves: a profile whose refs describe one + # revision and whose pin names another is exactly what the + # all-or-nothing rule exists to prevent. Recaling alone used to + # leave that state behind, and a later bump then read the recaled + # refs at the old pin and saw them shifted again. target = Path(scratch) / path.name target.write_bytes(path.read_bytes()) recale, bumped = ( @@ -2714,10 +2742,8 @@ def _apply_writes(args, name: str, profile: dict, report: ProfileReport) -> None applied = [] if args.rebase_refs and not report.skipped: applied = rebase_refs(target, report, args.accept_changed) - moved = bool(args.bump_commit) and bump_commit( - target, report, args.accept_changed - ) - if both and applied and not moved: + moved = bump_commit(target, report, args.accept_changed) + if args.rebase_refs and applied and not moved: print( f"{name}: refs recaled but the pin will not follow, so nothing " "was written; the prose or an annotated ref has to move first", @@ -2783,7 +2809,13 @@ def _print_detection(args, profile: dict, report: ProfileReport, repo) -> None: def _print_extras(args, profile: dict, report: ProfileReport) -> None: - """Optional per-profile sections beyond the ref report.""" + """Optional per-profile sections beyond the ref report. + + The ref report mutes a forge that refuses; the extras read the same + forge again, so a refusal here is reported on the profile's own line + instead of ending a pass over every profile. A quota signal still + stops everything. + """ wants = ( args.check_version or args.detect_new_files @@ -2796,7 +2828,15 @@ def _print_extras(args, profile: dict, report: ProfileReport) -> None: repo = select_repo(profile) if repo is None: return + try: + _print_extras_from(args, profile, report, repo) + except upstream.RateLimitError: + raise + except upstream.UpstreamError as exc: + print(f" extras skipped: {exc}") + +def _print_extras_from(args, profile: dict, report: ProfileReport, repo) -> None: if args.check_version: _print_version(args, profile, report, repo) if args.detect_new_files or args.watch_hashes: @@ -2855,6 +2895,9 @@ def _print_triage(args, selected: dict, reports: list[ProfileReport]) -> None: ranked.append((drift_score(report, version, commits), report)) for score, report in sorted(ranked, key=lambda item: -item[0]): state = report.skipped or f"{report.needs_review()} to review" + unchecked = (report.counts or {}).get("UNCHECKED", 0) + if unchecked and not report.skipped: + state += f", {unchecked} unchecked" print(f"{score:5d} {report.name:30s} {state}") diff --git a/scripts/upstream.py b/scripts/upstream.py index 9e6e900d..5142fca9 100644 --- a/scripts/upstream.py +++ b/scripts/upstream.py @@ -36,6 +36,13 @@ _HOSTS: dict[str, tuple[str, str, str]] = { "https://raw.githubusercontent.com", ), "gitlab.com": ("gitlab", "https://gitlab.com/api/v4", "https://gitlab.com"), + # libretro builds a growing share of its cores from its own GitLab, and + # several ports (wqxemu, cemu-libretro, libretro-radio) live only there. + "git.libretro.com": ( + "gitlab", + "https://git.libretro.com/api/v4", + "https://git.libretro.com", + ), "codeberg.org": ( "forgejo", "https://codeberg.org/api/v1", diff --git a/tests/test_profile_sync.py b/tests/test_profile_sync.py index 1d813fd2..2e580c34 100644 --- a/tests/test_profile_sync.py +++ b/tests/test_profile_sync.py @@ -58,6 +58,7 @@ from profile_sync import ( watch_hashes, worst_status, ) +import upstream from upstream import CompareResult, FileChange @@ -474,6 +475,17 @@ class TestVerifyAtPin(unittest.TestCase): profile_sync.verify_at_pin(part, None, ["deadbeef"]).status, "GONE" ) + def test_an_entry_with_nothing_to_look_for_is_unchecked_not_anchored(self): + """race's analysis.npbios entry carries a `filename:` and no hash: at a + pin equal to HEAD its ref was reported anchored while the cited block + had been deleted upstream. No token means no verdict.""" + part = RefPart("a.c", 3, 3, "a.c:3") + result = profile_sync.verify_at_pin(part, self.LINES, []) + self.assertEqual(result.status, "UNCHECKED") + self.assertNotIn("UNCHECKED", profile_sync.REVIEW_STATUSES) + self.assertEqual(profile_sync.worst_status(["ANCHORED", "UNCHECKED"]), "UNCHECKED") + self.assertEqual(profile_sync.worst_status(["UNCHECKED", "SHIFTED"]), "SHIFTED") + def test_line_beyond_the_file(self): part = RefPart("a.c", 99, 99, "a.c:99") self.assertEqual( @@ -487,11 +499,13 @@ class TestVerifyAtPin(unittest.TestCase): "ANCHORED", ) - def test_an_entry_declaring_nothing_is_accepted(self): + def test_an_entry_declaring_nothing_is_accepted_but_not_called_anchored(self): + # Nothing to re-read, so no review is demanded; but the verdict says + # that nothing was checked rather than claiming the ref anchors. part = RefPart("a.c", 1, 1, "a.c:1") - self.assertEqual( - profile_sync.verify_at_pin(part, self.LINES, []).status, "ANCHORED" - ) + result = profile_sync.verify_at_pin(part, self.LINES, []) + self.assertEqual(result.status, "UNCHECKED") + self.assertNotIn(result.status, profile_sync.REVIEW_STATUSES) class TestExternalCitation(unittest.TestCase): @@ -1176,6 +1190,21 @@ class TestBuildReport(unittest.TestCase): "test", {"emulator": "T", "source": "https://www.6809.org.uk/"}, self.dir ) self.assertIn("unsupported host", report.skipped) + self.assertEqual(report.unread, ["https://www.6809.org.uk/"]) + + def test_unsupported_source_is_named_when_upstream_answers(self): + profile = self._profile(["a.c:1"]) + profile["source"] = "https://forge.example.invalid/o/n" + profile["upstream"] = "https://github.com/o/n" + self.files[("pinsha", "a.c")] = ["x"] + self.files[("headsha", "a.c")] = ["x"] + report = build_report("test", profile, self.dir) + self.assertIsNone(report.skipped) + self.assertEqual(report.unread, ["https://forge.example.invalid/o/n"]) + self.assertIn( + "not read (unsupported host): https://forge.example.invalid/o/n", + format_report(report), + ) def test_profile_without_refs_is_reported_not_empty(self): profile = { @@ -1705,6 +1734,49 @@ class TestElidedSummary(unittest.TestCase): self.assertEqual(buffer.getvalue(), "") +class TestExtrasSurviveARefusingForge(unittest.TestCase): + """A pass over every profile used to die on the first forge answering + 403 to the detection reads, after the ref report had already muted it.""" + + def _args(self): + return argparse.Namespace( + check_version=False, detect_new_files=True, watch_hashes=False, + full_diff=False, tree_diff=False, cache_dir="x", offline=False, + ref=None, context=3, + ) + + def _profile(self): + return {"emulator": "T", "source": "https://github.com/o/n", "files": []} + + def test_a_refusal_is_printed_on_the_profile_not_raised(self): + original = upstream.fetch_file + + def refuse(*_args, **_kwargs): + raise upstream.UpstreamError("https://x/y: HTTP 403") + + upstream.fetch_file = refuse + try: + buffer = io.StringIO() + with contextlib.redirect_stdout(buffer): + profile_sync._print_extras(self._args(), self._profile(), _sample_report()) + finally: + upstream.fetch_file = original + self.assertIn("extras skipped: https://x/y: HTTP 403", buffer.getvalue()) + + def test_a_quota_signal_still_stops_the_pass(self): + original = upstream.fetch_file + + def quota(*_args, **_kwargs): + raise upstream.RateLimitError("quota") + + upstream.fetch_file = quota + try: + with self.assertRaises(upstream.RateLimitError): + profile_sync._print_extras(self._args(), self._profile(), _sample_report()) + finally: + upstream.fetch_file = original + + class TestReportToDict(unittest.TestCase): def test_round_trips_through_json(self): payload = report_to_dict(_sample_report()) @@ -2152,6 +2224,36 @@ class TestWriteDryRun(unittest.TestCase): self.assertNotIn("would", output) self.assertIn('source_ref: "a.c:30-32"', self.path.read_text()) + def test_a_recale_carries_the_pin(self): + """Refs rewritten for HEAD describe HEAD, so the pin follows in the + same write. Left behind, the profile named one revision and cited + another, and the next pass read the new refs at the old pin.""" + output = self._run( + self._args(rebase_refs=True, dry_run=False), self._shifted() + ) + self.assertIn("source_commit -> newhead", output) + self.assertIn('source_commit: "newhead"', self.path.read_text()) + + def test_a_recale_the_pin_cannot_follow_writes_nothing(self): + """One ref recales, another is CHANGED and unread: neither the refs + nor the pin move, and the file is left as it was.""" + before = self.path.read_text() + shifted = PartResult( + RefPart("a.c", 10, 12, "a.c:10-12"), "SHIFTED", None, 30, 32, [] + ) + changed = PartResult(RefPart("b.c", 5, 5, "b.c:5"), "CHANGED", None, 7, 7, []) + report = ProfileReport( + name="p", repo="o/n", pin="pin", head="newhead", + entries=[ + EntryReport("a.bin", "a.c:10-12", "SHIFTED", [shifted]), + EntryReport("b.bin", "b.c:5", "CHANGED", [changed]), + ], + counts={"SHIFTED": 1, "CHANGED": 1}, + ) + output = self._run(self._args(rebase_refs=True, dry_run=False), report) + self.assertNotIn("source_commit", output) + self.assertEqual(self.path.read_text(), before) + def test_a_planned_bump_reads_the_prose_the_rebase_would_have_left(self): """The pin is blocked by prose until the rebase moves it. diff --git a/tests/test_upstream.py b/tests/test_upstream.py index 828fed10..05d2dc35 100644 --- a/tests/test_upstream.py +++ b/tests/test_upstream.py @@ -53,6 +53,12 @@ class TestParseRepo(unittest.TestCase): repo = parse_repo("https://gitlab.com/recalbox/recalbox") self.assertEqual(repo.family, "gitlab") + def test_libretro_gitlab_is_a_gitlab(self): + repo = parse_repo("https://git.libretro.com/libretro/wqxemu.git") + self.assertEqual(repo.family, "gitlab") + self.assertEqual(repo.api_base, "https://git.libretro.com/api/v4") + self.assertEqual(repo.slug, "libretro/wqxemu") + def test_codeberg_is_forgejo(self): self.assertEqual(parse_repo("https://codeberg.org/a/b").family, "forgejo")