-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expose some additional nixosModules #261660
base: master
Are you sure you want to change the base?
Conversation
These are “general” modules that provide options without config and are already relied on by other projects (such as home-manager). Exposing them this way makes them an explicit part of the nixpkgs flake interface and can allow other projects to use, e.g., nixpkgs.nixosModules.assertions rather than something like (pkgs.path + "/nixos/modules/misc/assertions.nix") which is bad not only because it treats the directory structure of nixpkgs as an interface, but also because it shouldn’t be necessary to have `pkgs` at the point modules are loaded (as opposed to when they are applied).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea, but we have some work to do before we make these modules official.
These modules are already loaded by NixOS, so the use case they serve is either
- for disabling
- in these cases: not feasible, unless replaced by equivalent module
- a very minor use case, for which it's fine or preferable to just use the paths
- in these cases: not feasible, unless replaced by equivalent module
- publish for use by other projects
- your use case
Publishing modules as attributes will come with some expectations that aren't met. Currently they don't have a well defined scope. Rather, they're quite arbitrary sets of implementation details. (And if you're ok with relying on implementation details, that's what file paths are for)
More concretely, because they haven't been developed as "stand alone" modules, they have problems, at least some of which I've highlighted in the file comments.
I think they're significant problems and I wouldn't be comfortable suggesting that others use it, and make all the other Nixpkgs maintainers half-responsible for those problems.
Before we make any module public, I think they should have the following:
- Designated maintainers
- Maybe not using
meta.maintainers
, because the module that declares that is optional; it would be an unnecessary constraint on module system applications that want to use these modules. A comment or CODEOWNERS entry would be sufficient.
- Maybe not using
- Unit tests, i.e. that don't load NixOS
- Independent documentation. Could be included in the Nixpkgs manual, but must not rely on NixOS for its context.
@@ -56,6 +56,9 @@ | |||
legacyPackages = forAllSystems (system: import ./. { inherit system; }); | |||
|
|||
nixosModules = { | |||
assertions = ./nixos/modules/misc/assertions.nix; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has only half of the logic, so I'm surprised that other projects use it.
Maybe #207187 is worth revisiting.
It's not a NixOS module if it's supposed to be used in other projects, so it should be moved to modules.generic
. (See also NixOS/nix#6257)
@@ -56,6 +56,9 @@ | |||
legacyPackages = forAllSystems (system: import ./. { inherit system; }); | |||
|
|||
nixosModules = { | |||
assertions = ./nixos/modules/misc/assertions.nix; | |||
lib = ./nixos/modules/misc/lib.nix; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one has a bug. Type should be lazyAttrsOf
.
Not really a NixOS module either.
@@ -56,6 +56,9 @@ | |||
legacyPackages = forAllSystems (system: import ./. { inherit system; }); | |||
|
|||
nixosModules = { | |||
assertions = ./nixos/modules/misc/assertions.nix; | |||
lib = ./nixos/modules/misc/lib.nix; | |||
meta = ./nixos/modules/misc/meta.nix; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Meta" is not a suitable scope for a module. This file has meta.buildDocsInSandbox
, meta.doc
and meta.maintainers
. It should probably be split into meta/docs.nix
and meta/maintainers.nix
, at least, but even then, those modules aren't usable on their own either.
Thanks! This is exactly what I was hoping for. I’ll work on changes in this vein. |
These are “general” modules that provide options without config and are already relied on by other projects (such as home-manager). Exposing them this way makes them an explicit part of the nixpkgs flake interface and can allow other projects to use, e.g.,
rather than something like
which is bad not only because it treats the directory structure of nixpkgs as an interface, but also because it shouldn’t be necessary to have
pkgs
at the point modules are loaded (as opposed to when they are applied).