-
Notifications
You must be signed in to change notification settings - Fork 54
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
imp: provide per channel modules paths to make it easy to "backport" #42
Conversation
719848b
to
361e2fe
Compare
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 this, it just adds more information to make backporting/updating modules possible.
I think this should go inside configurationBuilder
instead and possibly under the optionalAttrs (output == "nixosConfigurations")
. It is a default, so its not terrible to put in evalHostArgs, but also its nixos specific and theres another place where modulesPath is edited, so it makes sense to go there.
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.
Looks good to me, tho let's not merge it just yet. I was about to fix existing issues in staging and actually do something about the release since we really need one
systemFlake.nix
Outdated
f = channelName: | ||
{ "${channelName}ModulesPath" = "${channels.${channelName}.input}/nixos/modules"; }; | ||
channelModulesPath = map f (attrNames channels); | ||
merge = lhs: rhs: lhs // rhs; |
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 don't think there is much of a point for this merge function :D
816d7d3
to
0d243e3
Compare
0d243e3
to
e7ae270
Compare
a module. imports = [ ${myChannelModulesPathk}/installer/... Signed-off-by: David Arnold <dar@xoe.solutions>
ac61a93
to
cbc521b
Compare
cbc521b
to
a5246e7
Compare
@@ -149,9 +159,9 @@ let | |||
]; | |||
}) | |||
] ++ host.modules; | |||
specialArgs = host.specialArgs; | |||
inherit 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.
this was in failure mode before. we have to remain careful about the builder generalization.
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.
There was actually a reason for this: setting modulesPath on a non-nixos builder to a nixos path would break things.
I'll address this in #44
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.
Ok thanks! We have to be really careful about rhose kind of things. There are now +- 5 design dimenstion in 3 lines of code.
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 we'd need to split them eventually apart completely and implement a generic builder and a specific nixos one on top.
a module. imports = [ ${myChannelModulesPathk}/installer/...
This allows users to relatively conveniently pull in modules from other channels. The chances of success are mixed depending on the channel distance, but if it works, it's great.