From 5f09e16f001a875e6fe7060dd49062bfe2312d76 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 8 Jun 2025 21:54:48 +0200 Subject: [PATCH] workflows/labels: manage approval labels The category 12 labels for number of approvals and approved by package maintainer are now automatically managed by the labels workflow. The logic is slightly different from the "by: package-maintainer" label. For approval, it's enough if *any one* maintainer approves the PR to have the label added, even if the PR touches multiple packages. The workflow only counts approved reviews, no matter whether there had been a push in the meantime or not. To achieve the currently used logic of "expiring approvals after push", we will have to set up a branch protection rule, which actually dismissed those reviews automatically. --- .github/workflows/labels.yml | 122 +++++++++++++++++++------ .github/workflows/review-submitted.yml | 17 ++++ 2 files changed, 111 insertions(+), 28 deletions(-) create mode 100644 .github/workflows/review-submitted.yml diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index 5d279d76d2be..6aa8ecd1b822 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -12,6 +12,11 @@ on: description: Name of the calling workflow. required: true type: string + workflow_run: + workflows: + - Review dismissed + - Review submitted + types: [completed] concurrency: group: ${{ inputs.caller }}-${{ github.workflow }}-${{ github.event_name }}-${{ github.head_ref || github.run_id }} @@ -27,56 +32,113 @@ jobs: runs-on: ubuntu-24.04-arm if: "!contains(github.event.pull_request.title, '[skip treewide]')" steps: + - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 + id: eval + with: + script: | + const run_id = (await github.rest.actions.listWorkflowRuns({ + owner: context.repo.owner, + repo: context.repo.repo, + workflow_id: 'eval.yml', + event: 'pull_request_target', + head_sha: context.payload.pull_request?.head.sha ?? context.payload.workflow_run.head_sha + })).data.workflow_runs[0]?.id + core.setOutput('run-id', run_id) + - name: Download the comparison results + if: steps.eval.outputs.run-id uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 with: + run-id: ${{ steps.eval.outputs.run-id }} + github-token: ${{ github.token }} pattern: comparison path: comparison merge-multiple: true - name: Labels from eval - if: ${{ github.event_name != 'pull_request' }} + if: steps.eval.outputs.run-id && github.event_name != 'pull_request' uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 with: script: | const { readFile } = require('node:fs/promises') - const pr = { - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: context.payload.pull_request.number + let pull_requests + if (context.payload.workflow_run) { + // PRs from forks don't have any PRs associated by default. + // Thus, we request the PR number with an API call *to* the fork's repo. + // Multiple pull requests can be open from the same head commit, either via + // different base branches or head branches. + const { head_repository, head_sha, repository } = context.payload.workflow_run + pull_requests = (await github.paginate(github.rest.repos.listPullRequestsAssociatedWithCommit, { + owner: head_repository.owner.login, + repo: head_repository.name, + commit_sha: head_sha + })).filter(pull_request => pull_request.base.repo.id == repository.id) + } else { + pull_requests = [ context.payload.pull_request ] } - // Get all currently set labels that we manage - const before = - (await github.paginate(github.rest.issues.listLabelsOnIssue, pr)) - .map(({ name }) => name) - .filter(name => name.startsWith('10.rebuild') || name == '11.by: package-maintainer') - - // And the labels that should be there - const after = JSON.parse(await readFile('comparison/changed-paths.json', 'utf-8')).labels - - // Remove the ones not needed anymore await Promise.all( - before.filter(name => !after.includes(name)) - .map(name => github.rest.issues.removeLabel({ - ...pr, - name - })) - ) + pull_requests.map(async (pull_request) => { + const pr = { + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: pull_request.number + } - // And add the ones that aren't set already - const added = after.filter(name => !before.includes(name)) - if (added.length > 0) { - await github.rest.issues.addLabels({ - ...pr, - labels: added + // Get all currently set labels that we manage + const before = + (await github.paginate(github.rest.issues.listLabelsOnIssue, pr)) + .map(({ name }) => name) + .filter(name => + name.startsWith('10.rebuild') || + name == '11.by: package-maintainer' || + name.startsWith('12.approvals:') || + name == '12.approved-by: package-maintainer' + ) + + const approvals = + (await github.paginate(github.rest.pulls.listReviews, { + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: pull_request.number + })) + .filter(review => review.state == 'APPROVED') + .map(review => review.user.id) + + const maintainers = Object.keys( + JSON.parse(await readFile('comparison/maintainers.json', 'utf-8')) + ) + + // And the labels that should be there + const after = JSON.parse(await readFile('comparison/changed-paths.json', 'utf-8')).labels + if (approvals.length > 0) after.push(`12.approvals: ${approvals.length > 2 ? '3+' : approvals.length}`) + if (maintainers.some(id => approvals.includes(id))) after.push('12.approved-by: package-maintainer') + + // Remove the ones not needed anymore + await Promise.all( + before.filter(name => !after.includes(name)) + .map(name => github.rest.issues.removeLabel({ + ...pr, + name + })) + ) + + // And add the ones that aren't set already + const added = after.filter(name => !before.includes(name)) + if (added.length > 0) { + await github.rest.issues.addLabels({ + ...pr, + labels: added + }) + } }) - } + ) - uses: actions/labeler@8558fd74291d67161a8a78ce36a881fa63b766a9 # v5.0.0 name: Labels from touched files if: | + github.event_name != 'workflow_run' && github.event.pull_request.head.repo.owner.login != 'NixOS' || !( github.head_ref == 'haskell-updates' || github.head_ref == 'python-updates' || @@ -87,9 +149,11 @@ jobs: repo-token: ${{ secrets.GITHUB_TOKEN }} configuration-path: .github/labeler.yml # default sync-labels: true + - uses: actions/labeler@8558fd74291d67161a8a78ce36a881fa63b766a9 # v5.0.0 name: Labels from touched files (no sync) if: | + github.event_name != 'workflow_run' && github.event.pull_request.head.repo.owner.login != 'NixOS' || !( github.head_ref == 'haskell-updates' || github.head_ref == 'python-updates' || @@ -100,12 +164,14 @@ jobs: repo-token: ${{ secrets.GITHUB_TOKEN }} configuration-path: .github/labeler-no-sync.yml sync-labels: false + - uses: actions/labeler@8558fd74291d67161a8a78ce36a881fa63b766a9 # v5.0.0 name: Labels from touched files (development branches) # Development branches like staging-next, haskell-updates and python-updates get special labels. # This is to avoid the mass of labels there, which is mostly useless - and really annoying for # the backport labels. if: | + github.event_name != 'workflow_run' && github.event.pull_request.head.repo.owner.login == 'NixOS' && ( github.head_ref == 'haskell-updates' || github.head_ref == 'python-updates' || diff --git a/.github/workflows/review-submitted.yml b/.github/workflows/review-submitted.yml new file mode 100644 index 000000000000..69663054f15b --- /dev/null +++ b/.github/workflows/review-submitted.yml @@ -0,0 +1,17 @@ +name: Review submitted + +on: + pull_request_review: + types: [submitted] + +permissions: {} + +defaults: + run: + shell: bash + +jobs: + trigger: + runs-on: ubuntu-24.04-arm + steps: + - run: echo This is a no-op only used as a trigger for workflow_run.