Avoiding duplicates with den.lib.parametric

[michaelBelsanti]

It seems that when using parametric, the owned modules are set twice, causing conflicts with options that can only be applied once:

eg:

  den.aspects.wayland._.cosmic = den.lib.parametric {
    includes = [ den.aspects.wayland._.base ];
    homeManager =
      { pkgs, lib, ... }:
      {
        gtk.iconTheme = {
          name = lib.mkForce "Cosmic";
          package = lib.mkForce pkgs.cosmic-icons;
        };
      };
};

Error:

nix-repl> nixosConfigurations.hades.config.home-manager.users.quasi.gtk.iconTheme
{
  name = "Cosmic";
  package = «error: The option `home-manager.users.quasi.gtk.iconTheme.package' is defined multiple times while it's expected to be unique.

Definition values:
- In `<unknown-file>, via option den.aspects.wayland.provides.cosmic.<function body>.includes."[definition 1-entry 2]".<function body>.homeManager': <derivation cosmic-icons-1.0.0-beta.5>
- In `<unknown-file>, via option den.aspects.wayland.provides.cosmic.<function body>.includes."[definition 1-entry 2]".<function body>.homeManager': <derivation cosmic-icons-1.0.0-beta.5>
Use `lib.mkForce value` or `lib.mkDefault value` to change the priority on any of these definitions.
»;
}

I found that moving the homeManager config to the includes fixed this:

  den.aspects.wayland._.cosmic = den.lib.parametric {
    includes = [
      den.aspects.wayland._.base
      (
        { host, ... }:
        {
          homeManager =
            { pkgs, lib, ... }:
            {
              xdg.configFile = {
                # using cosmics automatic gtk theming
                "gtk-4.0/assets".enable = false;
                "gtk-4.0/gtk.css".enable = false;
                "gtk-4.0/gtk-dark.css".enable = false;
              };
              gtk.iconTheme = {
                name = lib.mkForce "Cosmic";
                package = lib.mkForce pkgs.cosmic-icons;
              };
            };
        }
      )
    ];
};

I wanted to see if there was a better solution.

[michaelBelsanti]

Minimal example here: https://github.com/michaelBelsanti/bogus-den/tree/withown-dupes

[vic]

Sure, feel free to add a failing test to the same file as I did in #100 and send a PR even if it is just a test, it would be of great help.

I'm curious about what usage patterns you are building :) thanks for pushing den forwards with it.

[vic]

Oh it looks like it has to do with the same tux aspect being shared on all hosts, maybe. Can you try setting a different aspect users.tux.aspect = "tux1" and see if that lowers the number of dups?

I will try your repo later today :)

[vic]

Oh, I see what you mean by chained parametric aspects. Hm, I have no idea, will try to explore as soon as I get home.

[michaelBelsanti]

I'm curious about what usage patterns you are building :) thanks for pushing den forwards with it.

I like to use these sorta meta-aspects that work like profiles for different types of systems, the ones I have currently are laptop, desktop, workstation, and server. laptop and desktop also both include workstation. These aspects import the majority of aspects I use, with each host only importing a few more modules specific to the host, like gaming on my gaming desktop. You can see the file handling most of that here: https://tangled.org/belsanti.xyz/nixconfig/blob/den/modules/systems.nix

Because of this design, all of those aspects need to pass any context to imported modules, so they're all parametric. Really, they should all probably be using parametric.atLeast rather than parametric, but there are currently some spots where I'm setting workstation.nixos and workstation.homeManager that I replace with a standalone module that gets imported.

[vic]

You can see the file handling most of that here:

Nice! I like how you are designing your infra. Do you allow me to add a link to your den branch on README ? Just to have a much more complete example than mine.

So funny that I haven't had much time for finally completing mine, as I dedicate most of my time to Den itself. Hopefully Den can become stable very soon, so I don't have to make more changes to it, and finally focus on other things :).

[vic]

I was able to create a failing test, by using a home-manager option. So the home-manager integration gets tested, all previous tests were OS-level configs.

here's the diff

diff --git a/templates/examples/foo.patch b/templates/examples/foo.patch
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/templates/examples/modules/_example/ci/parametric-with-owned.nix b/templates/examples/modules/_example/ci/parametric-with-owned.nix
index 708150c5b4..6b69534026 100644
--- a/templates/examples/modules/_example/ci/parametric-with-owned.nix
+++ b/templates/examples/modules/_example/ci/parametric-with-owned.nix
@@ -1,7 +1,7 @@
 { den, lib, ... }:
 let
   # a test module to check context was forwarded
-  fwdModule.nixos.options.fwd = {
+  fwdModule.options.fwd = {
     a = strOpt;
     b = strOpt;
     c = strOpt;
@@ -19,10 +19,12 @@
 {
 
   den.aspects.rockhopper.includes = [
-    fwdModule
+    { nixos.imports = [ fwdModule ]; }
+    { homeManager.imports = [ fwdModule ]; }
     den.aspects.fwd._.first
   ];
   den.aspects.rockhopper.nixos.fwd.c = "host owned C";
+  den.aspects.rockhopper.homeManager.fwd.a = "host home-managed A";
 
   # this aspect will take any context and also forward it
   # into any includes function that can take same context.
@@ -33,6 +35,11 @@
         fwd.a = "First owned A";
         fwd.pkg = pkgs.hello;
       };
+    homeManager =
+      { pkgs, ... }:
+      {
+        fwd.pkg = pkgs.vim;
+      };
     includes = [
       den.aspects.fwd._.second
       { nixos.fwd.d = "First static includes D"; }
@@ -73,7 +80,12 @@
     };
 
   perSystem =
-    { checkCond, rockhopper, ... }:
+    {
+      checkCond,
+      rockhopper,
+      alice-at-rockhopper,
+      ...
+    }:
     {
       checks.parametric-fwd-a = checkCond "fwd-a" (rockhopper.config.fwd.a == "First owned A");
       checks.parametric-fwd-b = checkCond "fwd-b" (
@@ -84,6 +96,13 @@
       checks.parametric-fwd-e = checkCond "fwd-e" (rockhopper.config.fwd.e == "Third Impact");
       checks.parametric-fwd-f = checkCond "fwd-f" (rockhopper.config.fwd.f == "Fifth Earth rockhopper");
       checks.parametric-fwd-pkg = checkCond "fwd-pkg" (lib.getName rockhopper.config.fwd.pkg == "hello");
+
+      checks.parametric-fwd-hm-a = checkCond "fwd-hm-a" (
+        alice-at-rockhopper.fwd.a == "host home-managed A"
+      );
+      checks.parametric-fwd-hm-pkg = checkCond "fwd-hm-pkg" (
+        lib.getName alice-at-rockhopper.fwd.pkg == "vim"
+      );
     };
 
 }

and error:

error: The option `home-manager.users.alice.fwd.pkg' is defined multiple times while it's expected to be unique.

       Definition values:
       - In `<unknown-file>, via option den.aspects.fwd.provides.first.<function body>.includes."[definition 1-entry 2]".includes."[definition 1-entry 1]".homeManager': <derivation vim-9.1.1833>
       - In `<unknown-file>, via option den.aspects.fwd.provides.first.<function body>.includes."[definition 1-entry 2]".includes."[definition 1-entry 1]".homeManager': <derivation vim-9.1.1833>
       

Use `lib.mkForce value` or `lib.mkDefault value` to change the priority on any of these definitions.

[michaelBelsanti]

Looking at the repro I made, it should also occur when using NixOS options. I unfortunately don't have the spare time atm to dive into the code, but if there's anything else I can do to help, just let me know. Otherwise, I'll take a look whenever I do get some spare time.

[michaelBelsanti]

It actually seems there's two different dupe bugs going on. One is the one I mentioned earlier where chaining parametric aspects will double the imports for each chain.

Seemingly separately, home manager modules are being imported twice, even without chaining any parametric aspects. It happens with only one declared host and user, so it isn't caused by reusing users: https://github.com/michaelBelsanti/bogus-den/tree/hm-dupes

[michaelBelsanti]

The home manager dupe issue doesn't exist prior to f6169af

[vic]

Fixed and tested at #105