From a34a22d8b9529df4e92e1e8c3670ac8da2542ab1 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 8 Jun 2025 18:51:00 +0200 Subject: [PATCH 1/3] workflows/review-{dismissed,minimize}: init This brings back the "minimize CI reviews after dismissal" job that was previously removed. The first time around, we had a single job triggered by the `pull_request_review` event. This lacks permission to do meaningful stuff, though. This time, we trigger an empty no-op job on `pull_request_review` and then run a second workflow on `workflow_run`. This can run with the proper permissions. --- .github/workflows/dismissed-review.yml | 65 ++++++++++++++++++++++++++ .github/workflows/review-dismissed.yml | 17 +++++++ 2 files changed, 82 insertions(+) create mode 100644 .github/workflows/dismissed-review.yml create mode 100644 .github/workflows/review-dismissed.yml diff --git a/.github/workflows/dismissed-review.yml b/.github/workflows/dismissed-review.yml new file mode 100644 index 000000000000..256627af6616 --- /dev/null +++ b/.github/workflows/dismissed-review.yml @@ -0,0 +1,65 @@ +name: Dismissed review + +on: + workflow_run: + workflows: + - Review dismissed + types: [completed] + +concurrency: + group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +permissions: + pull-requests: write + +defaults: + run: + shell: bash + +jobs: + # The `check-cherry-picks` workflow creates review comments which reviewers + # are encouraged to manually dismiss if they're not relevant. + # When a CI-generated review is dismissed, this job automatically minimizes + # it, preventing it from cluttering the PR. + minimize: + name: Minimize as resolved + runs-on: ubuntu-24.04-arm + steps: + - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 + with: + script: | + // 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 + await Promise.all( + (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) + .map(async (pull_request) => + Promise.all( + (await github.paginate(github.rest.pulls.listReviews, { + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: pull_request.number + })).filter(review => + review.user.login == 'github-actions[bot]' && + review.state == 'DISMISSED' + ).map(review => github.graphql(` + mutation($node_id:ID!) { + minimizeComment(input: { + classifier: RESOLVED, + subjectId: $node_id + }) + { clientMutationId } + }`, + { node_id: review.node_id } + )) + ) + ) + ) diff --git a/.github/workflows/review-dismissed.yml b/.github/workflows/review-dismissed.yml new file mode 100644 index 000000000000..988b4a47df14 --- /dev/null +++ b/.github/workflows/review-dismissed.yml @@ -0,0 +1,17 @@ +name: Review dismissed + +on: + pull_request_review: + types: [dismissed] + +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. From 2d0bcd7165516e90df43582667ec6c1918ba48f1 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 8 Jun 2025 20:06:00 +0200 Subject: [PATCH 2/3] workflows/labels: move labels logic from eval workflow This moves the actual labeling from the eval workflow to the labels workflow. At this stage, this only has a disadvantage: Adding the topic-labels to the pull request will now only happen after eval has finished, instead of instantly. We will only benefit from this later, when we manage approval related events. With this change, we will have the comparison results and thus the package maintainer info available. --- .github/workflows/eval.yml | 50 ++++++------------------------ .github/workflows/labels.yml | 60 ++++++++++++++++++++++++++++++++++-- 2 files changed, 67 insertions(+), 43 deletions(-) diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index bb99c0dfbc68..98535776a98d 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -213,46 +213,6 @@ jobs: name: comparison path: comparison/* - - name: Labelling pull request - if: ${{ github.event_name == 'pull_request_target' }} - 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 - } - - // 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 - })) - ) - - // 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 - }) - } - - name: Add eval summary to commit statuses if: ${{ github.event_name == 'pull_request_target' }} uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 @@ -285,6 +245,16 @@ jobs: target_url }) + labels: + name: Labels + needs: [ compare ] + uses: ./.github/workflows/labels.yml + permissions: + issues: write + pull-requests: write + with: + caller: ${{ github.workflow }} + reviewers: name: Reviewers # No dependency on "compare", so that it can start at the same time. diff --git a/.github/workflows/labels.yml b/.github/workflows/labels.yml index 5cdec70b62ce..5d279d76d2be 100644 --- a/.github/workflows/labels.yml +++ b/.github/workflows/labels.yml @@ -6,14 +6,18 @@ name: "Label PR" on: - pull_request_target: + workflow_call: + inputs: + caller: + description: Name of the calling workflow. + required: true + type: string concurrency: - group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.head_ref || github.run_id }} + group: ${{ inputs.caller }}-${{ github.workflow }}-${{ github.event_name }}-${{ github.head_ref || github.run_id }} cancel-in-progress: true permissions: - contents: read issues: write # needed to create *new* labels pull-requests: write @@ -23,7 +27,55 @@ jobs: runs-on: ubuntu-24.04-arm if: "!contains(github.event.pull_request.title, '[skip treewide]')" steps: + - name: Download the comparison results + uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 + with: + pattern: comparison + path: comparison + merge-multiple: true + + - name: Labels from eval + if: ${{ 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 + } + + // 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 + })) + ) + + // 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.pull_request.head.repo.owner.login != 'NixOS' || !( github.head_ref == 'haskell-updates' || @@ -36,6 +88,7 @@ jobs: 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.pull_request.head.repo.owner.login != 'NixOS' || !( github.head_ref == 'haskell-updates' || @@ -48,6 +101,7 @@ jobs: 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. From 5f09e16f001a875e6fe7060dd49062bfe2312d76 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 8 Jun 2025 21:54:48 +0200 Subject: [PATCH 3/3] 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.