Every now and then, the nixpkgs-vet CI job currently fails with one of:
error: creating symlink
'/build/.local/share/nix/root/nix/var/nix/gcroots/profiles' ->
'/build/.local/share/nix/root/nix/var/nix/profiles': File exists
error: SQLite database
'/build/.local/share/nix/root/nix/var/nix/db/db.sqlite' is busy
It's hard to reproduce for me, so just taking a guess with the required
changes.
Instead of rolling our own update script which only works for a single
pin, let's use npins. We can then use it for the treefmtNix pin as well,
which was mostly unmaintained, so far.
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.
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.
The current setup causes the Security team and the other owners of
.github/workflows to **not** be pinged for the
check-format/codeowners-v2/nixpkgs-vet workflows. This was highly likely
unintended when adding those additional rules, so removing them.
Also, we have some owners looking after `workflows/`, but not `ci/` -
and some the other way around. This doesn't make much sense to me, since
both parts depend on each other very much.
Even though there is only a small window where a commit is not on
staging, but already on staging-next, it is technically valid to
backport commits from staging-next, too.
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.
request-maintainers.sh script can be a bit unreliable, declaring ownership of certain paths allows
notification even when it is misbehaving. https://github.com/NixOS/nixpkgs/pull/404791#issuecomment-2856635870
wildcard paths are used intentionally so we don't have to change this often
if new packages are added/removed
Signed-off-by: John Titor <50095635+JohnRTitor@users.noreply.github.com>
When cherry-picking without -x or not cherry-picking at all, the
check-cherry-picks job would usually remain green. This is annoying to
deal with for reviewers, because "all green" still needs attention -
have all commits been cherry-picked properly?
If a commit was not cherry-picked correctly, either without -x or not at
all, because it's a genuine commit to begin with, the reviewers
attention is required anyway. Thus we can also let the job fail in this
case.
This makes the job significantly faster when the commit can't be found
on master or staging directly. Before this change, the script would have
had to iterate through 20+ release branches before finding the latest
one. With lazy fetching for git enabled, this would take a few minutes.
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.
The default is to checkout a contributors fork as "origin", thus the
NixOS/nixpkgs remote is most likely named differently. But not everybody
keeps their fork's main branches up-to-date all the time. Thus the
script would fail locally.
We recently moved the $commits variable out of a "subshell in a
herestring", let's do the same for the list of branches, where errors
would be silently swallowed as well.
Also reformat the expressions slightly, we have enough line-length.
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.
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 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.
Instead of parsing a plain text file with jq, we can make nix-env output
JSON directly, which is significantly faster.
This saves about 8 out of 10 seconds for the combine step.
compare/maintainers.nix needs to access the current checkout to check
attrpaths, but makes the mistake of using lib from that checkout as
well. All other code in ci/ uses the pinned nixpkgs instance, so
maintainers.nix should do so as well.
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.
As one of the resident Nixpkgs licensing pedants and reviewer of
several recent changes to this file I think it makes sense to make
the de facto the de jure.
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.
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.
