-
-
Notifications
You must be signed in to change notification settings - Fork 15.1k
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
Add minimal NixOS entrypoint #153211
Add minimal NixOS entrypoint #153211
Conversation
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.
Some ideas.
I still don't really see the point of the new files (I do see the point in a minimal module list however!) when I could just do this instead:
{
eval = import <nixpkgs/lib/eval-config.nix> {
system = "";
modules = [ whatever ];
};
}
@@ -0,0 +1,46 @@ | |||
let |
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.
Are you sure this is the right place for evaluating NixOS? I'd have expected this to be in nixos/default.nix
(but I know that place is already used). Maybe someone else has a good idea but this location makes it seem like importing that file gives me library functions around NixOS and not NixOS
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.
It is not great, I admit, and as you've noted that's mostly due to nixos/default.nix
being taken. I've considered turning the existing nixos/default.nix
into a __functor
, but I've seen the command line tools behave weird around __functor
and I don't want to touch such an essential file for an experimental feature.
I've also considered nixos/eval-config.nix
. While familiarity is good, it is too easily confused with nixos/lib/eval-config.nix
, which behaves rather differently, both wrt module list and a ton of legacy behavior we don't need.
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.
What about nixos/minimal-eval.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.
nixos/minimal-eval.nix
does sound less confusing. or maybe nixos/lib/minimal-eval.nix
? anything that doesn't break the pattern of import ./lib
pulling in all lib functions, really.
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.
anything that doesn't break the pattern of
import ./lib
pulling in all lib functions
Actually, that's exactly what this is doing, when looking at the flake.nix
change. Its lib.nixos
is simply this file.
So then it should be lib/default.nix
and the fact that the functions are about minimal evaluation is currently a coincidence.
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.
from the angle of what the flake exports you're totally right. but there's a lot of other stuff in nixos/lib
(like that now-standard eval-config.nix) that isn't available from import ./nixos/lib
. so i guess it's fine either way, and we retract the suggestion.
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.
Right, I now better understand your suggestion. Depending on whether those are intended for "public" consumption, the functions in those other files can later be exposed through nixos/lib/default.nix
or moved to a nixos/common
directory where exposure is not desirable.
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.
we have nixos/lib/eval-config.nix
, so I would propose nixos/lib/eval-config-minimal.nix
. Also nixos/lib/eval-config.nix
should be using nixos/lib/eval-config-minimal.nix
, so prove it is minimal and therefore the status quo cna be built on top of it.
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've moved the implementation as @Ericson2314 suggested, kept nixos/lib/default.nix
for the purpose of being lib.nixos
in the flake and anyone who likes the lib
style.
I'm undecided whether it only proves a point or whether it should be a requirement. If we find a reason to disconnect the two, proving a point shouldn't stop us from doing that. I can imagine that we find a useful change along the way, that we don't want to apply to conventional NixOS, yet I do hope the two can remain as similar as possible.
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.
Yeah if we have need to direct them, sure. I am not yet familiar with the more ambitious plans discussed before.
nixos/lib/default.nix
Outdated
|
||
modules: A list of modules that constitute the configuration. | ||
|
||
specialArgs: An attribute set of module arguments. Unlike |
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.
Maybe also add to the description that these are not meant for general use and should only be used when _module.args can't be used.
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.
Now it's
specialArgs: An attribute set of module arguments. Unlike
`config._module.args`, these are available for use in
`imports`.
`config._module.args` should be preferred when possible.
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.
config._module.args
should be preferred when possible.
Doing more in from inside the modules is good I suppose and I think maybe it could be typed this way as well.
What would @infinisil, who I believe came up with this recommendation, tell the reader?
1f5e2ba
to
aa1d542
Compare
The new function is specifically for working with a minimal module list, which you don't get out of |
While this pr is low risk, it is important code, so I won't merge it for another couple of days, so you can give more feedback. |
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 test does not run because nixpkgs.nix
includes meta attributes without loading the meta.nix
module. looks pretty good otherwise :)
1c3adbc
to
ca40dac
Compare
ca40dac
to
3168017
Compare
Will merge tomorrow, unless anyone has more feedback. |
I don't like to self-merge, but this is blocking other work. It's marked as experimental, so we can change it after merging anyway. |
Looking forward to seeing it 😄 |
Motivation for this change
See #148456
In short: improve modularity, enabling new use cases, improving performance. This is just a first step; doing this in reviewable increments.
This adds a new entrypoint, similar to the PoC, but incorporating some feedback, making the entrypoint itself more minimal.
It also marks the new function as experimental, because using a minimal module list may be a little rough at first and I know it to be incomplete. However, in the spirit of minimalism, this is ok and things that can't be made optional can be added later if necessary.
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)nixos/doc/manual/md-to-db.sh
to update generated release notes