-
Notifications
You must be signed in to change notification settings - Fork 107
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
mkDevos and evalDevosArgs: Move flake implementation logic into lib #206
Changes from all commits
7e0ef75
0a00a3a
052d870
2ce0be4
5e8ebea
01d4eb2
b81f426
a485d6c
80f6ed7
6b23bd7
558b926
077cd91
58fb2d4
e7f972f
6dd8438
1a1a9a6
0e19655
06f9b7b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
# Use mkDevos | ||
You can also use devos as a flake input and get the advantage of using | ||
nix flakes to sync with upstream changes in devos. | ||
|
||
You can either use the default template or use the 'mkdevos' template which only | ||
includes the necessary files for `mkDevos` usage. It can be pulled with: | ||
```sh | ||
nix flake init -t github:divnix/devos#mkdevos | ||
``` | ||
|
||
Once you have a template, edit your flake.nix to add devos as an input: | ||
```nix | ||
inputs = { | ||
... | ||
devos.url = "github:divnix/devos"; | ||
}; | ||
``` | ||
then replace the outputs section with this: | ||
```nix | ||
outputs = { self, devos, ... }: devos.lib.mkDevos { inherit self; }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From a clean naming point of view this should be "Make a flake with devos". Else we immediatly have a reasoning issue: What is this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Comment on lines
+13
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider instead of specifying individual changes be like "this is how your flake should look like" and users can add extra attributes if necessary There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like that idea. I think docs should be its own PR too, discussion for that can happen on its own. |
||
``` | ||
|
||
Now you can sync with upstream devos changes just like any other flake input. | ||
|
||
If you would like to change your directory's structure, you can pass | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not a big fan of nobleing this into the docs as a stand-alone use case: I'm not sure if preference is a use case motivated all by itself. Also we should not throw options at users without clear guidance in what situations they have to use which option. |
||
arguments to `mkDevos` to outline where certain things are. You can take a | ||
look at [evalDevosArgs](../../lib/devos/evalDevosArgs.nix) to learn about | ||
the arguments that it can take. | ||
|
||
> ##### Notes: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A few too many notes, I guess. If it merits exhibition, then it probably also deserves more than a note. We should find other means for them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My thoughts too, the docs need some work |
||
> - All inputs used in devos must also be added to your flake, due to an | ||
> [issue](https://github.com/NixOS/nix/pull/4641) with the `follows` attribute. | ||
> - You can append `/community` to the `devos.url` to get access to community modules | ||
> and packages which can be added in [extern](../../extern). | ||
> - Many folders in devos will no longer be needed and can, optionally, be deleted. | ||
> - To use community profiles, you will have to copy them to your tree | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? — asks the reader (not me). Is it relevant (enough) to mention? Can't it be solved with better UX? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well its because profiles are not exported anywhere in the flake. I don't see the point in adding the |
||
> - If you use this method, you will no longer be able to edit devos internals. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this can be dropped. Rather we need to define clear guidance when users are expected to use which method:
I think maybe we should avoid (end user) documenting this PR alltogether at this moment and rather have a discussion about what usage modes we recommend for which situation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would appreciate being told this, just to make it clear the cons of using mkDevos. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,68 +28,11 @@ | |
pkgs.inputs.nixpkgs.follows = "nixos"; | ||
}; | ||
|
||
outputs = inputs@{ deploy, nixos, nur, self, utils, ... }: | ||
let | ||
inherit (self) lib; | ||
inherit (lib) os; | ||
|
||
extern = import ./extern { inherit inputs; }; | ||
|
||
multiPkgs = os.mkPkgs; | ||
|
||
outputs = { | ||
nixosConfigurations = | ||
import ./hosts (nixos.lib.recursiveUpdate inputs { | ||
inherit multiPkgs extern; | ||
defaultSystem = "x86_64-linux"; | ||
lib = nixos.lib.extend (final: prev: { | ||
dev = self.lib; | ||
}); | ||
}); | ||
|
||
homeConfigurations = os.mkHomeConfigurations; | ||
|
||
nixosModules = | ||
let moduleList = import ./modules/module-list.nix; | ||
in lib.pathsToImportedAttrs moduleList; | ||
|
||
homeModules = | ||
let moduleList = import ./users/modules/module-list.nix; | ||
in lib.pathsToImportedAttrs moduleList; | ||
|
||
overlay = import ./pkgs; | ||
overlays = lib.pathsToImportedAttrs (lib.pathsIn ./overlays); | ||
|
||
lib = import ./lib { inherit nixos self inputs; }; | ||
|
||
templates.flk.path = ./.; | ||
templates.flk.description = "flk template"; | ||
defaultTemplate = self.templates.flk; | ||
|
||
deploy.nodes = os.mkNodes deploy self.nixosConfigurations; | ||
outputs = inputs@{ deploy, nixos, nur, self, utils, ... }: | ||
let | ||
lib = import ./lib { inherit self nixos inputs; }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a little idea for the future - separate the library implementation from this template repository (probably not relevant as part of this PR) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I would be for that. @nrdxp @blaggacao thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm definitely up for it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitly, in the coming days, until 1.0 we might need the benefit of atomic spanning PRs, still, though. |
||
in | ||
lib.mkDevos { | ||
inherit self; | ||
}; | ||
|
||
systemOutputs = utils.lib.eachDefaultSystem (system: | ||
let pkgs = multiPkgs.${system}; in | ||
{ | ||
checks = | ||
let | ||
tests = nixos.lib.optionalAttrs (system == "x86_64-linux") | ||
(import ./tests { inherit self pkgs; }); | ||
deployHosts = nixos.lib.filterAttrs | ||
(n: _: self.nixosConfigurations.${n}.config.nixpkgs.system == system) self.deploy.nodes; | ||
deployChecks = deploy.lib.${system}.deployChecks { nodes = deployHosts; }; | ||
in | ||
nixos.lib.recursiveUpdate tests deployChecks; | ||
|
||
packages = utils.lib.flattenTreeSystem system | ||
(os.mkPackages { inherit pkgs; }); | ||
|
||
devShell = import ./shell { | ||
inherit self system; | ||
}; | ||
} | ||
); | ||
in | ||
nixos.lib.recursiveUpdate outputs systemOutputs; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,9 @@ rec { | |
paths' = lib.filter (lib.hasSuffix ".nix") paths; | ||
in | ||
genAttrs' paths' (path: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In Nix usually, collection itself (in this case |
||
name = lib.removeSuffix ".nix" (baseNameOf path); | ||
name = lib.removeSuffix ".nix" | ||
# Safe as long this is just used as a name | ||
(builtins.unsafeDiscardStringContext (baseNameOf path)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's motivating this change? (And typo in the comment) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. see pr that changes |
||
value = import path; | ||
}); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,10 +7,21 @@ | |
config = { allowUnfree = true; }; | ||
}; | ||
|
||
importIf = path: fallback: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does not allow to declare things inline (within the flake). There is no reason to enforce library users to use any sort of file hierarchy. To reflect that, rather let's use this implementation: https://github.com/numtide/flake-utils/blob/master/simpleFlake.nix#L31 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Big reverbaration step of mine, see other comment about my thought process, here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah with the planned |
||
if builtins.pathExists path then import path else fallback; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Won't it crash if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but this is supposed to be a thin wrapper around nix's |
||
|
||
profileMap = map (profile: profile.default); | ||
|
||
evalDevosArgs = dev.callLibs ./evalDevosArgs.nix; | ||
|
||
mkDevos = dev.callLibs ./mkDevos.nix; | ||
|
||
mkNodes = dev.callLibs ./mkNodes.nix; | ||
|
||
mkHosts = dev.callLibs ./mkHosts.nix; | ||
|
||
mkSuites = dev.callLibs ./mkSuites.nix; | ||
|
||
mkProfileAttrs = dev.callLibs ./mkProfileAttrs.nix; | ||
|
||
mkPkgs = dev.callLibs ./mkPkgs.nix; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,166 @@ | ||
{ self, dev, nixos, inputs, ... }: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
What is it? (Can we make that evident from its name?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't like dev either, it is kind of confusing. I'm not sure how to fix it though outside of better documentation. Its just a side effect of needing to export lib as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. perhaps simply renaming to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
{ args }: | ||
let | ||
argOpts = with nixos.lib; { config, options, ... }: | ||
let | ||
inherit (dev) os; | ||
inherit (os) importIf; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could that be Or to align with how we import:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looka like it would be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm also not sold on not declaring it locally as long as it is not of actual use in another place. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could just declare it in evalDevosArgs, its really only useful there. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should not be necessary with |
||
|
||
cfg = config; | ||
inherit (cfg) self; | ||
|
||
inputAttrs = types.functionTo types.attrs; | ||
in | ||
{ | ||
options = with types; { | ||
self = mkOption { | ||
type = addCheck attrs nixos.lib.isStorePath; | ||
description = "The flake to create the devos outputs for"; | ||
}; | ||
hosts = mkOption { | ||
type = path; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not convinced of any benefits of encoding any specific file hierarchy into the library's API. The library should require to abstractly fulfill its own requirements and nothing more. How people organize their files does not matter as long as their contents fulfill the This stance complements my call for Note, that flake.nix will shrink dramatically. Hence the only sane purpose we can give it is "index". We should not hide that away into a library function. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An alternative is to layer another funktion:
So we shoudn't do a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will address in planned |
||
default = "${self}/hosts"; | ||
defaultText = "\${self}/hosts"; | ||
description = "Path to directory containing host configurations"; | ||
}; | ||
packages = mkOption { | ||
# functionTo changes arg names which breaks flake check | ||
type = types.anything // { | ||
check = builtins.isFunction; | ||
description = "Nixpkgs overlay"; | ||
}; | ||
default = importIf "${self}/pkgs" (final: prev: {}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It makes me wonder if users won't get confused at first that something gets picked up by just creating a folder (without manually declaring it) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, I should be able to address this soon when switching to |
||
defaultText = "\${self}/pkgs"; | ||
description = '' | ||
Overlay for custom packages that will be included in treewide 'pkgs'. | ||
This should follow the standard nixpkgs overlay format - two argument function | ||
that returns an attrset. | ||
''; | ||
}; | ||
modules = mkOption { | ||
type = listOf anything; | ||
default = importIf "${self}/modules/module-list.nix" []; | ||
defaultText = "\${self}/modules/module-list.nix"; | ||
apply = dev.pathsToImportedAttrs; | ||
description = "list of modules to include in confgurations"; | ||
}; | ||
userModules = mkOption { | ||
type = listOf anything; | ||
default = importIf "${self}/users/modules/module-list.nix" []; | ||
defaultText = "\${self}/users/modules/module-list.nix"; | ||
apply = dev.pathsToImportedAttrs; | ||
description = "list of modules to include in home-manager configurations"; | ||
}; | ||
profiles = mkOption { | ||
type = path; | ||
default = "${self}/profiles"; | ||
defaultText = "\${self}/profiles"; | ||
apply = os.mkProfileAttrs; | ||
description = "path to profiles folder"; | ||
}; | ||
userProfiles = mkOption { | ||
type = path; | ||
default = "${self}/users/profiles"; | ||
defaultText = "\${self}/users/profiles"; | ||
apply = os.mkProfileAttrs; | ||
description = "path to user profiles folder"; | ||
}; | ||
suites = | ||
let | ||
defaults = { user = {}; system = {}; }; | ||
in | ||
mkOption { | ||
type = inputAttrs; | ||
default = importIf "${self}/suites" ({...}: defaults); | ||
defaultText = "\${self}/suites"; | ||
apply = suites: defaults // os.mkSuites { | ||
inherit suites; | ||
inherit (config) profiles users userProfiles; | ||
}; | ||
description = '' | ||
function with inputs 'users' and 'profiles' that returns attribute 'system' | ||
which defines suites passed to configurations as the suites specialArg | ||
''; | ||
}; | ||
users = mkOption { | ||
type = path; | ||
default = "${self}/users"; | ||
defaultText = "\${self}/users"; | ||
apply = os.mkProfileAttrs; | ||
description = "path to folder containing user profiles"; | ||
}; | ||
extern = | ||
let | ||
defaults = { | ||
modules = []; overlays = []; specialArgs = {}; | ||
userModules = []; userSpecialArgs = []; | ||
}; | ||
in | ||
mkOption { | ||
type = inputAttrs; | ||
default = importIf "${self}/extern" ({...}: defaults); | ||
defaultText = "\${self}/extern"; | ||
# So unneeded extern attributes can safely be deleted | ||
apply = x: defaults // (x { inputs = inputs // self.inputs; }); | ||
description = '' | ||
Function with argument 'inputs' with all devos and this flake's inputs. | ||
The function should return an attribute set with modules, overlays, and | ||
specialArgs to be included across devos | ||
''; | ||
}; | ||
overlays = mkOption { | ||
type = path; | ||
default = "${self}/overlays"; | ||
defaultText = "\${self}/overlays"; | ||
apply = x: dev.pathsToImportedAttrs (dev.pathsIn x); | ||
description = "path to folder containing overlays which will be applied to pkgs"; | ||
}; | ||
overrides = | ||
let | ||
defaults = { modules = []; disabledModules = []; packages = _: _: _: {}; }; | ||
in | ||
mkOption { | ||
type = attrs; | ||
default = importIf "${self}/overrides" defaults; | ||
apply = x: defaults // x; | ||
defaultText = "\${self}/overrides"; | ||
description = "attrset of packages and modules that will be pulled from nixpkgs master"; | ||
}; | ||
genDoc = mkOption { | ||
type = functionTo attrs; | ||
internal = true; | ||
description = "function that returns a generated options doc derivation given nixpkgs"; | ||
}; | ||
}; | ||
config.genDoc = | ||
let | ||
singleDoc = name: value: '' | ||
## ${name} | ||
${value.description} | ||
|
||
${optionalString (value ? type) '' | ||
*_Type_*: | ||
${value.type} | ||
''} | ||
${optionalString (value ? default) '' | ||
*_Default_* | ||
``` | ||
${builtins.toJSON value.default} | ||
``` | ||
''} | ||
${optionalString (value ? example) '' | ||
*_Example_* | ||
``` | ||
${value.example} | ||
``` | ||
''} | ||
''; | ||
in | ||
pkgs: with pkgs; writeText "devosOptions.md" | ||
(concatStringsSep "" (mapAttrsToList singleDoc (nixosOptionsDoc { inherit options; }).optionsNix)); | ||
}; | ||
in | ||
nixos.lib.evalModules { | ||
modules = [ argOpts args ]; | ||
} |
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'm not a big fan of just throwing options at the user. It increases complexity and defeats the purpose of being easy-onboarding.
We should clearly blueprint the use case in simple english so that users can identify themselves as "yes that's me, I'll do it this way".
devos.lib
If you think devos template is tpp complicated ...
Once we are 1.0, don't fork this (!), use it as a lib instead...
...
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 think for now it should be documented as a 'next step' and another option. My reasoning is that there is some work to switch to the
mkDevos
method, and it's not an easy change.