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 +''