From b2579d36ffbf8691ed1e9fcdc001d5880277ae14 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 18 May 2025 21:21:44 +0200 Subject: [PATCH 1/5] workflows/eval: consistently avoid "result" in arguments Everything is a result, especially when nix-build uses "result" as its default output. This becomes confusing, when re-wiring the different parts later. Thus, consistently name those things after some of their properties and avoid the term result. --- .github/workflows/eval.yml | 33 +++++++++++++++++---------------- ci/eval/compare/default.nix | 14 +++++++------- ci/eval/default.nix | 14 +++++++------- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 0063ad1a12e9..1432aec3b0d1 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -74,14 +74,15 @@ jobs: run: | nix-build untrusted/ci -A eval.singleSystem \ --argstr evalSystem "$MATRIX_SYSTEM" \ - --arg chunkSize 10000 + --arg chunkSize 10000 \ + --out-link merged # If it uses too much memory, slightly decrease chunkSize - name: Upload the output paths and eval stats uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: - name: intermediate-${{ matrix.system }} - path: result/* + name: merged-${{ matrix.system }} + path: merged/* process: name: Process @@ -93,8 +94,8 @@ jobs: - name: Download output paths and eval stats for all systems uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 with: - pattern: intermediate-* - path: intermediate + pattern: merged-* + path: merged merge-multiple: true - name: Check out the PR at the test merge commit @@ -111,14 +112,14 @@ jobs: - name: Combine all output paths and eval stats run: | nix-build untrusted/ci -A eval.combine \ - --arg resultsDir ./intermediate \ - -o prResult + --arg evalDir ./merged \ + --out-link combined - name: Upload the combined results uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: - name: result - path: prResult/* + name: combined + path: combined/* - name: Get target run id if: needs.prepare.outputs.targetSha @@ -156,8 +157,8 @@ jobs: - uses: actions/download-artifact@v4 if: steps.targetRunId.outputs.targetRunId with: - name: result - path: targetResult + name: combined + path: target merge-multiple: true github-token: ${{ github.token }} run-id: ${{ steps.targetRunId.outputs.targetRunId }} @@ -174,15 +175,15 @@ jobs: # Use the target branch to get accurate maintainer info nix-build trusted/ci -A eval.compare \ - --arg beforeResultDir ./targetResult \ - --arg afterResultDir "$(realpath prResult)" \ + --arg beforeDir ./target \ + --arg afterDir "$(realpath combined)" \ --arg touchedFilesJson ./touched-files.json \ --argstr githubAuthorId "$AUTHOR_ID" \ - -o comparison + --out-link comparison cat comparison/step-summary.md >> "$GITHUB_STEP_SUMMARY" - - name: Upload the combined results + - name: Upload the comparison results if: steps.targetRunId.outputs.targetRunId uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: @@ -211,7 +212,7 @@ jobs: permission-members: read permission-pull-requests: write - - name: Download process result + - name: Download comparison result uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 with: name: comparison diff --git a/ci/eval/compare/default.nix b/ci/eval/compare/default.nix index 68456135629a..01c735a4c218 100644 --- a/ci/eval/compare/default.nix +++ b/ci/eval/compare/default.nix @@ -7,8 +7,8 @@ python3, }: { - beforeResultDir, - afterResultDir, + beforeDir, + afterDir, touchedFilesJson, githubAuthorId, byName ? false, @@ -20,7 +20,7 @@ let --- Inputs: - - beforeResultDir, afterResultDir: The evaluation result from before and after the change. + - beforeDir, afterDir: The evaluation result from before and after the change. They can be obtained by running `nix-build -A ci.eval.full` on both revisions. --- @@ -83,8 +83,8 @@ let data = builtins.unsafeDiscardStringContext raw; in builtins.fromJSON data; - beforeAttrs = getAttrs beforeResultDir; - afterAttrs = getAttrs afterResultDir; + beforeAttrs = getAttrs beforeDir; + afterAttrs = getAttrs afterDir; # Attrs # - keys: "added", "changed" and "removed" @@ -149,8 +149,8 @@ runCommand "compare" maintainers = builtins.toJSON maintainers; passAsFile = [ "maintainers" ]; env = { - BEFORE_DIR = "${beforeResultDir}"; - AFTER_DIR = "${afterResultDir}"; + BEFORE_DIR = "${beforeDir}"; + AFTER_DIR = "${afterDir}"; }; } '' diff --git a/ci/eval/default.nix b/ci/eval/default.nix index 2e5ac008312f..f60dd46efd3e 100644 --- a/ci/eval/default.nix +++ b/ci/eval/default.nix @@ -193,9 +193,9 @@ let combine = { - resultsDir, + evalDir, }: - runCommand "combined-result" + runCommand "combined-eval" { nativeBuildInputs = [ jq @@ -205,11 +205,11 @@ let mkdir -p $out # Combine output paths from all systems - cat ${resultsDir}/*/paths.json | jq -s add > $out/outpaths.json + cat ${evalDir}/*/paths.json | jq -s add > $out/outpaths.json mkdir -p $out/stats - for d in ${resultsDir}/*; do + for d in ${evalDir}/*; do cp -r "$d"/stats-by-chunk $out/stats/$(basename "$d") done ''; @@ -225,8 +225,8 @@ let quickTest ? false, }: let - results = symlinkJoin { - name = "results"; + evals = symlinkJoin { + name = "evals"; paths = map ( evalSystem: singleSystem { @@ -236,7 +236,7 @@ let }; in combine { - resultsDir = results; + evalDir = evals; }; in From 13f5aa304ef68e02a35d0a26812217bb32d73be2 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 25 May 2025 17:34:38 +0200 Subject: [PATCH 2/5] workflows/eval: run trusted code in process step We don't really need to run the combine and comparison steps from the untrusted merge commit. By switching to the trusted target commit, we can avoid adding another worktree - and lay the foundation to later do those steps in the tag job, which has access to secrets. --- .github/workflows/eval.yml | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 1432aec3b0d1..0f5a3e90eea9 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -98,11 +98,11 @@ jobs: path: merged merge-multiple: true - - name: Check out the PR at the test merge commit + - name: Check out the PR at the target commit uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - ref: ${{ needs.prepare.outputs.mergedSha }} - path: untrusted + ref: ${{ needs.prepare.outputs.targetSha }} + path: trusted - name: Install Nix uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 @@ -111,7 +111,7 @@ jobs: - name: Combine all output paths and eval stats run: | - nix-build untrusted/ci -A eval.combine \ + nix-build trusted/ci -A eval.combine \ --arg evalDir ./merged \ --out-link combined @@ -168,9 +168,8 @@ jobs: env: AUTHOR_ID: ${{ github.event.pull_request.user.id }} run: | - git -C untrusted fetch --depth 1 origin ${{ needs.prepare.outputs.targetSha }} - git -C untrusted worktree add ../trusted ${{ needs.prepare.outputs.targetSha }} - git -C untrusted diff --name-only ${{ needs.prepare.outputs.targetSha }} \ + git -C trusted fetch --depth 1 origin ${{ needs.prepare.outputs.mergedSha }} + git -C trusted diff --name-only ${{ needs.prepare.outputs.mergedSha }} \ | jq --raw-input --slurp 'split("\n")[:-1]' > touched-files.json # Use the target branch to get accurate maintainer info From a6b659b08a58ed914c52a6042ad8b92ece8dea03 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 18 May 2025 13:50:19 +0200 Subject: [PATCH 3/5] workflows/eval: fetch target results in outpaths job This is an intermediate step towards more efficiency. At this stage, the outpaths job pulls the result from the matching outpaths job on the target branch and uploads both results together. The process job then downloads both results at once and does the comparison as usual. This is slightly more inefficient, because the intermediate results are essentially stored as artifacts twice. But that inefficiency will go away in the next step, this refactor is split to make it slightly more reviewable and testable. On the other side, this allows us to save the process job on push events entirely, which is a win, because most of it is setup and nix download anyway. --- .github/workflows/eval.yml | 131 +++++++++++++++++++++---------------- 1 file changed, 76 insertions(+), 55 deletions(-) diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 0f5a3e90eea9..6f86d8587200 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -84,20 +84,84 @@ jobs: name: merged-${{ matrix.system }} path: merged/* + - name: Get target run id + if: needs.prepare.outputs.targetSha + id: targetRunId + env: + GH_TOKEN: ${{ github.token }} + MATRIX_SYSTEM: ${{ matrix.system }} + REPOSITORY: ${{ github.repository }} + TARGET_SHA: ${{ needs.prepare.outputs.targetSha }} + run: | + # Get the latest eval.yml workflow run for the PR's target commit + if ! run=$(gh api --method GET /repos/"$REPOSITORY"/actions/workflows/eval.yml/runs \ + -f head_sha="$TARGET_SHA" -f event=push \ + --jq '.workflow_runs | sort_by(.run_started_at) | .[-1]') \ + || [[ -z "$run" ]]; then + echo "Could not find an eval.yml workflow run for $TARGET_SHA, cannot make comparison" + exit 1 + fi + echo "Comparing against $(jq .html_url <<< "$run")" + runId=$(jq .id <<< "$run") + + if ! job=$(gh api --method GET /repos/"$REPOSITORY"/actions/runs/"$runId"/jobs \ + --jq ".jobs[] | select (.name == \"Outpaths ($MATRIX_SYSTEM)\")") \ + || [[ -z "$job" ]]; then + echo "Could not find the Outpaths ($MATRIX_SYSTEM) job for workflow run $runId, cannot make comparison" + exit 1 + fi + jobId=$(jq .id <<< "$job") + conclusion=$(jq -r .conclusion <<< "$job") + + while [[ "$conclusion" == null || "$conclusion" == "" ]]; do + echo "Job not done, waiting 10 seconds before checking again" + sleep 10 + conclusion=$(gh api /repos/"$REPOSITORY"/actions/jobs/"$jobId" --jq '.conclusion') + done + + if [[ "$conclusion" != "success" ]]; then + echo "Job was not successful (conclusion: $conclusion), cannot make comparison" + exit 1 + fi + + echo "targetRunId=$runId" >> "$GITHUB_OUTPUT" + + - uses: actions/download-artifact@v4 + if: steps.targetRunId.outputs.targetRunId + with: + run-id: ${{ steps.targetRunId.outputs.targetRunId }} + name: merged-${{ matrix.system }} + path: target + github-token: ${{ github.token }} + merge-multiple: true + + - name: Upload the output paths and eval stats + if: steps.targetRunId.outputs.targetRunId + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: target-${{ matrix.system }} + path: target/* + process: name: Process runs-on: ubuntu-24.04-arm needs: [ prepare, outpaths ] - outputs: - targetRunId: ${{ steps.targetRunId.outputs.targetRunId }} + if: needs.prepare.outputs.targetSha steps: - - name: Download output paths and eval stats for all systems + - name: Download output paths and eval stats for all systems (PR) uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 with: pattern: merged-* path: merged merge-multiple: true + - name: Download output paths and eval stats for all systems (target) + uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 + with: + pattern: target-* + path: target + merge-multiple: true + - name: Check out the PR at the target commit uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: @@ -109,62 +173,20 @@ jobs: with: extra_nix_config: sandbox = true - - name: Combine all output paths and eval stats + - name: Combine all output paths and eval stats (PR) run: | nix-build trusted/ci -A eval.combine \ --arg evalDir ./merged \ - --out-link combined + --out-link combinedMerged - - name: Upload the combined results - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 - with: - name: combined - path: combined/* - - - name: Get target run id + - name: Combine all output paths and eval stats (target) if: needs.prepare.outputs.targetSha - id: targetRunId - env: - REPOSITORY: ${{ github.repository }} - TARGET_SHA: ${{ needs.prepare.outputs.targetSha }} - GH_TOKEN: ${{ github.token }} run: | - # Get the latest eval.yml workflow run for the PR's target commit - if ! run=$(gh api --method GET /repos/"$REPOSITORY"/actions/workflows/eval.yml/runs \ - -f head_sha="$TARGET_SHA" -f event=push \ - --jq '.workflow_runs | sort_by(.run_started_at) | .[-1]') \ - || [[ -z "$run" ]]; then - echo "Could not find an eval.yml workflow run for $TARGET_SHA, cannot make comparison" - exit 1 - fi - echo "Comparing against $(jq .html_url <<< "$run")" - runId=$(jq .id <<< "$run") - conclusion=$(jq -r .conclusion <<< "$run") - - while [[ "$conclusion" == null || "$conclusion" == "" ]]; do - echo "Workflow not done, waiting 10 seconds before checking again" - sleep 10 - conclusion=$(gh api /repos/"$REPOSITORY"/actions/runs/"$runId" --jq '.conclusion') - done - - if [[ "$conclusion" != "success" ]]; then - echo "Workflow was not successful (conclusion: $conclusion), cannot make comparison" - exit 1 - fi - - echo "targetRunId=$runId" >> "$GITHUB_OUTPUT" - - - uses: actions/download-artifact@v4 - if: steps.targetRunId.outputs.targetRunId - with: - name: combined - path: target - merge-multiple: true - github-token: ${{ github.token }} - run-id: ${{ steps.targetRunId.outputs.targetRunId }} + nix-build trusted/ci -A eval.combine \ + --arg evalDir ./target \ + -o combinedTarget - name: Compare against the target branch - if: steps.targetRunId.outputs.targetRunId env: AUTHOR_ID: ${{ github.event.pull_request.user.id }} run: | @@ -174,8 +196,8 @@ jobs: # Use the target branch to get accurate maintainer info nix-build trusted/ci -A eval.compare \ - --arg beforeDir ./target \ - --arg afterDir "$(realpath combined)" \ + --arg beforeDir "$(realpath combinedTarget)" \ + --arg afterDir "$(realpath combinedMerged)" \ --arg touchedFilesJson ./touched-files.json \ --argstr githubAuthorId "$AUTHOR_ID" \ --out-link comparison @@ -183,7 +205,6 @@ jobs: cat comparison/step-summary.md >> "$GITHUB_STEP_SUMMARY" - name: Upload the comparison results - if: steps.targetRunId.outputs.targetRunId uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: name: comparison @@ -194,7 +215,7 @@ jobs: name: Tag runs-on: ubuntu-24.04-arm needs: [ prepare, process ] - if: needs.process.outputs.targetRunId + if: needs.prepare.outputs.targetSha permissions: pull-requests: write statuses: write From 8a39ce4a48fab9b79be46e9ad28f410749f079d9 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sun, 18 May 2025 15:31:48 +0200 Subject: [PATCH 4/5] workflows/eval: diff outpaths immediately This moves the diff of outpaths into the outpaths job, mainly as a preparation to allow future improvements. For example, this will allow running the purity release checks only on changed outpaths instead of the whole eval. This also removes the inefficiency introduced in the last commit about uploading the intermediate paths twice. Now, only the diff is passed on. Also, technically, the diff is now run in parallel across 4 jobs. This should be *slightly* faster than before, where outpaths from all systems were combined first and then diffed. It's probably only a few seconds, though. --- .github/workflows/eval.yml | 46 +++++++++++++--------------- ci/eval/compare/default.nix | 22 +++---------- ci/eval/compare/utils.nix | 26 ---------------- ci/eval/default.nix | 41 +++++++++++++++++++------ ci/eval/diff.nix | 61 +++++++++++++++++++++++++++++++++++++ 5 files changed, 117 insertions(+), 79 deletions(-) create mode 100644 ci/eval/diff.nix diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 6f86d8587200..2d22270c4874 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -135,12 +135,23 @@ jobs: github-token: ${{ github.token }} merge-multiple: true - - name: Upload the output paths and eval stats + - name: Compare outpaths against the target branch + if: steps.targetRunId.outputs.targetRunId + env: + MATRIX_SYSTEM: ${{ matrix.system }} + run: | + nix-build untrusted/ci -A eval.diff \ + --arg beforeDir ./target \ + --arg afterDir "$(readlink ./merged)" \ + --argstr evalSystem "$MATRIX_SYSTEM" \ + --out-link diff + + - name: Upload outpaths diff and stats if: steps.targetRunId.outputs.targetRunId uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: - name: target-${{ matrix.system }} - path: target/* + name: diff-${{ matrix.system }} + path: diff/* process: name: Process @@ -148,18 +159,11 @@ jobs: needs: [ prepare, outpaths ] if: needs.prepare.outputs.targetSha steps: - - name: Download output paths and eval stats for all systems (PR) + - name: Download output paths and eval stats for all systems uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 with: - pattern: merged-* - path: merged - merge-multiple: true - - - name: Download output paths and eval stats for all systems (target) - uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 - with: - pattern: target-* - path: target + pattern: diff-* + path: diff merge-multiple: true - name: Check out the PR at the target commit @@ -173,18 +177,11 @@ jobs: with: extra_nix_config: sandbox = true - - name: Combine all output paths and eval stats (PR) + - name: Combine all output paths and eval stats run: | nix-build trusted/ci -A eval.combine \ - --arg evalDir ./merged \ - --out-link combinedMerged - - - name: Combine all output paths and eval stats (target) - if: needs.prepare.outputs.targetSha - run: | - nix-build trusted/ci -A eval.combine \ - --arg evalDir ./target \ - -o combinedTarget + --arg diffDir ./diff \ + --out-link combined - name: Compare against the target branch env: @@ -196,8 +193,7 @@ jobs: # Use the target branch to get accurate maintainer info nix-build trusted/ci -A eval.compare \ - --arg beforeDir "$(realpath combinedTarget)" \ - --arg afterDir "$(realpath combinedMerged)" \ + --arg combinedDir "$(realpath ./combined)" \ --arg touchedFilesJson ./touched-files.json \ --argstr githubAuthorId "$AUTHOR_ID" \ --out-link comparison diff --git a/ci/eval/compare/default.nix b/ci/eval/compare/default.nix index 01c735a4c218..f6cf1ebe7856 100644 --- a/ci/eval/compare/default.nix +++ b/ci/eval/compare/default.nix @@ -7,8 +7,7 @@ python3, }: { - beforeDir, - afterDir, + combinedDir, touchedFilesJson, githubAuthorId, byName ? false, @@ -66,7 +65,6 @@ let Example: { name = "python312Packages.numpy"; platform = "x86_64-linux"; } */ inherit (import ./utils.nix { inherit lib; }) - diff groupByKernel convertToPackagePlatformAttrs groupByPlatform @@ -74,22 +72,10 @@ let getLabels ; - getAttrs = - dir: - let - raw = builtins.readFile "${dir}/outpaths.json"; - # The file contains Nix paths; we need to ignore them for evaluation purposes, - # else there will be a "is not allowed to refer to a store path" error. - data = builtins.unsafeDiscardStringContext raw; - in - builtins.fromJSON data; - beforeAttrs = getAttrs beforeDir; - afterAttrs = getAttrs afterDir; - # Attrs # - keys: "added", "changed" and "removed" # - values: lists of `packagePlatformPath`s - diffAttrs = diff beforeAttrs afterAttrs; + diffAttrs = builtins.fromJSON (builtins.readFile "${combinedDir}/combined-diff.json"); rebuilds = diffAttrs.added ++ diffAttrs.changed; rebuildsPackagePlatformAttrs = convertToPackagePlatformAttrs rebuilds; @@ -149,8 +135,8 @@ runCommand "compare" maintainers = builtins.toJSON maintainers; passAsFile = [ "maintainers" ]; env = { - BEFORE_DIR = "${beforeDir}"; - AFTER_DIR = "${afterDir}"; + BEFORE_DIR = "${combinedDir}/before"; + AFTER_DIR = "${combinedDir}/after"; }; } '' diff --git a/ci/eval/compare/utils.nix b/ci/eval/compare/utils.nix index 6e75b2a62790..064d2cf57ea1 100644 --- a/ci/eval/compare/utils.nix +++ b/ci/eval/compare/utils.nix @@ -93,32 +93,6 @@ rec { in uniqueStrings (builtins.map (p: p.name) packagePlatformAttrs); - /* - Computes the key difference between two attrs - - { - added: [ ], - removed: [ ], - changed: [ ], - } - */ - diff = - let - filterKeys = cond: attrs: lib.attrNames (lib.filterAttrs cond attrs); - in - old: new: { - added = filterKeys (n: _: !(old ? ${n})) new; - removed = filterKeys (n: _: !(new ? ${n})) old; - changed = filterKeys ( - n: v: - # Filter out attributes that don't exist anymore - (new ? ${n}) - - # Filter out attributes that are the same as the new value - && (v != (new.${n})) - ) old; - }; - /* Group a list of `packagePlatformAttr`s by platforms diff --git a/ci/eval/default.nix b/ci/eval/default.nix index f60dd46efd3e..82f69e1c9a44 100644 --- a/ci/eval/default.nix +++ b/ci/eval/default.nix @@ -191,9 +191,11 @@ let cat "$chunkOutputDir"/result/* | jq -s 'add | map_values(.outputs)' > $out/${evalSystem}/paths.json ''; + diff = callPackage ./diff.nix { }; + combine = { - evalDir, + diffDir, }: runCommand "combined-eval" { @@ -205,12 +207,22 @@ let mkdir -p $out # Combine output paths from all systems - cat ${evalDir}/*/paths.json | jq -s add > $out/outpaths.json + cat ${diffDir}/*/diff.json | jq -s ' + reduce .[] as $item ({}; { + added: (.added + $item.added), + changed: (.changed + $item.changed), + removed: (.removed + $item.removed) + }) + ' > $out/combined-diff.json - mkdir -p $out/stats + mkdir -p $out/before/stats + for d in ${diffDir}/before/*; do + cp -r "$d"/stats-by-chunk $out/before/stats/$(basename "$d") + done - for d in ${evalDir}/*; do - cp -r "$d"/stats-by-chunk $out/stats/$(basename "$d") + mkdir -p $out/after/stats + for d in ${diffDir}/after/*; do + cp -r "$d"/stats-by-chunk $out/after/stats/$(basename "$d") done ''; @@ -225,18 +237,26 @@ let quickTest ? false, }: let - evals = symlinkJoin { - name = "evals"; + diffs = symlinkJoin { + name = "diffs"; paths = map ( evalSystem: - singleSystem { - inherit quickTest evalSystem chunkSize; + let + eval = singleSystem { + inherit quickTest evalSystem chunkSize; + }; + in + diff { + inherit evalSystem; + # Local "full" evaluation doesn't do a real diff. + beforeDir = eval; + afterDir = eval; } ) evalSystems; }; in combine { - evalDir = evals; + diffDir = diffs; }; in @@ -244,6 +264,7 @@ in inherit attrpathsSuperset singleSystem + diff combine compare # The above three are used by separate VMs in a GitHub workflow, diff --git a/ci/eval/diff.nix b/ci/eval/diff.nix new file mode 100644 index 000000000000..629b4f8d3a6a --- /dev/null +++ b/ci/eval/diff.nix @@ -0,0 +1,61 @@ +{ + lib, + runCommand, + writeText, +}: + +{ + beforeDir, + afterDir, + evalSystem, +}: + +let + /* + Computes the key difference between two attrs + + { + added: [ ], + removed: [ ], + changed: [ ], + } + */ + diff = + let + filterKeys = cond: attrs: lib.attrNames (lib.filterAttrs cond attrs); + in + old: new: { + added = filterKeys (n: _: !(old ? ${n})) new; + removed = filterKeys (n: _: !(new ? ${n})) old; + changed = filterKeys ( + n: v: + # Filter out attributes that don't exist anymore + (new ? ${n}) + + # Filter out attributes that are the same as the new value + && (v != (new.${n})) + ) old; + }; + + getAttrs = + dir: + let + raw = builtins.readFile "${dir}/${evalSystem}/paths.json"; + # The file contains Nix paths; we need to ignore them for evaluation purposes, + # else there will be a "is not allowed to refer to a store path" error. + data = builtins.unsafeDiscardStringContext raw; + in + builtins.fromJSON data; + + beforeAttrs = getAttrs beforeDir; + afterAttrs = getAttrs afterDir; + diffAttrs = diff beforeAttrs afterAttrs; + diffJson = writeText "diff.json" (builtins.toJSON diffAttrs); +in +runCommand "diff" { } '' + mkdir -p $out/${evalSystem} + + cp -r ${beforeDir} $out/before + cp -r ${afterDir} $out/after + cp ${diffJson} $out/${evalSystem}/diff.json +'' From b942fb47dc604f25e4bb710a1072f30e675c973a Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Mon, 19 May 2025 20:14:13 +0200 Subject: [PATCH 5/5] workflows/eval: drop process job Since process doesn't need to run on push events anymore, we can just as well remove it entirely. The little bit of combine and comparison can be done in the tag job, even with elevated privileges. That's because those parts can be done entirely from the target commit, which is trusted. This saves startup, installing nix, downloading tools and artifacts for one job. It saves about 1 minute per run, start to finish. --- .github/workflows/eval.yml | 59 +++++++++++--------------------------- 1 file changed, 17 insertions(+), 42 deletions(-) diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 2d22270c4874..b9a9c7e9dcbe 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -153,11 +153,14 @@ jobs: name: diff-${{ matrix.system }} path: diff/* - process: - name: Process + tag: + name: Tag runs-on: ubuntu-24.04-arm needs: [ prepare, outpaths ] if: needs.prepare.outputs.targetSha + permissions: + pull-requests: write + statuses: write steps: - name: Download output paths and eval stats for all systems uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 @@ -206,46 +209,6 @@ jobs: name: comparison path: comparison/* - # Separate job to have a very tightly scoped PR write token - tag: - name: Tag - runs-on: ubuntu-24.04-arm - needs: [ prepare, process ] - if: needs.prepare.outputs.targetSha - permissions: - pull-requests: write - statuses: write - steps: - # See ./codeowners-v2.yml, reuse the same App because we need the same permissions - # Can't use the token received from permissions above, because it can't get enough permissions - - uses: actions/create-github-app-token@df432ceedc7162793a195dd1713ff69aefc7379e # v2.0.6 - if: vars.OWNER_APP_ID - id: app-token - with: - app-id: ${{ vars.OWNER_APP_ID }} - private-key: ${{ secrets.OWNER_APP_PRIVATE_KEY }} - permission-administration: read - permission-members: read - permission-pull-requests: write - - - name: Download comparison result - uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 - with: - name: comparison - path: comparison - - - name: Install Nix - uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 - - # Important: This workflow job runs with extra permissions, - # so we need to make sure to not run untrusted code from PRs - - name: Check out Nixpkgs at the target commit - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - with: - ref: ${{ needs.prepare.outputs.targetSha }} - path: trusted - sparse-checkout: ci - - name: Build the requestReviews derivation run: nix-build trusted/ci -A requestReviews @@ -303,6 +266,18 @@ jobs: "/repos/$GITHUB_REPOSITORY/statuses/$PR_HEAD_SHA" \ -f "context=Eval / Summary" -f "state=success" -f "description=$description" -f "target_url=$target_url" + # See ./codeowners-v2.yml, reuse the same App because we need the same permissions + # Can't use the token received from permissions above, because it can't get enough permissions + - uses: actions/create-github-app-token@df432ceedc7162793a195dd1713ff69aefc7379e # v2.0.6 + if: vars.OWNER_APP_ID + id: app-token + with: + app-id: ${{ vars.OWNER_APP_ID }} + private-key: ${{ secrets.OWNER_APP_PRIVATE_KEY }} + permission-administration: read + permission-members: read + permission-pull-requests: write + - name: Requesting maintainer reviews if: ${{ steps.app-token.outputs.token && github.repository_owner == 'NixOS' }} env: