Release on merging a release PR from staging into master

.github/release/checks.py

@@ -3,10 +3,14 @@
 import subprocess
 
 
+def is_ancestor(commit, ref, repo_root, runner=subprocess.run):
+    """Whether the commit is the ref or one of its ancestors."""
+    return runner(["git", "-C", str(repo_root), "merge-base", "--is-ancestor", commit, ref], capture_output=True).returncode == 0
+
+
 def branch_problems(commit, branch, repo_root, runner=subprocess.run):
     """Problems unless the commit is an ancestor of origin/<branch>."""
-    result = runner(["git", "-C", str(repo_root), "merge-base", "--is-ancestor", commit, f"origin/{branch}"], capture_output=True)
-    return [] if result.returncode == 0 else [f"{commit} is not on origin/{branch}; the tag must point to that branch"]
+    return [] if is_ancestor(commit, f"origin/{branch}", repo_root, runner) else [f"{commit} is not on origin/{branch}; the tag must point to that branch"]
 
 
 def missing_checks(check_runs, required):

.github/release/pending.py

@@ -0,0 +1,55 @@
+"""What a release PR from staging into master releases: one GitHub release per tag rule of staging whose version has
+no tag yet, on the PR's head commit, once the merge is checked to hold exactly that commit."""
+
+import subprocess
+from dataclasses import dataclass
+
+import checks
+import sources
+from plan import checkout_tags, plan_for_tag
+
+
+@dataclass(frozen=True)
+class PendingRelease:
+    """One GitHub release the merge creates."""
+
+    tag: str
+    prerelease: bool
+    latest: bool
+    notes: str
+
+    def to_json(self):
+        """Serializable form, written to pending.json for the workflow; the notes go to a file of their own."""
+        return {"tag": self.tag, "prerelease": self.prerelease, "latest": self.latest}
+
+
+def merge_problems(head, merge, repo_root, runner=subprocess.run):
+    """Problems unless the merge commit contains the PR's head commit and holds exactly its tree, so the commit the
+    releases are tagged on is what master holds."""
+    if not checks.is_ancestor(head, merge, repo_root, runner):
+        return [f"{merge} does not contain {head}; merge the release PR with a merge commit, not a squash or a rebase"]
+    trees = runner(["git", "-C", str(repo_root), "rev-parse", f"{head}^{{tree}}", f"{merge}^{{tree}}"], capture_output=True, text=True, check=True).stdout.split()
+    if trees[0] != trees[1]:
+        return ["master holds changes staging lacks; merge master into staging, then update the release PR"]
+    return []
+
+
+def existing_tags(repo_root, runner=subprocess.run):
+    """The tags of the checkout."""
+    return set(runner(["git", "-C", str(repo_root), "tag", "--list"], capture_output=True, text=True, check=True).stdout.split())
+
+
+def pending_releases(config, repo_root, branch, tags):
+    """The releases the checkout makes from the branch, in tag rule order, and the problems that must stop all of them.
+    A tag that exists is skipped; every other one needs its version sources and changelog ready. Only a final release of
+    the plain version tag is marked latest. Raises PlanError when a version is unreadable."""
+    releases = []
+    problems = []
+    for tag in checkout_tags(config, repo_root, branch):
+        if tag in tags:
+            continue
+        plan = plan_for_tag(config, tag)
+        notes, notes_problems = sources.release_notes(repo_root, plan, config)
+        problems += sources.check_version_sources(repo_root, plan, config) + notes_problems
+        releases.append(PendingRelease(tag, plan.prerelease, tag == plan.version and not plan.prerelease, notes))
+    return releases, list(dict.fromkeys(problems))

.github/release/release.py

@@ -16,6 +16,7 @@
 import index
 import layout
 import manifest as manifest_module
+import pending
 import sbom
 import sign
 import sources
@@ -116,6 +117,28 @@ def cmd_checkout_tags(args):
     summary([f"### Tags {args.branch} would release"] + [f"- {tag}" for tag in tags or ["none"]])
 
 
