Unverified Commit 3334170f authored by Philip Taron's avatar Philip Taron Committed by GitHub
Browse files

{workflows/eval,ci/github-script}: check for mass and NixOS test rebuilds... 

{workflows/eval,ci/github-script}: check for mass and NixOS test rebuilds targeting master/release-* branches (#481205)
parents 73f0bc1a a4d5f8a6

.github/workflows/eval.yml

+12 −0
Original line number Diff line number Diff line
@@ -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:

.github/workflows/merge-group.yml

+1 −0
Original line number Diff line number Diff line
@@ -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 }}

.github/workflows/pull-request-target.yml

+1 −0
Original line number Diff line number Diff line
@@ -81,6 +81,7 @@ jobs: 
    uses: ./.github/workflows/eval.yml

    permissions:

      # compare

      pull-requests: write

      statuses: write

    with:

      artifact-prefix: ${{ inputs.artifact-prefix }}

ci/github-script/check-target-branch.js

0 → 100644
+135 −0
Original line number Diff line number Diff line 
/// @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<import('@actions/github/lib/utils').GitHub>,

 *  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

ci/github-script/reviews.js

+25 −8
Original line number Diff line number Diff line 
const eventToState = {

  COMMENT: 'COMMENTED',

  REQUEST_CHANGES: 'CHANGES_REQUESTED',

}



/**

 * @param {{

 *  github: InstanceType<import('@actions/github/lib/utils').GitHub>,

 *  context: import('@actions/github/lib/context').Context

 *  core: import('@actions/core')

 *  dry: boolean

 * }} CheckTargetBranchProps

 */

async function dismissReviews({ github, context, dry }) {

  const pull_number = context.payload.pull_request.number
@@ -19,13 +32,13 @@ async function dismissReviews({ github, context, dry }) { 
            ...context.repo,

            pull_number,

            review_id: review.id,

            message: 'All good now, thank you!',

            message: 'Review dismissed automatically',

          })

        }

        await github.graphql(

          `mutation($node_id:ID!) {

            minimizeComment(input: {

              classifier: RESOLVED,

              classifier: OUTDATED,

              subjectId: $node_id

            })

            { clientMutationId }
@@ -36,7 +49,14 @@ async function dismissReviews({ github, context, dry }) { 
  )

}



async function postReview({ github, context, core, dry, body }) {

async function postReview({

  github,

  context,

  core,

  dry,

  body,

  event = 'REQUEST_CHANGES',

}) {

  const pull_number = context.payload.pull_request.number



  const pendingReview = (
@@ -49,10 +69,7 @@ async function postReview({ github, context, core, dry, body }) { 
      review.user?.login === 'github-actions[bot]' &&

      // If a review is still pending, we can just update this instead

      // of posting a new one.

      (review.state === 'CHANGES_REQUESTED' ||

        // No need to post a new review, if an older one with the exact

        // same content had already been dismissed.

        review.body === body),

      review.state === eventToState[event],

  )



  if (dry) {
@@ -72,7 +89,7 @@ async function postReview({ github, context, core, dry, body }) { 
      await github.rest.pulls.createReview({

        ...context.repo,

        pull_number,

        event: 'REQUEST_CHANGES',

        event,

        body,

      })

    }