diff --git a/.github/workflows/issue-status.yml b/.github/workflows/issue-status.yml new file mode 100644 index 0000000..09c2862 --- /dev/null +++ b/.github/workflows/issue-status.yml @@ -0,0 +1,101 @@ +name: Issue status + +# Moves issues across the project board as their work progresses. GitHub does +# not link an issue to a pull request based on dev, so the closing keywords are +# parsed out of the body here rather than read back from the API. +on: + push: + branches: ['issue-*'] + pull_request: + types: [opened, reopened, ready_for_review, edited, closed] + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: false + +permissions: {} + +env: + PROJECT_NUMBER: '1' + GH_TOKEN: ${{ secrets.RELEASE_TOKEN }} + +jobs: + in-progress: + # A branch deletion after its pull request merges can also fire a push + # event, with no branch left to check out. + if: github.event_name == 'push' && github.event.deleted == false + runs-on: ubuntu-latest + steps: + - name: Check out + uses: actions/checkout@v4 + + - name: Move the issue to In progress + env: + BRANCH: ${{ github.ref_name }} + run: | + set -uo pipefail + issue="$(printf '%s' "$BRANCH" | sed -nE 's/^issue-([0-9]+)-.*/\1/p')" + if [ -z "$issue" ]; then + echo "Branch $BRANCH does not name an issue - nothing to do." + exit 0 + fi + # Todo or unset only, so a later push cannot pull it out of In review. + bash scripts/set-issue-status.sh "$issue" "In progress" Todo unset + + pull-request: + # Fork runs get no secrets, so GH_TOKEN would be empty and every external + # contribution would show a red check it has no way to fix. + if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-latest + steps: + - name: Check out + uses: actions/checkout@v4 + + - name: Move the referenced issues + env: + # Through the environment, never interpolated into the script: a body + # is attacker controllable text. + BODY: ${{ github.event.pull_request.body }} + MERGED: ${{ github.event.pull_request.merged }} + BASE: ${{ github.event.pull_request.base.ref }} + ACTION: ${{ github.event.action }} + run: | + set -uo pipefail + source scripts/issue-status.sh + + if [ "$ACTION" = "closed" ]; then + if [ "$MERGED" != "true" ] || [ "$BASE" != "dev" ]; then + echo "Closed without merging into dev - nothing to do." + exit 0 + fi + status="Ready for release" + # Every state except Done: a merge into dev always beats an + # earlier state, but a down-merge from master can repeat a + # Closes reference for an issue that already shipped, and that + # must not drag it backwards out of Done. + allowed=(Todo unset "In progress" "In review" "Ready for release") + else + status="In review" + # Guards an edit on an already merged pull request from pulling + # the issue back out of Ready for release or Done. + allowed=(Todo unset "In progress") + fi + + refs="$(printf '%s' "$BODY" | closing_refs)" + if [ -z "$refs" ]; then + echo "No closing references in the body - nothing to do." + exit 0 + fi + + failures=0 + while IFS= read -r issue; do + [ -n "$issue" ] || continue + if ! bash scripts/set-issue-status.sh "$issue" "$status" "${allowed[@]}"; then + printf 'FAILED: could not set #%s to %s\n' "$issue" "$status" >&2 + failures=$((failures + 1)) + fi + done <<< "$refs" + + if [ "$failures" -gt 0 ]; then + exit 1 + fi diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ef44386..b813681 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -16,6 +16,8 @@ concurrency: permissions: contents: write + issues: write + pull-requests: read jobs: release: @@ -96,6 +98,17 @@ jobs: echo "current=$CURRENT" echo "next=$NEXT" + - name: Dry run - list the issues this release would close + if: steps.plan.outputs.release == 'true' && inputs.dry_run + env: + GH_TOKEN: ${{ github.token }} + VERSION: ${{ steps.plan.outputs.next }} + DRY_RUN: '1' + run: | + set -uo pipefail + PREV_TAG="$(git describe --tags --abbrev=0 2>/dev/null || true)" \ + bash scripts/close-released-issues.sh + - name: Dry run - resolve BukkitDev game versions if: steps.plan.outputs.release == 'true' && inputs.dry_run env: @@ -116,6 +129,17 @@ jobs: end ' + # Must run before Commit and tag creates v$NEXT, or the range below is + # empty and nothing closes. + - name: Record the previous release tag + id: prev + if: steps.plan.outputs.release == 'true' + run: | + set -uo pipefail + tag="$(git describe --tags --abbrev=0 2>/dev/null || true)" + echo "tag=$tag" >> "$GITHUB_OUTPUT" + echo "Previous release tag: ${tag:-none}" + - name: Commit and tag if: steps.plan.outputs.release == 'true' && !inputs.dry_run env: @@ -153,3 +177,15 @@ jobs: gh release create "v$VERSION" "SetHomesTwo.V$VERSION.jar" \ --title "SetHomesTwo V$VERSION" \ --notes "$NOTES" + + # An issue closes when the commit that fixed it is contained in the commit + # being released. That is what keeps work sitting on dev open when an + # immediate fix ships from master, and closes an immediate fix that never + # passed through dev. + - name: Close the issues this release contains + if: steps.plan.outputs.release == 'true' && !inputs.dry_run + env: + GH_TOKEN: ${{ github.token }} + PREV_TAG: ${{ steps.prev.outputs.tag }} + VERSION: ${{ steps.plan.outputs.next }} + run: bash scripts/close-released-issues.sh diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 52a4d79..7eab0bd 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -51,6 +51,9 @@ jobs: - name: Run the BukkitDev publish tests run: bash scripts/test-publish-bukkitdev.sh + - name: Run the issue status parsing tests + run: bash scripts/test-issue-status.sh + # Rehearses the release on every pull request. The changelog heading went # missing in a docs change, and nothing noticed until a release ran. - name: Check a release could be applied diff --git a/scripts/close-released-issues.sh b/scripts/close-released-issues.sh new file mode 100644 index 0000000..19ef826 --- /dev/null +++ b/scripts/close-released-issues.sh @@ -0,0 +1,87 @@ +#!/usr/bin/env bash +# Closes the issues contained in the commit being released. +# +# Reads PREV_TAG (may be empty on a first release) and VERSION from the +# environment. With DRY_RUN=1 it prints what it would close and closes nothing. +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=./issue-status.sh +source "$SCRIPT_DIR/issue-status.sh" + +VERSION="${VERSION:?VERSION must be set}" +PREV_TAG="${PREV_TAG:-}" +DRY_RUN="${DRY_RUN:-0}" + +if [ -z "$PREV_TAG" ] && [ -n "$(git tag --list)" ]; then + echo "Error: PREV_TAG is empty but the repository already has tags - refusing to scan all of history." >&2 + echo "This usually means the checkout is missing tags or full history. Fix the checkout rather than closing every referenced issue." >&2 + exit 1 +fi + +range="HEAD" +if [ -n "$PREV_TAG" ]; then + range="$PREV_TAG..HEAD" +fi +printf 'Range: %s\n' "$range" + +# Subjects only. A commit body may mention a pull request it did not come from. +prs="$(git log --format=%s "$range" | pr_numbers_from_log)" +if [ -z "$prs" ]; then + echo "No pull requests in the range - nothing to close." + exit 0 +fi + +issues="" +pr_count=0 +pr_failures=0 +while IFS= read -r pr; do + [ -n "$pr" ] || continue + pr_count=$((pr_count + 1)) + if ! body="$(gh pr view "$pr" --json body --jq '.body')"; then + printf 'Warning: could not look up pull request #%s - skipping it.\n' "$pr" >&2 + pr_failures=$((pr_failures + 1)) + continue + fi + refs="$(printf '%s' "$body" | closing_refs)" + [ -n "$refs" ] || continue + issues="$(printf '%s\n%s' "$issues" "$refs")" +done <<< "$prs" + +if [ "$pr_count" -gt 0 ] && [ "$pr_failures" -eq "$pr_count" ]; then + printf 'Error: all %d pull request lookups failed - cannot tell what this release closes.\n' "$pr_count" >&2 + exit 1 +fi + +issues="$(printf '%s' "$issues" | grep -E '^[1-9][0-9]*$' | awk '!seen[$0]++')" +if [ -z "$issues" ]; then + echo "No closing references among those pull requests - nothing to close." + exit 0 +fi + +close_failures=0 +while IFS= read -r issue; do + [ -n "$issue" ] || continue + + state="$(gh issue view "$issue" --json state --jq '.state')" || continue + if [ "$state" != "OPEN" ]; then + printf '#%s is already %s - skipping.\n' "$issue" "$state" + continue + fi + + if [ "$DRY_RUN" = "1" ]; then + printf 'Would close #%s (Released in v%s)\n' "$issue" "$VERSION" + continue + fi + + if gh issue close "$issue" --reason completed --comment "Released in v$VERSION"; then + printf 'Closed #%s\n' "$issue" + else + printf 'Failed to close #%s - close it by hand.\n' "$issue" >&2 + close_failures=$((close_failures + 1)) + fi +done <<< "$issues" + +if [ "$close_failures" -gt 0 ]; then + exit 1 +fi diff --git a/scripts/issue-status.sh b/scripts/issue-status.sh new file mode 100644 index 0000000..dfef119 --- /dev/null +++ b/scripts/issue-status.sh @@ -0,0 +1,30 @@ +#!/usr/bin/env bash +# Parses issue and pull request references out of text. +# +# Pure text handling, no network, so the workflows that call it stay thin and +# the parsing is unit tested. + +# GitHub's own closing keyword set. A bare #NN must not close anything: pull +# request bodies here routinely mention issues they do not resolve. +CLOSING_KEYWORDS='close|closes|closed|fix|fixes|fixed|resolve|resolves|resolved' + +# Reads text on stdin, writes the issue numbers it closes to stdout, one per +# line, first appearance order, deduplicated. Always exits 0. +closing_refs() { + grep -oiE "\\b(${CLOSING_KEYWORDS})[[:space:]]*:?[[:space:]]+#[0-9]+" \ + | grep -oE '[0-9]+' \ + | grep -E '^[1-9][0-9]*$' \ + | awk '!seen[$0]++' + return 0 +} + +# Reads `git log --format=%s` output on stdin and writes the pull request +# numbers it contains to stdout, one per line, deduplicated. Subjects only: +# a body may mention a pull request the commit did not come from. +pr_numbers_from_log() { + grep -oE '(Merge pull request #[0-9]+|\(#[0-9]+\))' \ + | grep -oE '[0-9]+' \ + | grep -E '^[1-9][0-9]*$' \ + | awk '!seen[$0]++' + return 0 +} diff --git a/scripts/set-issue-status.sh b/scripts/set-issue-status.sh new file mode 100644 index 0000000..6150b80 --- /dev/null +++ b/scripts/set-issue-status.sh @@ -0,0 +1,130 @@ +#!/usr/bin/env bash +# Sets an issue's Status on the SetHomesTwo project board. +# +# bash scripts/set-issue-status.sh 54 "In review" +# bash scripts/set-issue-status.sh 54 "In progress" Todo unset +# +# The optional trailing arguments guard the write: the status is set only when +# the item currently holds one of them, where `unset` means no status yet. That +# is what stops a later push to an issue branch pulling the issue back out of +# In review. +# +# Needs GH_TOKEN with the project scope; GITHUB_TOKEN cannot reach an +# organization project. +set -uo pipefail + +ORG="Blockframe-Studios" +REPO="SetHomesTwo" +PROJECT_NUMBER="${PROJECT_NUMBER:?PROJECT_NUMBER must be set}" + +ISSUE="${1:?issue number required}" +TARGET_STATUS="${2:?target status required}" +shift 2 +ALLOWED_CURRENT=("$@") + +PROJECT_QUERY=' + query($org:String!, $number:Int!) { + organization(login:$org) { + projectV2(number:$number) { + id + field(name:"Status") { + ... on ProjectV2SingleSelectField { id options { id name } } + } + } + } + }' + +# gh has jq built in. Standalone jq is not installed on the development machine, +# and every line here has to run locally as well as on a runner. +ids="$(gh api graphql -f query="$PROJECT_QUERY" -f org="$ORG" \ + -F number="$PROJECT_NUMBER" \ + --jq '[.data.organization.projectV2.id, + .data.organization.projectV2.field.id] | @tsv')" || exit 1 +IFS=$'\t' read -r project_id field_id <<<"$ids" + +options="$(gh api graphql -f query="$PROJECT_QUERY" -f org="$ORG" \ + -F number="$PROJECT_NUMBER" \ + --jq '.data.organization.projectV2.field.options[] | [.name, .id] | @tsv')" || exit 1 + +# Matched in awk rather than inside the jq program, so the status name is never +# interpolated into a query. +option_id="$(printf '%s\n' "$options" \ + | awk -F'\t' -v n="$TARGET_STATUS" '$1 == n { print $2; exit }')" + +if [ -z "$option_id" ]; then + printf 'No Status option named [%s] on the project.\n' "$TARGET_STATUS" >&2 + exit 1 +fi + +ISSUE_QUERY=' + query($owner:String!, $repo:String!, $issue:Int!) { + repository(owner:$owner, name:$repo) { + issue(number:$issue) { + id + projectItems(first:20) { + nodes { + id + project { id } + fieldValueByName(name:"Status") { + ... on ProjectV2ItemFieldSingleSelectValue { name } + } + } + } + } + } + }' + +issue_id="$(gh api graphql -f query="$ISSUE_QUERY" -f owner="$ORG" -f repo="$REPO" \ + -F issue="$ISSUE" --jq '.data.repository.issue.id')" || exit 1 +if [ -z "$issue_id" ]; then + printf 'Issue #%s not found - nothing to do.\n' "$ISSUE" + exit 0 +fi + +items="$(gh api graphql -f query="$ISSUE_QUERY" -f owner="$ORG" -f repo="$REPO" \ + -F issue="$ISSUE" \ + --jq '.data.repository.issue.projectItems.nodes[] + | [.project.id, .id, (.fieldValueByName.name // "unset")] | @tsv')" || exit 1 + +item_id="$(printf '%s\n' "$items" \ + | awk -F'\t' -v p="$project_id" '$1 == p { print $2; exit }')" +current_status="$(printf '%s\n' "$items" \ + | awk -F'\t' -v p="$project_id" '$1 == p { print $3; exit }')" + +if [ -z "$item_id" ] || [ "$item_id" = "null" ]; then + current_status="unset" + # Idempotent: returns the existing item when the issue is already on the board. + item_id="$(gh api graphql -f query=' + mutation($project:ID!, $content:ID!) { + addProjectV2ItemById(input:{projectId:$project, contentId:$content}) { + item { id } + } + }' -f project="$project_id" -f content="$issue_id" \ + --jq '.data.addProjectV2ItemById.item.id')" || exit 1 +fi + +if [ "${#ALLOWED_CURRENT[@]}" -gt 0 ]; then + allowed=1 + for candidate in "${ALLOWED_CURRENT[@]}"; do + if [ "$candidate" = "$current_status" ]; then + allowed=0 + break + fi + done + if [ "$allowed" -ne 0 ]; then + printf '#%s is [%s], not one of [%s] - leaving it alone.\n' \ + "$ISSUE" "$current_status" "${ALLOWED_CURRENT[*]}" + exit 0 + fi +fi + +gh api graphql -f query=' + mutation($project:ID!, $item:ID!, $field:ID!, $option:String!) { + updateProjectV2ItemFieldValue(input:{ + projectId:$project, itemId:$item, fieldId:$field, + value:{ singleSelectOptionId:$option } + }) { projectV2Item { id } } + }' -f project="$project_id" -f item="$item_id" -f field="$field_id" \ + -f option="$option_id" > /dev/null || exit 1 + +printf '#%s: %s -> %s\n' "$ISSUE" "$current_status" "$TARGET_STATUS" diff --git a/scripts/test-issue-status.sh b/scripts/test-issue-status.sh new file mode 100644 index 0000000..4328d49 --- /dev/null +++ b/scripts/test-issue-status.sh @@ -0,0 +1,199 @@ +#!/usr/bin/env bash +# Defines the issue-status.sh contract. +# +# Sources issue-status.sh so the tests can call its functions in-process. +# +# Run with: bash scripts/test-issue-status.sh +set -u + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +ISSUE_STATUS_SH="$SCRIPT_DIR/issue-status.sh" + +# shellcheck source=./issue-status.sh +source "$ISSUE_STATUS_SH" + +PASS=0 +FAIL=0 + +pass() { + PASS=$((PASS + 1)) + printf 'ok - %s\n' "$1" +} + +fail() { + FAIL=$((FAIL + 1)) + printf 'FAIL - %s\n' "$1" + if [ -n "${2:-}" ]; then + printf ' %s\n' "$2" + fi +} + +assert_equals() { + local desc="$1" expected="$2" actual="$3" + if [ "$expected" = "$actual" ]; then + pass "$desc" + else + fail "$desc" "expected [$expected] got [$actual]" + fi +} + +# Runs closing_refs over a body and joins the result with commas, so an +# expectation reads as one string. +refs_of() { + printf '%s' "$1" | closing_refs | paste -sd, - +} + +# -- which references close an issue -- + +test_each_keyword_matches() { + assert_equals "close" "1" "$(refs_of 'close #1')" + assert_equals "closes" "2" "$(refs_of 'closes #2')" + assert_equals "closed" "3" "$(refs_of 'closed #3')" + assert_equals "fix" "4" "$(refs_of 'fix #4')" + assert_equals "fixes" "5" "$(refs_of 'fixes #5')" + assert_equals "fixed" "6" "$(refs_of 'fixed #6')" + assert_equals "resolve" "7" "$(refs_of 'resolve #7')" + assert_equals "resolves" "8" "$(refs_of 'resolves #8')" + assert_equals "resolved" "9" "$(refs_of 'resolved #9')" +} + +test_keywords_are_case_insensitive() { + assert_equals "Closes" "10" "$(refs_of 'Closes #10')" + assert_equals "FIXES" "11" "$(refs_of 'FIXES #11')" +} + +test_a_colon_and_extra_space_are_tolerated() { + assert_equals "colon form" "12" "$(refs_of 'Closes: #12')" + assert_equals "wide space" "13" "$(refs_of 'Closes #13')" +} + +test_trailing_punctuation_is_not_part_of_the_number() { + assert_equals "full stop" "53" "$(refs_of 'Closes #53.')" + assert_equals "comma" "53" "$(refs_of 'Closes #53, and more')" +} + +test_a_bare_mention_does_not_close() { + assert_equals "bare hash" "" "$(refs_of 'See #41 for background')" +} + +test_a_keyword_inside_a_word_does_not_count() { + assert_equals "supercloses" "" "$(refs_of 'supercloses #5')" +} + +test_a_real_pull_request_body_yields_only_the_closed_issue() { + local body + body='Closes #53. First of the five sub-issues split out of #41. + +The alternatives are recorded on #41 and were rejected. + +- #46 (found during the in game check) +- The README change slightly overlaps #56.' + assert_equals "PR 58 body closes only 53" "53" "$(refs_of "$body")" +} + +test_several_references_are_all_returned() { + assert_equals "two keywords" "1,2" "$(refs_of 'Closes #1 and fixes #2')" +} + +test_a_repeated_reference_is_returned_once() { + assert_equals "deduped" "7" "$(refs_of 'Closes #7. Also closes #7.')" +} + +test_a_body_with_no_reference_is_empty_and_clean() { + assert_equals "no refs" "" "$(refs_of 'Just a description.')" + printf '%s' 'Just a description.' | closing_refs > /dev/null + assert_equals "exit status" "0" "$?" +} + +test_an_empty_body_is_empty_and_clean() { + assert_equals "empty" "" "$(refs_of '')" + printf '%s' '' | closing_refs > /dev/null + assert_equals "exit status" "0" "$?" +} + +test_issue_zero_is_rejected() { + assert_equals "hash zero" "" "$(refs_of 'Closes #0')" +} + +test_a_non_numeric_reference_is_rejected() { + assert_equals "hash word" "" "$(refs_of 'Closes #abc')" +} + +# Runs pr_numbers_from_log over log subjects and joins the result with commas. +prs_of() { + printf '%s' "$1" | pr_numbers_from_log | paste -sd, - +} + +# -- which pull requests reached a commit range -- + +test_a_merge_subject_yields_its_number() { + assert_equals "merge commit" "58" \ + "$(prs_of 'Merge pull request #58 from Blockframe-Studios/issue-53-refuse-alongside-v1')" +} + +test_a_squash_subject_yields_its_number() { + assert_equals "squash commit" "44" \ + "$(prs_of 'Tab completion matches anywhere in a name (#44)')" +} + +test_a_release_commit_yields_nothing() { + assert_equals "release commit" "" "$(prs_of 'chore(release): 1.2.3')" +} + +test_an_ordinary_commit_yields_nothing() { + assert_equals "plain commit" "" "$(prs_of 'fix: send the BukkitDev metadata with --form-string')" +} + +# A subject may cite an issue without the commit having come from that pull +# request. Only the merge and squash forms count. +test_a_mention_in_a_subject_is_not_a_pull_request() { + assert_equals "bare mention" "" "$(prs_of 'fix: address feedback on #41')" +} + +test_an_empty_range_is_empty_and_clean() { + assert_equals "empty range" "" "$(prs_of '')" + printf '%s' '' | pr_numbers_from_log > /dev/null + assert_equals "exit status" "0" "$?" +} + +test_repeated_numbers_collapse() { + local log + log='Merge pull request #31 from Blockframe-Studios/fix/bukkitdev-metadata-semicolon +Merge pull request #31 from Blockframe-Studios/fix/bukkitdev-metadata-semicolon' + assert_equals "deduped" "31" "$(prs_of "$log")" +} + +test_several_merges_keep_first_appearance_order() { + local log + log='Merge pull request #58 from Blockframe-Studios/issue-53-refuse-alongside-v1 +Merge pull request #52 from Blockframe-Studios/issue-49-import-report-colour-codes' + assert_equals "order kept" "58,52" "$(prs_of "$log")" +} + +test_each_keyword_matches +test_keywords_are_case_insensitive +test_a_colon_and_extra_space_are_tolerated +test_trailing_punctuation_is_not_part_of_the_number +test_a_bare_mention_does_not_close +test_a_keyword_inside_a_word_does_not_count +test_a_real_pull_request_body_yields_only_the_closed_issue +test_several_references_are_all_returned +test_a_repeated_reference_is_returned_once +test_a_body_with_no_reference_is_empty_and_clean +test_an_empty_body_is_empty_and_clean +test_issue_zero_is_rejected +test_a_non_numeric_reference_is_rejected +test_a_merge_subject_yields_its_number +test_a_squash_subject_yields_its_number +test_a_release_commit_yields_nothing +test_an_ordinary_commit_yields_nothing +test_a_mention_in_a_subject_is_not_a_pull_request +test_an_empty_range_is_empty_and_clean +test_repeated_numbers_collapse +test_several_merges_keep_first_appearance_order + +printf '\n%d passed, %d failed\n' "$PASS" "$FAIL" +if [ "$FAIL" -gt 0 ]; then + exit 1 +fi +exit 0