Skip to content

move necessary parts of core to mkHosts#223

Closed
Pacman99 wants to merge 3 commits intodivnix:corefrom
Pacman99:core-module
Closed

move necessary parts of core to mkHosts#223
Pacman99 wants to merge 3 commits intodivnix:corefrom
Pacman99:core-module

Conversation

@Pacman99
Copy link
Member

@Pacman99 Pacman99 commented Mar 27, 2021

Title doesn't match the patch yet.

I want to move all the necessary nix features of core to mkHosts, and go back to importing core through suites so we don't subvert the standard devos api. Then to remove core, you can just drop the line in suites/delete the core profile.

@Pacman99 Pacman99 requested review from blaggacao and nrdxp and removed request for nrdxp March 27, 2021 22:13
@nrdxp
Copy link
Collaborator

nrdxp commented Mar 28, 2021

What is the semantic for disabling core? From my brief review of the code, it still seems to be included.

@Pacman99
Copy link
Member Author

Pacman99 commented Mar 28, 2021

Yup its still included by default. But now you could do:

devos.core.enable = false;

in a host file to drop core. In the future we could add more options, so you could, for example, just add the nix stuff and not the aliases. But for now I wanted to keep this simple.

@blaggacao
Copy link
Contributor

blaggacao commented Mar 28, 2021

I'm not so sure about this, as it might blur the lines between profiles and modules.

So having modules enabled by default might be a dangerous precedent. I see it is perfectly valid nix, but people should define things they always want in a common / core profile.

If they do, profiles are a purely constructive index of your environment.

If people start enabling modules by default, nobody can tell the state of the world by only looking at profiles alone.

This is a recipe to complexity.

We actually might error out on modules that are enabled by default to enforce sane contracts about modules.

I'm not sure if it is currently documented, but as much as we "forbid" to define options in profiles, we probably shouls "forbid" altering the system state in modules.

@Pacman99
Copy link
Member Author

modules definitely should alter system state, but I think your point is they shouldn't by default. And thats where I disagree. Even nixos has many modules enabled by default to ensure you get a proper linux system running. And in the same way, this core module could be enabled by default to get a proper devos system - you can't use devos without unstable features.

There are other parts of this module that probably shouldn't be enabled by default like aliases and fonts. And those should probably go under more options. An alternative is to enable by default in mkHosts/hosts - another place where system state is altered thats not in profiles.

Also I messed this up and forgot to add mkIf to the config, but thats a code issue.

@nrdxp
Copy link
Collaborator

nrdxp commented Mar 29, 2021

This is an excellent discussion to have an one that needs to happen. FWIW, I have been "feeling" like there should be a way to disable core, but some of it may have to moved to mkHosts to ensure critical options are always set, such as nix.package. Other than those options, core just tries to provide some useful tools, aliases and the like without getting too overwhelming; not 100% critical stuff (so safely removable), but useful to have by default.

if I were going to do it, this is the cleanest implementation for sure. I think it's okay to move forward on this as long as we ensure nothing absolutely critical for NixOS to function properly with flakes is left in core.

@Pacman99
Copy link
Member Author

Yeah I agree with that. Since this PR isn't blocking anything else, we can take some time to figure out what can be moved and actually add a few more options. Also possibly rename it to something else, if its no longer critical, I'm not sure core is the right name anymore. Maybe something along the line of defaults - from flake-utils-plus - or features.

@blaggacao
Copy link
Contributor

blaggacao commented Mar 29, 2021

By modularizing it, we extend the devos api to modules, and that's a bit unwarranted.

We should be able to include the devos dependencies into mkHosts and keep an henceforth optional core profile in the core template, as a convenience offering (which I would not want to miss).

@Pacman99
Copy link
Member Author

Pacman99 commented Mar 29, 2021

By modularizing it, we extend the devos api to modules, and that's a bit unwarranted.

I've actually been wanting to extend devos api to modules. I think its one of the most powerful features of nixpkgs, and instead of writing our own api and methods to get information from users, the module system has a a great and perfected method of passing and storing data. The PR for multi-arch support already established this somewhat, by getting information from the user through the use of nixpkgs.system. And I have many ideas, for example - configSystem an option that could be set to darwin or robotnix to establish what configuration system a host should be evaluated with.

We should be able to include the devos dependencies into mkHosts and keep an henceforth optional core profile in the core template, as a convenience offering (which I would not want to miss).

So I think we are in agreement in the first part that the necessary bits of core should go to mkHosts. But I believe the remaining parts should be in modules, so we can make use of options to allow users to enable specific defaults. Like defaultShellAliases. And in the future we have a place to add more options to let users optionally pass more information about their hosts.

@Pacman99
Copy link
Member Author

Moved all the nix related config to mkHosts and changed the module options to devos.defaults. Then enable all defaults in mkHosts with mkDefault. I think the nix related parts are the 'required' parts of the core module and belong in mkHosts. The rest are optional and should be allowed to be disabled. I actually want to keep the module itself called core, so we can add more options into it in the future that are 'core' to devos.

Pacman99 added 3 commits March 31, 2021 08:11
allows core to be disabled and prevents auto-import as a profile
move all nix related config to mkHosts
create new options for devos defaults and enable by default in hosts
@Pacman99
Copy link
Member Author

Pacman99 commented Apr 3, 2021

I'm thinking of making this PR just a move of all required core features to mkHosts. Then core can just be a standard profile, and I think it should be imported through suites, so mkHosts isn't subverting the standard devos api by separately importing core.
This might be a better way of making core optional, if you don't want the defaults, you can just drop the profile and the relevant line in suites.

@Pacman99 Pacman99 changed the title core: move to modules move necessary parts of core to mkHosts Apr 3, 2021
@Pacman99 Pacman99 marked this pull request as draft April 3, 2021 00:51
@Pacman99
Copy link
Member Author

Pacman99 commented Apr 3, 2021

Made a new PR for what I actually now want to accomplish, #229, so closing this.

@Pacman99 Pacman99 closed this Apr 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants