From 8824c563a706dc743a66511beed58b7f2ecd710b Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Fri, 22 Aug 2025 18:11:21 +0200 Subject: [PATCH 1/3] workflows/{merge-group,pr}: post "no PR failures" status manually Posting the status manually allows us to avoid the strange "skipped == success" logic and properly skip the `unlock` job for pull_request events in the next commit. This should be much easier to understand than the previous logic. (cherry picked from commit 2c25cb0891d23f052baac7db22004ec39e683921) --- .github/workflows/merge-group.yml | 30 ++++++++++++++++++--------- .github/workflows/pr.yml | 34 ++++++++++++++++++------------- .github/workflows/test.yml | 7 +++++-- 3 files changed, 45 insertions(+), 26 deletions(-) diff --git a/.github/workflows/merge-group.yml b/.github/workflows/merge-group.yml index dac02065debf2..e7404d5a69759 100644 --- a/.github/workflows/merge-group.yml +++ b/.github/workflows/merge-group.yml @@ -26,19 +26,29 @@ jobs: mergedSha: ${{ inputs.mergedSha || github.event.merge_group.head_sha }} targetSha: ${{ inputs.targetSha || github.event.merge_group.base_sha }} - # This job's only purpose is to serve as a target for the "Required Status Checks" branch ruleset. + # This job's only purpose is to create the target for the "Required Status Checks" branch ruleset. # It "needs" all the jobs that should block the Merge Queue. - # 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: + unlock: # Modify this list to add or remove jobs from required status checks. needs: - lint - # WARNING: - # Do NOT change the name of this job, otherwise the rule will not catch it anymore. - # This would prevent all PRs from passing the merge queue. - name: no PR failures - if: ${{ failure() }} runs-on: ubuntu-24.04-arm + permissions: + statuses: write steps: - - run: exit 1 + - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 + with: + script: | + const { serverUrl, repo, runId, payload } = context + const target_url = + `${serverUrl}/${repo.owner}/${repo.repo}/actions/runs/${runId}` + await github.rest.repos.createCommitStatus({ + ...repo, + sha: payload.merge_group.head_sha, + // WARNING: + // Do NOT change the name of this, otherwise the rule will not catch it anymore. + // This would prevent all PRs from merging. + context: 'no PR failures', + state: 'success', + target_url, + }) diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index a7720df7226ad..b280e7c4dd359 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -119,26 +119,32 @@ jobs: baseBranch: ${{ needs.prepare.outputs.baseBranch }} mergedSha: ${{ needs.prepare.outputs.mergedSha }} - # This job's only purpose is to serve as a target for the "Required Status Checks" branch ruleset. + # This job's only purpose is to create the 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: + unlock: # 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 - # A single job is "cancelled" when it hits its timeout. This is not the same - # as "skipped", which happens when the `if` condition doesn't apply. - # The "cancelled()" function only checks the whole workflow, but not individual - # jobs. - if: ${{ failure() || contains(needs.*.result, 'cancelled') }} runs-on: ubuntu-24.04-arm + permissions: + statuses: write steps: - - run: exit 1 + - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 + with: + script: | + const { serverUrl, repo, runId, payload } = context + const target_url = + `${serverUrl}/${repo.owner}/${repo.repo}/actions/runs/${runId}?pr=${payload.pull_request.number}` + await github.rest.repos.createCommitStatus({ + ...repo, + sha: payload.pull_request.head.sha, + // WARNING: + // Do NOT change the name of this, otherwise the rule will not catch it anymore. + // This would prevent all PRs from merging. + context: 'no PR failures', + state: 'success', + target_url, + }) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f26c371a55639..b3c2c6c598631 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -76,6 +76,9 @@ jobs: name: Merge Group needs: [prepare] uses: ./.github/workflows/merge-group.yml + # Those are actually only used on the merge_group event, but will throw an error if not set. + permissions: + statuses: write secrets: CACHIX_AUTH_TOKEN: ${{ secrets.CACHIX_AUTH_TOKEN }} with: @@ -87,7 +90,7 @@ jobs: name: PR needs: [prepare] uses: ./.github/workflows/pr.yml - # Those are not actually used on pull_request, but will throw an error if not set. + # Those are actually only used on the pull_request_target event, but will throw an error if not set. permissions: issues: write pull-requests: write @@ -102,7 +105,7 @@ jobs: name: Push needs: [prepare] uses: ./.github/workflows/push.yml - # Those are not actually used on push, but will throw an error if not set. + # Those are not actually used on the push or pull_request events, but will throw an error if not set. permissions: statuses: write secrets: From fc237c6b07dff0804a9c2d300b3477f60148c17f Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Fri, 22 Aug 2025 18:14:07 +0200 Subject: [PATCH 2/3] workflows/{merge-group,pr}: avoid posting "no PR failures" for pull_request trigger The required status checks should depend on exactly one workflow, triggered via pull_request_target or merge_group. Anything that is triggered by pull_request is for testing purposes of the workflows themselves only. (cherry picked from commit 5ff32763b23369e3e1c06d3728e71a7bace4e0e3) --- .github/workflows/merge-group.yml | 1 + .github/workflows/pr.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/merge-group.yml b/.github/workflows/merge-group.yml index e7404d5a69759..9af5cf0ebb71f 100644 --- a/.github/workflows/merge-group.yml +++ b/.github/workflows/merge-group.yml @@ -29,6 +29,7 @@ jobs: # This job's only purpose is to create the target for the "Required Status Checks" branch ruleset. # It "needs" all the jobs that should block the Merge Queue. unlock: + if: github.event_name != 'pull_request' # Modify this list to add or remove jobs from required status checks. needs: - lint diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index b280e7c4dd359..936a9173e7c46 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -122,6 +122,7 @@ jobs: # This job's only purpose is to create the target for the "Required Status Checks" branch ruleset. # It "needs" all the jobs that should block merging a PR. unlock: + if: github.event_name != 'pull_request' # Modify this list to add or remove jobs from required status checks. needs: - check From 78287e87bed9cc3297074a650e0ecf3a6bf25975 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Fri, 22 Aug 2025 18:44:19 +0200 Subject: [PATCH 3/3] workflows/check: allow owners to fail when ci/OWNERS is untouched The owners check is not reproducible, because it depends on the state of the NixOS org on GitHub. Owners can rename their accounts or they can leave the organisation and access to Nixpkgs can be removed from teams. All of this breaks the owners check for reasons unrelated to the PR at hand. This PR makes the check for the owners file conditionally required: Only when the ci/OWNERS file is actually modified a failed check will block merging the PR. When that's not the case, the check will still fail visibily in the checklist, but the failure can be ignored. This is especially relevant for the Merge Queue, which should not be entirely blocked whenever any of these events happen. Also, it allows passing the checks in a fork when testing, where the owners check will *always* fail, because the respective teams and members are never part of the "user org" that a fork is. (cherry picked from commit 956d0a744d2fa17a392bb04b2a25d10aef642813) --- .github/workflows/check.yml | 4 ++++ .github/workflows/pr.yml | 1 + ci/github-script/prepare.js | 6 ++++-- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index ded8bad536c47..9192264266a1c 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -12,6 +12,9 @@ on: mergedSha: required: true type: string + ownersCanFail: + required: true + type: boolean targetSha: required: true type: string @@ -94,6 +97,7 @@ jobs: # handling untrusted PR input. owners: runs-on: ubuntu-24.04-arm + continue-on-error: ${{ inputs.ownersCanFail }} timeout-minutes: 5 steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 936a9173e7c46..81f0f1d0f5074 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -61,6 +61,7 @@ jobs: headBranch: ${{ needs.prepare.outputs.headBranch }} mergedSha: ${{ needs.prepare.outputs.mergedSha }} targetSha: ${{ needs.prepare.outputs.targetSha }} + ownersCanFail: ${{ !contains(fromJSON(needs.prepare.outputs.touched), 'owners') }} lint: name: Lint diff --git a/ci/github-script/prepare.js b/ci/github-script/prepare.js index fb000cb6820a6..0fcec880f3763 100644 --- a/ci/github-script/prepare.js +++ b/ci/github-script/prepare.js @@ -76,8 +76,10 @@ module.exports = async ({ github, context, core }) => { }) ).map((file) => file.filename) - if (files.includes('ci/pinned.json')) core.setOutput('touched', ['pinned']) - else core.setOutput('touched', []) + const touched = [] + if (files.includes('ci/pinned.json')) touched.push('pinned') + if (files.includes('ci/OWNERS')) touched.push('owners') + core.setOutput('touched', touched) return }