workflows/eval: drop process job#410852
Merged
Mic92 merged 5 commits intoNixOS:masterfrom May 25, 2025
Merged
Conversation
Contributor
Author
|
All job failures expected, because of renaming things. Those will only work after merge. |
mweinelt
reviewed
May 25, 2025
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.
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.
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.
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.
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.
778d924 to
b942fb4
Compare
13 tasks
Contributor
|
Successfully created backport PR for |
1 task
1 task
Contributor
|
Successfully created backport PR for |
Contributor
|
Git push to origin failed for release-25.05 with exitcode 1 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With a bit of rewiring, we can run the diff of attrpaths separately for each system in the outpaths jobs. We then only pass the diff onwards - directly to the
tagjob. Thecomparestep then only consists of figuring out the rebuild labels to add and which maintainers to ping. Thus, we can leave out the process job entirely. Some of this was described in #406825 (comment).This gives us:
nixpkgs-reviewdoesn't really benefit, the comparison results still come in around the same time.I tried to split the commits up to make it reviewable, hopefully that works well.
Things done
Add a 👍 reaction to pull requests you find important.