BUG: Duplicate values when multiple modules contribute to the same aspect

[musjj]

Description

Reproduction repo: https://github.com/musjj/bogus-duplication

imports = [
  {  
    den.aspects.tux-groups = den.lib.parametric {
      nixos.users.users.tux.extraGroups = [ "foo" ];
    };
  }
  {  
    den.aspects.tux-groups = den.lib.parametric {
      nixos.users.users.tux.extraGroups = [ "bar" ];
    };
  }
];

den.hosts.x86_64-linux.igloo.users.tux = { };

den.aspects.tux.includes = [ den.aspects.tux-groups ];

In this case, you would expect extraGroups to be [ "foo" "bar" ] (or [ "bar" "foo" ]), but in reality you get:

$ nix eval .#nixosConfigurations.igloo.config.users.users.tux.extraGroups
[ "foo" "bar" "foo" "bar" ]

It looks like that duplication occurs when there are multiple modules that contributes to the same aspect.

Might be related to: #131

[vic]

Thanks for reporting, hope to have time in two or three days to investigate. If you find something be sure to share here any insight (and/or solution/workaround)

[vic]

Hey @musjj,

Just arrived home and tried this bogus test, thanks for your code.

Here are some things I've noticed so far.

Turns out that if I change each den.lib.parametric {...} for _: {...} (a function ignoring its arguments) we get the same error, so parametric is out of the equation.

Then I tried changing both to be not function _: {...} -> {...} works as expected.

Then I made one of them a function and the other NOT a function, also works as expected.

Only happens if both are functions.

[vic]

The following also makes it work,

Setting the module key to different values. So the module system knows they are different modules.

    imports = [
      {
          den.aspects.tux-groups = _: {
              nixos = {
                 key = "one";
                 users.users.tux.extraGroups = [ "foo" ];
              };
          };
      }
      {
          den.aspects.tux-groups = _: {
              nixos = {
                key = "two";
                users.users.tux.extraGroups = [ "bar" ];
              };
          };
      }
   ];

[vic]

The test is very clean, I disabled HM, commented ctx.host.into.{user,default} and ctx.user.into.default and ctx.host._.user in context/os.nix trying to isolate the problem more and do not involve den bidirectional-dependencies (all HM/maid/hjem disabled in working copy), and I see the same results.

[vic]

Using both as functions: _: { ... }, without key.

But explicitly applying the tux-groups aspect, WORKS

den.aspects.tux.includes = [ (den.aspects.tux-groups true) ];

So we are back at parametric (since den.aspects.tux is parametric as created by Den)

[vic]

Tomorrow I'll try to make a test with only den.lib without any of den.ctx, etc, trying to isolate the bug more.

[vic]

Pure nix test works (as expected)

  flake.tests.bogus = {
    test-pure-nix =
      let
        aspectType = lib.types.lazyAttrsOf lib.types.deferredModule;
        funToMod = lib.types.either (lib.types.functionTo aspectType) aspectType;

        tux-groups-modules = [
          {
            options.tux-groups = lib.mkOption { type = funToMod; };
            options.includes = lib.mkOption { type = lib.types.listOf funToMod; };
          }
          {
            tux-groups = _: {
              nixos.names = [ "foo" ];
            };
          }
          {
            tux-groups = _: {
              nixos.names = [ "bar" ];
            };
          }
          (
            { config, ... }:
            {
              config.includes = [ config.tux-groups ];
            }
          )
        ];

        ev = lib.evalModules {
          modules = tux-groups-modules;
        };

        namesMod.options.names = lib.mkOption {
          type = lib.types.listOf lib.types.str;
        };

        ev2 = lib.evalModules {
          modules = [ namesMod ] ++ (map (i: (i true).nixos) ev.config.includes);
        };

      in
      {
        expr = ev2.config.names;
        expected = [
          "foo"
          "bar"
        ];
      };
  };

[vic]

flake-aspects test without den.parametric WORKS

  flake.tests.bogus = {
    test-flake-aspects-without-den-parametric =
      let
        inherit (den-lib) aspects;

        my-aspects-namespace =
          { config, ... }:
          aspects.new (option: _: {
            options.my-aspects = option;
          }) config.my-aspects;

        evModules = [
          my-aspects-namespace
          { options.den.lib = lib.mkOption { default = den-lib; }; }
          {
            my-aspects.tux._.names =
              { x }:
              {
                nixos.names = [ "foo" ];
              };
          }
          {
            my-aspects.tux._.names =
              { x }:
              {
                nixos.names = [ "bar" ];
              };
          }
          (
            { config, ... }:
            {
              my-aspects.tux.includes =
                map (f: f { x = true; }) # APPLY CONTEXT: simulate den.lib.parametric
                  [ config.my-aspects.tux._.names ];
            }
          )
          (
            { config, ... }:
            {
              options.nixosMod = lib.mkOption {
                type = lib.types.deferredModule;
                default = config.my-aspects.tux.resolve { class = "nixos"; };
              };
            }
          )
        ];

        ev = lib.evalModules { modules = evModules; };
        mod = ev.config.nixosMod;

        namesMod.options.names = lib.mkOption { type = lib.types.listOf lib.types.str; };

        ev2 = lib.evalModules {
          modules = [
            mod
            namesMod
          ];
        };
      in
      {
        expr = ev2.config.names;
        expected = [
          "foo"
          "bar"
        ];
      };
  };

[vic]

