From a4d5f8a6b11edf4c66858fb4dcd9a1bfde49df60 Mon Sep 17 00:00:00 2001 From: Michael Daniels Date: Sat, 17 Jan 2026 19:42:08 -0500 Subject: [PATCH] {workflows/eval,ci/github-script}: check for mass rebuilds targeting master/release-* branches --- .github/workflows/eval.yml | 12 ++ .github/workflows/merge-group.yml | 1 + .github/workflows/pull-request-target.yml | 1 + ci/github-script/check-target-branch.js | 135 ++++++++++++++++++++++ ci/github-script/run | 11 ++ 5 files changed, 160 insertions(+) create mode 100644 ci/github-script/check-target-branch.js diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 28b5e8c99f61..390f19d96e2f 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -168,6 +168,7 @@ jobs: needs: [eval] if: ${{ !cancelled() && !failure() }} permissions: + pull-requests: write statuses: write timeout-minutes: 5 steps: @@ -254,6 +255,17 @@ jobs: description, target_url }) + - name: Request changes if PR is against an inappropriate branch + if: ${{ github.event_name == 'pull_request_target' }} + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + with: + script: | + require('./nixpkgs/trusted/ci/github-script/check-target-branch.js')({ + github, + context, + core, + dry: context.eventName == 'pull_request', + }) # Creates a matrix of Eval performance for various versions and systems. report: diff --git a/.github/workflows/merge-group.yml b/.github/workflows/merge-group.yml index 299f56c09dfe..0720817c6da7 100644 --- a/.github/workflows/merge-group.yml +++ b/.github/workflows/merge-group.yml @@ -84,6 +84,7 @@ jobs: # even though they are unused when working with the merge queue. permissions: # compare + pull-requests: write statuses: write secrets: CACHIX_AUTH_TOKEN_GHA: ${{ secrets.CACHIX_AUTH_TOKEN_GHA }} diff --git a/.github/workflows/pull-request-target.yml b/.github/workflows/pull-request-target.yml index 9609a440ae89..c49e2ae60d37 100644 --- a/.github/workflows/pull-request-target.yml +++ b/.github/workflows/pull-request-target.yml @@ -81,6 +81,7 @@ jobs: uses: ./.github/workflows/eval.yml permissions: # compare + pull-requests: write statuses: write with: artifact-prefix: ${{ inputs.artifact-prefix }} diff --git a/ci/github-script/check-target-branch.js b/ci/github-script/check-target-branch.js new file mode 100644 index 000000000000..2b671715d37d --- /dev/null +++ b/ci/github-script/check-target-branch.js @@ -0,0 +1,135 @@ +/// @ts-check + +// TODO: should this be combined with the branch checks in prepare.js? +// They do seem quite similar, but this needs to run after eval, +// and prepare.js obviously doesn't. + +const { classify, split } = require('../supportedBranches.js') +const { readFile } = require('node:fs/promises') +const { postReview } = require('./reviews.js') + +/** + * @param {{ + * github: InstanceType, + * context: import('@actions/github/lib/context').Context + * core: import('@actions/core') + * dry: boolean + * }} CheckTargetBranchProps + */ +async function checkTargetBranch({ github, context, core, dry }) { + const changed = JSON.parse( + await readFile('comparison/changed-paths.json', 'utf-8'), + ) + const pull_number = context.payload.pull_request?.number + if (!pull_number) { + core.warning( + 'Skipping checkTargetBranch: no pull_request number (is this being run as part of a merge group?)', + ) + return + } + const prInfo = ( + await github.rest.pulls.get({ + ...context.repo, + pull_number, + }) + ).data + const base = prInfo.base.ref + const head = prInfo.head.ref + const baseClassification = classify(base) + const headClassification = classify(head) + + // Don't run on, e.g., staging-nixos to master merges. + if (headClassification.type.includes('development')) { + core.info( + `Skipping checkTargetBranch: PR is from a development branch (${head})`, + ) + return + } + // Don't run on PRs against staging branches, wip branches, haskell-updates, etc. + if (!baseClassification.type.includes('primary')) { + core.info( + `Skipping checkTargetBranch: PR is against a non-primary base branch (${base})`, + ) + return + } + + const maxRebuildCount = Math.max( + ...Object.values(changed.rebuildCountByKernel), + ) + const rebuildsAllTests = + changed.attrdiff.changed.includes('nixosTests.simple') ?? false + core.info( + `checkTargetBranch: PR causes ${maxRebuildCount} rebuilds and ${rebuildsAllTests ? 'does' : 'does not'} rebuild all NixOS tests.`, + ) + + if (maxRebuildCount >= 1000) { + const desiredBranch = + base === 'master' ? 'staging' : `staging-${split(base).version}` + const body = [ + `The PR's base branch is set to \`${base}\`, but this PR causes ${maxRebuildCount} rebuilds.`, + 'It is therefore considered a mass rebuild.', + `Please [change the base branch](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-base-branch-of-a-pull-request) to [the right base branch for your changes](https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#branch-conventions) (probably \`${desiredBranch}\`).`, + ].join('\n') + + await postReview({ + github, + context, + core, + dry, + body, + event: 'REQUEST_CHANGES', + }) + + throw new Error('This PR is against the wrong branch.') + } else if (rebuildsAllTests) { + let branchText + if (base === 'master' && maxRebuildCount >= 500) { + branchText = '(probably either `staging-nixos` or `staging`)' + } else if (base === 'master') { + branchText = '(probably `staging-nixos`)' + } else { + branchText = `(probably \`staging-${split(base).version}\`)` + } + const body = [ + `The PR's base branch is set to \`${base}\`, but this PR rebuilds all NixOS tests.`, + base === 'master' && maxRebuildCount >= 500 + ? `Since this PR also causes ${maxRebuildCount} rebuilds, it may also be considered a mass rebuild.` + : '', + `Please [change the base branch](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-base-branch-of-a-pull-request) to [the right base branch for your changes](https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#branch-conventions) ${branchText}.`, + ].join('\n') + + await postReview({ + github, + context, + core, + dry, + body, + event: 'REQUEST_CHANGES', + }) + + throw new Error('This PR is against the wrong branch.') + } else if (maxRebuildCount >= 500) { + const stagingBranch = + base === 'master' ? 'staging' : `staging-${split(base).version}` + const body = [ + `The PR's base branch is set to \`${base}\`, and this PR causes ${maxRebuildCount} rebuilds.`, + `Please consider whether this PR causes a mass rebuild according to [our conventions](https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#branch-conventions).`, + `If it does cause a mass rebuild, please [change the base branch](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-base-branch-of-a-pull-request) to [the right base branch for your changes](https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#branch-conventions) (probably \`${stagingBranch}\`).`, + `If it does not cause a mass rebuild, this message can be ignored.`, + ].join('\n') + + await postReview({ + github, + context, + core, + dry, + body, + event: 'COMMENT', + }) + } else { + // Any existing reviews were dismissed by commits.js + core.info('checkTargetBranch: this PR is against an appropriate branch.') + } +} + +module.exports = checkTargetBranch diff --git a/ci/github-script/run b/ci/github-script/run index 095768f317da..744e49f018a4 100755 --- a/ci/github-script/run +++ b/ci/github-script/run @@ -105,4 +105,15 @@ program await run(checkCommitMessages, owner, repo, pr, options) }) +program + .command('check-target-branch') + .description('Check that the PR is made against the correct branch') + .argument('', 'Owner of the GitHub repository to run on (Example: NixOS)') + .argument('', 'Name of the GitHub repository to run on (Example: nixpkgs)') + .argument('', 'Number of the Pull Request to run on') + .action(async (owner, repo, pr, options) => { + const checkCommitMessages = (await import('./check-target-branch.js')).default + await run(checkCommitMessages, owner, repo, pr, options) + }) + await program.parse()