catppuccin / nix

❄️ Soothing pastel theme for Nix
https://nix.catppuccin.com/
MIT License
303 stars 47 forks source link

Global `catppuccin.enable` option may produce errors #205

Closed danielgafni closed 1 week ago

danielgafni commented 1 month ago

I just encountered an issue when enabling the global catppuccin.enable setting.

For whatever reason I had swaylock installed & configured via packages and xdg.configFile."swaylock" instead of programs.swaylock.enable.

This caused a recursion error when setting catppuccin.enable = true;. It took me a while to understand the issue.

It seems like this could be handled better. catppuccin-nix probably should have no effect on a package which is not installed via programs.<x>.enable.

isabelroses commented 1 month ago

For some modules it makes sense to do what you say. But for others it makes just as much sense to do something like https://github.com/catppuccin/nix/blob/2c7661c9fa26a920b8088300ef87d14179c71a27/modules/home-manager/bottom.nix#L12

Which is particularly important for a module like hyprland, since it loads var for users to utilise.

Its also also perticularly easy to enforce this standard.

danielgafni commented 1 month ago

Oh right, makes sense.

In any case, recursion errors should not happen :)

isabelroses commented 1 month ago

In any case, recursion errors should not happen :)

This is 100% true.

getchoo commented 1 month ago

This caused a recursion error when setting catppuccin.enable = true;. It took me a while to understand the issue.

would you be able to provide a log or git revision of this? this seems like a pretty big issue, but i'm not at all sure where it could be coming from

configured via packages and xdg.configFile."swaylock" instead of programs.swaylock.enable

my only guess is that this may have to with both you and the module setting the configFile option, but that should never apply unless our module is enabled (with global catppuccin.enable it will be) and the home-manager swaylock module (source)

danielgafni commented 1 month ago

Here is a recent error message from my GitHub Actions

It started happening after this commit where I simply removed swaylock.

isabelroses commented 1 month ago

I've heard Nobbz say config = mkIf cond is a dangerous pattern. Unsure if this is related but a number of other programs don't use this pattern. Might be worth changing it for the sake of testing.

getchoo commented 2 weeks ago

hi! sorry for the wait, been a bit busy with some things IRL

i've done a bit of digging into this and i haven't quite been able to figure out what exactly caused this in your configuration, or where it could possibly pop up in others. given the following minimum viable configuration of our modules

nix-repl> (pkgs.nixos {
            imports = [ ./modules/nixos inputs.home-manager.nixosModules.home-manager ];
            users.users.test = { isNormalUser = true; home = "/home/test"; };
            home-manager.users.test = { imports = [ ./modules/home-manager ]; catppuccin.enable = true; home.stateVersion = "24.11"; };
            system.stateVersion = "24.11";
          }).config.system.build.vm

this seems to work as expected. when i have the time i'll try to debug this a bit more, but given that this doesn't seem to be easy to trigger by just using the global catppuccin.enable and no one else reporting this, i'm going to take it off the 1.0.0 milestone for now as it seems to be an isolated issue

y-usuzumi commented 1 week ago

Hi! I have run into the same issue today. I'm not a Nix expert so I'm not sure how I can help. But I also found "swaylock" in the stacktrace which I have never used. I'm running Plasma 6 only.

https://pastebin.com/8KDKJjrL

Edit: A workaround for me is simply giving programs.swaylock.enable = false.

getchoo commented 1 week ago

@danielgafni @y-usuzumi could you check if #243 fixes your issue? you should be able to test this by changing your flake input ref from github:catppuccin/nix to github:catppuccin/nix?ref=pull/243/head

i am fairly sure this should fix daniel's issue, as github:danielgafni/nixos/3f04e515a48d29d91364765c595bfed59e0209cb#homeConfigurations.'dan@framnix'.activationPackage is able to evaluate with it applied to the revision of this repository it was using at the time

danielgafni commented 1 week ago

My config is different now, and since you've checked the fix with the old commit, I believe it should be fine. Thanks for the fix!