From 456a4697b1bbafb6c85c0632ad1fca1d6aa0c24a Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 11 May 2025 20:55:26 +0200 Subject: [PATCH 1/3] workflows/eval: load supportedSystems from JSON file This is a refactor to prepare the next commit. It doesn't do much on its own, but is separated for ease of review. --- .github/workflows/eval.yml | 7 ++----- .github/workflows/get-merge-commit.yml | 10 +++++++++- ci/eval/README.md | 2 +- ci/eval/default.nix | 7 ++----- ci/supportedSystems.json | 6 ++++++ ci/supportedSystems.nix | 6 ------ pkgs/top-level/release-haskell.nix | 2 +- pkgs/top-level/release-outpaths.nix | 2 +- pkgs/top-level/release.nix | 2 +- 9 files changed, 23 insertions(+), 21 deletions(-) create mode 100644 ci/supportedSystems.json delete mode 100644 ci/supportedSystems.nix diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 8cc523d1bef3..95130ec85d6b 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -26,7 +26,6 @@ jobs: if: needs.get-merge-commit.outputs.mergedSha outputs: targetSha: ${{ steps.targetSha.outputs.targetSha }} - systems: ${{ steps.systems.outputs.systems }} steps: - name: Check out the PR at the test merge commit uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 @@ -47,11 +46,9 @@ jobs: with: extra_nix_config: sandbox = true - - name: Evaluate the list of all attributes and get the systems matrix - id: systems + - name: Evaluate the list of all attributes run: | nix-build nixpkgs/ci -A eval.attrpathsSuperset - echo "systems=$(> "$GITHUB_OUTPUT" - name: Upload the list of all attributes uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 @@ -66,7 +63,7 @@ jobs: strategy: fail-fast: false matrix: - system: ${{ fromJSON(needs.attrs.outputs.systems) }} + system: ${{ fromJSON(needs.get-merge-commit.outputs.systems) }} steps: - name: Enable swap run: | diff --git a/.github/workflows/get-merge-commit.yml b/.github/workflows/get-merge-commit.yml index cb38438cae3b..7ce1d53b0249 100644 --- a/.github/workflows/get-merge-commit.yml +++ b/.github/workflows/get-merge-commit.yml @@ -6,6 +6,9 @@ on: mergedSha: description: "The merge commit SHA" value: ${{ jobs.resolve-merge-commit.outputs.mergedSha }} + systems: + description: "The supported systems" + value: ${{ jobs.resolve-merge-commit.outputs.systems }} permissions: {} @@ -14,6 +17,7 @@ jobs: runs-on: ubuntu-24.04-arm outputs: mergedSha: ${{ steps.merged.outputs.mergedSha }} + systems: ${{ steps.systems.outputs.systems }} steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: @@ -40,4 +44,8 @@ jobs: fi ;; esac - rm -rf base + + - name: Load supported systems + id: systems + run: | + echo "systems=$(jq -c > "$GITHUB_OUTPUT" diff --git a/ci/eval/README.md b/ci/eval/README.md index 0436a028ed69..011f3dd74ed0 100644 --- a/ci/eval/README.md +++ b/ci/eval/README.md @@ -11,7 +11,7 @@ nix-build ci -A eval.full \ --arg evalSystems '["x86_64-linux" "aarch64-darwin"]' ``` -- `--max-jobs`: The maximum number of derivations to run at the same time. Only each [supported system](../supportedSystems.nix) gets a separate derivation, so it doesn't make sense to set this higher than that number. +- `--max-jobs`: The maximum number of derivations to run at the same time. Only each [supported system](../supportedSystems.json) gets a separate derivation, so it doesn't make sense to set this higher than that number. - `--cores`: The number of cores to use for each job. Recommended to set this to the amount of cores on your system divided by `--max-jobs`. - `chunkSize`: The number of attributes that are evaluated simultaneously on a single core. Lowering this decreases memory usage at the cost of increased evaluation time. If this is too high, there won't be enough chunks to process them in parallel, and will also increase evaluation time. - `evalSystems`: The set of systems for which `nixpkgs` should be evaluated. Defaults to the four official platforms (`x86_64-linux`, `aarch64-linux`, `x86_64-darwin` and `aarch64-darwin`). diff --git a/ci/eval/default.nix b/ci/eval/default.nix index 639e75ec4211..99445e953183 100644 --- a/ci/eval/default.nix +++ b/ci/eval/default.nix @@ -26,14 +26,14 @@ let "nixos" "pkgs" ".version" - "ci/supportedSystems.nix" + "ci/supportedSystems.json" ] ); }; nix = nixVersions.nix_2_24; - supportedSystems = import ../supportedSystems.nix; + supportedSystems = builtins.fromJSON (builtins.readFile ../supportedSystems.json); attrpathsSuperset = runCommand "attrpaths-superset.json" @@ -43,8 +43,6 @@ let nix time ]; - env.supportedSystems = builtins.toJSON supportedSystems; - passAsFile = [ "supportedSystems" ]; } '' export NIX_STATE_DIR=$(mktemp -d) @@ -58,7 +56,6 @@ let --option restrict-eval true \ --option allow-import-from-derivation false \ --arg enableWarnings false > $out/paths.json - mv "$supportedSystemsPath" $out/systems.json ''; singleSystem = diff --git a/ci/supportedSystems.json b/ci/supportedSystems.json new file mode 100644 index 000000000000..44c18f1abf0e --- /dev/null +++ b/ci/supportedSystems.json @@ -0,0 +1,6 @@ +[ + "aarch64-linux", + "aarch64-darwin", + "x86_64-linux", + "x86_64-darwin" +] diff --git a/ci/supportedSystems.nix b/ci/supportedSystems.nix deleted file mode 100644 index 471f84b92fc2..000000000000 --- a/ci/supportedSystems.nix +++ /dev/null @@ -1,6 +0,0 @@ -[ - "aarch64-linux" - "aarch64-darwin" - "x86_64-linux" - "x86_64-darwin" -] diff --git a/pkgs/top-level/release-haskell.nix b/pkgs/top-level/release-haskell.nix index deb2290fb521..59ee41b013ea 100644 --- a/pkgs/top-level/release-haskell.nix +++ b/pkgs/top-level/release-haskell.nix @@ -10,7 +10,7 @@ $ hydra-eval-jobs -I . pkgs/top-level/release-haskell.nix */ { - supportedSystems ? import ../../ci/supportedSystems.nix, + supportedSystems ? builtins.fromJSON (builtins.readFile ../../ci/supportedSystems.json), }: let diff --git a/pkgs/top-level/release-outpaths.nix b/pkgs/top-level/release-outpaths.nix index e6caabec04e9..8ca14fc95007 100644 --- a/pkgs/top-level/release-outpaths.nix +++ b/pkgs/top-level/release-outpaths.nix @@ -13,7 +13,7 @@ attrNamesOnly ? false, # Set this to `null` to build for builtins.currentSystem only - systems ? import ../../ci/supportedSystems.nix, + systems ? builtins.fromJSON (builtins.readFile ../../ci/supportedSystems.json), }: let lib = import (path + "/lib"); diff --git a/pkgs/top-level/release.nix b/pkgs/top-level/release.nix index 31fe8fb38e13..7126252d7976 100644 --- a/pkgs/top-level/release.nix +++ b/pkgs/top-level/release.nix @@ -19,7 +19,7 @@ system ? builtins.currentSystem, officialRelease ? false, # The platform doubles for which we build Nixpkgs. - supportedSystems ? import ../../ci/supportedSystems.nix, + supportedSystems ? builtins.fromJSON (builtins.readFile ../../ci/supportedSystems.json), # The platform triples for which we build bootstrap tools. bootstrapConfigs ? [ "aarch64-apple-darwin" From 962836d4d0bdad936e973194a8935363f34a2678 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 11 May 2025 21:28:40 +0200 Subject: [PATCH 2/3] workflows/get-merge-commit: return targetSha We can fetch the targetSha directly with the mergedSha from the API. This avoids a checkout with fetch-depth: 2 for a small performance improvement. --- .github/workflows/eval.yml | 20 +++++--------------- .github/workflows/get-merge-commit.yml | 10 +++++++--- ci/README.md | 4 ++-- ci/get-merge-commit.sh | 5 ++++- 4 files changed, 18 insertions(+), 21 deletions(-) diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 95130ec85d6b..024e7b823b62 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -24,23 +24,13 @@ jobs: runs-on: ubuntu-24.04-arm needs: get-merge-commit if: needs.get-merge-commit.outputs.mergedSha - outputs: - targetSha: ${{ steps.targetSha.outputs.targetSha }} steps: - 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 }} - fetch-depth: 2 path: nixpkgs - - name: Determine target commit - if: github.event_name == 'pull_request_target' - id: targetSha - run: | - targetSha=$(git -C nixpkgs rev-parse HEAD^1) - echo "targetSha=$targetSha" >> "$GITHUB_OUTPUT" - - name: Install Nix uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 with: @@ -143,7 +133,7 @@ jobs: path: prResult/* - name: Get target run id - if: needs.attrs.outputs.targetSha + if: needs.get-merge-commit.outputs.targetSha id: targetRunId run: | # Get the latest eval.yml workflow run for the PR's target commit @@ -172,7 +162,7 @@ jobs: echo "targetRunId=$runId" >> "$GITHUB_OUTPUT" env: REPOSITORY: ${{ github.repository }} - TARGET_SHA: ${{ needs.attrs.outputs.targetSha }} + TARGET_SHA: ${{ needs.get-merge-commit.outputs.targetSha }} GH_TOKEN: ${{ github.token }} - uses: actions/download-artifact@v4 @@ -186,8 +176,8 @@ jobs: - name: Compare against the target branch if: steps.targetRunId.outputs.targetRunId run: | - git -C nixpkgs worktree add ../target ${{ needs.attrs.outputs.targetSha }} - git -C nixpkgs diff --name-only ${{ needs.attrs.outputs.targetSha }} \ + git -C nixpkgs worktree add ../target ${{ needs.get-merge-commit.outputs.targetSha }} + git -C nixpkgs diff --name-only ${{ needs.get-merge-commit.outputs.targetSha }} \ | jq --raw-input --slurp 'split("\n")[:-1]' > touched-files.json # Use the target branch to get accurate maintainer info @@ -241,7 +231,7 @@ jobs: - name: Check out Nixpkgs at the base commit uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - ref: ${{ needs.attrs.outputs.targetSha }} + ref: ${{ needs.get-merge-commit.outputs.targetSha }} path: base sparse-checkout: ci diff --git a/.github/workflows/get-merge-commit.yml b/.github/workflows/get-merge-commit.yml index 7ce1d53b0249..7d8dd03fbf66 100644 --- a/.github/workflows/get-merge-commit.yml +++ b/.github/workflows/get-merge-commit.yml @@ -6,6 +6,9 @@ on: 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 }} @@ -17,6 +20,7 @@ jobs: runs-on: ubuntu-24.04-arm outputs: mergedSha: ${{ steps.merged.outputs.mergedSha }} + targetSha: ${{ steps.merged.outputs.targetSha }} systems: ${{ steps.systems.outputs.systems }} steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 @@ -35,9 +39,9 @@ jobs: echo "mergedSha=${{ github.sha }}" >> "$GITHUB_OUTPUT" ;; pull_request_target) - if mergedSha=$(base/ci/get-merge-commit.sh ${{ github.repository }} ${{ github.event.number }}); then - echo "Checking the merge commit $mergedSha" - echo "mergedSha=$mergedSha" >> "$GITHUB_OUTPUT" + if commits=$(base/ci/get-merge-commit.sh ${{ github.repository }} ${{ github.event.number }}); then + echo "Checking the commits:\n$commits" + echo "$commits" >> "$GITHUB_OUTPUT" else # Skipping so that no notifications are sent echo "Skipping the rest..." diff --git a/ci/README.md b/ci/README.md index a1b327de4e5d..6ef665e8b099 100644 --- a/ci/README.md +++ b/ci/README.md @@ -44,14 +44,14 @@ Why not just build the tooling right from the PRs Nixpkgs version? ## `get-merge-commit.sh GITHUB_REPO PR_NUMBER` Check whether a PR is mergeable and return the test merge commit as -[computed by GitHub](https://docs.github.com/en/rest/guides/using-the-rest-api-to-interact-with-your-git-database?apiVersion=2022-11-28#checking-mergeability-of-pull-requests). +[computed by GitHub](https://docs.github.com/en/rest/guides/using-the-rest-api-to-interact-with-your-git-database?apiVersion=2022-11-28#checking-mergeability-of-pull-requests) and its parent. Arguments: - `GITHUB_REPO`: The repository of the PR, e.g. `NixOS/nixpkgs` - `PR_NUMBER`: The PR number, e.g. `1234` Exit codes: -- 0: The PR can be merged, the test merge commit hash is returned on stdout +- 0: The PR can be merged, the hashes of the test merge commit and the target commit are returned on stdout - 1: The PR cannot be merged because it's not open anymore - 2: The PR cannot be merged because it has a merge conflict - 3: The merge commit isn't being computed, GitHub is likely having internal issues, unknown if the PR is mergeable diff --git a/ci/get-merge-commit.sh b/ci/get-merge-commit.sh index c62bb56dd993..c233f7f91691 100755 --- a/ci/get-merge-commit.sh +++ b/ci/get-merge-commit.sh @@ -55,7 +55,10 @@ done if [[ "$mergeable" == "true" ]]; then log "The PR can be merged" - jq -r .merge_commit_sha <<< "$prInfo" + mergedSha="$(jq -r .merge_commit_sha <<< "$prInfo")" + echo "mergedSha=$mergedSha" + targetSha="$(gh api "/repos/$repo/commits/$mergedSha" --jq '.parents[0].sha')" + echo "targetSha=$targetSha" else log "The PR has a merge conflict" exit 2 From af6faf876022f53031aee0bf9fdb6aa1b0842995 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 11 May 2025 19:49:54 +0200 Subject: [PATCH 3/3] workflows/eval: remove attrs step Previously, the attrs step consisted of: - 7s queue time - 1m 15s run time Only 25s of this were spent preparing the attr paths. A bit more than a minute was just spent for queuing, checking out the repo, downloading nix, downloading dependencies, uploading the artifacts - and then downloading them again in the next step. All of that can be avoided if we collect the attrs as part of the outpaths job. By running the attrs step as part of each outpaths step the attrpaths will be collected 4x, but: - We save a minute for each eval run to complete. - We save a full job, giving us more free runners and *possibly* less queue times for other jobs in the repo. - We reduce complexity in the workflow file. --- .github/workflows/eval.yml | 40 +++----------------------------------- ci/eval/default.nix | 3 +-- 2 files changed, 4 insertions(+), 39 deletions(-) diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 024e7b823b62..c887638ab9b8 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -19,37 +19,10 @@ jobs: get-merge-commit: uses: ./.github/workflows/get-merge-commit.yml - attrs: - name: Attributes - runs-on: ubuntu-24.04-arm - needs: get-merge-commit - if: needs.get-merge-commit.outputs.mergedSha - steps: - - 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 }} - path: nixpkgs - - - name: Install Nix - uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 - with: - extra_nix_config: sandbox = true - - - name: Evaluate the list of all attributes - run: | - nix-build nixpkgs/ci -A eval.attrpathsSuperset - - - name: Upload the list of all attributes - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 - with: - name: paths - path: result/* - outpaths: name: Outpaths runs-on: ubuntu-24.04-arm - needs: [ attrs, get-merge-commit ] + needs: [ get-merge-commit ] strategy: fail-fast: false matrix: @@ -62,12 +35,6 @@ jobs: sudo mkswap /swap sudo swapon /swap - - name: Download the list of all attributes - uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 - with: - name: paths - path: paths - - name: Check out the PR at the test merge commit uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: @@ -85,7 +52,6 @@ jobs: run: | nix-build nixpkgs/ci -A eval.singleSystem \ --argstr evalSystem "$MATRIX_SYSTEM" \ - --arg attrpathFile ./paths/paths.json \ --arg chunkSize 10000 # If it uses too much memory, slightly decrease chunkSize @@ -98,7 +64,7 @@ jobs: process: name: Process runs-on: ubuntu-24.04-arm - needs: [ outpaths, attrs, get-merge-commit ] + needs: [ outpaths, get-merge-commit ] outputs: targetRunId: ${{ steps.targetRunId.outputs.targetRunId }} steps: @@ -200,7 +166,7 @@ jobs: tag: name: Tag runs-on: ubuntu-24.04-arm - needs: [ attrs, process ] + needs: [ process ] if: needs.process.outputs.targetRunId permissions: pull-requests: write diff --git a/ci/eval/default.nix b/ci/eval/default.nix index 99445e953183..e3f035e18d56 100644 --- a/ci/eval/default.nix +++ b/ci/eval/default.nix @@ -65,7 +65,7 @@ let # because `--argstr system` would only be passed to the ci/default.nix file! evalSystem, # The path to the `paths.json` file from `attrpathsSuperset` - attrpathFile, + attrpathFile ? "${attrpathsSuperset}/paths.json", # The number of attributes per chunk, see ./README.md for more info. chunkSize, checkMeta ? true, @@ -286,7 +286,6 @@ let name = evalSystem; path = singleSystem { inherit quickTest evalSystem chunkSize; - attrpathFile = attrpathsSuperset + "/paths.json"; }; }) evalSystems );