Using den.lib.parametric WORKS ! :O

  flake.tests.bogus = {
    test-den-parametric =
      let
        inherit (den-lib) parametric aspects;

        my-aspects-namespace =
          { config, ... }:
          aspects.new (option: _: {
            options.my-aspects = option;
          }) config.my-aspects;

        evModules = [
          my-aspects-namespace
          { options.den.lib = lib.mkOption { default = den-lib; }; }
          {
            my-aspects.tux._.names =
              { x }:
              {
                nixos.names = [ "foo" ];
              };
          }
          {
            my-aspects.tux._.names =
              { x }:
              {
                nixos.names = [ "bar" ];
              };
          }
          (
            { config, ... }:
            {
	      my-aspects.tux = parametric {  # <--- PARAMETRIC like den created aspects
	        includes = [ config.my-aspects.tux._.names ];
	      };
            }
          )
          (
            { config, ... }:
            {
              options.nixosMod = lib.mkOption {
                type = lib.types.deferredModule;
                default = (config.my-aspects.tux { x = 1; }).resolve { class = "nixos"; };
              };
            }
          )
        ];

        ev = lib.evalModules { modules = evModules; };
        mod = ev.config.nixosMod;

        namesMod.options.names = lib.mkOption { type = lib.types.listOf lib.types.str; };

        ev2 = lib.evalModules {
          modules = [
            mod
            namesMod
          ];
        };
      in
      {
        expr = ev2.config.names;
        expected = [
          "foo"
          "bar"
        ];
      };
  };

[musjj]

Wow, thanks that's really interesting!

But for some reason using parametric on the nested aspects causes the test to fail:

{
  my-aspects.tux._.names =
    parametric
    {
      nixos.names = [ "foo" ];
    };
}
{
  my-aspects.tux._.names =
    parametric
    {
      nixos.names = [ "bar" ];
    };
}

This fails with:

error: The option `names' was accessed but has no value defined. Try setting the option.

I wonder why.

Another thing I found was that static contexts also work:

{
  den.aspects.tux-groups = { class, aspect-chain }: {
    nixos.users.users.tux.extraGroups = [ "foo" ];
  };
}
{
  den.aspects.tux-groups = { class, aspect-chain }: {
    nixos.users.users.tux.extraGroups = [ "bar" ];
  };
}

They also work when curried:

{
  den.aspects.tux-groups = { host, user }: { class, aspect-chain }: {
    nixos.users.users.tux.extraGroups = [ "foo" ];
  };
}
{
  den.aspects.tux-groups = { host, user }: { class, aspect-chain }: {
    nixos.users.users.tux.extraGroups = [ "bar" ];
  };
}

I'm not really sure why.

[vic]

Ok, I've done the following. Merged #230 which exposes den.{lib,ctx,aspects} without any of NixOS domain (no den.{hosts,homes,default} or any other NixOS stuff). This helped me create another test using only den.lib.

I was able to reproduce the problem with this code (tried to reproduce as much as possible how Den-framework works!):

{
  lib,
  inputs,
  ...
}:
let

  denModule = (import inputs.den).nixModule inputs;
  testBogus = bogusModule: 
      let
        ev = lib.evalModules { modules = [ denModule bogusModule ]; };
        fooAspect = ev.config.den.ctx.foo { x = 0; y = 1; };
        fooModule = fooAspect.resolve { class = "foo"; };

        namesModule.options.names = lib.mkOption { type = lib.types.listOf lib.types.str; };
        ev2 = lib.evalModules { modules = [ fooModule namesModule ]; };

        expr = ev2.config.names;
        expected = [ "foo" "bar" ];
      in { inherit expr expected; };

in
{
  flake.tests.bogus = {

    test-something =
      testBogus ({ den, ... }:
          let
            inherit (den.lib) parametric;
          in
          {
            imports = [
              {
                den.aspects.groups = parametric {
                  foo = {
                    names = [ "foo" ];
                  };
                };
              }
              {
                den.aspects.groups = parametric {
                  foo = {
                    names = [ "bar" ];
                  };
                };
              }
              {
                den.aspects.foo = parametric { };
              }
              {
                den.aspects.foo.includes = [ den.aspects.groups ];
              }
              {
                den.ctx.foo.provides.foo = { x, y }@ctx: parametric.fixedTo ctx den.aspects.foo;
              }
            ];
          });

  };
}

So clearly something is happenning in den core.

[vic]

Ok, I think I found the issue, the problem is both groups = parametric do the following:

groups.__functor = <the parametric functor that passes context>

Now, the problem is we are doing that twice, once on each module that sets groups.__functor, the module system tries to merge their results because <Aspect>.__functor is a lib.types.functionTo ... in flake-aspects' types.nix.

So the default behaviour of types.functionTo x is to merge the values of different x produced by different definitions of the same __functor. This is why we are getting duplicates from parametric here.

The fix is not in Den, but in flake-aspects' aspect.__functor type, it should not merge, it must take only the latest definition (changing the __functor on a single aspect does not makes sense and that is what those two modules of ours do in groups).

I'm fixing this in flake-aspects, with a test there and then also add your test case here in den for posterity.

[musjj]

So the default behaviour of types.functionTo x is to merge the values of different x produced by different definitions of the same __functor. This is why we are getting duplicates from parametric here.

Damn, I did not know that the module system can merge functions like that. Tested it out on a REPL:

nix-repl> modules = lib.evalModules {
  modules = [
    { options.foo = lib.mkOption { type = lib.types.functionTo lib.types.int; }; }
    { foo = _: lib.trace "first" 0; }
    { foo = _: lib.trace "second" 0; }
  ];
}


nix-repl> modules.config.foo null
trace: second
trace: first
0

This is a wildly unintuitive behavior!