nix-community / impermanence

Modules to help you handle persistent state on systems with ephemeral root storage [maintainer=@talyz]
MIT License
1.01k stars 76 forks source link

Add global enable switches for nixos and home-manager #171

Closed 0xf09f95b4 closed 2 weeks ago

0xf09f95b4 commented 4 months ago

In #138 and #167, requests for some global enable option were made.

I've also found some use-cases where I'd like to disable all side-effects of the impermanence modules for specific configurations while globally importing the modules. Though setting environment.persistence to {} generally works, it has two drawbacks:

This PR adds two additional options to impermanence:

I'd rather have this module be disabled by default (as most nixos modules are) but this would break every current install when updating which would be very bad. Instead, these new options are enabled by default, which means current installs would not be affected at all by these changes.

I put the nixos switch into boot because I felt that this module mostly affects the boot process (though some systemd services are started as well). I'm open to any suggestions for better placement.

Kreyren commented 4 months ago

I'd rather have this module be disabled by default (as most nixos modules are) but this would break every current install when updating which would be very bad. Instead, these new options are enabled by default, which means current installs would not be affected at all by these changes. -- @0xf09f95b4 (https://github.com/nix-community/impermanence/pull/171#issue-2150772607)

I would argue for these to be set to false by default as in the worst case the user will have to reboot and change configuration and in the present usecase it seems that it would make a permanent breaking changes to all my systems that don't have impermenance integrated in a reliable way yet https://github.com/Kreyren/nixos-config/tree/aaf809b0212d63ad694054139a74dc97c32ff497

Kreyren commented 3 months ago

ping @0xf09f95b4 @talyz could this be merged or brainstormed? This feature is a major roadblock for my nixos configuration atm

0xf09f95b4 commented 3 months ago

I'm ready to get this merged ;) I'm currently just using my fork for my projects. I don't think I can make the decision if impermanence should be disabled by default though. I still see the danger of breaking existing configurations.

talyz commented 3 months ago

For the NixOS module, there's already an enable option that can be set for each declared persistent storage path, i.e. environment.persistence."/persistent".enable. It seems to be undocumented, though. It should be added to the home-manager module as well.

0xf09f95b4 commented 2 months ago

It might seem a bit nitpicky, but disabling all storage paths is not completely side-effect free, even environment.persistence = {} still leads to a few empty activation script files added to the system.

I'd still really love a global enable/disable switch for impermanence.

Adding individual enable options to the home-manager module would be a good idea regardless.

talyz commented 2 months ago

It shouldn't be a problem to make it side-effect free, and if we do, I don't see the need for global switches.

Kreyren commented 1 month ago

environment.persistence."/persistent".enable

It shouldn't be a problem to make it side-effect free, and if we do, I don't see the need for global switches.

On a more complicated setup that is using various directories it's really pain to manage and i want to implement a logic that would on demand enable/disable impermanence per system, so the proposed options would make programming logic practically usable on complex setups.

ElvishJerricco commented 1 month ago

@talyz

It shouldn't be a problem to make it side-effect free, and if we do, I don't see the need for global switches.

Well, it's not uncommon one module to set options for another module in case the other module is enabled. In order to do this, you really need an enable switch to check in a mkIf, or you need it to be that the options you set will have no effect if the user has chosen to disable the other module.

Neither method is possible with impermanence right now, short of the user knowing to do environment.persistence = lib.mkForce {};

talyz commented 1 month ago

But we already have enable switches, they're just on a submodule level instead of a module one. Why would this not be enough?

Usage example:

{
  config = lib.mkIf config.environment.persistence.main.enable {
    ...
  }
}

or with more than one persistent storage path

{
  config = lib.mkIf (config.environment.persistence.main.enable && config.environment.persistence.backup.enable) {
    ...
  }
}
talyz commented 1 month ago

It might seem a bit nitpicky, but disabling all storage paths is not completely side-effect free, even environment.persistence = {} still leads to a few empty activation script files added to the system.

Should be solved by #189. Please check it out.

bddvlpr commented 1 month ago

