BUG: Can not use parametric aspects from imported namespace

[nikolaiser]

Description

Repro https://github.com/nikolaiser/den-repro-parametric-namespace

When including a parametric aspect from an imported namespace it fails with

  error: expected a Boolean but found a list: [ «thunk» ]
       at «github:vic/den/1751a01eafffbdcea9ddc203f2d72bd6587fffc6?narHash=sha256-HbOuxsoL2NOWfLrRBXbvVhweIdDeKjmTnvNpWNhS98s%3D»/nix/fn-can-take.nix:8:49:
            7|       fargs = lib.functionArgs func;
            8|       required = lib.filterAttrs (_: optional: !optional) fargs;
             |                                                 ^
            9|       reqNames = builtins.attrNames required;

If I comment out the broken test in my repro the working test (that is using a non-parametric aspect from the same namespace) succeeds

[vic]

Hey, thank you very much for the repo, just a few hours ago I was adding suport for parameteric aspects from other namespaces, looks like I missed that case. Thanks a lot for the reproduction code, that is very helpful. I'll take a look later today.

[nikolaiser]

Thanks a lot for the quick response!

A bit more context. If I wrap the provider.broken into den.lib parametric then it works.

  provider.broken =
    let
      aspect =
        { host, ... }:
        {
          ${host.class} = {
            environment.sessionVariables.PROVIDER_HOST = host.name;
          };
        };
    in
    den.lib.parametric { includes = [ aspect ]; };

But it's not needed for the aspects in the same flake, only in the imported one. I added another test case to my repro - workingLocal. It uses the same code as provider.broken

UPDATE:

And also I noticed this when migrating a pre 0.9.0 config to the latest den, and before it worked just fine

[vic]

Oh, yes, that's another thing I'm working at currently, trying to make all aspects in Den parametric by default. I have documented about the need for parametric and how it works and why it is needed, but looks like it would be better if all aspects were parametric by default. Including anonymous aspects that people create at includes list. I'm almost done with default-parametric, if that gets merged then I'll try to run this repo again and see if we have more to fix. Thanks for the context :)

[vic]

An attrset { includes = [ ... ]; } is not parametric by itself, it is just a list of functions, but nothing is passing context to them, that is what parametric { includes = [ ... ]; } is for. With the default-parametric branch, those plain-attrsets (that have not an explicit __functor) will be treated as if __functor = parametric so they can automatically forward context.

[nikolaiser]

I see, thank you for the explanation. I still don't get 2 things though:

1. Why does it work when the same aspect without the den.lib.parametric is defined in the same flake and only breaks for the imported ones (workingLocal in my repro)?
2. Why did it work before 0.9.0? Probably because of the context flow changes?

[vic]

1. Depends on which aspects you refer, if you are talking about den.aspects.<hostName> or den.aspects.<userName> they are defined as parametric { } for you. See Den Created Aspects. All others, like den.aspects.foo._.bar = {} are not parametric unless you explicitly assign the ctx-forwarding __functor on it (that is what parametric { } does, it sets __functor on its argument).
2. Hm, yeah 0.9.0 changed a lot of things, I even re-wrote all of our test suite (we are currently at 165 tests), so maybe I missed something.

[vic]

Read also: Search for Enhance the igloo host aspect https://den.oeiuwq.com/guides/from-zero-to-den/

[nikolaiser]

Depends on which aspects you refer, if you are talking about den.aspects. or den.aspects. they are defined as parametric { } for you. See Den Created Aspects. All others, like den.aspects.foo._.bar = {} are not parametric unless you explicitly assign the ctx-forwarding __functor on it (that is what parametric { } does, it sets __functor on its argument).

Sorry, I forgot to push the latest commit in my repro 🤦

workingLocal = denTest (
      {
        den,
        igloo, # igloo = nixosConfigurations.igloo.config
        provider,
        ...
      }:
      {

        imports = [
          (inputs.den.namespace "provider" [
            true
            inputs.provider
          ])
        ];

        provider.workingLocal =
          { host, ... }:
          {
            ${host.class} = {
              environment.sessionVariables.PROVIDER_HOST = host.name;
            };
          };

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

        den.aspects.igloo = den.lib.parametric {
          includes = [ provider.workingLocal ];
        };

        expr = igloo.environment.sessionVariables.PROVIDER_HOST ? _;
        expected = "igloo";
      }
    );

This test is working. But if I move the provider.workingLocal aspect into the provider flake without any other changes (broken test case) it will fail with

  error: expected a Boolean but found a list: [ «thunk» ]
       at «github:vic/den/1751a01eafffbdcea9ddc203f2d72bd6587fffc6?narHash=sha256-HbOuxsoL2NOWfLrRBXbvVhweIdDeKjmTnvNpWNhS98s%3D»/nix/fn-can-take.nix:8:49:
            7|       fargs = lib.functionArgs func;
            8|       required = lib.filterAttrs (_: optional: !optional) fargs;
             |                                                 ^
            9|       reqNames = builtins.attrNames required;

[vic]

That location is can-take.nix trying to find what functionArgs (context) is needed. lib.functionArgs must return booleans! fargs = lib.functionArgs func. :O

[vic]

I will take a deeper look later today when I get home. But if you are willing to explore, feel free and share any insight.

[vic]

Looks like I'm done with default parametric aspects: See #243 tests

[vic]

ok still failing in main, I believe this is due to my changes in flake-aspects regarding no-function merge on aspect.__functor option. denful/flake-aspects#38

[vic]

Added failing test simulating a remote flake, based in your code: https://github.com/vic/den/pull/266/changes