nix-community / home-manager

Manage a user environment using Nix [maintainer=@rycee]
https://nix-community.github.io/home-manager/
MIT License
6.69k stars 1.76k forks source link

bug: lib function toHyprconf doesn't support paths #5743

Open w-lfchen opened 4 weeks ago

w-lfchen commented 4 weeks ago

Are you following the right branch?

Is there an existing issue for this?

Issue description

My flake-based configuration was making use of self.outPath a lot, which, as I found out the hard way, leads to a lot of unnecessary config builds since the files depending on it need to be rebuilt every time anything in my flake changes, even if it's only whitespace. When fixing my code, I tried to replace with relative paths since those don't seem to cause that. However, as the issue title states, this lead to errors. While the modules (the offending one was Hyprlock, but this should apply to all modules making use of this function, Hyprland had the same problem) accept paths, the underlying function is unable to process them, leading to the following error:

error: evaluation aborted with the following error message: 'generators.mkValueStringDefault: this value is not supported: "<path>"'

This leads me to believe that this is an issue on a type level, since setting the path to different values does not change the error message. A minimal config to reproduce this could be:

{ ... }:
{
  programs.hyprlock = {
    enable = true;
    settings.background = [
      {
        monitor = "";
        path = ./.;
      }
    ];
  };
}

I have also found a workaround that seems to work with the modules while not leading to the aforementioned unnecessary file generations: Using "${<path>}" instead of the path directly works. I am not sure whether this is a bug or a feature request, but since every module's settings option states that it accepts paths, I decided it is the former. I hope this is enough information, if there's anything else I might be able to help with, feel free to ping me. I would've loved to attempt to fix this myself and open a pr instead, but currently, I am lacking both the time and experience required to do so.

Maintainer CC

@khaneliman @fufexan

System information

- system: `"x86_64-linux"`
 - host os: `Linux 6.6.41, NixOS, 24.11 (Vicuna), 24.11.20240725.5ad6a14`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.18.5`
 - channels(root): `"nixos"`
 - channels(wolf): `"home-manager-23.11.tar.gz"`
 - nixpkgs: `/nix/store/xblysc3prg1zqsi59fr36nj6wdiqryzp-source`
fufexan commented 4 weeks ago

I think this is a misunderstanding. Hypr*'s path is different than Nix's path type. So you should always use strings.

w-lfchen commented 4 weeks ago

If one should always use strings, I'd suggest not accepting paths, every module does this: https://github.com/nix-community/home-manager/blob/086f619dd991a4d355c07837448244029fc2d9ab/modules/services/window-managers/hyprland.nix#L130-L140 Or am I missing something again?

fufexan commented 4 weeks ago

That may have been an oversight. From what I've seen, we can probably include special handling for path types, or we can remove that entirely.

w-lfchen commented 4 weeks ago

I think making use of the path type would be nice since it gives consumers more options which is always good imo, but I can't judge the feasibility of implementing this.

fufexan commented 3 weeks ago

I've cooked something up. I'm now letting tests run to see whether I've missed anything.

fufexan commented 3 weeks ago

I'm not sure I can make this work after all, I'm receiving the same error you get if I add /path/to/plugin2 to the plugins list in the simple-config test. @rycee would you know how to fix this? Here's what I've tried so far

diff --git a/modules/lib/generators.nix b/modules/lib/generators.nix
index 2aebd92c..446e8113 100644
--- a/modules/lib/generators.nix
+++ b/modules/lib/generators.nix
@@ -7,6 +7,8 @@
         all concatMapStringsSep concatStrings concatStringsSep filterAttrs foldl
         generators hasPrefix isAttrs isList mapAttrsToList replicate;

+      inherit (builtins) mapAttrs removeAttrs typeOf;
+
       initialIndent = concatStrings (replicate indentLevel "  ");

       toHyprconf' = indent: attrs:
@@ -37,11 +39,17 @@

           importantFields = filterAttrs isImportantField allFields;

-          fields = builtins.removeAttrs allFields
-            (mapAttrsToList (n: _: n) importantFields);
+          pathFields = filterAttrs (_: v: typeOf v == "path") allFields;
+
+          path2StringFields = mapAttrs (_: toString) pathFields;
+
+          excludedFields = importantFields // pathFields;
+
+          fields = removeAttrs allFields (mapAttrsToList (n: _: n) excludedFields);
         in mkFields importantFields
         + concatStringsSep "\n" (mapAttrsToList mkSection sections)
-        + mkFields fields;
+        + mkFields fields
+        + mkFields path2StringFields;
     in toHyprconf' initialIndent attrs;

   toKDL = { }:
diff --git a/tests/modules/services/window-managers/hyprland/simple-config.conf b/tests/modules/services/window-managers/hyprland/simple-config.conf
index 29dcd7cd..420df1cd 100644
--- a/tests/modules/services/window-managers/hyprland/simple-config.conf
+++ b/tests/modules/services/window-managers/hyprland/simple-config.conf
@@ -1,6 +1,7 @@
 exec-once = /nix/store/00000000000000000000000000000000-dbus/bin/dbus-update-activation-environment --systemd DISPLAY HYPRLAND_INSTANCE_SIGNATURE WAYLAND_DISPLAY XDG_CURRENT_DESKTOP && systemctl --user stop hyprland-session.target && systemctl --user start hyprland-session.target
 plugin=/path/to/plugin1
 plugin=/nix/store/00000000000000000000000000000000-foo/lib/libfoo.so
+plugin=/path/type/to/plugin2
 $mod=SUPER
 bezier=smoothOut, 0.36, 0, 0.66, -0.56
 bezier=smoothIn, 0.25, 1, 0.5, 1
diff --git a/tests/modules/services/window-managers/hyprland/simple-config.nix b/tests/modules/services/window-managers/hyprland/simple-config.nix
index e7b11ffe..d59f6344 100644
--- a/tests/modules/services/window-managers/hyprland/simple-config.nix
+++ b/tests/modules/services/window-managers/hyprland/simple-config.nix
@@ -5,8 +5,11 @@
     enable = true;
     package = lib.makeOverridable
       (attrs: config.lib.test.mkStubPackage { name = "hyprland"; }) { };
-    plugins =
-      [ "/path/to/plugin1" (config.lib.test.mkStubPackage { name = "foo"; }) ];
+    plugins = [
+      "/path/to/plugin1"
+      (config.lib.test.mkStubPackage { name = "foo"; })
+      /path/type/to/plugin2
+    ];
     settings = {
       source = [ "sourced.conf" ];
L-Trump commented 2 weeks ago

It seems like an issue from the nixpkgs.lib.generators.toKeyValue. It doesn't deal with the path automatically. A simple workaround is using builtins.path to make the files available in nix store.

{ ... }:
{
  programs.hyprlock = {
    enable = true;
    settings.background = [
      {
        monitor = "";
        path = builtins.path { path = ./.; };
      }
    ];
  };
}