For the NixOS module, there's already an enable option that can be set for each declared persistent storage path, i.e. environment.persistence."/persistent".enable. It seems to be undocumented, though. It should be added to the home-manager module as well.

To follow up on this, is there a specific reason this isn't implemented in home-manager or has it just not been developed yet?

talyz commented 1 month ago

To follow up on this, is there a specific reason this isn't implemented in home-manager or has it just not been developed yet?

No reason, it just hasn't been developed yet.

ElvishJerricco commented 4 weeks ago

But we already have enable switches, they're just on a submodule level instead of a module one. Why would this not be enough?

I'm not following. The point I made was that a module would like to set environment.persistence."/foo" so that it only takes effect if the config is using impermanence at all. Right now it's not possible to mkIf that kind of condition. So a module either configures /foo to be an impermanence directory or it doesn't, and it cannot decide whether or not to do that based on whether impermanence is in use.

talyz commented 4 weeks ago

There's no need for mkIf in that case: the module can just go ahead and always set the options in environment.persistence."/foo"; whether that configuration will be active can be decided by other modules through environment.persistence."/foo".enable.

ElvishJerricco commented 2 weeks ago

I think that's missing the point. If an external module X creates several environment.persistent entries, then I the user of X have to be aware of exactly which entries X made and include the appropriate enable = false lines for each of them in any config that I don't want persistence for. That is extremely error prone and un-ergonomic. As is, the best way to accomplish this is for X to have its own usePersistence module option, which is just a global enable switch made by someone else instead.

Kreyren commented 2 weeks ago

I forked the repo and implemented this commit, referencing the real life usecase: https://github.com/search?q=repo%3ANiXium-org%2FNiXium+boot.impermanence.enable&type=code

So that now i can declaratively rebuild any system with impermanence and without it on demand.

talyz commented 2 weeks ago

@ElvishJerricco In what scenario would an external module create persistentStoragePaths? If it would, you would have bigger issues anyway, such as providing backing storage for them. Disregarding that, it would also be pretty strange for an external module to create many different persistentStoragePaths as most people only need one or maybe two of them and the reason you would need more would certainly be outside the scope of the module anyway.

@Kreyren For your use, you could just use environment.persistence."/nix/persist/system".enable instead - just set it to false for your non-impermanence systems.

Kreyren commented 2 weeks ago

@Kreyren For your use, you could just use environment.persistence."/nix/persist/system".enable instead - just set it to false for your non-impermanence systems. -- @talyz (https://github.com/nix-community/impermanence/pull/171#issuecomment-2184038544)

The problem is that environment.persistent.<path> has a variable so automating that is a problem as in real-life usage you standardize /persistence for non-disko, but for disko setup it's more convinient to use /nix/persist to be able to e.g. just declare the whole /nix as BTRFS with @persist sub-volume (other filesystem setups have different needs depending on the projected data type that they will hold).

Then for users i am providing /nix/persist/users through system and for invidual users using /nix/persist/users/<user> which gets enabled per user so the check for mkIf config.environment.persistent.<path>.enable becomes unusable with complexity as i would have to do add checks for each user to the condition.

Additionally I expect the setup on system-level to get significantly more complicated with integrating https://github.com/NiXium-org/NiXium/issues/16

ElvishJerricco commented 2 weeks ago

@talyz I'm pretty tired of explaining the use case, and it's frustrating that your answer continues to be "I wouldn't be in that scenario".

It's standard practice for a module to have a global enable flag. The amount of discussion on this PR is ridiculous. I'm out.

talyz commented 2 weeks ago

@ElvishJerricco I was hoping that informing about the current enable options and how to use them would be enough to keep this discussion short, but obviously not. Instead, I'll do what I was hoping to avoid spending time on and explain my current stance on this and how to use the module in different scenarios at length.

My current stance

Usage in different, progressively more niche, scenarios

ElvishJerricco commented 2 weeks ago

This is exactly what I'm talking about. This is a real "you're holding it wrong" moment.

talyz commented 2 weeks ago

I've presented my take on this and shown how the current option can be used. I don't think there's much more I can do or add here, and my patience for your demands is running out.