There is no point in passing these arguments around between all the
different parts of the eval chain - these global settings should surely
not be modified between different steps.
This also makes it simpler to add new arguments like extra nixpkgs
configuration.
This creates a simple mapping of all packages to github ids of all their
maintainers. This is uploaded as an artifact and is then available for
download on the latest commit of each branch with a merge queue. This
will allow scheduled jobs to use this information for setting
maintainer-related labels, to request reviewers and to implement the
merge-bot.
The advantage over querying this information directly via Nix in each
case: The scheduled job does not need to install Nix and does not need
to checkout the target branch.
Compared to obtaining the maintainer information just for a single PR
during Eval, this will allow retroactively changing maintainers for a
package: For example, it allows to request a new maintainer as reviewer
for a PR that was created before they became maintainer, but is still
open - and similarly for maintainer labels and merge-bot rights.
None of these extensions are implemented by this PR, yet.
Update the README.md to document what gets returned.
We might in the future split these up into other attrsets but I don't see a usecase for that at the moment.
These files are tightly coupled with the code in ci/eval and not used
anywhere else. They are subject to the same backporting requirements as
the remaining CI code. They are better placed next here.
This allows running a full comparison between two commits locally.
What was previously `eval.full` is now called `eval.all`. The new
`eval.full` takes a `baseline` argument for the comparison.
We had set a default of 5000 for local evaluation earlier for
`singleSystem`, it makes sense to also use that for `full`.
The README is also a bit outdated, because Nix 2.30 significantly
changed the memory requirements. Rewriting the README to also show the
ability to directly evaluate the current system only.
Instead of uploading the outpaths as artifact, this uploads them via
cachix. Most of all, this makes CI less brittle, because Eval in PRs
will still be able to succeed, even if no workflow run for the push
event could be found on the target branch. It will just take longer.
This also makes moving Eval into the Merge Queue easier to do: When
downloading artifacts from a different run, these would always have to
match on the right event, too. By pulling from cachix, the same workflow
can support target branches with merge queue and without merge queue at
the same time. The latter would still use the push event, while the
former could use the merge_group event.
Last but not least, this should fix Eval on PRs targeting `wip-`
branches and any other branches that the push event doesn't trigger on.
These would never find an Eval result from the target branch and could
never show rebuilds accurately. Now these PRs should work at a slightly
higher runtime cost.
Using these defaults makes it possible to just run `nix-build ci -A
eval.singleSystem` without passing additional arguments and get a sane
result back. Especially helpful when testing or debugging.
A `chunkSize` of 5000 is conservative to be able to run on systems with
less memory as well. Run-time is not impacted by that, as recent
benchmarks show.
`procps` pulls in 180 MB of systemd, but busybox also provides `kill`.
`busybox` also ships `time`, so no need for that extra dependency.
Using `nativeBuildInputs` pulls in all the -dev outputs of the listed
packages - which we don't need. We only need to run these tools, thus
map to their bin outputs.
Brings down the closure size from 500+ MB to 193 MB for the Eval job.
This probably saves ~10 seconds for the job.
No need to return bytes in these files. Also busybox has problems to
render `free -b` with sizes > 100 GB properly in the next commit,
leading to extraction errors with awk.
When a package's attrpath is renamed it is currently treated as a
rebuild, even though the outpath already exists and is already cached.
This also happens when adding new names for packagesets that already
exist, for example when starting to eval `perlPackages` in CI, which is
just the same as `perl540Packages` currently. It would also happen when
`perlPackages` is switched from `perl540Packages` to `perl999Packages`.
Assuming that `perl999Packages` had already been built before, this
doesn't really cause any rebuilds.
This doesn't fail on *all* asserts, yet, because nix-env still ignores
these in the main eval step. But it already gives some converage during
the attrpath generation.
This file takes attrpaths as input and returns chunks of derivations,
aka a subset of release-outpaths - while release-attrpaths-superset
returns only an attrpath, not any derivations.
Thus, as a specialization of release-outpaths, the name
release-outpaths-parallel matches much better.
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.
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.
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.
We really can't expect packages that are marked as broken to evaluate,
and *especially* not on unsupported platforms.
For context, we were attempting to eval them *past* the broken throw
previously, which caused fun side effects like [0].
When we set `includeBroken = true` before, this also included unfree
packages. Those would now be excluded, which is not what we want. Thus,
we explicitly enable them separately.
Commit by winterqt, message slightly reworded by wolfgangwalther.
[0]:
https://github.com/NixOS/nixpkgs/issues/355847#issuecomment-2878873137
Right now, there are some paths that don't even get exposed to certain
systems (notably Darwin, but some outliers exist for Linux such as the
Darwin-specific Hackage overlay) for one reason or another, usually
because of assertions like `stdenv.isLinux`. To catch these scenarios,
this change implements a way to specify the system to evaluate attrpaths
on, and makes it default to the system that we're evaluating outpaths
for.
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.
We commonly use platform-dependent conditional patterns like
`lib.meta.availableOn stdenv.hostPlatform` and `stdenv.hostPlatform.isLinux`
to enable different features in a given derivation or to evaluate
completely different derivations based on the platform.
For example, source builds of a given derivation may only be available
on linux but not on darwin. The use of such conditionals allow us to
fall back to patched binaries on darwin instead.
In `chromedriver` (pkgs/development/tools/selenium/chromedriver/default.nix), we use
~~~nix
if lib.meta.availableOn stdenv.hostPlatform chromium then
callPackage ./source.nix { }
else
callPackage ./binary.nix { }
~~~
To provide some context, `chromedriver` source builds are based on `chromium.mkDerivation`
and `chromium` is limited to `lib.platforms.linux`.
Based on the same `chromium.mkDerivation`, we also do source builds for
`electron` (pkgs/top-level/all-packages.nix):
~~~nix
electron_33 = if lib.meta.availableOn stdenv.hostPlatform electron-source.electron_33 then electron-source.electron_33 else electron_33-bin;
electron_34 = electron_34-bin;
electron = electron_34;
~~~
And finally, the top-level `jdk` (Java) attribute has a lot of
indirection, but eventually also boils down to `stdenv.hostPlatform.isLinux`
for source builds and binaries for x86_64-darwin and aarch64-darwin.
A surprising amount of electron and jdk consumers use variations of
`meta.platforms = electron.meta.platforms` in their own meta block.
Due to internal implementation details, the conditionals in those
top-level attributes like `chromedriver`, `electron` and `jdk` are
evaluated based on the value from `builtins.currentSystem` and not the
system passed to `import <nixpkgs> { }`.
This then causes `chromedriver`, `electron`, `jdk` and all dependents
that inherit those `meta.platforms` to appear only available on linux
despite also being available on darwin. Hydra is affected similarly, but
it's a lot more nuanced and in practice not actually *that* bad.
The addition of `--eval-system` ensures that `builtins.currentSystem`
matches the requested platform.
As a bonus, this also fixes the store paths of an impure test that
should probably be made pure:
~~~diff
@@ -885069,13 +886119,13 @@
"out": "/nix/store/lb2500hc69czy4sfga9mbh2k679cr1rp-test-compressDrv"
},
"tests.config.allowPkgsInPermittedInsecurePackages.aarch64-darwin": {
- "out": "/nix/store/0l5h8svrpzwymq35mnpvx82gyc7nf8s4-hello-2.12.1"
+ "out": "/nix/store/v1zjb688mp4y2132b6chii43d5kkxnpa-hello-2.12.1"
},
"tests.config.allowPkgsInPermittedInsecurePackages.aarch64-linux": {
- "out": "/nix/store/0l5h8svrpzwymq35mnpvx82gyc7nf8s4-hello-2.12.1"
+ "out": "/nix/store/hb21z2zdk03dwygsw5lvpa8zc3fbr500-hello-2.12.1"
},
"tests.config.allowPkgsInPermittedInsecurePackages.x86_64-darwin": {
- "out": "/nix/store/0l5h8svrpzwymq35mnpvx82gyc7nf8s4-hello-2.12.1"
+ "out": "/nix/store/gljdqsf0mxv1j8zb04phx9ws09pp7z3l-hello-2.12.1"
},
"tests.config.allowPkgsInPermittedInsecurePackages.x86_64-linux": {
"out": "/nix/store/0l5h8svrpzwymq35mnpvx82gyc7nf8s4-hello-2.12.1"
~~~
Diff stats between two full evals based on 75c8548d81
with and without this fix on x86_64-linux:
~~~bash
# git diff --no-index --stat /nix/store/659l3xp78255wx7abbahggsnrlj3a1la-combined-result/outpaths.json /nix/store/4fhlq4g5qa65cxbibskq9pma40zigrx7-combined-result/outpaths.json
/nix/store/{659l3xp78255wx7abbahggsnrlj3a1la-combined-result => 4fhlq4g5qa65cxbibskq9pma40zigrx7-combined-result}/outpaths.json | 1416 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 1405 insertions(+), 11 deletions(-)
~~~
The full diff is available as a gist at <https://gist.github.com/emilylange/d40c50031fc332bbcca133ad56d224f6>.
When we added `electron_34` only as binary instead of the usual source
on linux with binary fallback in cfed9a19cb
and made the unversioned `electron` top-level point to the newly added
`electron_34` instead of `electron_33`, the GitHub workflow suddenly
reported 20 new packages. Of those 20 reported packages, 17 where
false-positives caused by dropping the wrongly evaluated conditional.
It's currently annoying to see the actual failure in the attrs step,
because `time -v` displays like 20 lines, which get repeated, therefore
requiring you to scroll up most of the time:
https://github.com/NixOS/nixpkgs/actions/runs/12290298121/job/34297218345#step:5:794
This commit fixes that by only displaying the most important stats, the
same ones as the chunked system-specific evals.
