Skip to content

workflows/eval: drop process job#410852

Merged
Mic92 merged 5 commits intoNixOS:masterfrom
wolfgangwalther:ci-eval-compose-after-compare
May 25, 2025
Merged

workflows/eval: drop process job#410852
Mic92 merged 5 commits intoNixOS:masterfrom
wolfgangwalther:ci-eval-compose-after-compare

Conversation

@wolfgangwalther
Copy link
Contributor

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 tag job. The compare step 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:

  • One job less, that's always good. Saves 1 min of CI resources per run, just doing setup, nix install and dependency download.
  • Slightly faster maintainer pings and ofc faster time to completion for a CI run - that 1 min from above. nixpkgs-review doesn't really benefit, the comparison results still come in around the same time.
  • The ability to implement very light-weight release-checks on top as discussed in ci/eval/release-checks: init #406825 (comment), because each outpaths job already knows about the diff for its system.

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.

@github-actions github-actions bot added 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 6.topic: policy discussion Discuss policies to work in and around Nixpkgs backport release-25.05 labels May 25, 2025
@wolfgangwalther
Copy link
Contributor Author

All job failures expected, because of renaming things. Those will only work after merge.

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.
@wolfgangwalther wolfgangwalther force-pushed the ci-eval-compose-after-compare branch from 778d924 to b942fb4 Compare May 25, 2025 17:26
@wolfgangwalther wolfgangwalther requested a review from winterqt May 25, 2025 17:28
@Mic92 Mic92 merged commit 886356e into NixOS:master May 25, 2025
15 of 23 checks passed
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented May 25, 2025

Successfully created backport PR for release-25.05:

@github-actions github-actions bot added the 8.has: port to stable This PR already has a backport to the stable release. label May 25, 2025
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented May 25, 2025

Successfully created backport PR for release-24.11:

@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented May 25, 2025

Git push to origin failed for release-25.05 with exitcode 1

@wolfgangwalther wolfgangwalther deleted the ci-eval-compose-after-compare branch May 25, 2025 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 6.topic: policy discussion Discuss policies to work in and around Nixpkgs 8.has: port to stable This PR already has a backport to the stable release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants