From e48d9d6174e77293235dd2f4e878f7214d0d8612 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Fri, 23 May 2025 13:47:14 +0200 Subject: [PATCH] workflows/get-merge-commit: move to composite action We don't need a separate workflow anymore, because we don't need to skip dependent jobs on failures anymore. The biggest failure mode was "conflict" previously, but we resolved that on the last commit. The remaining failure modes are so rare, that it's OK to just fail the jobs in this case instead of marking them as "skipped". Especially, because the resolve-merge-commit job would have previously failed anyway. By moving this to an action we avoid running separate jobs each time we need the merge commit. This also makes the check list in PRs much cleaner. --- .github/actions/get-merge-commit/action.yml | 66 +++++++++++++++ .github/workflows/check-format.yml | 14 ++-- .github/workflows/codeowners-v2.yml | 16 ++-- .github/workflows/eval-aliases.yml | 13 +-- .github/workflows/eval.yml | 45 ++++++++--- .github/workflows/get-merge-commit.yml | 90 --------------------- .github/workflows/lib-tests.yml | 14 ++-- .github/workflows/nix-parse-v2.yml | 15 ++-- .github/workflows/nixpkgs-vet.yml | 14 ++-- ci/OWNERS | 1 + 10 files changed, 149 insertions(+), 139 deletions(-) create mode 100644 .github/actions/get-merge-commit/action.yml delete mode 100644 .github/workflows/get-merge-commit.yml diff --git a/.github/actions/get-merge-commit/action.yml b/.github/actions/get-merge-commit/action.yml new file mode 100644 index 000000000000..1505d582efd8 --- /dev/null +++ b/.github/actions/get-merge-commit/action.yml @@ -0,0 +1,66 @@ +name: Get merge commit + +description: 'Checks whether the Pull Request is mergeable and returns two commit hashes: 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.' + +outputs: + mergedSha: + description: "The merge commit SHA" + value: ${{ fromJSON(steps.merged.outputs.result).mergedSha }} + targetSha: + description: "The target commit SHA" + value: ${{ fromJSON(steps.merged.outputs.result).targetSha }} + +runs: + using: composite + steps: + - id: merged + uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 + with: + script: | + if (context.eventName == 'push') return { mergedSha: context.sha } + + for (const retryInterval of [5, 10, 20, 40, 80]) { + console.log("Checking whether the pull request can be merged...") + const prInfo = (await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number + })).data + + if (prInfo.state != 'open') throw new Error ("PR is not open anymore.") + + if (prInfo.mergeable == null) { + console.log(`GitHub is still computing whether this PR can be merged, waiting ${retryInterval} seconds before trying again...`) + await new Promise(resolve => setTimeout(resolve, retryInterval * 1000)) + continue + } + + if (prInfo.mergeable) { + console.log("The PR can be merged.") + + const mergedSha = prInfo.merge_commit_sha + const targetSha = (await github.rest.repos.getCommit({ + owner: context.repo.owner, + repo: context.repo.repo, + ref: prInfo.merge_commit_sha + })).data.parents[0].sha + + console.log(`Checking the commits:\nmerged:${mergedSha}\ntarget:${targetSha}`) + + return { mergedSha, targetSha } + } else { + console.log("The PR has a merge conflict.") + + const mergedSha = prInfo.head.sha + const targetSha = (await github.rest.repos.compareCommitsWithBasehead({ + owner: context.repo.owner, + repo: context.repo.repo, + basehead: `${prInfo.base.sha}...${prInfo.head.sha}` + })).data.merge_base_commit.sha + + console.log(`Checking the commits:\nmerged:${mergedSha}\ntarget:${targetSha}`) + + return { mergedSha, targetSha } + } + } + throw new Error("Not retrying anymore. It's likely that GitHub is having internal issues: check https://www.githubstatus.com.") diff --git a/.github/workflows/check-format.yml b/.github/workflows/check-format.yml index cdc0176b2671..58c76f97da6b 100644 --- a/.github/workflows/check-format.yml +++ b/.github/workflows/check-format.yml @@ -9,18 +9,20 @@ on: permissions: {} jobs: - get-merge-commit: - uses: ./.github/workflows/get-merge-commit.yml - nixos: name: fmt-check runs-on: ubuntu-24.04-arm - needs: get-merge-commit - if: needs.get-merge-commit.outputs.mergedSha steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - ref: ${{ needs.get-merge-commit.outputs.mergedSha }} + sparse-checkout: .github/actions + - name: Check if the PR can be merged and get the test merge commit + uses: ./.github/actions/get-merge-commit + id: get-merge-commit + + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ steps.get-merge-commit.outputs.mergedSha }} - uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 with: diff --git a/.github/workflows/codeowners-v2.yml b/.github/workflows/codeowners-v2.yml index 16bfd417cb7f..8d36d9b03a1e 100644 --- a/.github/workflows/codeowners-v2.yml +++ b/.github/workflows/codeowners-v2.yml @@ -37,17 +37,19 @@ env: DRY_MODE: ${{ github.event.pull_request.draft && '1' || '' }} jobs: - get-merge-commit: - if: github.repository_owner == 'NixOS' - uses: ./.github/workflows/get-merge-commit.yml - # Check that code owners is valid check: name: Check runs-on: ubuntu-24.04-arm - needs: get-merge-commit - if: github.repository_owner == 'NixOS' && needs.get-merge-commit.outputs.mergedSha + if: github.repository_owner == 'NixOS' steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + sparse-checkout: .github/actions + - name: Check if the PR can be merged and get the test merge commit + uses: ./.github/actions/get-merge-commit + id: get-merge-commit + - uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 - uses: cachix/cachix-action@0fc020193b5a1fa3ac4575aa3a7d3aa6a35435ad # v16 @@ -77,7 +79,7 @@ jobs: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - ref: ${{ needs.get-merge-commit.outputs.mergedSha }} + ref: ${{ steps.get-merge-commit.outputs.mergedSha }} path: pr - name: Validate codeowners diff --git a/.github/workflows/eval-aliases.yml b/.github/workflows/eval-aliases.yml index 7f9b658081f1..941ffd378ef7 100644 --- a/.github/workflows/eval-aliases.yml +++ b/.github/workflows/eval-aliases.yml @@ -9,18 +9,21 @@ on: permissions: {} jobs: - get-merge-commit: - uses: ./.github/workflows/get-merge-commit.yml - eval-aliases: name: Eval nixpkgs with aliases enabled runs-on: ubuntu-24.04-arm - needs: [ get-merge-commit ] steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + sparse-checkout: .github/actions + - 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: Check out the PR at the test merge commit uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - ref: ${{ needs.get-merge-commit.outputs.mergedSha }} + ref: ${{ steps.get-merge-commit.outputs.mergedSha }} path: nixpkgs - name: Install Nix diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index ecb763043425..416fe1afbc50 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -19,17 +19,36 @@ on: permissions: {} jobs: - get-merge-commit: - uses: ./.github/workflows/get-merge-commit.yml + prepare: + name: 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" outpaths: name: Outpaths runs-on: ubuntu-24.04-arm - needs: [ get-merge-commit ] + needs: [ prepare ] strategy: fail-fast: false matrix: - system: ${{ fromJSON(needs.get-merge-commit.outputs.systems) }} + system: ${{ fromJSON(needs.prepare.outputs.systems) }} steps: - name: Enable swap run: | @@ -41,7 +60,7 @@ jobs: - name: Check out the PR at the test merge commit uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - ref: ${{ needs.get-merge-commit.outputs.mergedSha }} + ref: ${{ needs.prepare.outputs.mergedSha }} path: nixpkgs - name: Install Nix @@ -67,7 +86,7 @@ jobs: process: name: Process runs-on: ubuntu-24.04-arm - needs: [ outpaths, get-merge-commit ] + needs: [ prepare, outpaths ] outputs: targetRunId: ${{ steps.targetRunId.outputs.targetRunId }} steps: @@ -80,7 +99,7 @@ jobs: - name: Check out the PR at the test merge commit uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - ref: ${{ needs.get-merge-commit.outputs.mergedSha }} + ref: ${{ needs.prepare.outputs.mergedSha }} fetch-depth: 2 path: nixpkgs @@ -102,7 +121,7 @@ jobs: path: prResult/* - name: Get target run id - if: needs.get-merge-commit.outputs.targetSha + if: needs.prepare.outputs.targetSha id: targetRunId run: | # Get the latest eval.yml workflow run for the PR's target commit @@ -131,7 +150,7 @@ jobs: echo "targetRunId=$runId" >> "$GITHUB_OUTPUT" env: REPOSITORY: ${{ github.repository }} - TARGET_SHA: ${{ needs.get-merge-commit.outputs.targetSha }} + TARGET_SHA: ${{ needs.prepare.outputs.targetSha }} GH_TOKEN: ${{ github.token }} - uses: actions/download-artifact@v4 @@ -145,8 +164,8 @@ jobs: - name: Compare against the target branch if: steps.targetRunId.outputs.targetRunId run: | - git -C nixpkgs worktree add ../target ${{ needs.get-merge-commit.outputs.targetSha }} - git -C nixpkgs diff --name-only ${{ needs.get-merge-commit.outputs.targetSha }} \ + git -C nixpkgs worktree add ../target ${{ needs.prepare.outputs.targetSha }} + git -C nixpkgs diff --name-only ${{ needs.prepare.outputs.targetSha }} \ | jq --raw-input --slurp 'split("\n")[:-1]' > touched-files.json # Use the target branch to get accurate maintainer info @@ -172,7 +191,7 @@ jobs: tag: name: Tag runs-on: ubuntu-24.04-arm - needs: [ get-merge-commit, process ] + needs: [ prepare, process ] if: needs.process.outputs.targetRunId permissions: pull-requests: write @@ -204,7 +223,7 @@ jobs: - name: Check out Nixpkgs at the base commit uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - ref: ${{ needs.get-merge-commit.outputs.targetSha }} + ref: ${{ needs.prepare.outputs.targetSha }} path: base sparse-checkout: ci diff --git a/.github/workflows/get-merge-commit.yml b/.github/workflows/get-merge-commit.yml deleted file mode 100644 index 8085b3d4d64b..000000000000 --- a/.github/workflows/get-merge-commit.yml +++ /dev/null @@ -1,90 +0,0 @@ -name: Get merge commit - -on: - pull_request: - paths: - - .github/workflows/get-merge-commit.yml - workflow_call: - outputs: - mergedSha: - description: "The merge commit SHA" - value: ${{ jobs.resolve-merge-commit.outputs.mergedSha }} - targetSha: - description: "The target commit SHA" - value: ${{ jobs.resolve-merge-commit.outputs.targetSha }} - systems: - description: "The supported systems" - value: ${{ jobs.resolve-merge-commit.outputs.systems }} - -permissions: {} - -jobs: - resolve-merge-commit: - runs-on: ubuntu-24.04-arm - outputs: - mergedSha: ${{ fromJSON(steps.merged.outputs.result).mergedSha }} - targetSha: ${{ fromJSON(steps.merged.outputs.result).targetSha }} - systems: ${{ steps.systems.outputs.systems }} - steps: - - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - with: - path: base - sparse-checkout: ci - - - name: Check if the PR can be merged and get the test merge commit - id: merged - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 - with: - script: | - if (context.eventName == 'push') return { mergedSha: context.sha } - - for (const retryInterval of [5, 10, 20, 40, 80]) { - console.log("Checking whether the pull request can be merged...") - const prInfo = (await github.rest.pulls.get({ - owner: context.repo.owner, - repo: context.repo.repo, - pull_number: context.payload.pull_request.number - })).data - - if (prInfo.state != 'open') throw new Error ("PR is not open anymore.") - - if (prInfo.mergeable == null) { - console.log(`GitHub is still computing whether this PR can be merged, waiting ${retryInterval} seconds before trying again...`) - await new Promise(resolve => setTimeout(resolve, retryInterval * 1000)) - continue - } - - if (prInfo.mergeable) { - console.log("The PR can be merged.") - - const mergedSha = prInfo.merge_commit_sha - const targetSha = (await github.rest.repos.getCommit({ - owner: context.repo.owner, - repo: context.repo.repo, - ref: prInfo.merge_commit_sha - })).data.parents[0].sha - - console.log(`Checking the commits:\nmerged:${mergedSha}\ntarget:${targetSha}`) - - return { mergedSha, targetSha } - } else { - console.log("The PR has a merge conflict.") - - const mergedSha = prInfo.head.sha - const targetSha = (await github.rest.repos.compareCommitsWithBasehead({ - owner: context.repo.owner, - repo: context.repo.repo, - basehead: `${prInfo.base.sha}...${prInfo.head.sha}` - })).data.merge_base_commit.sha - - console.log(`Checking the commits:\nmerged:${mergedSha}\ntarget:${targetSha}`) - - return { mergedSha, targetSha } - } - } - throw new Error("Not retrying anymore. It's likely that GitHub is having internal issues: check https://www.githubstatus.com.") - - - name: Load supported systems - id: systems - run: | - echo "systems=$(jq -c > "$GITHUB_OUTPUT" diff --git a/.github/workflows/lib-tests.yml b/.github/workflows/lib-tests.yml index d55a109aa9f5..345d59bb9cc4 100644 --- a/.github/workflows/lib-tests.yml +++ b/.github/workflows/lib-tests.yml @@ -12,18 +12,20 @@ on: permissions: {} jobs: - get-merge-commit: - uses: ./.github/workflows/get-merge-commit.yml - nixpkgs-lib-tests: name: nixpkgs-lib-tests runs-on: ubuntu-24.04 - needs: get-merge-commit - if: needs.get-merge-commit.outputs.mergedSha steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - ref: ${{ needs.get-merge-commit.outputs.mergedSha }} + sparse-checkout: .github/actions + - name: Check if the PR can be merged and get the test merge commit + uses: ./.github/actions/get-merge-commit + id: get-merge-commit + + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ steps.get-merge-commit.outputs.mergedSha }} - uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 with: diff --git a/.github/workflows/nix-parse-v2.yml b/.github/workflows/nix-parse-v2.yml index 4b670331308e..6ae66e964474 100644 --- a/.github/workflows/nix-parse-v2.yml +++ b/.github/workflows/nix-parse-v2.yml @@ -9,18 +9,21 @@ on: permissions: {} jobs: - get-merge-commit: - uses: ./.github/workflows/get-merge-commit.yml - tests: name: nix-files-parseable-check runs-on: ubuntu-24.04-arm - needs: get-merge-commit - if: "needs.get-merge-commit.outputs.mergedSha && !contains(github.event.pull_request.title, '[skip treewide]')" + if: "!contains(github.event.pull_request.title, '[skip treewide]')" steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - ref: ${{ needs.get-merge-commit.outputs.mergedSha }} + sparse-checkout: .github/actions + - name: Check if the PR can be merged and get the test merge commit + uses: ./.github/actions/get-merge-commit + id: get-merge-commit + + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ steps.get-merge-commit.outputs.mergedSha }} - uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 with: diff --git a/.github/workflows/nixpkgs-vet.yml b/.github/workflows/nixpkgs-vet.yml index 160bc27697e8..761ecbf08f2f 100644 --- a/.github/workflows/nixpkgs-vet.yml +++ b/.github/workflows/nixpkgs-vet.yml @@ -17,21 +17,23 @@ permissions: {} # There is a feature request for suppressing notifications on concurrency-canceled runs: https://github.com/orgs/community/discussions/13015 jobs: - get-merge-commit: - uses: ./.github/workflows/get-merge-commit.yml - check: name: nixpkgs-vet # This needs to be x86_64-linux, because we depend on the tooling being pre-built in the GitHub releases. runs-on: ubuntu-24.04 # This should take 1 minute at most, but let's be generous. The default of 6 hours is definitely too long. timeout-minutes: 10 - needs: get-merge-commit - if: needs.get-merge-commit.outputs.mergedSha steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - ref: ${{ needs.get-merge-commit.outputs.mergedSha }} + sparse-checkout: .github/actions + - name: Check if the PR can be merged and get the test merge commit + uses: ./.github/actions/get-merge-commit + id: get-merge-commit + + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ steps.get-merge-commit.outputs.mergedSha }} # Fetches the merge commit and its parents fetch-depth: 2 diff --git a/ci/OWNERS b/ci/OWNERS index cad846ee537e..aafefb88086f 100644 --- a/ci/OWNERS +++ b/ci/OWNERS @@ -15,6 +15,7 @@ # CI /.github/*_TEMPLATE* @SigmaSquadron +/.github/actions @NixOS/Security @Mic92 @zowoq @infinisil @azuwis @wolfgangwalther /.github/workflows @NixOS/Security @Mic92 @zowoq @infinisil @azuwis @wolfgangwalther /.github/workflows/check-format.yml @infinisil @wolfgangwalther /.github/workflows/codeowners-v2.yml @infinisil @wolfgangwalther