From b6bbf7b2509cf967934baa3bbe5502cdd7fa7151 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Thu, 21 Aug 2025 15:08:23 +0200 Subject: [PATCH] workflows/check: always run commits job This is the very first step to extending the commits job to do more than just cherry-picks in the future: It could check reverts or merge commits, but also the commit message format and more. Of course, cherry-picks are still just checked on the stable branches as before. For now, this allows us to run the part that dismisses automated reviews automatically. This helps us when we do branch related checks in the prepare step, which would also create such a review. To avoid cluttering multiple reviews across a PR, we'll want all of these reviews to be handled by the same code, thus this change. --- .github/workflows/check.yml | 11 ++++++----- ci/github-script/commits.js | 20 +++++++++++++------- ci/github-script/run | 12 +++++++----- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index ded8bad536c4..cace4dd2259c 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -42,10 +42,7 @@ jobs: EOF exit 1 - cherry-pick: - if: | - github.event_name == 'pull_request' || - (fromJSON(inputs.baseBranch).stable && !contains(fromJSON(inputs.headBranch).type, 'development')) + commits: permissions: pull-requests: write runs-on: ubuntu-24.04-arm @@ -65,16 +62,20 @@ jobs: GH_TOKEN: ${{ github.token }} run: gh api /rate_limit | jq - - name: Check cherry-picks + - name: Check commits id: check uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 + env: + TARGETS_STABLE: ${{ fromJSON(inputs.baseBranch).stable && !contains(fromJSON(inputs.headBranch).type, 'development') }} with: script: | + const targetsStable = JSON.parse(process.env.TARGETS_STABLE) require('./trusted/ci/github-script/commits.js')({ github, context, core, dry: context.eventName == 'pull_request', + cherryPicks: context.eventName == 'pull_request' || targetsStable, }) - name: Log current API rate limits diff --git a/ci/github-script/commits.js b/ci/github-script/commits.js index 242abda88658..92e1265c4747 100644 --- a/ci/github-script/commits.js +++ b/ci/github-script/commits.js @@ -1,4 +1,4 @@ -module.exports = async ({ github, context, core, dry }) => { +module.exports = async ({ github, context, core, dry, cherryPicks }) => { const { execFileSync } = require('node:child_process') const { classify } = require('../supportedBranches.js') const withRateLimit = require('./withRateLimit.js') @@ -16,7 +16,7 @@ module.exports = async ({ github, context, core, dry }) => { run_id: context.runId, per_page: 100, }) - ).find(({ name }) => name.endsWith('Check / cherry-pick')).html_url + + ).find(({ name }) => name.endsWith('Check / commits')).html_url + '?pr=' + pull_number @@ -137,10 +137,14 @@ module.exports = async ({ github, context, core, dry }) => { } } - const commits = await github.paginate(github.rest.pulls.listCommits, { - ...context.repo, - pull_number, - }) + // For now we short-circuit the list of commits when cherryPicks should not be checked. + // This will not run any checks, but still trigger the "dismiss reviews" part below. + const commits = !cherryPicks + ? [] + : await github.paginate(github.rest.pulls.listCommits, { + ...context.repo, + pull_number, + }) const extracted = await Promise.all(commits.map(extract)) @@ -185,6 +189,8 @@ module.exports = async ({ github, context, core, dry }) => { // Only create step summary below in case of warnings or errors. // Also clean up older reviews, when all checks are good now. + // An empty results array will always trigger this condition, which is helpful + // to clean up reviews created by the prepare step when on the wrong branch. if (results.every(({ severity }) => severity === 'info')) { if (!dry) { await Promise.all( @@ -201,7 +207,7 @@ module.exports = async ({ github, context, core, dry }) => { ...context.repo, pull_number, review_id: review.id, - message: 'All cherry-picks are good now, thank you!', + message: 'All good now, thank you!', }) } await github.graphql( diff --git a/ci/github-script/run b/ci/github-script/run index ae107df73b51..17dac5ab3a2d 100755 --- a/ci/github-script/run +++ b/ci/github-script/run @@ -7,7 +7,7 @@ import { program } from 'commander' import * as core from '@actions/core' import { getOctokit } from '@actions/github' -async function run(action, owner, repo, pull_number, dry = true) { +async function run(action, owner, repo, pull_number, options = {}) { const token = execSync('gh auth token', { encoding: 'utf-8' }).trim() const github = getOctokit(token) @@ -35,7 +35,8 @@ async function run(action, owner, repo, pull_number, dry = true) { }, }, core, - dry, + dry: true, + ...options, }) } @@ -56,9 +57,10 @@ program .argument('', 'Owner of the GitHub repository to check (Example: NixOS)') .argument('', 'Name of the GitHub repository to check (Example: nixpkgs)') .argument('', 'Number of the Pull Request to check') - .action(async (owner, repo, pr) => { + .option('--no-cherry-picks', 'Do not expect cherry-picks.') + .action(async (owner, repo, pr, options) => { const commits = (await import('./commits.js')).default - run(commits, owner, repo, pr) + run(commits, owner, repo, pr, options) }) program @@ -74,7 +76,7 @@ program try { process.env.GITHUB_WORKSPACE = tmp process.chdir(tmp) - run(labels, owner, repo, pr, options.dry) + run(labels, owner, repo, pr, options) } finally { rmSync(tmp, { recursive: true }) }