+def cmd_pending(args):
+    """Lists the GitHub releases a release PR from staging into master creates and writes their notes, after checking
+    the merge; fails on anything that would stop one of them, so nothing is created."""
+    config = load_config()
+    problems = pending.merge_problems(args.head, args.merge, REPO_ROOT)
+    releases = []
+    if not problems:
+        try:
+            releases, problems = pending.pending_releases(config, REPO_ROOT, args.branch, pending.existing_tags(REPO_ROOT))
+        except (PlanError, OSError) as error:
+            problems = [str(error)]
+    if not problems:
+        notes = Path(args.notes)
+        notes.mkdir(parents=True, exist_ok=True)
+        for release in releases:
+            (notes / f"{release.tag}.md").write_bytes(release.notes.encode("utf-8"))
+        Path(args.out).write_bytes(json.dumps({"releases": [release.to_json() for release in releases]}, indent=2).encode("utf-8"))
+        lines = [f"- {release.tag}{' (pre-release)' if release.prerelease else ''}" for release in releases]
+        summary(["### Releases of this merge"] + (lines or ["- none, every version is tagged already"]))
+    finish(problems, "Release plan")
+
+
 def cmd_check_branch(args):
     """The tag commit must be on the plan's release branch."""
     _, plan = read_plan(args.plan)
@@ -327,6 +350,14 @@ def main(argv=None):
     command.add_argument("--branch", required=True)
     command.set_defaults(func=cmd_checkout_tags)
 
+    command = commands.add_parser("pending")
+    command.add_argument("--head", required=True)
+    command.add_argument("--merge", required=True)
+    command.add_argument("--branch", required=True)
+    command.add_argument("--out", required=True)
+    command.add_argument("--notes", required=True)
+    command.set_defaults(func=cmd_pending)
+
     command = commands.add_parser("check-branch")
     command.add_argument("--plan", required=True)
     command.add_argument("--commit", required=True)

.github/release/sources.py

@@ -1,11 +1,15 @@
-"""Checks that every version source of the tag's artifacts carries the tag's version."""
+"""Checks that every version source of the tag's artifacts carries the tag's version, and reads the release notes from
+the changelogs."""
 
 import re
 from pathlib import Path
 
+PLACEHOLDER = "XX.XX.XX"
+
 
 def check_version_sources(repo_root, plan, config):
-    """Returns problems; empty when every version source and, for final releases, every changelog heading matches."""
+    """Returns problems; empty when every version source and, for final releases, every changelog heading matches, with
+    no '## XX.XX.XX' heading left."""
     problems = []
     root = Path(repo_root)
     for artifact in plan.artifacts:
@@ -22,7 +26,53 @@ def check_version_sources(repo_root, plan, config):
                 problems.append(f"{source['file']}: version is {match.group(1).strip()}, the tag says {plan.version}")
         changelog = spec.get("changelog")
         if changelog and not plan.prerelease:
-            text = (root / changelog).read_text(encoding="utf-8")
-            if re.search(rf"^## {re.escape(plan.version)}\s*$", text, re.MULTILINE) is None:
+            path = root / changelog
+            if not path.is_file():
+                problems.append(f"{changelog} does not exist")
+                continue
+            text = path.read_text(encoding="utf-8")
+            if changelog_section(text, plan.version) is None:
                 problems.append(f"{changelog}: no '## {plan.version}' heading")
+            if changelog_section(text, PLACEHOLDER) is not None:
+                problems.append(f"{changelog}: still has a '## {PLACEHOLDER}' heading; move its lines under '## {plan.version}'")
     return list(dict.fromkeys(problems))
+
+
+def release_notes(repo_root, plan, config):
+    """The notes of the plan's GitHub release, the changelog section of its version for each of its artifacts; a
+    pre-release takes the '## XX.XX.XX' section when there is one. Returns the notes and the problems that leave a
+    changelog without them."""
+    notes = ""
+    problems = []
+    for artifact in plan.artifacts:
+        changelog = config["artifacts"][artifact.key].get("changelog")
+        if not changelog:
+            continue
+        path = Path(repo_root) / changelog
+        if not path.is_file():
+            problems.append(f"{changelog} does not exist")
+            continue
+        text = path.read_text(encoding="utf-8")
+        section = (changelog_section(text, PLACEHOLDER) if plan.prerelease else None) or changelog_section(text, plan.version)
+        if section is None:
+            problems.append(f"{changelog}: no '## {plan.version}' heading")
+        notes += section or ""
+    return notes, problems
+
+
+def changelog_section(text, heading):
+    """The lines under the '## <heading>' heading of a changelog, up to the next '## ' heading outside a fenced code
+    block, without the blank lines around them; None when the changelog has no such heading."""
+    lines = text.splitlines()
+    start = next((index for index, line in enumerate(lines) if re.fullmatch(rf"## {re.escape(heading)}\s*", line)), None)
+    if start is None:
+        return None
+    section = []
+    fenced = False
+    for line in lines[start + 1:]:
+        if line.lstrip().startswith("```"):
+            fenced = not fenced
+        elif not fenced and line.startswith("## "):
+            break
+        section.append(line)
+    return "\n".join(section).strip("\n") + "\n"

