diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 0063ad1a12e96..b9a9c7e9dcbec 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -74,59 +74,24 @@ 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/* - - process: - name: Process - runs-on: ubuntu-24.04-arm - needs: [ prepare, outpaths ] - outputs: - targetRunId: ${{ steps.targetRunId.outputs.targetRunId }} - steps: - - name: Download output paths and eval stats for all systems - uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 - with: - pattern: intermediate-* - path: intermediate - merge-multiple: true - - - name: Check out the PR at the test merge commit - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - with: - ref: ${{ needs.prepare.outputs.mergedSha }} - path: untrusted - - - name: Install Nix - uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 - with: - extra_nix_config: sandbox = true - - - name: Combine all output paths and eval stats - run: | - nix-build untrusted/ci -A eval.combine \ - --arg resultsDir ./intermediate \ - -o prResult - - - name: Upload the combined results - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 - with: - name: result - path: prResult/* + 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 }} - 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 \ @@ -138,16 +103,24 @@ jobs: fi echo "Comparing against $(jq .html_url <<< "$run")" runId=$(jq .id <<< "$run") - conclusion=$(jq -r .conclusion <<< "$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 "Workflow not done, waiting 10 seconds before checking again" + echo "Job not done, waiting 10 seconds before checking again" sleep 10 - conclusion=$(gh api /repos/"$REPOSITORY"/actions/runs/"$runId" --jq '.conclusion') + conclusion=$(gh api /repos/"$REPOSITORY"/actions/jobs/"$jobId" --jq '.conclusion') done if [[ "$conclusion" != "success" ]]; then - echo "Workflow was not successful (conclusion: $conclusion), cannot make comparison" + echo "Job was not successful (conclusion: $conclusion), cannot make comparison" exit 1 fi @@ -156,78 +129,85 @@ jobs: - uses: actions/download-artifact@v4 if: steps.targetRunId.outputs.targetRunId with: - name: result - path: targetResult - merge-multiple: true - github-token: ${{ github.token }} run-id: ${{ steps.targetRunId.outputs.targetRunId }} + name: merged-${{ matrix.system }} + path: target + github-token: ${{ github.token }} + merge-multiple: true - - name: Compare against the target branch + - name: Compare outpaths against the target branch if: steps.targetRunId.outputs.targetRunId env: - AUTHOR_ID: ${{ github.event.pull_request.user.id }} + MATRIX_SYSTEM: ${{ matrix.system }} 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 }} \ - | jq --raw-input --slurp 'split("\n")[:-1]' > touched-files.json - - # Use the target branch to get accurate maintainer info - nix-build trusted/ci -A eval.compare \ - --arg beforeResultDir ./targetResult \ - --arg afterResultDir "$(realpath prResult)" \ - --arg touchedFilesJson ./touched-files.json \ - --argstr githubAuthorId "$AUTHOR_ID" \ - -o comparison - - cat comparison/step-summary.md >> "$GITHUB_STEP_SUMMARY" + nix-build untrusted/ci -A eval.diff \ + --arg beforeDir ./target \ + --arg afterDir "$(readlink ./merged)" \ + --argstr evalSystem "$MATRIX_SYSTEM" \ + --out-link diff - - name: Upload the combined results + - name: Upload outpaths diff and stats if: steps.targetRunId.outputs.targetRunId uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: - name: comparison - path: comparison/* + name: diff-${{ matrix.system }} + path: diff/* - # 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.process.outputs.targetRunId + needs: [ prepare, outpaths ] + 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 + - name: Download output paths and eval stats for all systems + uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 with: - app-id: ${{ vars.OWNER_APP_ID }} - private-key: ${{ secrets.OWNER_APP_PRIVATE_KEY }} - permission-administration: read - permission-members: read - permission-pull-requests: write + pattern: diff-* + path: diff + merge-multiple: true - - name: Download process result - uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 + - name: Check out the PR at the target commit + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - name: comparison - path: comparison + ref: ${{ needs.prepare.outputs.targetSha }} + path: trusted - name: Install Nix uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31 + with: + extra_nix_config: sandbox = true - # 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 + - name: Combine all output paths and eval stats + run: | + nix-build trusted/ci -A eval.combine \ + --arg diffDir ./diff \ + --out-link combined + + - name: Compare against the target branch + 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 }} \ + | jq --raw-input --slurp 'split("\n")[:-1]' > touched-files.json + + # Use the target branch to get accurate maintainer info + nix-build trusted/ci -A eval.compare \ + --arg combinedDir "$(realpath ./combined)" \ + --arg touchedFilesJson ./touched-files.json \ + --argstr githubAuthorId "$AUTHOR_ID" \ + --out-link comparison + + cat comparison/step-summary.md >> "$GITHUB_STEP_SUMMARY" + + - name: Upload the comparison results + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: - ref: ${{ needs.prepare.outputs.targetSha }} - path: trusted - sparse-checkout: ci + name: comparison + path: comparison/* - name: Build the requestReviews derivation run: nix-build trusted/ci -A requestReviews @@ -286,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: diff --git a/ci/eval/compare/default.nix b/ci/eval/compare/default.nix index 68456135629a5..f6cf1ebe78569 100644 --- a/ci/eval/compare/default.nix +++ b/ci/eval/compare/default.nix @@ -7,8 +7,7 @@ python3, }: { - beforeResultDir, - afterResultDir, + combinedDir, touchedFilesJson, githubAuthorId, byName ? false, @@ -20,7 +19,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. --- @@ -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 beforeResultDir; - afterAttrs = getAttrs afterResultDir; - # 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 = "${beforeResultDir}"; - AFTER_DIR = "${afterResultDir}"; + BEFORE_DIR = "${combinedDir}/before"; + AFTER_DIR = "${combinedDir}/after"; }; } '' diff --git a/ci/eval/compare/utils.nix b/ci/eval/compare/utils.nix index 6e75b2a627900..064d2cf57ea19 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 2e5ac008312ff..82f69e1c9a44f 100644 --- a/ci/eval/default.nix +++ b/ci/eval/default.nix @@ -191,11 +191,13 @@ let cat "$chunkOutputDir"/result/* | jq -s 'add | map_values(.outputs)' > $out/${evalSystem}/paths.json ''; + diff = callPackage ./diff.nix { }; + combine = { - resultsDir, + diffDir, }: - runCommand "combined-result" + runCommand "combined-eval" { nativeBuildInputs = [ jq @@ -205,12 +207,22 @@ let mkdir -p $out # Combine output paths from all systems - cat ${resultsDir}/*/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 ${resultsDir}/*; 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 - results = symlinkJoin { - name = "results"; + 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 { - resultsDir = results; + 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 0000000000000..629b4f8d3a6a0 --- /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 +''