The change to use `builtins.storePath` was good - for when the store
path *is* already part of the nix store. In all my tests so far, that
was already the case, because I was iterating on the solution and the
Eval results stayed the same.
But when this is run on a entirely new commit, these the values for
`afterDir` and `combinedDir` are *not* in the store, yet. As part of
running `eval.full` on a new commit they will be created. `eval.full` is
linked up, so that the values passed around there will actually be
derivations, which might not be realized, yet.
Checking whether the input is a path or not fixes this for both cases.
Due to how we pass in existing store paths via CLI arguments for the
diff and combine scripts, Nix didn't register a dependency on the store
paths properly. This meant that some of the derivations that were built,
didn't have the right store paths made available in the sandbox -
leading to all kinds of "not found" errors.
We worked around this in CI by resolving the symlinks to the nix store
beforehand. We tried to work around this locally by storing the nix
store path in BASELINE, but this didn't fully work. By explicitly
registering these store paths as dependencies, this should work across
the board - without any magic required by the caller.
This indicates that the NixOS test-driver changed and all NixOS tests
have to be rebuilt. It can be used to either re-target to staging or to
batch this with other similar changes, at least.
By now, these files have been changed enough to not need the "vendored
from" notes anymore. These links would still be there when going through
the history of the file, but today GHA CI has not many similarities
anymore to what ofborg did, so these are not really helpful.
It makes no sense to check newly added attrpaths for maintainers on the
target branch - by definition these attrpaths won't exist, yet. We can
avoid falling back to `null` for these etc.
This change pings maintainers of actually removed packages, aka where
the package's expression is deleted.
This will not ping maintainers of packages that become invisible,
because a (transitive) dependency of them is marked as insecure or
broken.
`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.
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.
Instead of deleting each label separately and then making another call
to add new labels, this replaces all labels at once, thus saving API
calls in some cases. Also, the labels are now managed in object-style
compared to the array-style before. This allows putting all the
knowledge about each label into a single place instead of in multiple
places. For example, the rebuild labels had to be special cased in the
workflow before - and the nix code to compare had to match that. Also,
the approval labels had to be considered in the `before` and `after`
phases.
The next commit shows how easy it is to add a new label now.
Those have not been working since before the migration from OfBorg.
Those `rebuildsByKernel` are an attrset of lists coming from
`groupByKernel` (also see lengthy comment at the top of the file) - thus
we need `lib.elem` instead.
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.
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.
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.
Sometimes it is quite useful to output names instead of GitHub IDs, e.g.
for maintainer scripts that show you who you would ping. Add this as an
option, but keep the existing default.
This suddenly appeared after updating Nix to v26, which then complained:
… while calling the 'fromJSON' builtin
at
/home/runner/work/nixpkgs/nixpkgs/target/ci/eval/compare/default.nix:74:19:
73|
74| getAttrs = dir: builtins.fromJSON (builtins.readFile
"${dir}/outpaths.json");
| ^
75| beforeAttrs = getAttrs beforeResultDir;
… while evaluating the first argument passed to builtins.fromJSON
error: the string '{
"AMB-plugins.aarch64-linux": {
"out":
"/nix/store/faw59ba5p6h4b177n8q2ilb3hlm7xlc2-AMB-plugins-0.8.1"
},
....
"zzuf.aarch64-linux": {
"out": "/nix/store/bqvm1h7jfd8smgnjc1v1gpmbwdgvwy5g-zzuf-0.15"
},
"zzuf.x86_64-linux": {
"out": "/nix/store/6qs4lnmzn1qlr3smqqxnmhnrcdcfiv6a-zzuf-0.15"
}
}
' is not allowed to refer to a store path (such as
'134m2q047vsr9miwh5l227j7sh9jb130-jq-1.7.1-bin')
By discard the unsafe string context, we explicitly allow loading those
store paths. It's unclear why this blew up now, especially because I was
not possible to consistently replicate this locally, so far.
Various improvements such as:
1. Avoiding deduplications when there can't be any duplicates
2. Avoiding O(n^2) deduplications
3. Using builtins.any to avoid list allocations
4. Using builtins.concatMap instead of lib.flatten when it's known that there's only one level of nesting
5. Using builtins.groupBy instead of folding with an accumulator
In particular 5. should fix CI exceeding the stack size on staging: https://github.com/NixOS/nixpkgs/actions/runs/12989244871/job/36240781244?pr=377253
While 2. in particular should make CI a lot faster.
Currently we need to rely on ofborg requesting reviews from package
maintainers, which takes a while with ofborg's eval queue. Since
recently we're doing faster evaluations with GitHub Actions, which contain all
necessary information to determine reviewers of changed packages the
same way ofborg does. This PR takes advantage of that.
