diff --git a/scripts/create_nori_release b/scripts/create_nori_release index d60eff22f..de861a8fc 100755 --- a/scripts/create_nori_release +++ b/scripts/create_nori_release @@ -125,10 +125,10 @@ def main(argv: list[str]) -> int: if args.dry_run: print("\n[DRY RUN] Would perform the following:") - print(f" 1. Fetch main branch head") + print(" 1. Fetch main branch head") print(f" 2. Create synthetic commit with Cargo.toml version = {version}") print(f" 3. Create annotated tag {tag_name}") - print(f" 4. Push tag to origin") + print(" 4. Push tag to origin") return 0 print("Fetching branch head...") @@ -420,32 +420,32 @@ def determine_version(args: argparse.Namespace) -> str: def list_tags() -> list[str]: - """List all tags matching TAG_PREFIX, returning version strings.""" - command = [ - "gh", "api", "--paginate", - f"/repos/{REPO}/git/refs/tags", - "-H", "Accept: application/vnd.github+json", - ] + """List all tags matching TAG_PREFIX, returning version strings. + + Uses `git ls-remote` rather than the REST API: it returns every matching + tag in a single git-protocol round trip with no pagination, no ref-count + cap, and no REST rate-limiting exposure. (The git/refs/tags REST endpoint + emits no Link header and silently truncates at 1000 refs, so `--paginate` + is a no-op and it cannot be listed correctly past that point.) + """ + command = ["git", "ls-remote", "--tags", "origin", f"refs/tags/{TAG_PREFIX}*"] try: result = subprocess.run(command, text=True, capture_output=True, timeout=60) except subprocess.TimeoutExpired: - raise ReleaseError("gh api call timed out listing tags") + raise ReleaseError("git ls-remote timed out listing tags") if result.returncode != 0: - message = result.stderr.strip() or result.stdout.strip() or "gh api call failed" + message = result.stderr.strip() or result.stdout.strip() or "git ls-remote failed" raise ReleaseError(message) - try: - response = json.loads(result.stdout or "[]") - except json.JSONDecodeError: - return [] - + prefix = f"refs/tags/{TAG_PREFIX}" tags: list[str] = [] - if isinstance(response, list): - for ref in response: - ref_name = ref.get("ref", "") - if ref_name.startswith(f"refs/tags/{TAG_PREFIX}"): - version = ref_name[len(f"refs/tags/{TAG_PREFIX}"):] - tags.append(version) + for line in result.stdout.splitlines(): + # Each line is "\t". Annotated tags also emit a peeled + # "^{}" line, which we skip to avoid duplicates. + _, _, ref = line.partition("\t") + if not ref.startswith(prefix) or ref.endswith("^{}"): + continue + tags.append(ref[len(prefix):]) return tags diff --git a/scripts/test_create_nori_release.py b/scripts/test_create_nori_release.py new file mode 100644 index 000000000..37ce3a9b3 --- /dev/null +++ b/scripts/test_create_nori_release.py @@ -0,0 +1,71 @@ +#!/usr/bin/env python3 +"""Tests for create_nori_release.""" + +import importlib.util +import os +import subprocess +import unittest +from importlib.machinery import SourceFileLoader +from unittest import mock + +# The release script has no .py extension, so point the loader at it directly. +_PATH = os.path.join(os.path.dirname(__file__), "create_nori_release") +_LOADER = SourceFileLoader("create_nori_release", _PATH) +_SPEC = importlib.util.spec_from_loader("create_nori_release", _LOADER) +release = importlib.util.module_from_spec(_SPEC) +_LOADER.exec_module(release) + + +def _ls_remote_output(versions: list[str]) -> str: + """Render `git ls-remote --tags` output for the given matching versions. + + Annotated tags emit both the tag ref and a peeled "^{}" ref; list_tags + must dedupe those. A non-matching ref is included to exercise filtering. + """ + lines = [] + for v in versions: + sha = "0" * 40 + lines.append(f"{sha}\trefs/tags/{release.TAG_PREFIX}{v}") + lines.append(f"{sha}\trefs/tags/{release.TAG_PREFIX}{v}^{{}}") + lines.append(f"{'1' * 40}\trefs/tags/some-other-tag") + return "\n".join(lines) + "\n" + + +def _completed(stdout: str, returncode: int = 0) -> subprocess.CompletedProcess: + return subprocess.CompletedProcess( + args=["git"], returncode=returncode, stdout=stdout, stderr="" + ) + + +class ListTagsTest(unittest.TestCase): + def test_parses_remote_tags_and_dedupes_peeled_refs(self): + """list_tags reads tags from the remote via git (not the REST API) in a + single call, strips the prefix, drops non-matching refs, and does not + double-count the peeled "^{}" refs that annotated tags produce. + """ + with mock.patch.object( + release.subprocess, + "run", + return_value=_completed(_ls_remote_output(["1.0.0", "1.1.0", "2.0.0"])), + ) as run: + tags = release.list_tags() + + self.assertEqual(run.call_count, 1) + # The fix is to use the git protocol, not the rate-limited/capped REST API. + self.assertEqual(run.call_args.args[0][0], "git") + self.assertEqual(sorted(tags), ["1.0.0", "1.1.0", "2.0.0"]) + self.assertNotIn("some-other-tag", tags) + + def test_raises_when_remote_query_fails(self): + """A failed remote query surfaces as a ReleaseError rather than being + swallowed into a wrong (empty) version computation. + """ + with mock.patch.object( + release.subprocess, "run", return_value=_completed("", returncode=128) + ): + with self.assertRaises(release.ReleaseError): + release.list_tags() + + +if __name__ == "__main__": + unittest.main()