This migrates the bash code to request reviewers to github-script. This
will allow multiple nice improvements later on, but at this stage it's
mostly a reduction in code and complexity.
`zizmor` is a tool that uses static analysis to find potential security
issues in GitHub Actions [0]. (Yes, it's a bit absurd that GitHub
made a CI system so complicated that tools like this were created, but
I digress.)
Given our increase in GHA usage recently, I think this is a good step
towards keeping our security posture in tip-top shape. (It also keeps
with the theme of automating as many things as possible!)
The rule related to the usages of dangerous-triggers have been disabled
to avoid false-positives. Explanations about the usage of
`pull_request_target` and expectations around its usage can be found in
`.github/workflows/README.md`.
[0]: https://woodruffw.github.io/zizmor/
Co-authored-by: Thomas Gerbet <thomas@gerbet.me>
This ensures we don't accidentally use aliases in the nixpkgs shell or
other places that depend on the CI-pinned pkgs instance.
Nixpkgs generally — and CI specifically — do not use aliases, because we
want to ensure they are not load-bearing and can be removed safely.
See: https://github.com/NixOS/nixpkgs/blob/ce9979ec1c/pkgs/top-level/release-outpaths.nix#L28
Currently treefmt-nix is still defaulting `programs.nixfmt.package` to
the `nixfmt-rfc-style` alias. This makes sense, as they do not know for
certain which revision of nixpkgs is in use.
We do know, however, so we can explicitly use the non-alias name.
This has been marked insecure a while ago, as some CVEs have not been
backported. Even if *some* CVEs are fixed, we'd need **all** of them to
be, to get it back into the cache.
Not having it in the cache means, we can not test it in CI. This means
we can't make sure to actually support this version to evaluate Nixpkgs.
The concept of this alias becomes questionable once we move past 2.18,
where Lix was forked. We should probably move to a feature-detection
based approach for lib/minver.nix eventually, too.
With this change, we start running Eval on all available Lix and Nix
versions. Because this requires a lot of resources, this complete test
is only run when `ci/pinned.json` is updated.
The resulting outpaths are checked for consistency with the target
branch. A difference will cause the `report` job to fail, thus blocking
the merge, ensuring Eval consistency for Nixpkgs across different
versions.
This implements a kind of "ratchet style" check: Since we originally
confirmed that the versions currently in Nixpkgs at the time of this
commit match Eval behavior of Nix 2.3, we can ensure consistency with
Nix 2.3 down the road, even without testing for it explicitly.
There had been one regression in Eval consistency for Nix between 2.18
and 2.24 - two tests in `tests.devShellTools` produce different results
between Lix 2.91+ (which was forked from Nix 2.18) and Nix 2.24+. I
assume it's unlikely that such a change would be "fixed" by now, thus I
added an exception for these.
As a bonus, we also present the total time in seconds it takes for Eval
to complete for every tested version in a summary table. This allows us
to easily see performance improvements for Eval due to version updates.
At this stage, this time only includes the "outpaths" step of Eval, but
not the generation of attrpaths beforehand.
This is slightly faster than downloading and extracting a tarball and
additionally allows a sparse checkout. No need to download docs or nixos
for our purpose.
The data is quite noisy, but suggests improvements from anywhere between
5-15 seconds for each job using the pinned nixpkgs.
This was run as a test in `doc/tests/check-nix-code-blocks.nix` before,
but its DX can be improved: By including it in `treefmt` we get better
error reporting and auto-fixing, as well as running it on *all* markdown
files (including READMEs etc.) for free.
This adds a build job for the tarball, which might help uncover eval
issues on attributes not normally touched by Eval, aka those added in
`pkgs/top-level/packages-config.nix`.
Except:
- Instances in documentation, because people in older versions
can't switch to nixfmt yet due to it having pointed to nixfmt-classic
before
- In code that runs based on a CI Nixpkgs version, which is also a bit
older still
- In update script shebangs, because many of them don't pin Nixpkgs, and run
with whatever is in NIX_PATH (and it's not easy to fix this, see
https://github.com/NixOS/nixpkgs/issues/425551)
Most workflow files are already well formatted, but to make it easier to
keep it that way, we can add yamlfmt.
I personally have a preference for non-indented arrays for YAML, but
wanted to avoid bigger diffs here - the status-quo clearly are indented
arrays.
Some changes are made manually to the get-merge-commit action and the
issue templates. Those would otherwise make yamlfmt misbehave on those.
Previously we were taking nixVersions and this made external use from
the Lix repo's CI annoying.
We should probably also test other nix versions than stable (i.e. also
latest and Lix), but this involves writing GitHub Actions about it and
maybe not running it on every single PR. Future work.
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.
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.
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.
This was run on .nix files only, but we recently added keep-sorted,
editorconfig-checker and actionlint to treefmt, so CI needs to check all
files instead.
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.
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.
We already have treefmt running for nixfmt, so it's easy to just add
another formatter to it. This gives a much better UX, because all
formatting errors are reported through the same channel.
It also saves us one CI job, which takes most of the time to just set up
the machine, clone the repo and download Nix - while doing a minimum of
actual work.
Total execution time for treefmt is ~10% slower:
- 38s only nixfmt
- 43s nixfmt + editorconfig-checker
Changes the Nix format checking workflow to now strictly enforce
formatting of all Nix files using the treefmt setup introduced
in the pre-previous commit.
This is in [accordance with the approved RFC 166](https://github.com/NixOS/rfcs/blob/master/rfcs/0166-nix-formatting.md#reformat-nixpkgs).
Note that the "skip treewide" thing is no longer necessary, already
before, because there's nothing that would fail for treewide changes.
Previously the problem was that the GitHub API would be bombarded.
Introduces treefmt with a simple nixfmt-rfc-style configuration to
format all Nix files.
This is only practically usable with the following commit that formats
all files accordingly.
Also post a comment in case base branch is wrong
This guides newcomers in how to smoothly handle the potentially scary
situation of having thousands of commits listed in a PR.
While CI shows the same, people might not even look at CI if the PR
looks botched.
