What do we think about `extraPackages` in modules?

Development
1

Some modules have an extraPackages option which simply adds entries to environment.systemPackages. For example:

  • services.xserver.windowManager.i3.extraPackages
  • programs.river.extraPackages
  • programs.sway.extraPackages
  • (and more)

The pattern is that extraPackages has a default value with some useful packages to use alongside the module. The extraPackages are then added to environment.systemPackages and the option isn’t used for anything else.

This strikes me as a bad pattern for a few reasons:

  1. The option isn’t doing anything that environment.systemPackages can’t.2. 3.
  2. The option can be used to add packages that have nothing to do with the module.
  3. Adding or removing a single package with this option requires restating the defaults.
  4. It’s difficult to safely migrate a package from extraPackages to a module option.

That last one is why I’m here, so I should explain that in more detail. As part of improving a recent PAM change, I’d like to modify services.xserver.windowManager.i3 so that it enables programs.i3lock instead of adding i3lock to environment.systemPackages. This is necessary so that the PAM module can see that i3lock is enabled and create the appropriate PAM rules.

The default case is easy: I can remove i3lock from extraPackages and set programs.i3lock.enable = mkDefault true. But what if a user has manually set extraPackages to a list that doesn’t include i3lock?

Some options I am considering:

  1. Remove extraPackages. Enable programs.i3lock by default. Add new options for the existing defaults that don’t have modules. For example, add a new option services.xserver.windowManager.i3.addDmenu that’s enabled by default.

  2. Keep extraPackages. Set the default value for programs.i3lock.enable to whether extraPackages contains i3lock.

  3. Same as (2), but additionally eval error if extraPackages contains anything that isn’t part of the default set. Any such packages should be added to environment.systemPackages instead.

6 Likes
2

programs.sway.extraPackages is atrocious. IMO an .extraPackages option should only make sense with additional checks and if it got added to a wrapper or ifbit was reused by multiple specialised options or so on. As you said, if all it’s doing is adding to environment.systemPackages, it’s not doing much.

However, here’s a counterargument: programs.sway.extraPackages is bundling packages that users expect to available for a basically usable sway installation, particularly given that the default config should Just Work. (Like, having a working terminal emulator, etc.) If users don’t want those packages, they have an easy opt-out via setting programs.sway.extraPackages to an empty list. If instead environment.extraPackages was set directly, it would be more difficult to remove those packages from that list while keeping other entries. I know there’s a PR to convert environment.systemPackages to an attrset, but it might ironically make this case even clunkier to handle.

For sway specifically, a migration step could include deprecating .extraPackages and adding a boolean option .includeDefaultPackages. It reduces granularity but at least makes a bit more sense.

5 Likes
3

How come? Wouldn’t the change then just be flipping environment.systemPackages.i3lock.enable = false; and setting programs.i3lock.enable = true? Module composition would handle the conflicts.

Or, if programs.i3lock set up some kind of wrapper, it could set environment.systemPackages.i3lock.enable = lib.mkForce false (or also just false and let users figure out what that means, or use an assert to tell users that a module is now available), so the entire breakage would have been avoided in the first place?

It’d actually even handle cases in which users are overriding i3lock, making it hard to remove from systemPackages.

Actually, as a side note, in this particular case, is it even necessary to do anything? Is adding a package to environment.systemPackages twice not a no-op?

1 Like
4

I was thinking of sway. Individually disabling a bunch of packages like foot and so on would be a pain especially if the set of packages change over time.

5

The problem (to the extent that there is one) is that a user might right now set extraPackages = [] if they explicitly don’t want the default packages. If I remove the package from the default value of extraPackages and set programs.i3lock.enable = mkDefault true instead, then users who have explicitly removed i3lock will have it enabled on them again without warning.

2 Likes