.github/release/tests/support.py

@@ -1,16 +1,40 @@
-"""Shared test helpers: the repository configuration and in-memory stand-ins for R2 and the web."""
+"""Shared test helpers: the repository configuration, fake checkouts and in-memory stand-ins for R2 and the web."""
 
 import hashlib
 from pathlib import Path
 
 from plan import load_config
 
+BUILD_GRADLE = 'allprojects {\n  ext.CLY_VERSION = "{version}"\n  ext.POWERMOCK_VERSION = "1.7.4"\n}\n'
+CONFIG_JAVA = 'public class Config {\n    protected String sdkVersion = "{version}";\n}\n'
+
 
 def repository_config():
     """The real config.json of this repository."""
     return load_config(Path(__file__).resolve().parents[1] / "config.json")
 
 
+def write_file(root, relative, text):
+    """Writes a file of a fake checkout."""
+    path = Path(root) / relative
+    path.parent.mkdir(parents=True, exist_ok=True)
+    path.write_bytes(text.encode("utf-8"))
+
+
+def write_core(root, properties_version, gradle_version, java_version, changelog):
+    """Writes the three version sources of the core SDK and its changelog into a fake checkout."""
+    write_file(root, "gradle.properties", f"VERSION_NAME={properties_version}\nGROUP=ly.count.sdk\n")
+    write_file(root, "build.gradle", BUILD_GRADLE.replace("{version}", gradle_version))
+    write_file(root, "sdk-java/src/main/java/ly/count/sdk/java/Config.java", CONFIG_JAVA.replace("{version}", java_version))
+    write_file(root, "CHANGELOG.md", changelog)
+
+
+def write_ui(root, version, changelog):
+    """Writes the version source of the UI package and its changelog into a fake checkout."""
+    write_file(root, "sdk-java-ui/gradle.properties", f"POM_ARTIFACT_ID=java-ui\nVERSION_NAME={version}\n")
+    write_file(root, "sdk-java-ui/CHANGELOG.md", changelog)
+
+
 class FakeBucket:
     """In-memory stand-in for s3.R2Bucket with MD5 ETags; `fail_on` makes one upload fail like a lost connection."""
 

.github/release/tests/test_pending.py

