_module.args.name gets merged on assignment

[furrycatherder]

Issue description

If I have an option "foo" of type attrsOf (submodule bar) and I assign an attrset with _module.args.name "bar", the configuration of submodule bar will see its name as "foobar".

This is because _module.args is types.unspecified which follows the default merging rules, but I'm not sure how to fix it (or if it's working as designed).

Steps to reproduce

Technical details

• system: "x86_64-linux"
• host os: Linux 4.14.91, NixOS, 18.09.1834.9d608a6f592 (Jellyfish)
• multi-user?: yes
• sandbox: yes
• version: nix-env (Nix) 2.1.3
• channels(sean): "nixos-19.03pre165037.eebd1a92637, nixpkgs-19.03pre165019.c06debd3f9d"
• channels(root): "nixos-18.09.1834.9d608a6f592"
• nixpkgs: /home/sean/.nix-defexpr/channels/nixpkgs

[furrycatherder]

#53443 This was my attempt but it seems to break things horribly.

[danbst]

I'm not saying, that issue will be fixed when you provide reproduction steps of your issue, but that would be a great start. For example, I don't follow where programs.neomutt.mailboxes option comes from

[furrycatherder]

@danbst Sure, let me see if I can provide a MWE so people can play with it. The root cause is in the issue description, though, afaict.

https://github.com/furrycatherder/mwe-nixpkgs-53458

This configuration:

{ config, lib, ... }:
{
  has."sensei" = { item = "chicken katsu"; };

  wants."jonas" = config.has."sensei";
}

nix-repl> mwe = import (fetchGit { url = "https://github.com/furrycatherder/mwe-nixpkgs-53458"; ref = "master"; })

nix-repl> mwe
fetching Git repository 'https://github.com/furrycatherder/mwe-nixpkgs-53458'warning: no common commits
{ config = { ... }; options = { ... }; }

nix-repl> mwe.config.wants.jonas
{ _module = { ... }; factoid = "senseijonas wants chicken katsu"; item = "chicken katsu"; }

[danbst]

@furrycatherder yeah, nasty bug. I've combined your example into one file:

with import <nixpkgs> {};
let

  option = lib: lib.mkOption {
    type = with lib.types; attrsOf (submodule {});
  };

  module = { config, lib, ... }: {
    options.o1 = option lib;
    options.o2 = option lib;
  };

  configuration = { config, ... }: {
    o1.a = {};

    o2.a = config.o1.a;
    # counterintuitive, but same as
    #o2.a = {
    #  _module.args.name = "a";
    #};

    # Causes infinite recursion
    #o1.b = config.o2.a;
  };

in with lib.evalModules {
  modules = [
    module
    configuration
  ];
};
config

And result of invocation:

$ nix eval '(import ./test.nix)' --json | jq
{
  "_module": {
    "args": {},
    "check": true
  },
  "o1": {
    "a": {
      "_module": {
        "args": {
          "name": "a"
        },
        "check": true
      }
    }
  },
  "o2": {
    "a": {
      "_module": {
        "args": {
          "name": "aa"
        },
        "check": true
      }
    }
  }
}

I can't think of what exactly is wrong here. Surely, name shouldn't merge. But should this code pattern be allowed at all? cc @nbp

[nbp]

The problem might be located here: https://github.com/NixOS/nixpkgs/blob/master/lib/types.nix#L375

But then the question becomes, should the name be inherited? What about:

  o2.foo = config.o1.bar;

Surely foo is expected to be the name.

[stale]

Hello, I'm a bot and I thank you in the name of the community for opening this issue.

To help our human contributors focus on the most-relevant reports, I check up on old issues to see if they're still relevant. This issue has had no activity for 180 days, and so I marked it as stale, but you can rest assured it will never be closed by a non-human.

The community would appreciate your effort in checking if the issue is still valid. If it isn't, please close it.

If the issue persists, and you'd like to remove the stale label, you simply need to leave a comment. Your comment can be as simple as "still important to me". If you'd like it to get more attention, you can ask for help by searching for maintainers and people that previously touched related code and @ mention them in a comment. You can use Git blame or GitHub's web interface on the relevant files to find them.

Lastly, you can always ask for help at our Discourse Forum or at #nixos' IRC channel.

[infinisil]

Still a problem, here's a repro that works after #82751:

with import ./lib;
evalModules {
  modules = [({ lib, ... }: {
    options.value = lib.mkOption {
      type = lib.types.attrsOf (lib.types.submodule ({ name, ... }: {
        options.nameValue = lib.mkOption {
          default = name;
        };
      }));
    };

    config.value.x = {
      _module.args.name = "y";
    };

  })];
}

[stale]

I marked this as stale due to inactivity. → More info

[roberth]

@infinisil, replacing types.unspecified by types.raw (#160489) seems excessive, losing the ability to mkForce. What about types.anything? It seems to have a reasonable merge function.

[infinisil]

@roberth See #160489 (comment), mkForce and co. still work. types.anything would be too strict for things like pkgs, where it would recurse into the whole thing

[dminuoso]

This change is unfortunate.

Right now we place utility functions into _module.args.utils, and this happens across several modules (split across several git repositories, even). This change breaks this ability and likely affects more users that haven't started migrating yet.

Could we perhaps change to a submodule with freeformType undefined, such that individual options like name can pick their suitable merge strategy?

[roberth]

Right now we place utility functions into _module.args.utils, and this happens across several modules (split across several git repositories, even).

This is asking for trouble. Logic is better represented by either

1. submodules that compute (e.g. mkOption { readOnly = true; ... }), or
2. library expressions that you can import as usual

The latter is easiest to test.

Could we perhaps change to a submodule with freeformType undefined,

We'll want to set its freeformType to lazyAttrsOf raw though, as _module.args has always been open for extension like that. The point of freeformType is that it doesn't prevent the improvement of types.
Since #156533, you don't even have to specify that you're type-merging with the _module.args submodule anymore, so you'd be able to say directly: _module.args.foo = mkOption { }.

[roberth]

@dminuoso I don't think we have a viable solution within the module system.

For now I can only recommend to avoid merging in _module.args.<name>. You can merge in another option beforehand if you must. E.g. { config, lib, ... }: { options.myOption = mkOption xyz; config._module.args.myOption = config.myOption; }.

[dminuoso]

The reason we have been using config._module.args.utils is because zhaofengli/colmena#94 has been preventing us from using readOnly functions straight in explicit options.

We'll want to set its freeformType to lazyAttrsOf raw though, as _module.args has always been open for extension like that. The point of freeformType is that it doesn't prevent the improvement of types. Since #156533, you don't even have to specify that you're type-merging with the _module.args submodule anymore, so you'd be able to say directly: _module.args.foo = mkOption { }.

Okay, wouldn't that work then? As long as #156533 exists, we can inject new options (and pick something other than raw with mergeable types).