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.
(cherry picked from commit b942fb47dc)
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.
(cherry picked from commit 8a39ce4a48)
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.
(cherry picked from commit a6b659b08a)
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.
(cherry picked from commit 13f5aa304e)
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.
(cherry picked from commit b2579d36ff)
This makes checking out the nixpkgs repo even more consistent and almost
forces us to use the trusted/untrusted path pattern.
(cherry picked from commit 0e1c284b13)
We have added nixpkgs-vet as a regular package to nixpkgs a while ago,
so we can now use it from pinned nixpkgs. This avoids pulling a
platform-specific binary version from upstream.
This change also allows to run the tool easily locally, the same way as
other tools:
nix-build ci -A nixpkgs-vet
This will do a full check of the repo with the exception of
nixpkgs-vet's "ratchet" checks: Those depend on having two branches to
compare, but the default is to only look at the head branch. Those
ratchet checks will still be run in CI, though.
(cherry picked from commit 942c377476)
By consistently checking out nixpkgs into the same location in every
workflow, it's easier to reason about the different workflows at once.
We also use crystal-clear names to make clear, which checkouts are
considered trusted, because they only contain target-branch-code and
which checkouts are untrusted, because they contain code from the head
branch. By naming the checkout directories trusted/untrusted, it's
obvious at the call-site.
One example of where we likely did the wrong thing is the nixpkgs-vet
workflow: Fetching the toolVersion from the untrusted checkout opens the
door for an injection into the download URL, thus code could be
downloaded from anywhere. This is not a problem, because this workflow
does not run with elevated privileges, but it's a scary oversight
nonetheless.
(cherry picked from commit 6720d25429)
In PRs with multiple commits and merge conflicts the logic "targetSha ==
immediate parent of mergedSha" doesn't hold anymore. The head and base
commits of the PR's branch have some commits inbetween them, instead.
Before this change, we'd get a "fatal: invalid reference" on the
"worktree add". Now, not anymore, because we fetch the right commit
directly.
(cherry picked from commit cd9a22d753)
Previously, `eval.full` organized the results for the supported systems
in a specific layout, i.e. with a folder with one subfolder per system.
Then, `eval.combine` relied on that.
When using `eval.singleSystem` and `eval.combine` directly, the caller
was responsible to recreate the same layout. This is annoying and
error-prone to do, when downloading artifacts from CI to recreate some
steps locally.
With this change, all the artifacts can be downloaded and extracted into
the same folder - because the result from `eval.singleSystem` already
contains the <system-name>/ subfolder.
(cherry picked from commit eccd9564ab)
`env` blocks are a bit like `let` blocks in Nix. They define a few
things, which are then used in the `run` block. The workflows are
considerably easier to read, if those definitions come first, making it
crystal clear where they belong and requiring less visual jumping.
(cherry picked from commit 82396d1f48)
This makes a difference for the case of a merge conflict: In that case,
the magic `.../merge` branch actually points to the *last test merge
commit* that was successful, which might not contain the latest head
commit in any way. Running the tests on that commit is heavily
misleading. By using the get-merge-commit action, we run on the PR's
head commit in this case, which is much better.
(cherry picked from commit 039a954892)
We don't need a separate workflow anymore, because we don't need to skip
dependent jobs on failures anymore. The biggest failure mode was
"conflict" previously, but we resolved that on the last commit. The
remaining failure modes are so rare, that it's OK to just fail the jobs
in this case instead of marking them as "skipped". Especially, because
the resolve-merge-commit job would have previously failed anyway.
By moving this to an action we avoid running separate jobs each time we
need the merge commit. This also makes the check list in PRs much
cleaner.
(cherry picked from commit e48d9d6174)
When a PR is having conflicts with the base branch, we used to skip most
jobs depending on the target branch. With this change, we still run
those jobs, but without actually merging the PR temporarily. That means
we compare the head of the PR with the merge-base of the PR's branch and
the target branch - i.e. the point where the PR branched off.
This is not 100% accurate, but that's not important, because after
resolving the merge conflicts, those workflows will run again anyway. It
allows to give early feedback, though, instead of just skipping all the
jobs.
(cherry picked from commit c77cfb9239)
The reason this was a separate shell script was, that this would be
included in multiple workflows separately. But a while ago this had been
changed to a re-usable workflow, so we can just as well inline the
script.
This also allows us to use actions/github-script, which makes for a much
more readable script than the bash script before.
(cherry picked from commit 277f7b998c)
After creating the backport successfully, we previously created the
"has: port to stable" label with the Nixpkgs CI App's token. This would
trigger another labeled event for the backport workflow. This only
appears as "skipped", so doesn't use any resources, but it clutters the
GitHub Actions output with useless skipped workflows.
Using `github.token` does not trigger any other workflows so avoids that
problem.
(cherry picked from commit 2566f9dcb4)
We intend to use the edited event to react to base branch changes - but
before this change, we also ran those jobs on simple edits like title or
description.
While this works for some of the quicker jobs, it will not be
sustainable for all evaluation-related jobs. But evaluation needs to be
re-triggered on a base branch change as well, thus this change.
(cherry picked from commit 9b01e09a35)
While OfBorg is still adding these, it takes a much longer time to do so
compared to the eval action. Since we're adding rebuild labels, I think
it'd be nice to just do it within the eval action.
(cherry picked from commit d52066e2b1)
All workflows where it's remotely useful now trigger on a pull_request
event when the workflow file itself is changed. This gives us basic
sanity testing of changes to workflow files itself and reduces the need
for manual tests in forks.
Previously, the attrs step consisted of:
- 7s queue time
- 1m 15s run time
Only 25s of this were spent preparing the attr paths. A bit more than a
minute was just spent for queuing, checking out the repo, downloading
nix, downloading dependencies, uploading the artifacts - and then
downloading them again in the next step. All of that can be avoided if
we collect the attrs as part of the outpaths job.
By running the attrs step as part of each outpaths step the attrpaths
will be collected 4x, but:
- We save a minute for each eval run to complete.
- We save a full job, giving us more free runners and *possibly* less
queue times for other jobs in the repo.
- We reduce complexity in the workflow file.
This adds the minimum nix version and the latest lix version to the
matrix of parse checks. Especially the minimum nix version is relevant,
because parsing routinely breaks because of introduction of newer
syntax.
Adding lix just completes the picture.
The nix-parse workflow can now be run locally the same way as in CI.
To do this, the CI's workflow was slightly adjusted. Instead of testing
only the changed files, we're now testing all files in the repository.
This is possible in two ways:
1. By calling nix-instantiate once with all files as arguments. This
will be rather fast, but only the first error is shown before it errors
out.
2. By calling nix-instantiate once for each file. This will be much
slower, but has the advantage that we see all errors at once.
To avoid running the long variant every time, we first do a quick check
with the fast version. If that fails, we run the slower one to report
the errors. This gives us the best of both.
ARM runners are supposed to be more energy efficient than x86. Also,
from limited testing, they appear to be faster for the eval jobs as
well. Average run time for the "Outpaths (x86_64-linux)" job was 4m 27s,
so far. In the first run, this job came in at 3m 9s. This effect did not
show for other jobs, yet.
The following two exceptions are made right now:
- nixpkgs-lib-tests currently fails on the ARM runner building Nix 2.3
- nixpkgs-vet is currently pinned to a x86_64-linux only binary release
I added a lint-action.sh script in .github/workflows a while ago while
fixing some warnings. But I haven't run it myself ever since. This needs
to be part of CI to make any use of it.
The eval-aliases job is independent of all the other eval jobs. By
splitting it into a separate workflow, we avoid running it in two cases:
1. When turning a PR "ready to review". In this case, the main eval
workflow needs to run to be able to tag reviewers - but not
eval-aliases.
2. On branches like master, staging, etc. We only need to run eval there
to have a result to compare against in PRs. eval-aliases doesn't
contribute to that.
Thus, this will avoid wasting resources.
The workflow has been disabled for 9 months. Except for the Eval
workflow, this is the most complex, yet unused, workflow. As discussed
in #332695, this needs a proper wrapper first. Chances are high, that
once a good CLI tool is available, the workflow would be implemented
entirely different: We could easily run it via treefmt as well, so that
we get the same results locally as in CI.
