From 69ab2f4347f82e3d28932e71ed5ef8630a41a19c Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Fri, 20 Jun 2025 09:22:17 +0200 Subject: [PATCH 1/5] workflows/eval: small improvements Some naming improvements after we introduced the PR / Push workflows and small refactors. --- .github/workflows/eval.yml | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index f7ce71e4dde1f..c17b1304deca9 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -92,14 +92,13 @@ jobs: let run_id try { run_id = (await github.rest.actions.listWorkflowRuns({ - owner: context.repo.owner, - repo: context.repo.repo, + ...context.repo, workflow_id: 'push.yml', event: 'push', head_sha: targetSha })).data.workflow_runs[0].id } catch { - throw new Error(`Could not find an push.yml workflow run for ${targetSha}.`) + throw new Error(`Could not find a push.yml workflow run for ${targetSha}.`) } core.setOutput('targetRunId', run_id) @@ -108,8 +107,7 @@ jobs: // Eval takes max 5-6 minutes, normally. for (let i = 0; i < 120; i++) { const result = await github.rest.actions.listWorkflowRunArtifacts({ - owner: context.repo.owner, - repo: context.repo.repo, + ...context.repo, run_id, name: `merged-${system}` }) @@ -224,10 +222,9 @@ jobs: `${serverUrl}/${repo.owner}/${repo.repo}/actions/runs/${runId}?pr=${payload.pull_request.number}` await github.rest.repos.createCommitStatus({ - owner: repo.owner, - repo: repo.repo, + ...repo, sha: payload.pull_request.head.sha, - context: 'Eval / Summary', + context: 'Eval Summary', state: 'success', description, target_url From 9422f30e470dbb9d1925306fc36a674475894ee1 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Wed, 18 Jun 2025 20:23:14 +0200 Subject: [PATCH 2/5] workflows/{pr,push}: move prepare job from eval workflow This is only a refactor at this stage, but split into a separate commit for better review. It's the base for the next two commits. --- .github/workflows/eval.yml | 53 ++++++++++++++------------------------ .github/workflows/pr.yml | 26 +++++++++++++++++++ .github/workflows/push.yml | 19 ++++++++++++++ 3 files changed, 65 insertions(+), 33 deletions(-) diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index c17b1304deca9..2db06fd9937c3 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -2,6 +2,15 @@ name: Eval on: workflow_call: + inputs: + mergedSha: + required: true + type: string + targetSha: + type: string + systems: + required: true + type: string secrets: OWNER_APP_PRIVATE_KEY: required: false @@ -13,34 +22,12 @@ defaults: shell: bash jobs: - prepare: - runs-on: ubuntu-24.04-arm - outputs: - mergedSha: ${{ steps.get-merge-commit.outputs.mergedSha }} - targetSha: ${{ steps.get-merge-commit.outputs.targetSha }} - systems: ${{ steps.systems.outputs.systems }} - steps: - - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - with: - sparse-checkout: | - .github/actions - ci/supportedSystems.json - - name: Check if the PR can be merged and get the test merge commit - uses: ./.github/actions/get-merge-commit - id: get-merge-commit - - - name: Load supported systems - id: systems - run: | - echo "systems=$(jq -c > "$GITHUB_OUTPUT" - eval: runs-on: ubuntu-24.04-arm - needs: [prepare] strategy: fail-fast: false matrix: - system: ${{ fromJSON(needs.prepare.outputs.systems) }} + system: ${{ fromJSON(inputs.systems) }} name: ${{ matrix.system }} steps: - name: Enable swap @@ -53,7 +40,7 @@ jobs: - name: Check out the PR at the test merge commit uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - ref: ${{ needs.prepare.outputs.mergedSha }} + ref: ${{ inputs.mergedSha }} path: untrusted - name: Install Nix @@ -78,12 +65,12 @@ jobs: path: merged/* - name: Get target run id - if: needs.prepare.outputs.targetSha + if: inputs.targetSha id: targetRunId uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 env: MATRIX_SYSTEM: ${{ matrix.system }} - TARGET_SHA: ${{ needs.prepare.outputs.targetSha }} + TARGET_SHA: ${{ inputs.targetSha }} with: script: | const system = process.env.MATRIX_SYSTEM @@ -145,8 +132,8 @@ jobs: compare: runs-on: ubuntu-24.04-arm - needs: [prepare, eval] - if: needs.prepare.outputs.targetSha + needs: [eval] + if: inputs.targetSha permissions: issues: write # needed to create *new* labels pull-requests: write @@ -162,7 +149,7 @@ jobs: - name: Check out the PR at the target commit uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - ref: ${{ needs.prepare.outputs.targetSha }} + ref: ${{ inputs.targetSha }} path: trusted - name: Install Nix @@ -180,8 +167,8 @@ jobs: env: AUTHOR_ID: ${{ github.event.pull_request.user.id }} run: | - git -C trusted fetch --depth 1 origin ${{ needs.prepare.outputs.mergedSha }} - git -C trusted diff --name-only ${{ needs.prepare.outputs.mergedSha }} \ + git -C trusted fetch --depth 1 origin ${{ inputs.mergedSha }} + git -C trusted diff --name-only ${{ inputs.mergedSha }} \ | jq --raw-input --slurp 'split("\n")[:-1]' > touched-files.json # Use the target branch to get accurate maintainer info @@ -243,8 +230,8 @@ jobs: # No dependency on "compare", so that it can start at the same time. # We only wait for the "comparison" artifact to be available, which makes the start-to-finish time # for the eval workflow considerably faster. - needs: [prepare, eval] - if: needs.prepare.outputs.targetSha + needs: [eval] + if: inputs.targetSha uses: ./.github/workflows/reviewers.yml secrets: OWNER_APP_PRIVATE_KEY: ${{ secrets.OWNER_APP_PRIVATE_KEY }} diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 924a60d3752d0..df2573b394148 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -18,6 +18,27 @@ concurrency: permissions: {} jobs: + prepare: + runs-on: ubuntu-24.04-arm + outputs: + mergedSha: ${{ steps.get-merge-commit.outputs.mergedSha }} + targetSha: ${{ steps.get-merge-commit.outputs.targetSha }} + systems: ${{ steps.systems.outputs.systems }} + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + sparse-checkout: | + .github/actions + ci/supportedSystems.json + - name: Check if the PR can be merged and get the test merge commit + uses: ./.github/actions/get-merge-commit + id: get-merge-commit + + - name: Load supported systems + id: systems + run: | + echo "systems=$(jq -c > "$GITHUB_OUTPUT" + check: name: Check uses: ./.github/workflows/check.yml @@ -31,6 +52,7 @@ jobs: eval: name: Eval + needs: [prepare] uses: ./.github/workflows/eval.yml permissions: # compare @@ -39,6 +61,10 @@ jobs: statuses: write secrets: OWNER_APP_PRIVATE_KEY: ${{ secrets.OWNER_APP_PRIVATE_KEY }} + with: + mergedSha: ${{ needs.prepare.outputs.mergedSha }} + targetSha: ${{ needs.prepare.outputs.targetSha }} + systems: ${{ needs.prepare.outputs.systems }} build: name: Build diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index a6836b186e068..407b77194f35f 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -18,8 +18,24 @@ on: permissions: {} jobs: + prepare: + runs-on: ubuntu-24.04-arm + outputs: + systems: ${{ steps.systems.outputs.systems }} + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + sparse-checkout: | + ci/supportedSystems.json + + - name: Load supported systems + id: systems + run: | + echo "systems=$(jq -c > "$GITHUB_OUTPUT" + eval: name: Eval + needs: [prepare] uses: ./.github/workflows/eval.yml # Those are not actually used on push, but will throw an error if not set. permissions: @@ -27,3 +43,6 @@ jobs: issues: write pull-requests: write statuses: write + with: + mergedSha: ${{ github.sha }} + systems: ${{ needs.prepare.outputs.systems }} From 09ddb1a8a0f94409ae121116b5d4b0f9a02915c0 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Wed, 18 Jun 2025 20:43:03 +0200 Subject: [PATCH 3/5] workflows: sync merge commits This fixes a problem where each workflow would get their own merge commit. This happens frequently when the target branch is merged into a the same time, different workflows in the same run will run get-merge-commit at different times and thus have different merge commits. Since the jobs don't really depend on each other, this doesn't cause practical problems, yet. But it has already led to strange CI failures in a still unmerged PR, which can be prevented from happening with this clean approach. And yes, this saves a few API calls on every run. --- .github/actions/get-merge-commit/action.yml | 15 +++++++++++---- .github/workflows/build.yml | 5 +++++ .github/workflows/lint.yml | 11 +++++++++++ .github/workflows/pr.yml | 7 +++++++ 4 files changed, 34 insertions(+), 4 deletions(-) diff --git a/.github/actions/get-merge-commit/action.yml b/.github/actions/get-merge-commit/action.yml index 3766ad4f4ea0d..aec17bf768587 100644 --- a/.github/actions/get-merge-commit/action.yml +++ b/.github/actions/get-merge-commit/action.yml @@ -3,9 +3,15 @@ name: Get merge commit description: 'Checks whether the Pull Request is mergeable and checks out the repo at up to two commits: The result of a temporary merge of the head branch into the target branch ("merged"), and the parent of that commit on the target branch ("target"). Handles push events and merge conflicts gracefully.' inputs: + mergedSha: + description: "The merge commit SHA, previously collected." + type: string merged-as-untrusted: description: "Whether to checkout the merge commit in the ./untrusted folder." type: boolean + targetSha: + description: "The target commit SHA, previously collected." + type: string target-as-trusted: description: "Whether to checkout the target commit in the ./trusted folder." type: boolean @@ -22,6 +28,7 @@ runs: using: composite steps: - id: commits + if: ${{ !inputs.mergedSha && !inputs.targetSha }} uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 with: script: | @@ -72,17 +79,17 @@ runs: } throw new Error("Not retrying anymore. It's likely that GitHub is having internal issues: check https://www.githubstatus.com.") - - if: inputs.merged-as-untrusted && steps.commits.outputs.mergedSha + - if: inputs.merged-as-untrusted && (inputs.mergedSha || steps.commits.outputs.mergedSha) # Would be great to do the checkouts in git worktrees of the existing spare checkout instead, # but Nix is broken with them: # https://github.com/NixOS/nix/issues/6073 uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - ref: ${{ steps.commits.outputs.mergedSha }} + ref: ${{ inputs.mergedSha || steps.commits.outputs.mergedSha }} path: untrusted - - if: inputs.target-as-trusted && steps.commits.outputs.targetSha + - if: inputs.target-as-trusted && (inputs.targetSha || steps.commits.outputs.targetSha) uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - ref: ${{ steps.commits.outputs.targetSha }} + ref: ${{ inputs.targetSha || steps.commits.outputs.targetSha }} path: trusted diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 897bde43b6455..5940819246049 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -2,6 +2,10 @@ name: Build on: workflow_call: + inputs: + mergedSha: + required: true + type: string secrets: CACHIX_AUTH_TOKEN: required: true @@ -39,6 +43,7 @@ jobs: - name: Check if the PR can be merged and checkout the merge commit uses: ./.github/actions/get-merge-commit with: + mergedSha: ${{ inputs.mergedSha }} merged-as-untrusted: true - uses: cachix/install-nix-action@17fe5fb4a23ad6cbbe47d6b3f359611ad276644c # v31 diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 19540306148ab..4bf917d800db8 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -2,6 +2,13 @@ name: Lint on: workflow_call: + inputs: + mergedSha: + required: true + type: string + targetSha: + required: true + type: string permissions: {} @@ -19,6 +26,7 @@ jobs: - name: Check if the PR can be merged and checkout the merge commit uses: ./.github/actions/get-merge-commit with: + mergedSha: ${{ inputs.mergedSha }} merged-as-untrusted: true - uses: cachix/install-nix-action@17fe5fb4a23ad6cbbe47d6b3f359611ad276644c # v31 @@ -50,6 +58,7 @@ jobs: - name: Check if the PR can be merged and checkout the merge commit uses: ./.github/actions/get-merge-commit with: + mergedSha: ${{ inputs.mergedSha }} merged-as-untrusted: true - uses: cachix/install-nix-action@17fe5fb4a23ad6cbbe47d6b3f359611ad276644c # v31 @@ -72,7 +81,9 @@ jobs: - name: Check if the PR can be merged and checkout merged and target commits uses: ./.github/actions/get-merge-commit with: + mergedSha: ${{ inputs.mergedSha }} merged-as-untrusted: true + targetSha: ${{ inputs.targetSha }} target-as-trusted: true - uses: cachix/install-nix-action@17fe5fb4a23ad6cbbe47d6b3f359611ad276644c # v31 diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index df2573b394148..51828270178f9 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -48,7 +48,11 @@ jobs: lint: name: Lint + needs: [prepare] uses: ./.github/workflows/lint.yml + with: + mergedSha: ${{ needs.prepare.outputs.mergedSha }} + targetSha: ${{ needs.prepare.outputs.targetSha }} eval: name: Eval @@ -68,6 +72,9 @@ jobs: build: name: Build + needs: [prepare] uses: ./.github/workflows/build.yml secrets: CACHIX_AUTH_TOKEN: ${{ secrets.CACHIX_AUTH_TOKEN }} + with: + mergedSha: ${{ needs.prepare.outputs.mergedSha }} From 9927d758e798620092b69b8ae1a1021c859e4419 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Wed, 18 Jun 2025 20:49:18 +0200 Subject: [PATCH 4/5] workflows/{labels,reviewers}: move from Eval to PR context This allows *not* depending on those two jobs with the required status checks in the next commit, which wouldn't really make sense. If labeling or pinging maintainers fails for obscure reasons or because the GitHub API is down, a PR might still pass all other tests and be merge-eligible. --- .github/workflows/eval.yml | 21 --------------------- .github/workflows/pr.yml | 18 ++++++++++++++++-- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 2db06fd9937c3..bbc50810f5d1b 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -135,8 +135,6 @@ jobs: needs: [eval] if: inputs.targetSha permissions: - issues: write # needed to create *new* labels - pull-requests: write statuses: write steps: - name: Download output paths and eval stats for all systems @@ -217,25 +215,6 @@ jobs: target_url }) - labels: - name: Labels - needs: [compare] - uses: ./.github/workflows/labels.yml - permissions: - issues: write - pull-requests: write - - reviewers: - name: Reviewers - # No dependency on "compare", so that it can start at the same time. - # We only wait for the "comparison" artifact to be available, which makes the start-to-finish time - # for the eval workflow considerably faster. - needs: [eval] - if: inputs.targetSha - uses: ./.github/workflows/reviewers.yml - secrets: - OWNER_APP_PRIVATE_KEY: ${{ secrets.OWNER_APP_PRIVATE_KEY }} - misc: if: ${{ github.event_name != 'push' }} runs-on: ubuntu-24.04-arm diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 51828270178f9..667a130287af4 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -60,8 +60,6 @@ jobs: uses: ./.github/workflows/eval.yml permissions: # compare - issues: write - pull-requests: write statuses: write secrets: OWNER_APP_PRIVATE_KEY: ${{ secrets.OWNER_APP_PRIVATE_KEY }} @@ -70,6 +68,22 @@ jobs: targetSha: ${{ needs.prepare.outputs.targetSha }} systems: ${{ needs.prepare.outputs.systems }} + labels: + name: Labels + needs: [eval] + uses: ./.github/workflows/labels.yml + permissions: + issues: write + pull-requests: write + + reviewers: + name: Reviewers + needs: [prepare, eval] + if: needs.prepare.outputs.targetSha + uses: ./.github/workflows/reviewers.yml + secrets: + OWNER_APP_PRIVATE_KEY: ${{ secrets.OWNER_APP_PRIVATE_KEY }} + build: name: Build needs: [prepare] From caf4ced100486e2058921c537089289d77fd5315 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Wed, 18 Jun 2025 20:58:04 +0200 Subject: [PATCH 5/5] workflows/pr: add required job This job serves as a target for the "Required Status Checks" branch protection rule. --- .github/workflows/pr.yml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 667a130287af4..8e92b5aed8169 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -92,3 +92,23 @@ jobs: CACHIX_AUTH_TOKEN: ${{ secrets.CACHIX_AUTH_TOKEN }} with: mergedSha: ${{ needs.prepare.outputs.mergedSha }} + + # This job's only purpose is to serve as a target for the "Required Status Checks" branch ruleset. + # It "needs" all the jobs that should block merging a PR. + # If they pass, it is skipped — which counts as "success" for purposes of the branch ruleset. + # However, if any of them fail, this job will also fail — thus blocking the branch ruleset. + no-pr-failures: + # Modify this list to add or remove jobs from required status checks. + needs: + - check + - lint + - eval + - build + # WARNING: + # Do NOT change the name of this job, otherwise the rule will not catch it anymore. + # This would prevent all PRs from merging. + name: no PR failures + if: ${{ failure() }} + runs-on: ubuntu-24.04-arm + steps: + - run: exit 1