workflows/eval: drop process job (#410852)

.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 }}
+          name: merged-${{ matrix.system }}
-          path: result/*
+          path: merged/*
-
-  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: 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 }}
+          nix-build untrusted/ci -A eval.diff \
-          git -C untrusted worktree add ../trusted ${{ needs.prepare.outputs.targetSha }}
+            --arg beforeDir ./target \
-          git -C untrusted diff --name-only ${{ needs.prepare.outputs.targetSha }} \
+            --arg afterDir "$(readlink ./merged)" \
-            | jq --raw-input --slurp 'split("\n")[:-1]' > touched-files.json
+            --argstr evalSystem "$MATRIX_SYSTEM" \
+            --out-link diff
 
-          # Use the target branch to get accurate maintainer info
+      - name: Upload outpaths diff and stats
-          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"
-
-      - name: Upload the combined results
         if: steps.targetRunId.outputs.targetRunId
         uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
         with:
-          name: comparison
+          name: diff-${{ matrix.system }}
-          path: comparison/*
+          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 ]
+    needs: [ prepare, outpaths ]
-    if: needs.process.outputs.targetRunId
+    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
+      - name: Download output paths and eval stats for all systems
-      # 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 process result
         uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8
         with:
-          name: comparison
+          pattern: diff-*
-          path: comparison
+          path: diff
+          merge-multiple: true
 
-      - name: Install Nix
+      - name: Check out the PR at the target commit
-        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: 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 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:
+          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:

ci/eval/compare/default.nix

@@ -7,8 +7,7 @@
   python3,
 }:
 {
-  beforeResultDir,
+  combinedDir,
-  afterResultDir,
   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}";
+      BEFORE_DIR = "${combinedDir}/before";
-      AFTER_DIR = "${afterResultDir}";
+      AFTER_DIR = "${combinedDir}/after";
     };
   }
   ''

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: [ <keys only in the second object> ],
-      removed: [ <keys only in the first object> ],
-      changed: [ <keys with different values between the two objects> ],
-    }
-  */
-  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
 

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
+        mkdir -p $out/after/stats
-          cp -r "$d"/stats-by-chunk $out/stats/$(basename "$d")
+        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 {
+      diffs = symlinkJoin {
-        name = "results";
+        name = "diffs";
         paths = map (
           evalSystem:
-          singleSystem {
+          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,

ci/eval/diff.nix

@@ -0,0 +1,61 @@
+{
+  lib,
+  runCommand,
+  writeText,
+}:
+
+{
+  beforeDir,
+  afterDir,
+  evalSystem,
+}:
+
+let
+  /*
+    Computes the key difference between two attrs
+
+    {
+      added: [ <keys only in the second object> ],
+      removed: [ <keys only in the first object> ],
+      changed: [ <keys with different values between the two objects> ],
+    }
+  */
+  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
+''