This introduces "check" as another category of jobs suitable for the
"Require status checks to pass" feature.
Checks as a category would include everything that is done before even
looking at the code: Right branch? Commit messages? Cherry-picked
correctly?
(commit messages are not checked, yet)
(cherry picked from commit f11b58a7a0)
They still run in separate jobs, but they give us a nice consistent
appearance in the check list: All of them are starting with "Lint /
...".
(cherry picked from commit 7de0345556)
This new workflow builds both manuals, the shell and the lib tests all
in a matrix of four jobs. This allows re-using the shared checkout and
the pinned nixpkgs download and saves time in the most likely cache: No
changes, just download from cache. Each step checks the cancelled
condition, which causes it to run even if the previous steps failed.
This way we get a full picture even if the first step fails immediately.
This could later be optimized to build more in parallel as well, but
we'll first need to clear the conditions on building the manuals on the
master branch only.
This reduces the number of jobs from up to 8 to 4 for this part.
(cherry picked from commit cd82aa54f5)
To enable *required status checks / workflows* in the future, we'd like
to run all workflows unconditionally. Since those workflows are already
using cachix, the additional runs will be very cheap. Yes, we'll run
additional jobs, but that will be temporary only, see next commits.
The immediate upside is, that we're not going to accidentally miss some
of the paths that would cause rebuilds as we did in the past.
(cherry picked from commit 540fd4e30f)
To actually download the eval results and then proceed to ping
maintainers after undrafting, the run-id must be specified explicitly.
Because we didn't run with `-o pipefail` up to the last commit, we
didn't notice that this workflow was silently failing with this error:
jq: error: Could not open file comparison/maintainers.json: No such
file or directory
(cherry picked from commit 530f896efa)
When backporting a change to 24.11 and 25.05 at the same time by adding
the two labels immediately *after* merging the PR, three backport jobs
will run concurrently: One for the merge and one for each label added.
Each of those jobs will try to create both PRs, which will lead to two
of the jobs failing for sure.
With a concurrency group and cancelling in-progress jobs, only one of
those jobs will remain. This reduces notification noise.
(cherry picked from commit 6276e09530)
After we removed the dismissed-review workflow, it can happen that
multiple non-minimized reviews exist after a force push. To avoid
cluttering the PR's history while it's actively worked on, we minimize
all of them instead of only the latest.
(cherry picked from commit 82c6ea2ddc)
In a pull_request event, vars are available, but secrets are not. Thus
the steps will run, even though there is no secret to request those
tokens - they will fail.
The intent was, to skip them entirely in that case.
(cherry picked from commit 50f6d8d909)
Any change to shell.nix or ci/ currently needs to rebuild treefmt on 4
systems from scratch. We can avoid that by using the existing cachix
cache.
Adding the cache to lib-tests won't do much on it's own, yet, but it
will with the next commit.
(cherry picked from commit f87dac4be3)
We can avoid running old jobs to completion, when pushing changes to a
pull request. This is done via concurrency groups. We set them on the
workflow level, with the following keys in the group name:
- `github.workflow` to only cancel / block the same workflow.
- `github.event_name` to avoid blocking between pull_request and
pull_request_target.
- `github.head_ref` which is unique for a PR, but the same when changing
it. This will cause PRs to cancel in progress jobs. Unset on pushes to
master & co.
- `github.run_id` as fallback for push events. In this case, the run_id
is unique for every push, thus *no* cancelling happens on the dev
branches.
(cherry picked from commit 6d4b1f8e30)
There is no reason not to build the NixOS manual on stable branches as
well.
Also removing the restricted-eval, because it turned out not to be
necessary for the nixpkgs-manual either.
(cherry picked from commit 7f6370832b)
github-script provides a better way to access the workflow's context
than bash variables + interpolation. Especially when considering future
changes, where you'll always be tempted to just use interpolation
directly in bash code.
(cherry picked from commit e344fdcc26)
Granting the "issues: write" permission allows creating the "port to
stable" label, if it doesn't exist, yet. This avoids failing the
workflow when testing in a fork without that label.
(cherry picked from commit e8906ae26c)
GitHub comments have a length limit, so we can't just dump everything.
The 10k limit is arbitrary, but the assumption is that reviewing the
range-diff is not the sensible thing to do once it becomes a certain
size - reviewing the regular diff and treating the commit as "new" is
easier to do in that case. Thus, truncating should work out fine,
especially when the full range-diff is still available in the runner
log.
This could still end up in with an error, if a PR has multiple commits,
which all hit the limit. Let's get there first, before we try to fix
that hypothetical case, too.
(cherry picked from commit 856792f93e)
Instead of failing the job, the workflow will now post review comments
as "Request Changes". This makes the feedback more readily visible and
avoids having to merge despite a failing CI job. It is also a
pre-requisite to enable required status checks / required workflows in
the future.
Committers are asked to confirm the differences by explicitly dismissing
the generated review. After dismissal, the related review comment will
automatically be marked as "resolved".
The comments only report warnings and errors. Reviews are automatically
dismissed when they have been addressed by the author and no problems
remain. If problems remain, existing, still pending, review comments
will be updated. If the same problems had already been dismissed
earlier, no new review comment will be created either.
(cherry picked from commit 515b174c42)
By using the pinned nixpkgs we have for CI, we can lift the restriction
of building the nixpkgs manual only in PRs targeting master.
At the same time, this uses the pinned nixpkgs for the doc/ folder's dev
shell. This allows entering that shell while working on a staging-based
branch and write documentation.
Why should staging be un(der)documented, after all?
Note: The package that is available in nixpkgs as pkgs.nixpkgs-manual
will still be built with the current nixpkgs checkout, not the pinned
version. This is the same that hydra builds.
(cherry picked from commit ef1077013b)
We don't need to run full eval when undrafting a PR. We already have an
eval result, so we can use that to do the maintainer pings.
We need to wait for eval to finish first, but we're already half there
because of how we're waiting for the artifact to appear. Since the
ready_for_review case is triggered in a different workflow run, we'll
need to fetch the ID of the relevant Eval workflow first, though.
(cherry picked from commit 8370024203)
This allows us to trigger only the reviewers job when undrafting a PR in
the next step. Split for ease of review. The code is copied 1:1 to
reviewers.yml.
(cherry picked from commit 4c2e23826c)
Splitting the job up into two is required to then move the reviews job
to a separate, re-usable workflow in the next step.
To avoid the eval workflow from taking more time to finish, after having
it optimized carefully recently, the reviews job starts in parallel to
the compare job, even though it requires the comparison results to
finish. We can then use the time to already set up nix, build
requestReviews and fetch our app-token.
This might seem overly complex, but given that we need to do the same
dance in the next commit when we run the re-usable workflow separately,
it's actually just easier to review that way, not more.
(cherry picked from commit ecf95fa458)
A while ago, I added those "owner == NixOS" conditions, because I
couldn't figure out why my fork kept failing those jobs, even though I
had set up the apps correctly.
Turns out, that when a label doesn't exist, those actions try to
implicitly create it. But to do that, they actually need write
permissions on the *issues* endpoint, the pull-requests endpoint is not
enough. Even though the docs state otherwise.
Thus, adding those permissions. This will also lead to new labels being
created when they are added via code (for example in
.github/labels.yml), even when they had not been created, yet. Labels
created this way will initially be grayish color and without description
- but we can always add those later, there is no point in failing
pipelines for everyone in that case.
(cherry picked from commit d2ff68b564)
Using a `tree:0` filter instead of `blob:none` reduces the checkout time
from over 3 minutes to about 45 seconds. The required trees/blobs will
then be fetched on-demand.
This on-demand fetching creates additional output for `git range-diff`
on stderr, so we hide that. This only happens the first time it's run,
so we don't need to adjust the other calls - which will still return any
real errors, should they happen.
(cherry picked from commit e575364ae6)
The script is part of CI and changes to it should be reviewed by the CI
owners. Thus moving it to ci/ is the most sensible thing to do.
(cherry picked from commit ad4b36d2d2)
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)
Using core.setOutput is much nicer than having to parse the json
"result" on the outside. This also avoids some very odd errors, when the
result can, for unknown reasons, *not* be parsed as JSON later on.
Also avoiding a bit of duplication between the "if mergeable" branches.
(cherry picked from commit 539e8d4f66)
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)
