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.
(cherry picked from commit 657c689842)
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.
(cherry picked from commit 80e011146b)
This is to be able to ignore the odd failure for some users, who are
listed as collaborators, but still fail to be requested properly.
(cherry picked from commit 034613f860)
This script assumed to get lowercased input before, but with the
addition of pinging maintainers that's not necessarily true anymore.
Since the checks for prAuthor and already-reviewed-by already lowercase,
make sure to lowercase the handles in the users array, too.
(cherry picked from commit 213dbf1774)
Fixes this problem for maintainer-based reviews when the maintainer
didn't yet accept or missed the automated invite:
gh: Reviews may only be requested from collaborators. One or more of
the users or teams you specified is not a collaborator of the
NixOS/nixpkgs repository. (HTTP 422)
(cherry picked from commit 077007a658)
The handles can change over time and there's nothing guaranteeing the
ones in the maintainer list are up-to-date. In comparison GitHub IDs
never change.
(cherry picked from commit b844cba4e6)
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.
(cherry picked from commit b9d800d468)
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.
(cherry picked from commit bd5c93ca3d)
The workaround to have ofborg ping chromium and ungoogled-chromium
maintainers when a change was only made to the upstream-info relied on
string context.
That string context was provided by the upstream-info being a nix file,
not a json file, and then holding on to that string context using
awkward attribute merges.
It was intended as a quick fix until the handling of this would improve
in ofborg itself and worked great.
That was until very recently when we switched from the chromium release
tarball to git source fetching in 8dd2f1add9.
Part of that change included going back from upstream-info.nix to
upstream-info.json and with that losing the string context and the base
on which this workaround used to work.
But this is fine. A lot has happened in the meantime.
CODEOWNERS was reimplemented and no longer requires every user listed in
it to have write permissions to the repository (commit bit).
Meaning we can accept that ofborg pings no longer work and instead rely
on CODEOWNERS exclusively.
It should, however, be noted that CODEOWNERS provide less granularity
than ofborg, meaning we can no longer differentiate between
ungoogled-chromium and chromium or even chromedriver.
Previously, implementing the workaround that is now essentially
reverted: 68c59791fb
(cherry picked from commit 674835a9d9)