@@ -0,0 +1,86 @@
+import subprocess
+import tempfile
+import unittest
+from pathlib import Path
+
+from pending import merge_problems, pending_releases
+from plan import PlanError
+from tests.support import repository_config, write_core, write_ui
+
+HEAD = "a" * 40
+MERGE = "b" * 40
+RELEASED = {"26.8.0", "ui-26.8.0"}
+
+
+def git_runner(contains_head, trees):
+    """A stand-in for subprocess.run answering the two git questions of merge_problems: whether the merge contains the
+    head, and the tree of each commit."""
+    def run(args, **kwargs):
+        if "merge-base" in args:
+            return subprocess.CompletedProcess(args, 0 if contains_head else 1)
+        return subprocess.CompletedProcess(args, 0, stdout="".join(trees[arg.split("^")[0]] + "\n" for arg in args[-2:]))
+    return run
+
+
+class PendingTest(unittest.TestCase):
+    def setUp(self):
+        self.config = repository_config()
+        self.root = Path(tempfile.mkdtemp())
+
+    def checkout(self, core, ui, changelog, ui_changelog, gradle=None):
+        """Writes the version sources and the changelogs of the core and the UI."""
+        write_core(self.root, core, gradle or core, core, changelog)
+        write_ui(self.root, ui, ui_changelog)
+
+    def test_new_versions_are_released_core_first_with_their_changelog_sections_and_only_the_core_is_latest(self):
+        self.checkout("26.9.0", "26.9.0", "## 26.9.0\n\n* Core change.\n  with a second line\n\n## 26.8.0\n* Old.\n", "## 26.9.0\n* UI change.\n")
+        releases, problems = pending_releases(self.config, self.root, "staging", RELEASED)
+        self.assertEqual(problems, [])
+        self.assertEqual([(release.tag, release.latest, release.prerelease) for release in releases], [("26.9.0", True, False), ("ui-26.9.0", False, False)])
+        self.assertEqual([release.notes for release in releases], ["* Core change.\n  with a second line\n", "* UI change.\n"])
+        self.assertEqual(releases[1].to_json(), {"tag": "ui-26.9.0", "prerelease": False, "latest": False})
+
+    def test_tagged_versions_are_skipped_so_an_unchanged_ui_or_a_repeated_merge_releases_nothing_more(self):
+        self.checkout("26.9.0", "26.8.0", "## 26.9.0\n* Core change.\n", "## 26.8.0\n* Old UI.\n")
+        releases, problems = pending_releases(self.config, self.root, "staging", RELEASED)
+        self.assertEqual(([release.tag for release in releases], problems), (["26.9.0"], []))
+        self.assertEqual(pending_releases(self.config, self.root, "staging", RELEASED | {"26.9.0"}), ([], []))
+
+    def test_a_changelog_or_a_version_source_not_ready_is_reported_for_every_release(self):
+        self.checkout("26.9.0", "26.9.0", "## XX.XX.XX\n* Still waiting.\n\n## 26.9.0\n* Core change.\n", "## 26.8.0\n", gradle="26.8.0")
+        _, problems = pending_releases(self.config, self.root, "staging", RELEASED)
+        self.assertEqual(problems, [
+            "build.gradle: version is 26.8.0, the tag says 26.9.0",
+            "CHANGELOG.md: still has a '## XX.XX.XX' heading; move its lines under '## 26.9.0'",
+            "sdk-java-ui/CHANGELOG.md: no '## 26.9.0' heading",
+        ])
+
+    def test_a_release_candidate_is_a_prerelease_never_latest_with_the_placeholder_section_as_notes(self):
+        self.checkout("26.9.0-rc1", "26.8.0", "## XX.XX.XX\n* Coming soon.\n\n## 26.8.0\n* Old.\n", "## 26.8.0\n")
+        releases, problems = pending_releases(self.config, self.root, "staging", RELEASED)
+        self.assertEqual(problems, [])
+        self.assertEqual([(release.tag, release.latest, release.prerelease, release.notes) for release in releases], [("26.9.0-rc1", False, True, "* Coming soon.\n")])
+
+    def test_a_release_candidate_falls_back_to_its_version_section_and_needs_one_of_them(self):
+        self.checkout("26.9.0-rc1", "26.8.0", "## 26.9.0-rc1\n* Ready early.\n", "## 26.8.0\n")
+        releases, problems = pending_releases(self.config, self.root, "staging", RELEASED)
+        self.assertEqual(([release.notes for release in releases], problems), (["* Ready early.\n"], []))
+        self.checkout("26.9.0-rc1", "26.8.0", "## 26.8.0\n* Old.\n", "## 26.8.0\n")
+        _, problems = pending_releases(self.config, self.root, "staging", RELEASED)
+        self.assertEqual(problems, ["CHANGELOG.md: no '## 26.9.0-rc1' heading"])
+
+    def test_a_version_outside_the_tag_grammar_stops_the_plan(self):
+        self.checkout("XX.XX.XX", "26.8.0", "## XX.XX.XX\n", "## 26.8.0\n")
+        with self.assertRaises(PlanError):
+            pending_releases(self.config, self.root, "staging", RELEASED)
+
+    def test_the_merge_must_contain_the_head_and_hold_exactly_its_tree(self):
+        self.assertEqual(merge_problems(HEAD, MERGE, "/repo", runner=git_runner(True, {HEAD: "tree", MERGE: "tree"})), [])
+        self.assertEqual(merge_problems(HEAD, MERGE, "/repo", runner=git_runner(False, {})),
+                         [f"{MERGE} does not contain {HEAD}; merge the release PR with a merge commit, not a squash or a rebase"])
+        self.assertEqual(merge_problems(HEAD, MERGE, "/repo", runner=git_runner(True, {HEAD: "tree", MERGE: "other"})),
+                         ["master holds changes staging lacks; merge master into staging, then update the release PR"])
+
+
+if __name__ == "__main__":
+    unittest.main()

