nixos/systemd-lib: don't fail on systemd.packages duplicates #77294
Conversation
In some cases like we've noticed in NixOS#76169, having duplicate packages in systemd.packages like ``` systemd.packages = [ gnome-shell gnome-shell gnome-session ]; ``` breaks. Here we use an associative array to ensure no duplicate paths when we symlink all the units listed in systemd.packages.
ofborg
bot
added
6.topic: nixos
| @@ -147,7 +147,13 @@ in rec { | |||
| done | |||
|
|
|||
| # Symlink all units provided listed in systemd.packages. | |||
| for i in ${toString cfg.packages}; do | |||
| packages="${toString cfg.packages}" | |||
There was a problem hiding this comment.
Would not toString (lib.unique cfg.packages) work?
There was a problem hiding this comment.
lib.unique is O(n^2), though not sure if that will realistically cause problems. Otherwise I'd say applying unique to systemd.packages would be close to ideal (having a built in set type would be the best though :).
There was a problem hiding this comment.
Wouldn't a built in set type be attrsOf? I think it would be un-intuitive for systemd.packages to have to be declared in anything other than a list, and because of systemPackages.
On the topic of being O(n^2), there's a deprecated type that is basically this suggestion uniqList.
When it comes to removing duplicates in bash versus lib.unique, idk.
There was a problem hiding this comment.
Wouldn't a built in set type be attrsOf?
Right, I was thinking more like a math set, ie. something like uniqList. Agreed that it makes the most UX sense for it to be list.
When it comes to removing duplicates in bash versus lib.unique, idk.
Yep, I'm fine with both. Doing it through the option's apply with unique is clean and will guard against any other uses, but O(n^2) isn't ideal so 🤷♂️
|
@GrahamcOfBorg test systemd gnome3 |
|
Systemd test was broken already https://hydra.nixos.org/job/nixos/trunk-combined/nixos.tests.systemd.x86_64-linux. |
In some cases like we've noticed in #76169,
having duplicate packages in systemd.packages like
breaks.
Here we use an associative array to ensure no
duplicate paths when we symlink all the units listed
in systemd.packages.
Motivation for this change
A certain situation popped up where
gnome3addsgnome-shellandgnome-sessiontosystemd.packages, butgdmalso depends ongnome-shellthere.Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)