diff --git a/.github/workflows/release-notes.yml b/.github/workflows/release-notes.yml index 9b119f15f..b01f2d1d9 100644 --- a/.github/workflows/release-notes.yml +++ b/.github/workflows/release-notes.yml @@ -1,6 +1,8 @@ -# Sets the GitHub Release body to notes generated from the merged pull -# requests, followed by dist's install instructions. dist's release.yml calls -# this once the Release is published. There is no CHANGELOG file. +# Sets the GitHub Release body to the bump PR's summary, then notes generated +# from the merged pull requests, then dist's install instructions; a tag with +# no bump PR summary gets the generated notes and install instructions alone. +# scripts/release-notes.sh builds the body. dist's release.yml calls this once +# the Release is published. There is no CHANGELOG file. name: Release notes on: @@ -15,13 +17,16 @@ jobs: runs-on: ubuntu-24.04 env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - REPO: ${{ github.repository }} + GH_REPO: ${{ github.repository }} TAG: ${{ fromJson(inputs.plan).announcement_tag }} INSTALL_NOTES: ${{ fromJson(inputs.plan).announcement_github_body }} steps: - # Builds the body from the plan rather than the current body, so a - # re-run doesn't repeat the notes. + - uses: actions/checkout@v7 + with: + persist-credentials: false + ref: ${{ env.TAG }} + # The script builds the body from the plan and the bump PR rather than + # the current body, so a re-run doesn't repeat the notes. - run: | - gh api "repos/$REPO/releases/generate-notes" -f tag_name="$TAG" --jq .body > notes.md - printf '\n%s\n' "$INSTALL_NOTES" >> notes.md - gh release edit "$TAG" --repo "$REPO" --notes-file notes.md + scripts/release-notes.sh "$TAG" "$INSTALL_NOTES" > notes.md + gh release edit "$TAG" --notes-file notes.md diff --git a/README.md b/README.md index e7308ef4c..b0492dffd 100644 --- a/README.md +++ b/README.md @@ -348,7 +348,7 @@ It works from `origin/main` in a temporary worktree, so your checked-out branch Before it pushes anything, it refuses a release that can't be cut and says why: a version that isn't plain `X.Y.Z` (no leading `v`), a version that isn't higher than the one in `Cargo.toml` on `origin/main`, a `v` tag that already exists locally or on origin, or a latest CI run on `origin/main` that failed or hasn't finished. -The tag starts the release workflow, generated by [`dist`](https://github.com/axodotdev/cargo-dist) from `dist-workspace.toml`. It builds the Linux and macOS binaries and the installer, checks that the Linux binary starts on Rocky Linux 9 and in WSL2 with Ubuntu 24.04, and only then publishes the GitHub Release. It then publishes the same version to crates.io, skipping it if it is already there, and finally replaces the Release's body with notes generated from the merged pull requests. +The tag starts the release workflow, generated by [`dist`](https://github.com/axodotdev/cargo-dist) from `dist-workspace.toml`. It builds the Linux and macOS binaries and the installer, checks that the Linux binary starts on Rocky Linux 9 and in WSL2 with Ubuntu 24.04, and only then publishes the GitHub Release. It then publishes the same version to crates.io, skipping it if it is already there, and finally replaces the Release's body with the bump pull request's summary, then notes generated from the merged pull requests, then the install instructions. [`scripts/release-notes.sh`](scripts/release-notes.sh) builds that body; for a tag pushed by hand, with no bump pull request summary, it is the generated notes and the install instructions alone. ## Credits and license diff --git a/dist-workspace.toml b/dist-workspace.toml index a8a94cde4..f2655aeec 100644 --- a/dist-workspace.toml +++ b/dist-workspace.toml @@ -25,7 +25,8 @@ global-artifacts-jobs = ["./release-smoke"] # Runs once the Release is created, which is only after the Rocky and WSL # checks pass; publishes the tagged version to crates.io. publish-jobs = ["./publish-crates"] -# Replaces the Release body with notes generated from the merged pull requests. +# Replaces the Release body with the bump PR's summary and notes generated from +# the merged pull requests. post-announce-jobs = ["./release-notes"] # The runner images are pinned to the platforms thirdshift supports, as in ci.yml. diff --git a/scripts/release-notes.sh b/scripts/release-notes.sh new file mode 100755 index 000000000..e87a712c4 --- /dev/null +++ b/scripts/release-notes.sh @@ -0,0 +1,64 @@ +#!/usr/bin/env bash +# Prints the body of the GitHub Release for : the summary section of the +# bump PR that release.sh merged as 's commit, then GitHub's generated +# notes, then , dist's install instructions. If no PR was merged +# as that commit (say, a tag pushed by hand), or its body has no summary +# section, the body is the generated notes and the install instructions alone. +# +# Usage: scripts/release-notes.sh +# +# release-notes.yml runs it in a checkout of once the Release is +# published. It builds the body from its inputs and the PR, never from the +# Release's current body, so running it again gives the same body. +set -euo pipefail +# So a failure inside $(…) stops the script too. +shopt -s inherit_errexit + +main() { + if [ "$#" -ne 2 ]; then + echo "usage: release-notes.sh " >&2 + exit 2 + fi + local commit summary + commit=$(git rev-parse --verify "$1^{commit}") + # In an if, so failing to read the PR falls back rather than stopping the + # script and leaving the Release with dist's body. + if ! summary=$(bump_summary "$commit"); then + echo "release-notes: warning: could not read the bump PR, so the body has no summary" >&2 + summary= + fi + if [ -n "${summary//[[:space:]]/}" ]; then + printf '%s\n\n' "$summary" + fi + gh api 'repos/{owner}/{repo}/releases/generate-notes' -f tag_name="$1" --jq .body + printf '\n%s\n' "$2" +} + +# bump_summary +# The summary section of the body of the PR merged as , or nothing. +# GitHub lists every PR the commit belongs to, including one whose head it +# is, so only a PR whose merge commit it is counts. +bump_summary() { + local number + # Errors are returned explicitly, as the caller's if turns errexit off. + number=$(gh api "repos/{owner}/{repo}/commits/$1/pulls" \ + --jq '.[] | "\(.merge_commit_sha) \(.number)"' | + awk -v commit="$1" '$1 == commit && !number { number = $2 } END { print number }') || + return + if [ -n "$number" ]; then + gh pr view "$number" --json body --jq .body | summary_section + fi +} + +# summary_section +# The lines of the PR body on stdin between the markers release.sh's pr_body +# writes around the summary, or nothing if either marker is missing. +summary_section() { + awk ' + { sub(/\r$/, "") } + $0 == "" && inside { printf "%s", text; inside = 0; done = 1 } + inside { text = text $0 "\n" } + $0 == "" && !done { inside = 1 }' +} + +main "$@" diff --git a/scripts/release.sh b/scripts/release.sh index 7bb5afe27..33d4d2d40 100755 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -181,8 +181,8 @@ rewrite() { # pr_body # The body of the bump PR on HEAD, which will name, off : the -# summary, between markers the Release page step can find, then the version -# diff. +# summary, between markers release-notes.sh finds to head the Release page +# with it, then the version diff. pr_body() { local diff summary diff=$(version_diff "$1") diff --git a/tests/fakes/gh b/tests/fakes/gh index 3e5e8eed5..16df6cfb7 100644 --- a/tests/fakes/gh +++ b/tests/fakes/gh @@ -43,6 +43,8 @@ generated notes: a line for each merged PR whose merge commit is reachable from `{owner}/{repo}` placeholder, here and in the commit endpoints. `gh api repos//actions/runs?head_sha=` answers with the workflow runs on : its check runs, read as the check-runs endpoint reads them. +`gh api repos//commits//pulls` answers with the PRs belongs +to, as GitHub does: those merged as and those whose head is . `gh run list` answers with the workflow runs in `runs`, newest first, that match its `--branch`, `--workflow` and `--event`. Runs there are only ones a @@ -353,15 +355,18 @@ def issue_close(state, positional, flags): def api(state, positional, flags): """The signed-in user, the two commit endpoints thirdshift reads CI from, - or the workflow runs on a commit that the release script waits for. Each + the workflow runs on a commit that the release script waits for, or the + PRs a commit belongs to that the Release notes script looks through. Each answers one page, each item of the list `--jq '.[]'` names on its - own line, or for the workflow runs one ` ` line - per run with `--jq` set to `WORKFLOW_RUN_LINES`.""" + own line, for the workflow runs one ` ` line + per run with `--jq` set to `WORKFLOW_RUN_LINES`, and for the PRs one + ` ` line per PR with `--jq` set to + `COMMIT_PR_LINES`.""" if positional[0] == "user" and not flags: print(json.dumps({"login": "runner", "email": state.get("user_email")})) return match = re.fullmatch( - r"repos/([^/]+/[^/]+)/(?:commits/([0-9a-f]+)/(check-runs|status)|actions/(runs)\?head_sha=([0-9a-f]+))(&.*|\?.*)?", + r"repos/([^/]+/[^/]+)/(?:commits/([0-9a-f]+)/(check-runs|status|pulls)|actions/(runs)\?head_sha=([0-9a-f]+))(&.*|\?.*)?", positional[0], ) if not match: @@ -374,6 +379,12 @@ def api(state, positional, flags): for run in read_checks(state, runs_sha): print(f"{run['status']} {run['conclusion'] or 'null'} {run['name']}") return + if endpoint == "pulls": + if flags.get("jq") != COMMIT_PR_LINES: + die(f"fake gh: unsupported --jq {flags.get('jq')}", 2) + for pr in commit_prs(state, sha): + print(f"{pr.get('mergeCommit') or 'null'} {pr['number']}") + return if endpoint == "check-runs": items, field = read_checks(state, sha), "check_runs" else: @@ -388,6 +399,18 @@ def api(state, positional, flags): print(json.dumps(item)) +def commit_prs(state, sha): + """The PRs merged as `sha` or whose head is `sha`, oldest first.""" + + def head(pr): + if "headRefOid" in pr: + return pr["headRefOid"] + tip = git(origin_repo(), "rev-parse", f"refs/heads/{pr['head']}") + return tip.stdout.strip() if tip.returncode == 0 else None + + return [pr for pr in state["prs"] if pr.get("mergeCommit") == sha or head(pr) == sha] + + def read_checks(state, sha): """The check runs on `sha`, each read once, after any `on-ci-read` script.""" @@ -413,6 +436,8 @@ GENERATE_NOTES = re.compile(r"repos/([^/]+/[^/]+)/releases/generate-notes") WORKFLOW_RUN_LINES = '.workflow_runs[] | "\\(.status) \\(.conclusion) \\(.name)"' +COMMIT_PR_LINES = '.[] | "\\(.merge_commit_sha) \\(.number)"' + def api_fields(args): """The `-f`/`-F` fields of a `gh api` call, by name.""" @@ -425,7 +450,8 @@ def api_fields(args): def generate_notes(state, args): - """GitHub's generated release notes for `tag_name` at `target_commitish`: + """GitHub's generated release notes for `tag_name` at `target_commitish`, + or at the tag itself without one: one line per merged PR whose merge commit is reachable from the target but not from `previous_tag_name`, oldest first, then the compare link. Only `--jq .body` is supported.""" @@ -434,7 +460,9 @@ def generate_notes(state, args): if jq != ".body": die(f"fake gh: unsupported --jq {jq}", 2) check_repo(state, {"repo": GENERATE_NOTES.fullmatch(args[0]).group(1)}) - tag, target = fields["tag_name"], fields["target_commitish"] + # GitHub ignores the target when the tag exists. + tag = fields["tag_name"] + target = fields.get("target_commitish", tag) previous = fields.get("previous_tag_name") def reachable(commit, ref): diff --git a/tests/release.rs b/tests/release.rs index ab512cc37..e021e89e7 100644 --- a/tests/release.rs +++ b/tests/release.rs @@ -248,6 +248,32 @@ impl Release { .expect("could not run release.sh") } + /// Runs the Release notes script for `tag` in the maintainer's clone, + /// once it has fetched the tags, with `INSTALL_NOTES` as dist's install + /// instructions. + fn run_notes_script(&self, tag: &str) -> Output { + git(&self.maintainer(), &["fetch", "-q", "--tags", "origin"]); + self.command( + &manifest_dir().join("scripts/release-notes.sh"), + &self.maintainer(), + ) + .args([tag, INSTALL_NOTES]) + .output() + .expect("could not run release-notes.sh") + } + + /// GitHub's generated notes for `tag`, from the fake `gh`. + fn generated_notes(&self, tag: &str) -> String { + self.gh(&[ + "api", + "repos/{owner}/{repo}/releases/generate-notes", + "-f", + &format!("tag_name={tag}"), + "--jq", + ".body", + ]) + } + fn gh_state(&self) -> Value { serde_json::from_str(&fs::read_to_string(self.root().join("gh-state.json")).unwrap()) .unwrap() @@ -294,6 +320,8 @@ fn stderr(output: &Output) -> String { const GREEN: &str = r#"[{"name": "test", "conclusion": "success", "pending_polls": 2}]"#; const RED: &str = r#"[{"name": "test", "conclusion": "failure"}]"#; const AGENT_SUMMARY: &str = "The headline is the frobnicator (#2)."; +const INSTALL_NOTES: &str = + "## Install thirdshift\n\n```sh\ncurl -LsSf https://example.com/install.sh | sh\n```"; #[test] #[cfg(target_os = "linux")] @@ -568,6 +596,97 @@ fn a_failure_to_generate_the_notes_stops_before_the_pr_is_opened() { assert_eq!(release.origin_tag("v0.2.0"), None); } +#[test] +#[cfg(target_os = "linux")] +fn the_release_body_is_the_bump_prs_summary_then_the_generated_notes_then_the_install_instructions() +{ + let release = Release::new(); + release.ci_reports(GREEN); + let output = release.run_script("0.2.0"); + assert!(output.status.success(), "{}", stderr(&output)); + + let first = release.run_notes_script("v0.2.0"); + let again = release.run_notes_script("v0.2.0"); + + assert!(first.status.success(), "{}", stderr(&first)); + let body = String::from_utf8(first.stdout).unwrap(); + let notes = release.generated_notes("v0.2.0"); + assert!(notes.contains("Add the frobnicator"), "{notes}"); + assert_eq!( + body, + format!("{AGENT_SUMMARY}\n\n{notes}\n\n{INSTALL_NOTES}\n") + ); + assert!(again.status.success(), "{}", stderr(&again)); + assert_eq!(String::from_utf8(again.stdout).unwrap(), body); +} + +#[test] +#[cfg(target_os = "linux")] +fn without_a_bump_pr_summary_the_release_body_is_the_generated_notes_and_install_instructions() { + let release = Release::new(); + // A PR with a summary section whose head commit, not its merge, is tagged. + release.merge_pr( + "summarised", + "Summarised", + "\nNot this release.\n", + ); + let contributor = release.contributor(); + git(&contributor, &["tag", "v0.1.2", "origin/summarised"]); + // A commit pushed straight to main, not through a PR. + fs::write(contributor.join("hand.txt"), "by hand").unwrap(); + git(&contributor, &["add", "-A"]); + git(&contributor, &["commit", "-q", "-m", "By hand"]); + git(&contributor, &["push", "-q", "origin", "HEAD:main"]); + git(&contributor, &["tag", "v0.1.3"]); + git(&contributor, &["push", "-q", "origin", "v0.1.2", "v0.1.3"]); + + for (case, tag) in [ + ("the merge of a PR without a summary section", "v0.1.0"), + ("the head of a PR, not its merge", "v0.1.2"), + ("a commit with no PR", "v0.1.3"), + ] { + let output = release.run_notes_script(tag); + + assert!(output.status.success(), "{case}: {}", stderr(&output)); + let notes = release.generated_notes(tag); + assert_eq!( + String::from_utf8(output.stdout).unwrap(), + format!("{notes}\n\n{INSTALL_NOTES}\n"), + "{case}" + ); + } +} + +#[test] +#[cfg(target_os = "linux")] +fn when_the_bump_pr_cannot_be_read_the_release_body_is_the_generated_notes_with_a_warning() { + let release = Release::new(); + release.ci_reports(GREEN); + let output = release.run_script("0.2.0"); + assert!(output.status.success(), "{}", stderr(&output)); + let merge = release.origin(&["rev-parse", "main"]); + release.gh(&[ + "fake", + "fails", + &format!("api repos/{{owner}}/{{repo}}/commits/{merge}/pulls"), + ]); + + let output = release.run_notes_script("v0.2.0"); + + assert!(output.status.success(), "{}", stderr(&output)); + let err = stderr(&output); + assert!( + err.lines() + .any(|line| line.starts_with("release-notes: warning:") && line.contains("summary")), + "{err}" + ); + let notes = release.generated_notes("v0.2.0"); + assert_eq!( + String::from_utf8(output.stdout).unwrap(), + format!("{notes}\n\n{INSTALL_NOTES}\n") + ); +} + /// Runs the script with `version` and asserts it refused before pushing /// anything, with a message on stderr containing each of `reasons`. fn assert_refused(release: &Release, version: &str, reasons: &[&str]) {