.github/release/tests/test_release_cli.py

@@ -1,18 +1,21 @@
 import json
 import os
+import shutil
 import subprocess
 import sys
 import tempfile
 import unittest
 from pathlib import Path
 
+from tests.support import write_core, write_ui
+
 RELEASE = Path(__file__).resolve().parents[1] / "release.py"
 
 
-def run_cli(*args, output=""):
+def run_cli(*args, output="", script=RELEASE):
     """Runs release.py like the workflow does; GitHub's step output file is written only when `output` names one."""
     env = dict(os.environ, GITHUB_OUTPUT=output, GITHUB_STEP_SUMMARY="")
-    return subprocess.run([sys.executable, str(RELEASE), *args], capture_output=True, text=True, env=env)
+    return subprocess.run([sys.executable, str(script), *args], capture_output=True, text=True, env=env)
 
 
 class ReleaseCliTest(unittest.TestCase):
@@ -61,6 +64,32 @@ def test_a_branch_without_releases_gets_the_empty_list_the_workflow_skips_on(sel
         self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
         self.assertEqual(output.read_text(encoding="utf-8").strip(), "tags=[]")
 
+    def test_pending_creates_nothing_when_the_merge_does_not_contain_the_head(self):
+        out = self.folder / "pending.json"
+        result = run_cli("pending", "--head", "0" * 40, "--merge", "HEAD", "--branch", "staging", "--out", str(out), "--notes", str(self.folder / "notes"))
+        self.assertEqual(result.returncode, 1, result.stdout + result.stderr)
+        self.assertIn("::error::", result.stdout)
+        self.assertFalse(out.exists())
+
+    def test_pending_writes_the_untagged_releases_and_their_notes_for_the_workflow(self):
+        """Runs a copy of the release tools in a throwaway repository whose core version has no tag yet."""
+        repo = self.folder / "repo"
+        shutil.copytree(RELEASE.parent, repo / ".github/release", ignore=shutil.ignore_patterns("tests", "__pycache__"))
+        write_core(repo, "26.9.0", "26.9.0", "26.9.0", "## 26.9.0\n* Core change.\n\n## 26.8.0\n* Old.\n")
+        write_ui(repo, "26.8.0", "## 26.8.0\n* Old UI.\n")
+        git = ["git", "-C", str(repo), "-c", "user.name=Release test", "-c", "user.email=release@example.com", "-c", "commit.gpgsign=false", "-c", "tag.gpgSign=false"]
+        subprocess.run(["git", "init", "-q", str(repo)], check=True)
+        subprocess.run(git + ["add", "-A"], check=True)
+        subprocess.run(git + ["commit", "-q", "-m", "Release 26.9.0"], check=True)
+        subprocess.run(git + ["tag", "ui-26.8.0"], check=True)
+        out = self.folder / "pending.json"
+        notes = self.folder / "notes"
+        result = run_cli("pending", "--head", "HEAD", "--merge", "HEAD", "--branch", "staging", "--out", str(out), "--notes", str(notes), script=repo / ".github/release/release.py")
+        self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
+        self.assertEqual(json.loads(out.read_text(encoding="utf-8")), {"releases": [{"tag": "26.9.0", "prerelease": False, "latest": True}]})
+        self.assertEqual(sorted(path.name for path in notes.iterdir()), ["26.9.0.md"])
+        self.assertEqual((notes / "26.9.0.md").read_text(encoding="utf-8"), "* Core change.\n")
+
     def test_prerelease_flag_must_match(self):
         cases = [("26.8.1", "true", 1), ("26.8.1-rc1", "false", 1), ("26.8.1", "unknown", 1), ("26.8.1-rc1", "true", 0), ("26.8.1", "false", 0)]
         for tag, prerelease, code in cases: