nathan/nixpkgs
1
0
Fork 0
mirror of https://github.com/NixOS/nixpkgs.git synced 2026-06-05 21:03:40 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert "{ci,workflows}: allow multiple blocking reviews"

Browse Source
This commit is contained in:
Michael Daniels
2026-05-03 13:21:39 -04:00
committed by GitHub
parent 8b6a8ead92
commit cd2e5a371b
7 changed files with 63 additions and 192 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
.github/workflows/check.yml vendored
View File

@@ -16,14 +16,6 @@ on:
required: true
type: string
secrets:
# Can be provided in pull requests because the job it is used in does
# not evaluate untrusted code.
NIXPKGS_COMMIT_CHECK_APP_PRIVATE_KEY:
required: false
# Can be provided in pull requests because the job it is used in does
# not evaluate untrusted code.
NIXPKGS_MANUAL_EDIT_CHECK_APP_PRIVATE_KEY:
required: false
# Should only be provided in the merge queue, not in pull requests,
# where we're evaluating untrusted code.
CACHIX_AUTH_TOKEN_GHA:
@@ -53,19 +45,9 @@ jobs:
- name: Install dependencies
run: npm install bottleneck@2.19.5
# It's fine to reuse this app in the 'pull-request-target / prepare' job,
# because that job has to run before this one.
- uses: actions/create-github-app-token@1b10c78c7865c340bc4f6099eb2f838309f1e8c3 # v3.1.1
if: github.event_name != 'pull_request' && vars.NIXPKGS_COMMIT_CHECK_CLIENT_ID
id: app-token
with:
client-id: ${{ vars.NIXPKGS_COMMIT_CHECK_CLIENT_ID }}
private-key: ${{ secrets.NIXPKGS_COMMIT_CHECK_APP_PRIVATE_KEY }}
permission-pull-requests: write
- name: Log current API rate limits
env:
GH_TOKEN: ${{ steps.app-token.outputs.token || github.token }}
GH_TOKEN: ${{ github.token }}
run: gh api /rate_limit | jq
- name: Check commits
@@ -74,7 +56,6 @@ jobs:
env:
TARGETS_STABLE: ${{ fromJSON(inputs.baseBranch).stable && !contains(fromJSON(inputs.headBranch).type, 'development') }}
with:
github-token: ${{ steps.app-token.outputs.token || github.token }}
script: |
const targetsStable = JSON.parse(process.env.TARGETS_STABLE)
require('./trusted/ci/github-script/commits.js')({
@@ -87,7 +68,7 @@ jobs:
- name: Log current API rate limits
env:
GH_TOKEN: ${{ steps.app-token.outputs.token || github.token }}
GH_TOKEN: ${{ github.token }}
run: gh api /rate_limit | jq
manual-file-edits:
@@ -104,35 +85,25 @@ jobs:
sparse-checkout: |
ci/github-script
- uses: actions/create-github-app-token@1b10c78c7865c340bc4f6099eb2f838309f1e8c3 # v3.1.1
if: github.event_name != 'pull_request' && vars.NIXPKGS_MANUAL_EDIT_CHECK_CLIENT_ID
id: app-token
with:
client-id: ${{ vars.NIXPKGS_MANUAL_EDIT_CHECK_CLIENT_ID }}
private-key: ${{ secrets.NIXPKGS_MANUAL_EDIT_CHECK_APP_PRIVATE_KEY }}
permission-pull-requests: write
- name: Log current API rate limits
env:
GH_TOKEN: ${{ steps.app-token.outputs.token || github.token }}
GH_TOKEN: ${{ github.token }}
run: gh api /rate_limit | jq
- name: Discourage manual edits to certain files
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
with:
github-token: ${{ steps.app-token.outputs.token || github.token }}
script: |
require('./trusted/ci/github-script/manual-file-edits.js')({
github,
context,
core,
dry: context.eventName == 'pull_request',
repoPath: 'trusted',
})
- name: Log current API rate limits
env:
GH_TOKEN: ${{ steps.app-token.outputs.token || github.token }}
GH_TOKEN: ${{ github.token }}
run: gh api /rate_limit | jq
owners:

14
.github/workflows/eval.yml vendored
View File

@@ -23,10 +23,6 @@ on:
default: false
type: boolean
secrets:
# Can be provided in pull requests because the job it is used in does
# not evaluate untrusted code.
NIXPKGS_BRANCH_CHECK_APP_PRIVATE_KEY:
required: false
# Should only be provided in the merge queue, not in pull requests,
# where we're evaluating untrusted code.
CACHIX_AUTH_TOKEN_GHA:
@@ -353,20 +349,10 @@ jobs:
description,
target_url
})
- uses: actions/create-github-app-token@1b10c78c7865c340bc4f6099eb2f838309f1e8c3 # v3.1.1
if: github.event_name == 'pull_request_target' && vars.NIXPKGS_BRANCH_CHECK_CLIENT_ID
id: app-token
with:
client-id: ${{ vars.NIXPKGS_BRANCH_CHECK_CLIENT_ID }}
private-key: ${{ secrets.NIXPKGS_BRANCH_CHECK_APP_PRIVATE_KEY }}
permission-pull-requests: write
- name: Request changes if PR is against an inappropriate branch
if: ${{ github.event_name == 'pull_request_target' }}
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
with:
github-token: ${{ steps.app-token.outputs.token || github.token }}
script: |
require('./nixpkgs/trusted/ci/github-script/check-target-branch.js')({
github,

22
.github/workflows/pull-request-target.yml vendored
View File

@@ -10,12 +10,6 @@ on:
secrets:
NIXPKGS_CI_APP_PRIVATE_KEY:
required: true
NIXPKGS_BRANCH_CHECK_APP_PRIVATE_KEY:
required: false
NIXPKGS_COMMIT_CHECK_APP_PRIVATE_KEY:
required: false
NIXPKGS_MANUAL_EDIT_CHECK_APP_PRIVATE_KEY:
required: false
concurrency:
group: pr-${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.run_id }}
@@ -42,17 +36,6 @@ jobs:
sparse-checkout-cone-mode: true # default, for clarity
sparse-checkout: |
ci/github-script
# It's fine to reuse this app in the 'check / commits' job,
# because this job has to run before that one.
- uses: actions/create-github-app-token@1b10c78c7865c340bc4f6099eb2f838309f1e8c3 # v3.1.1
if: vars.NIXPKGS_COMMIT_CHECK_CLIENT_ID
id: app-token
with:
client-id: ${{ vars.NIXPKGS_COMMIT_CHECK_CLIENT_ID }}
private-key: ${{ secrets.NIXPKGS_COMMIT_CHECK_APP_PRIVATE_KEY }}
permission-pull-requests: write
- id: prepare
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
with:
@@ -77,9 +60,6 @@ jobs:
permissions:
# cherry-picks
pull-requests: write
secrets:
NIXPKGS_COMMIT_CHECK_APP_PRIVATE_KEY: ${{ secrets.NIXPKGS_COMMIT_CHECK_APP_PRIVATE_KEY }}
NIXPKGS_MANUAL_EDIT_CHECK_APP_PRIVATE_KEY: ${{ secrets.NIXPKGS_MANUAL_EDIT_CHECK_APP_PRIVATE_KEY }}
with:
baseBranch: ${{ needs.prepare.outputs.baseBranch }}
headBranch: ${{ needs.prepare.outputs.headBranch }}
@@ -102,8 +82,6 @@ jobs:
# compare
pull-requests: write
statuses: write
secrets:
NIXPKGS_BRANCH_CHECK_APP_PRIVATE_KEY: ${{ secrets.NIXPKGS_BRANCH_CHECK_APP_PRIVATE_KEY }}
with:
artifact-prefix: ${{ inputs.artifact-prefix }}
mergedSha: ${{ needs.prepare.outputs.mergedSha }}

10
ci/github-script/check-target-branch.js
View File

@@ -151,9 +151,11 @@ async function checkTargetBranch({ github, context, core, dry }) {
core,
dry,
body,
event: 'REQUEST_CHANGES',
event: 'COMMENT',
reviewKey,
})
throw new Error('This PR is against the wrong branch.')
} else if (rebuildsAllTests && !isExemptKernelUpdate) {
let branchText
if (base === 'master' && maxRebuildCount >= 500) {
@@ -177,9 +179,11 @@ async function checkTargetBranch({ github, context, core, dry }) {
core,
dry,
body,
event: 'REQUEST_CHANGES',
event: 'COMMENT',
reviewKey,
})
throw new Error('This PR is against the wrong branch.')
} else if (
maxRebuildCount >= 500 &&
!isExemptKernelUpdate &&
@@ -200,7 +204,7 @@ async function checkTargetBranch({ github, context, core, dry }) {
core,
dry,
body,
event: 'REQUEST_CHANGES',
event: 'COMMENT',
reviewKey,
})
} else {

26
ci/github-script/manual-file-edits.js
View File

@@ -7,13 +7,9 @@ const { getCommitDetailsForPR } = require('./get-pr-commit-details')
* context: import('@actions/github/lib/context').Context,
* core: import('@actions/core'),
* repoPath?: string,
* dry: boolean,
* }} CheckManualFileEditsProps
*/
async function checkManualFileEdits({ github, context, core, repoPath, dry }) {
const { dismissReviews, postReview } = require('./reviews.js')
const reviewKey = 'manual-file-edits'
async function checkManualFileEdits({ github, context, core, repoPath }) {
const pull_number = context.payload.pull_request?.number
if (!pull_number) {
core.info('This is not a pull request. Skipping checks.')
@@ -39,13 +35,8 @@ async function checkManualFileEdits({ github, context, core, repoPath, dry }) {
changedPaths.includes('maintainers/github-teams.json'),
)
) {
postReview({
github,
context,
core,
dry,
event: 'REQUEST_CHANGES',
body: [
core.setFailed(
[
'maintainers/github-teams.json is supposed to accurately reflect the state of the teams in GitHub.\n',
'Therefore, it should not be edited manually.\n',
'All changes to teams listed in maintainers/github-teams.json should be performed in GitHub by a team maintainer.\n',
@@ -57,16 +48,7 @@ async function checkManualFileEdits({ github, context, core, repoPath, dry }) {
(prev, curr) => prev + (!prev || prev.endsWith('\n') ? '' : ' ') + curr,
'',
),
reviewKey,
})
} else {
dismissReviews({
github,
context,
core,
dry,
reviewKey,
})
)
}
}

16
ci/github-script/prepare.js
View File

@@ -172,20 +172,14 @@ module.exports = async ({ github, context, core, dry }) => {
' ```',
].join('\n')
await postReview({
github,
context,
core,
dry,
body,
event: 'REQUEST_CHANGES',
reviewKey,
})
await postReview({ github, context, core, dry, body, reviewKey })
throw new Error(`The PR contains commits from a different base.`)
}
} else {
await dismissReviews({ github, context, core, dry, reviewKey })
}
await dismissReviews({ github, context, core, dry, reviewKey })
let mergedSha, targetSha
if (prInfo.mergeable) {

130
ci/github-script/reviews.js
View File

@@ -5,28 +5,10 @@ const eventToState = {
REQUEST_CHANGES: 'CHANGES_REQUESTED',
}
// Use substring checks in order to allow testing in forks
// Usernames must also end in "[bot]"
const reviewUsers = [
'github-actions',
'nixpkgs-ci',
'branch-check',
'commit-check',
'manual-edit',
]
/**
* @typedef {InstanceType<import('@actions/github/lib/utils').GitHub>} GitHub
* @typedef {typeof import('@actions/github').context} Context
*
* @typedef {Awaited<ReturnType<GitHub['rest']['pulls']['listReviews']>>['data'][number]} Review
* @typedef {Review & { user: NonNullable<Review['user']> }} ReviewWithNonNullUser
*/
/**
* @param {{
* github: GitHub,
* context: Context,
* github: InstanceType<import('@actions/github/lib/utils').GitHub>,
* context: import('@actions/github/lib/context').Context,
* core: import('@actions/core'),
* dry: boolean,
* reviewKey?: string,
@@ -43,32 +25,18 @@ async function dismissReviews({ github, context, core, dry, reviewKey }) {
return
}
const allReviews = await github.paginate(github.rest.pulls.listReviews, {
...context.repo,
pull_number,
})
const reviews = /** @type {ReviewWithNonNullUser[]} */ (
allReviews.filter(
(review) =>
review.user &&
review.state !== 'DISMISSED' &&
review.user.login.endsWith('[bot]') &&
reviewUsers.some((substr) => review.user?.login.includes(substr)),
)
const reviews = (
await github.paginate(github.rest.pulls.listReviews, {
...context.repo,
pull_number,
})
).filter(
(review) =>
review.user?.login === 'github-actions[bot]' &&
review.state !== 'DISMISSED',
)
const reviewsByUser = reviews.reduce(
(prev, curr) => {
if (!(curr.user.login in prev)) {
prev[curr.user.login] = []
}
prev[curr.user.login].push(curr)
return prev
},
/** @type {Record<string, ReviewWithNonNullUser[]> } */ ({}),
const changesRequestedReviews = reviews.filter(
(review) => review.state === 'CHANGES_REQUESTED',
)
const commentRegex = new RegExp(
@@ -82,8 +50,8 @@ async function dismissReviews({ github, context, core, dry, reviewKey }) {
)
let reviewsToMinimize = reviews
const /** @type {ReviewWithNonNullUser[]} */ reviewsToDismiss = []
const /** @type {ReviewWithNonNullUser[]} */ reviewsToResolve = []
let /** @type {typeof reviews} */ reviewsToDismiss = []
let /** @type {typeof reviews} */ reviewsToResolve = []
if (reviewKey && reviews.every((review) => commentRegex.test(review.body))) {
reviewsToMinimize = reviews.filter((review) =>
@@ -91,39 +59,29 @@ async function dismissReviews({ github, context, core, dry, reviewKey }) {
)
}
for (const reviewsForUser of Object.values(reviewsByUser)) {
// Make sure that we don't dismiss all reviews by a user if they
// have any reviews we don't want to dismiss.
if (
reviewsForUser.every(
(review) =>
commentResolvedRegex.test(review.body) ||
(reviewKey && reviewKeyRegex.test(review.body)) ||
// If we are called by check-commits and the review body is clearly
// from `commits.js`, then we can safely dismiss the review.
// This helps with pre-existing reviews (before the comments were added).
(reviewKey &&
reviewKey === 'check-commits' &&
review.body.includes('PR / Check / cherry-pick')),
)
) {
reviewsToDismiss.push(
...reviewsForUser.filter(
(review) => review.state === 'CHANGES_REQUESTED',
),
)
} else {
reviewsToResolve.push(
...reviewsForUser.filter(
(review) =>
review.state === 'CHANGES_REQUESTED' &&
!commentResolvedRegex.test(review.body) &&
reviewsToMinimize.some(
(toMinimize) => toMinimize.node_id === review.node_id,
),
),
)
}
// If we want to dismiss all reviews with the key reviewKey,
// but there are other requested changes from CI, we can't dismiss,
// because then the other requested changes will be dismissed too.
if (
changesRequestedReviews.every(
(review) =>
commentResolvedRegex.test(review.body) ||
(reviewKey && reviewKeyRegex.test(review.body)) ||
// If we are called by check-commits and the review body is clearly
// from `commits.js`, then we can safely dismiss the review.
// This helps with pre-existing reviews (before the comments were added).
(reviewKey &&
reviewKey === 'check-commits' &&
review.body.includes('PR / Check / cherry-pick')),
)
) {
reviewsToDismiss = changesRequestedReviews
} else if (reviewsToMinimize.length) {
reviewsToResolve = reviewsToMinimize.filter(
(review) =>
review.state === 'CHANGES_REQUESTED' &&
!commentResolvedRegex.test(review.body),
)
}
await Promise.all([
@@ -163,8 +121,8 @@ async function dismissReviews({ github, context, core, dry, reviewKey }) {
/**
* @param {{
* github: GitHub,
* context: Context,
* github: InstanceType<import('@actions/github/lib/utils').GitHub>,
* context: import('@actions/github/lib/context').Context
* core: import('@actions/core'),
* dry: boolean,
* body: string,
@@ -200,13 +158,11 @@ async function postReview({
})
).filter(
(review) =>
review.user &&
review.state !== 'DISMISSED' &&
review.user.login.endsWith('[bot]') &&
reviewUsers.some((substr) => review.user?.login.includes(substr)),
review.user?.login === 'github-actions[bot]' &&
review.state !== 'DISMISSED',
)
/** @type {null | Review} */
/** @type {null | typeof reviews[number]} */
let pendingReview
const matchingReviews = reviews.filter((review) =>
reviewKeyRegex.test(review.body),