Skip to content

Commit 886356e

Browse files
authored
workflows/eval: drop process job (#410852)
2 parents cb02c5e + b942fb4 commit 886356e

File tree

5 files changed

+182
-148
lines changed

5 files changed

+182
-148
lines changed

.github/workflows/eval.yml

Lines changed: 84 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -74,59 +74,24 @@ jobs:
7474
run: |
7575
nix-build untrusted/ci -A eval.singleSystem \
7676
--argstr evalSystem "$MATRIX_SYSTEM" \
77-
--arg chunkSize 10000
77+
--arg chunkSize 10000 \
78+
--out-link merged
7879
# If it uses too much memory, slightly decrease chunkSize
7980
8081
- name: Upload the output paths and eval stats
8182
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
8283
with:
83-
name: intermediate-${{ matrix.system }}
84-
path: result/*
85-
86-
process:
87-
name: Process
88-
runs-on: ubuntu-24.04-arm
89-
needs: [ prepare, outpaths ]
90-
outputs:
91-
targetRunId: ${{ steps.targetRunId.outputs.targetRunId }}
92-
steps:
93-
- name: Download output paths and eval stats for all systems
94-
uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8
95-
with:
96-
pattern: intermediate-*
97-
path: intermediate
98-
merge-multiple: true
99-
100-
- name: Check out the PR at the test merge commit
101-
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
102-
with:
103-
ref: ${{ needs.prepare.outputs.mergedSha }}
104-
path: untrusted
105-
106-
- name: Install Nix
107-
uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31
108-
with:
109-
extra_nix_config: sandbox = true
110-
111-
- name: Combine all output paths and eval stats
112-
run: |
113-
nix-build untrusted/ci -A eval.combine \
114-
--arg resultsDir ./intermediate \
115-
-o prResult
116-
117-
- name: Upload the combined results
118-
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
119-
with:
120-
name: result
121-
path: prResult/*
84+
name: merged-${{ matrix.system }}
85+
path: merged/*
12286

12387
- name: Get target run id
12488
if: needs.prepare.outputs.targetSha
12589
id: targetRunId
12690
env:
91+
GH_TOKEN: ${{ github.token }}
92+
MATRIX_SYSTEM: ${{ matrix.system }}
12793
REPOSITORY: ${{ github.repository }}
12894
TARGET_SHA: ${{ needs.prepare.outputs.targetSha }}
129-
GH_TOKEN: ${{ github.token }}
13095
run: |
13196
# Get the latest eval.yml workflow run for the PR's target commit
13297
if ! run=$(gh api --method GET /repos/"$REPOSITORY"/actions/workflows/eval.yml/runs \
@@ -138,16 +103,24 @@ jobs:
138103
fi
139104
echo "Comparing against $(jq .html_url <<< "$run")"
140105
runId=$(jq .id <<< "$run")
141-
conclusion=$(jq -r .conclusion <<< "$run")
106+
107+
if ! job=$(gh api --method GET /repos/"$REPOSITORY"/actions/runs/"$runId"/jobs \
108+
--jq ".jobs[] | select (.name == \"Outpaths ($MATRIX_SYSTEM)\")") \
109+
|| [[ -z "$job" ]]; then
110+
echo "Could not find the Outpaths ($MATRIX_SYSTEM) job for workflow run $runId, cannot make comparison"
111+
exit 1
112+
fi
113+
jobId=$(jq .id <<< "$job")
114+
conclusion=$(jq -r .conclusion <<< "$job")
142115
143116
while [[ "$conclusion" == null || "$conclusion" == "" ]]; do
144-
echo "Workflow not done, waiting 10 seconds before checking again"
117+
echo "Job not done, waiting 10 seconds before checking again"
145118
sleep 10
146-
conclusion=$(gh api /repos/"$REPOSITORY"/actions/runs/"$runId" --jq '.conclusion')
119+
conclusion=$(gh api /repos/"$REPOSITORY"/actions/jobs/"$jobId" --jq '.conclusion')
147120
done
148121
149122
if [[ "$conclusion" != "success" ]]; then
150-
echo "Workflow was not successful (conclusion: $conclusion), cannot make comparison"
123+
echo "Job was not successful (conclusion: $conclusion), cannot make comparison"
151124
exit 1
152125
fi
153126
@@ -156,78 +129,85 @@ jobs:
156129
- uses: actions/download-artifact@v4
157130
if: steps.targetRunId.outputs.targetRunId
158131
with:
159-
name: result
160-
path: targetResult
161-
merge-multiple: true
162-
github-token: ${{ github.token }}
163132
run-id: ${{ steps.targetRunId.outputs.targetRunId }}
133+
name: merged-${{ matrix.system }}
134+
path: target
135+
github-token: ${{ github.token }}
136+
merge-multiple: true
164137

165-
- name: Compare against the target branch
138+
- name: Compare outpaths against the target branch
166139
if: steps.targetRunId.outputs.targetRunId
167140
env:
168-
AUTHOR_ID: ${{ github.event.pull_request.user.id }}
141+
MATRIX_SYSTEM: ${{ matrix.system }}
169142
run: |
170-
git -C untrusted fetch --depth 1 origin ${{ needs.prepare.outputs.targetSha }}
171-
git -C untrusted worktree add ../trusted ${{ needs.prepare.outputs.targetSha }}
172-
git -C untrusted diff --name-only ${{ needs.prepare.outputs.targetSha }} \
173-
| jq --raw-input --slurp 'split("\n")[:-1]' > touched-files.json
174-
175-
# Use the target branch to get accurate maintainer info
176-
nix-build trusted/ci -A eval.compare \
177-
--arg beforeResultDir ./targetResult \
178-
--arg afterResultDir "$(realpath prResult)" \
179-
--arg touchedFilesJson ./touched-files.json \
180-
--argstr githubAuthorId "$AUTHOR_ID" \
181-
-o comparison
182-
183-
cat comparison/step-summary.md >> "$GITHUB_STEP_SUMMARY"
143+
nix-build untrusted/ci -A eval.diff \
144+
--arg beforeDir ./target \
145+
--arg afterDir "$(readlink ./merged)" \
146+
--argstr evalSystem "$MATRIX_SYSTEM" \
147+
--out-link diff
184148
185-
- name: Upload the combined results
149+
- name: Upload outpaths diff and stats
186150
if: steps.targetRunId.outputs.targetRunId
187151
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
188152
with:
189-
name: comparison
190-
path: comparison/*
153+
name: diff-${{ matrix.system }}
154+
path: diff/*
191155

192-
# Separate job to have a very tightly scoped PR write token
193156
tag:
194157
name: Tag
195158
runs-on: ubuntu-24.04-arm
196-
needs: [ prepare, process ]
197-
if: needs.process.outputs.targetRunId
159+
needs: [ prepare, outpaths ]
160+
if: needs.prepare.outputs.targetSha
198161
permissions:
199162
pull-requests: write
200163
statuses: write
201164
steps:
202-
# See ./codeowners-v2.yml, reuse the same App because we need the same permissions
203-
# Can't use the token received from permissions above, because it can't get enough permissions
204-
- uses: actions/create-github-app-token@df432ceedc7162793a195dd1713ff69aefc7379e # v2.0.6
205-
if: vars.OWNER_APP_ID
206-
id: app-token
165+
- name: Download output paths and eval stats for all systems
166+
uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8
207167
with:
208-
app-id: ${{ vars.OWNER_APP_ID }}
209-
private-key: ${{ secrets.OWNER_APP_PRIVATE_KEY }}
210-
permission-administration: read
211-
permission-members: read
212-
permission-pull-requests: write
168+
pattern: diff-*
169+
path: diff
170+
merge-multiple: true
213171

214-
- name: Download process result
215-
uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8
172+
- name: Check out the PR at the target commit
173+
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
216174
with:
217-
name: comparison
218-
path: comparison
175+
ref: ${{ needs.prepare.outputs.targetSha }}
176+
path: trusted
219177

220178
- name: Install Nix
221179
uses: cachix/install-nix-action@526118121621777ccd86f79b04685a9319637641 # v31
180+
with:
181+
extra_nix_config: sandbox = true
222182

223-
# Important: This workflow job runs with extra permissions,
224-
# so we need to make sure to not run untrusted code from PRs
225-
- name: Check out Nixpkgs at the target commit
226-
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
183+
- name: Combine all output paths and eval stats
184+
run: |
185+
nix-build trusted/ci -A eval.combine \
186+
--arg diffDir ./diff \
187+
--out-link combined
188+
189+
- name: Compare against the target branch
190+
env:
191+
AUTHOR_ID: ${{ github.event.pull_request.user.id }}
192+
run: |
193+
git -C trusted fetch --depth 1 origin ${{ needs.prepare.outputs.mergedSha }}
194+
git -C trusted diff --name-only ${{ needs.prepare.outputs.mergedSha }} \
195+
| jq --raw-input --slurp 'split("\n")[:-1]' > touched-files.json
196+
197+
# Use the target branch to get accurate maintainer info
198+
nix-build trusted/ci -A eval.compare \
199+
--arg combinedDir "$(realpath ./combined)" \
200+
--arg touchedFilesJson ./touched-files.json \
201+
--argstr githubAuthorId "$AUTHOR_ID" \
202+
--out-link comparison
203+
204+
cat comparison/step-summary.md >> "$GITHUB_STEP_SUMMARY"
205+
206+
- name: Upload the comparison results
207+
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
227208
with:
228-
ref: ${{ needs.prepare.outputs.targetSha }}
229-
path: trusted
230-
sparse-checkout: ci
209+
name: comparison
210+
path: comparison/*
231211

232212
- name: Build the requestReviews derivation
233213
run: nix-build trusted/ci -A requestReviews
@@ -286,6 +266,18 @@ jobs:
286266
"/repos/$GITHUB_REPOSITORY/statuses/$PR_HEAD_SHA" \
287267
-f "context=Eval / Summary" -f "state=success" -f "description=$description" -f "target_url=$target_url"
288268
269+
# See ./codeowners-v2.yml, reuse the same App because we need the same permissions
270+
# Can't use the token received from permissions above, because it can't get enough permissions
271+
- uses: actions/create-github-app-token@df432ceedc7162793a195dd1713ff69aefc7379e # v2.0.6
272+
if: vars.OWNER_APP_ID
273+
id: app-token
274+
with:
275+
app-id: ${{ vars.OWNER_APP_ID }}
276+
private-key: ${{ secrets.OWNER_APP_PRIVATE_KEY }}
277+
permission-administration: read
278+
permission-members: read
279+
permission-pull-requests: write
280+
289281
- name: Requesting maintainer reviews
290282
if: ${{ steps.app-token.outputs.token && github.repository_owner == 'NixOS' }}
291283
env:

ci/eval/compare/default.nix

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@
77
python3,
88
}:
99
{
10-
beforeResultDir,
11-
afterResultDir,
10+
combinedDir,
1211
touchedFilesJson,
1312
githubAuthorId,
1413
byName ? false,
@@ -20,7 +19,7 @@ let
2019
2120
---
2221
Inputs:
23-
- beforeResultDir, afterResultDir: The evaluation result from before and after the change.
22+
- beforeDir, afterDir: The evaluation result from before and after the change.
2423
They can be obtained by running `nix-build -A ci.eval.full` on both revisions.
2524
2625
---
@@ -66,30 +65,17 @@ let
6665
Example: { name = "python312Packages.numpy"; platform = "x86_64-linux"; }
6766
*/
6867
inherit (import ./utils.nix { inherit lib; })
69-
diff
7068
groupByKernel
7169
convertToPackagePlatformAttrs
7270
groupByPlatform
7371
extractPackageNames
7472
getLabels
7573
;
7674

77-
getAttrs =
78-
dir:
79-
let
80-
raw = builtins.readFile "${dir}/outpaths.json";
81-
# The file contains Nix paths; we need to ignore them for evaluation purposes,
82-
# else there will be a "is not allowed to refer to a store path" error.
83-
data = builtins.unsafeDiscardStringContext raw;
84-
in
85-
builtins.fromJSON data;
86-
beforeAttrs = getAttrs beforeResultDir;
87-
afterAttrs = getAttrs afterResultDir;
88-
8975
# Attrs
9076
# - keys: "added", "changed" and "removed"
9177
# - values: lists of `packagePlatformPath`s
92-
diffAttrs = diff beforeAttrs afterAttrs;
78+
diffAttrs = builtins.fromJSON (builtins.readFile "${combinedDir}/combined-diff.json");
9379

9480
rebuilds = diffAttrs.added ++ diffAttrs.changed;
9581
rebuildsPackagePlatformAttrs = convertToPackagePlatformAttrs rebuilds;
@@ -149,8 +135,8 @@ runCommand "compare"
149135
maintainers = builtins.toJSON maintainers;
150136
passAsFile = [ "maintainers" ];
151137
env = {
152-
BEFORE_DIR = "${beforeResultDir}";
153-
AFTER_DIR = "${afterResultDir}";
138+
BEFORE_DIR = "${combinedDir}/before";
139+
AFTER_DIR = "${combinedDir}/after";
154140
};
155141
}
156142
''

ci/eval/compare/utils.nix

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -93,32 +93,6 @@ rec {
9393
in
9494
uniqueStrings (builtins.map (p: p.name) packagePlatformAttrs);
9595

96-
/*
97-
Computes the key difference between two attrs
98-
99-
{
100-
added: [ <keys only in the second object> ],
101-
removed: [ <keys only in the first object> ],
102-
changed: [ <keys with different values between the two objects> ],
103-
}
104-
*/
105-
diff =
106-
let
107-
filterKeys = cond: attrs: lib.attrNames (lib.filterAttrs cond attrs);
108-
in
109-
old: new: {
110-
added = filterKeys (n: _: !(old ? ${n})) new;
111-
removed = filterKeys (n: _: !(new ? ${n})) old;
112-
changed = filterKeys (
113-
n: v:
114-
# Filter out attributes that don't exist anymore
115-
(new ? ${n})
116-
117-
# Filter out attributes that are the same as the new value
118-
&& (v != (new.${n}))
119-
) old;
120-
};
121-
12296
/*
12397
Group a list of `packagePlatformAttr`s by platforms
12498

0 commit comments

Comments
 (0)