search

nix-community / nixbox

NixOS Vagrant boxes [maintainer=@ifurther]
https://app.vagrantup.com/nixbox/
MIT License
304 stars 101 forks source link

Adding build time ERB/erubis dependency? #51

Open ersinakinci opened 4 years ago

ersinakinci commented 4 years ago

Would folks be open to adding ERB/erubis as a dependency to this project to customize config files during the provisioning step?

Use case

I'm adding support for the parallels-iso builder, which entails setting the hardware.parallels.enable = true option in NixOS to enable Parallels guest OS support. However, I'm currently stuck because this line in parallels-guest.nix conflicts with this line in configuration.nix, resulting in the following error when running packer build:

    parallels-iso: building the configuration in /mnt/etc/nixos/configuration.nix...
==> parallels-iso: error: The option `services.timesyncd.enable' has conflicting definitions, in `/mnt/etc/nixos/hardware-builder.nix' and `/mnt/etc/nixos/configuration.nix' and `/nix/var/nix/profiles/per-user/root/channels/nixos/nixos/modules/virtualisation/parallels-guest.nix'.
==> parallels-iso: (use '--show-trace' to show detailed location information)
==> parallels-iso: Unregistering virtual machine...
==> parallels-iso: Deleting output directory...
Build 'parallels-iso' errored: Script exited with non-zero exit status: 1.Allowed exit codes are: [0]

In my opinion, the proper way to deal with this is to generate configuration.nix from a template during the provisioning step before it's downloaded by install.sh. In this case, we would stick a conditional that would either emit or omit services.timesyncd.enable = false; from configuration.nix depending on which builder is being used.

I think that using ERB makes sense since Packer and Vagrant use Ruby and erubis is already a Packer dependency.

PierreR commented 4 years ago

Do we really need templating ? We can either move the line services.timesyncd.enable in each builder with the appropriate flag (easy) or use a boolean condition based on the value of config.virtualisation.vmware.guest.enable and cie. I don't really see why the second option would be better than the first in this case.

ersinakinci commented 4 years ago

I hadn't thought of the option of moving services.timesyncd.enable to the builders, that makes sense. Although now that I think about it, I feel that getting rid of all the builders and replacing them with a templated configuration.nix would be a cleaner approach. If we move services.timesyncd.enable, we'll end up duplicating it across several builders, plus it will decrease visibility by burying it in expressions that are meant to set up virtualization-specific configuration.

Templating is also more scalable for future changes, allowing for more complex logic as we add support for more VM's. And as I mentioned, erubis is already a Vagrant dependency, so it's not like we'll be asking most people to download something new.

I personally wouldn't want to introduce runtime checks in configuration.nix for enabling services depending on whether certain virtualization options have been enabled. That feels brittle.

PierreR commented 4 years ago

Fair enough. As an extra note, there had been some chat about replacing packer with nixops here: https://github.com/nix-community/nixbox/issues/38