lib.systems.inspect: deprecate isEfi#264297
Conversation
This predicate never did what it claimed. (U)EFI support is a property of a specific motherboard, not of a platform or an entire architecture. This predicate was used only by two packages. This commit relocates the data previously held by `inspect.isEfi` to the `meta.platforms` of `gnu-efi` and the `withEfi` default value of `systemd`. We need to make room for `isEfi` (or perhaps `isUefi`) in preparation for #231951 so we should start the deprecation process sooner rather than later.
ghost
mentioned this pull request
github-actions
bot
added
6.topic: systemd
| # Unfortunately a strictness bug in lib.systems.equals means that | ||
| # we can't lib.warn on accesses to boolean values. If | ||
| # https://github.com/NixOS/nixpkgs/pull/238331 is merged we can go | ||
| # back to using Nix `==` and this warning can be enabled. | ||
| # | ||
| #predicates = result.predicates // { | ||
| # isEfi = argument: warnEfiDeprecated (result.predicates.isEfi argument); | ||
| #}; |
There was a problem hiding this comment.
This problem was introduced by
After that PR, we are no longer able to deprecate boolean-valued attributes of platform objects because lib.systems.equals is too strict.
We could get back to using ==, which is lazy enough to handle this correctly, by merging
ghost
marked this pull request as ready for review
ofborg
bot
added
10.rebuild-darwin: 0
| platforms = platforms.linux; | ||
| platforms = with lib.systems.inspect.patterns; map (pat: pat // isLinux) ([ | ||
| isArmv6 ] ++ isArmv7 ++ [ isArmv8 isRiscV isx86 | ||
| ]); |
There was a problem hiding this comment.
Using doubles strings rather than patterns would make the code here less complicated, and have the same effect, right?
There was a problem hiding this comment.
I thought @Ericson2314 wanted to get rid of double strings but maybe I misinterpreted him.
There was a problem hiding this comment.
It's not something we're going to move away from one package at a time regardless…
There was a problem hiding this comment.
Using doubles strings rather than patterns would make the code here less complicated, and have the same effect, right?
Not exactly.
The isEfi predicate that I removed uses a pattern, so to get precisely the same effect I used a pattern here. The main issue is that the { cpu = { family = "arm"; version = 6,7; }; } patterns will match any string with the prefix armv6/armv7, and likewise for version = 8 with aarch64. There are quite a lot of these, with more being added all the time apparently (aarch64ec!?)
Also double-strings are icky gross.
It's not something we're going to move away from one package at a time regardless…
I dunno, isn't incremental migration the easy way to do it?
2 participants
Description of changes
This predicate never did what it claimed.
(U)EFI support is a property of a specific motherboard, not of a platform or an entire architecture. This predicate was used only by two packages. This commit relocates the data previously held by
inspect.isEfito themeta.platformsofgnu-efiand thewithEfidefault value ofsystemd.We need to make room for
isEfi(or perhapsisUefi) in preparation for #231951 so we should start the deprecation process sooner rather than later.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)