Skip to content
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

Expose internal modules as flake nixosModules. #19

Closed
wants to merge 1 commit into from

Conversation

gcoakes
Copy link

@gcoakes gcoakes commented Mar 16, 2021

No description provided.

Copy link

@Hoverbear Hoverbear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested using this module in my own flake (https://github.com/Hoverbear-Consulting/flake/tree/wsl) and it works fine. Hope you don't mind if I keep using it! 😉

I was originally just trying to package syschdemd (https://github.com/Hoverbear/NixOS-WSL/tree/packages.syschdemd) but your approach is much better.

@Trundle
Copy link
Member

Trundle commented Sep 27, 2021

The issue with this PR is that it breaks the non-flakes case: you can't nixos-rebuild from inside WSL.

@gcoakes gcoakes force-pushed the modularize branch 9 times, most recently from e4dac2f to f3ddb4f Compare March 5, 2022 19:28
@gcoakes
Copy link
Author

gcoakes commented Mar 5, 2022

I stopped using WSL for a bit, but I decided to jump back into it a bit ago. I forgot this PR was even still up, and I started from the latest master a few days ago. The latest state of the branch should be fully backwards compatible even with the non-flake use case. @Trundle, I know you stepped back a bit from maintaining this project, but would you be able to comment whether the current iteration of this alleviates your previous concerns?

To give some background on my specific motivation for the changes, I use flakes to manage multiple system configurations with some shared modules (like my dev environment). I just want to consume the unique parts of NixOS-WSL like syschdemd and system.build.tarball, and I want to include the wsl module without conditional imports. So, these changes move most of the functionality behind enable options like wsl.enable and wsl.tarball.enable. I also added options to disable copying the config to /etc/nixos and setting the channels in the tarball. I keep my config in my home directory and just run sudo nixos-rebuild switch --flake .#work, so I don't want the default configs copied. Also, I don't use channels, so I don't want that copied either.

This would be breaking to the existing behavior except that all the defaults replicate the existing behavior faithfully. So, even people who consumed NixOS-WSL downstream by directly importing files shouldn't be impacted. configuration.nix imports module.nix and sets wsl.enable = true;. build-tarball.nix has all it's options set to true by default meaning that importers will see it almost as if it were written as { config.system.build.tarball = ... } rather than with options.

Consuming the flake downstream is as simple as adding NixOS-WSL.nixosModule to the list of modules and setting wsl.enable = true;.

@gcoakes gcoakes force-pushed the modularize branch 2 times, most recently from b646267 to 3cab1b7 Compare March 5, 2022 20:42
Create module.nix with most of the contents of configuration.nix behind
an `enable` option. Import that module and enable the option in
configuration.nix to retain backwards compatibility. This allows the
flake to be consumed by other system configurations without doing
conditional imports.
@nzbr
Copy link
Member

nzbr commented Mar 6, 2022

I have made similar changes to this in #56. It uses flake-compat in order to have everything (including the default system config) inside the flake. Having the option to disable copying the config and channels to the tarball sounds great to me

@gcoakes
Copy link
Author

gcoakes commented Mar 6, 2022

@nzbr, it looks like your PR has significantly more changes fixing a few issues I haven't even encountered yet. I'm more than happy to drop this PR if yours gets merged as it seems to accomplish the same goals.

I was very careful to keep my change backwards compatible even with the names of the files. This was in case someone downstream was consuming it by just importing build-tarball.nix or something similar. I did this so it was more palatable from a maintenance perspective. I would hate to delay upstreaming modularization because I did too many changes in one PR.

@nzbr
Copy link
Member

nzbr commented Mar 24, 2022

I have moved your idea to a new issue (#61), so I am closing this PR. Thank you though!

@nzbr nzbr closed this Mar 24, 2022
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.

None yet

4 participants