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
configure the evaluation of modules in a modular way #51894
Conversation
@@ -10,26 +10,20 @@ with lib.types; | |||
|
|||
rec { | |||
|
|||
/* Evaluate a set of modules. The result is a set of two | |||
evalModules = import ./eval-modules.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 would equate the non-modular attributes allowed with that of nixos/lib/eval-config.nix
. It should only allow the non-modular attributes of evalModules
that were allowed before.
attributes: ‘options’: the nested set of all option declarations, | ||
and ‘config’: the nested set of all option values. | ||
!!! Please think twice before adding to this argument list! The more |
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 comment should probably be retained and put above the new evalModules
and maybe indicate that they are there only for backwards compatibility, and as such should be considered deprecated.
}: | ||
|
||
let extraArgs_ = extraArgs; pkgs_ = pkgs; | ||
extraModules = let e = builtins.getEnv "NIXOS_EXTRA_MODULE_PATH"; |
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.
Although there is no need anymore for this environment variable, in theory at least, in practice people might still want to use it, so this should probably be kept somehow for backwards compatibility.
evalModule = rec { | ||
_file = toString ./eval-modules.nix; | ||
key = _file; | ||
config = lib.mkMerge [ |
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.
Or should I have gone with lib.optionalAttrs
to add pkgs
to _module.args
?
then x callArgs' | ||
else callArgs' // x // builtins.listToAttrs (map | ||
(name: { inherit name; value = callArgs'.${name} // x.${name}; }) | ||
(builtins.filter (name: x ? ${name}) [ "args" "specialArgs" ])); |
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.
If configureModules
is just an attrset rather than a function mapping the existing attrset, I would expect the attributes args
and specialArgs
to be merged with the existing attrset.
(name: { inherit name; value = callArgs'.${name} // x.${name}; }) | ||
(builtins.filter (name: x ? ${name}) [ "args" "specialArgs" ])); | ||
|
||
configArgs = let len = builtins.length configurers; in if len == 1 |
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 wish I had a better heuristic to determine the entry point module, but all we got is a module list at this point, so this is the best I could come up with.
in lib.evalModules' { | ||
inherit prefix; | ||
modules = modules ++ [ evalModule ]; | ||
specialArgs = { modulesPath = toString ../nixos/modules; } // specialArgs // { inherit lib; }; |
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.
The modulesPath
is NixOS-specific. This might be undesirable if others projects (e.g. home-manager) want to use nixpkgs' module system.
Although I still like the idea behind this PR, I am no longer in favor of the current implementation, because it makes the configuration less composable, e.g. you can no longer pass your configuration to a function that does something like this: configuration: import <nixpkgs/nixos> { configuration = [ configuration ./other.nix ]; } |
Motivation for this change
After realizing that all solutions I could come up with to replace the lib module argument in my NixOS configuration had some shortcoming or another (#51797), I realized that was I really just wanted to be able to do, was to supply arguments to
nixos/lib/eval-config.nix
based on my configuration (i.e. as part of my configuration). This would allow me to supply my configuration with a custom lib in a pure manner, i.e. no ad hoc extension mechanism through an impure environment variable, and would work with all existing code. Of course this purity cannot be enforced, you could always still callgetEnv
or similar, but this solution should not reduce purity to my knowledge. If it does, please do tell. This solution also addresses the non-modular arguments toevalModules
, because except for themodules
attribute to determine theconfigureModules
attribute, everything else can be set through thisconfigureModules
attribute. And all the convenience attributes have been kept, but rewritten to their modular equivalent, so they could now be considered syntax sugar when usingconfigureModules
, but will still be allowed non-modular when supplied directly tonixos/lib/eval-config.nix
for backwards compatibility. I thought it best to only allow the one module to have aconfigureModules
attribute, which ought to be the entry point (e.g./etc/nixos/configuration.nix
), but unless it is safe to assume the first item in the list of modules is this entry point, I have no way to enforce this.I have tested it with some complex configurations, but the code is not yet ready for a potential merge it its current state, so keep this in mind when giving feedback, but I thought it best to already make a PR so I could already get feedback, so that I know sooner rather than later whether it is a good idea to begin with and what I could do to refine it. Here is an example of it in use:
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)