Improved module errors

[infinisil]

Motivation for this change

This updates NixOS module system errors to print the actual values of the definitions. It also updates the error formatting a bit. Related issues:

• Something like lib/modules.nix: provide detailed error locations #65024 could improve these errors with line and column info
• Improve generators.toPretty #97133 will improve generators.toPretty, which is used by these error messages

Ping @Profpatsch @roberth @grahamc

Here is an overview of the new error messages, reproducable with the following file:

Details

{ lib, config, pkgs, options, ... }: {

  _file = "/etc/nixos/configuration.nix";

  imports = [
    ({ lib, config, pkgs, ... }: {

      _file = "nixos/modules/services/foo.nix";

      options.services.foo = {
        dataDir = lib.mkOption {
          type = lib.types.str;
          readOnly = true;
          default = "/var/lib/foo";
          description = "";
        };

        dataDir' = lib.mkOption {
          type = lib.types.str;
          default = "/var/lib/foo";
          description = "";
        };

        fileContents = lib.mkOption {
          type = lib.types.lines;
          default = "";
          description = "";
        };

        arbitrary = lib.mkOption {
          description = "";
        };

        pkg = lib.mkOption {
          type = lib.types.package;
          description = "";
        };

        port = lib.mkOption {
          type = lib.types.port;
          description = "";
        };
      };

      config.environment.etc.foo.text = builtins.toJSON config.services.foo;
    })
    {
      _file = "/etc/nixos/extra.nix";
      services.foo.arbitrary = "hello";
      services.foo.pkg = pkgs.hello;
      services.foo.port = 8080;
    }
  ];

  config = {

    # Uncomment a single line of the ones below for a different error each

    #services.foo.enable = true;
    #services.foo.dataDir = "/data/foo";
    #services.foo.fileContents = { port = 10; foo = "foo"; };
    #services.foo.arbitrary = { foo = "foo"; };
    #services.foo.pkg = pkgs.openssh;
    #services.foo.port = 8000;


    boot.loader.grub.device = "nodev";
    fileSystems."/".device = "main/root";

  };
}

Non-existant option

error: The option `services.foo.enable' does not exist. Definition values:
- In `/etc/nixos/configuration.nix': true

Multiple readOnly definitions

error: The option `services.foo.dataDir' is read-only, but it's set multiple times. Definition values:
- In `nixos/modules/services/foo.nix': "/var/lib/foo"
- In `/etc/nixos/configuration.nix': "/data/foo"

Wrong type

error: A definition for option `services.foo.fileContents' is not of type `strings concatenated with "\n"'. Definition values:
- In `/etc/nixos/configuration.nix':
    {
      foo = "foo";
      port = 10;
    }

Unmergable definitions (when no type specified)

error: Cannot merge definitions of `services.foo.arbitrary'. Definition values:
- In `/etc/nixos/extra.nix': "hello"
- In `/etc/nixos/configuration.nix':
    {
      foo = "foo";
    }

Unique option defined multiple times

error: The unique option `services.foo.pkg' is defined multiple times. Definition values:
- In `/etc/nixos/extra.nix': <derivation /nix/store/5rj29hp44c8p6281imxz9h1klqa23ijs-hello-2.10.drv>
- In `/etc/nixos/configuration.nix': <derivation /nix/store/86zf5amc82hgs8g47xhbnfp34zsch51i-openssh-8.3p1.drv>

Note: This is pretty much the same error condition for readOnly options, however this failure is specified by the type itself, rather than the option. This happens e.g. with uniq (listOf str) or types.package

Conflicting definitions

error: The option `services.foo.port' has conflicting definitions values:
- In `/etc/nixos/extra.nix': 8080
- In `/etc/nixos/configuration.nix': 8000

Things done

• Updated the tests for the new error messages

[grahamc]

I think this is really great, and the evaluation performance report indicates this is basically free -- nice!

One concern: - In /etc/nixos/configuration.nix': <δ:openssh-8.3p1>uses theδsymbol. However, as an uneducated plebe I didn't know whatδ` was, and didn't know how to ask about it beyond "what is this funny symbol?". I think it would be better to use Nix repl's description:

«derivation /nix/store/4pmrswlhqyclwpv12l1h7mr9qkfhpd1c-hello-2.10.drv»

I like this option better because it is a word somebody can say without needing to know more than English. (@infinisil suggested this specific change in a chat with him.)

[grahamc]

Furthermore, as @andir pointed out, the lambda and delta aren't rendered by the kernel in a TTY. (« and » do)

[edolstra]

What happens if definitions are very large values (or even circular/infinite, like derivations)?

[infinisil]

@edolstra toPretty doesn't recurse into derivations, those are just printed as shown above. Though I'll think about maybe adding a recursion and element limit to toPretty.

[domenkozar]

Not that it needs to be of scope in this PR (as it already improves the status quo),
but suggesting what users can do would really make it easier for beginners.

For example when there's a conflict the user can either remove one value or use mkForce.

[infinisil]

@domenkozar Oh great idea! I think I'll leave this for another PR indeed though.

@grahamc I just pushed a change to the toPretty improvement PR that will not use those special symbols anymore: #97133 (comment)

[Ma27]

Awesome work!

[Profpatsch]

@edolstra toPretty doesn't recurse into derivations, those are just printed as shown above. Though I'll think about maybe adding a recursion and element limit to toPretty.

Check it out:

nixpkgs/lib/debug.nix

Lines 88 to 109 in 93fd4b5

	/* Like `traceSeq`, but only evaluate down to depth n.
	This is very useful because lots of `traceSeq` usages
	lead to an infinite recursion.
	Example:
	traceSeqN 2 { a.b.c = 3; } null
	trace: { a = { b = {…}; }; }
	=> null
	*/
	traceSeqN = depth: x: y: with lib;
	let snip = v: if isList v then noQuotes "[…]" v
	else if isAttrs v then noQuotes "{…}" v
	else v;
	noQuotes = str: v: { __pretty = const str; val = v; };
	modify = n: fn: v: if (n == 0) then fn v
	else if isList v then map (modify (n - 1) fn) v
	else if isAttrs v then mapAttrs
	(const (modify (n - 1) fn)) v
	else v;
	in trace (generators.toPretty { allowPrettyValues = true; }
	(modify depth snip x)) y;

[Profpatsch]

Turns out this is what the allowPrettyValues argument to toPretty was for all along 😛

[infinisil]

Changes since last revision:

• The toPretty improvement PR is merged now. I rebased this PR on top of it. With it, the values are now printed on multiple lines, and derivations and functions use <derivation ..> and <function ..> instead of δ and λ
• Values for error display are now evaluated with builtins.tryEval. If evaluation fails, no value is displayed
• The pretty printed value is now limited to 5 lines only. If there's more than 5 lines, a ... is printed
• The readOnly error message now doesn't show the module-internal _type attributes and co.
• The error format is slightly updated: The value is now printed on a new line from the start if it has multiple lines

As an example showcasing all of this, the error you get when you set the system.requiredKernelConfig option to have readOnly = true is

error: The option `system.requiredKernelConfig' is read-only, but it's set multiple times. Definition values:
- In `/home/infinisil/src/nixpkgs/nixos/modules/system/boot/kernel.nix': [ ]
- In `/home/infinisil/src/nixpkgs/nixos/modules/virtualisation/xen-dom0.nix'
- In `/home/infinisil/src/nixpkgs/nixos/modules/system/boot/systemd.nix':
    [
      {
        assertion = <function>;
        configLine = "CONFIG_DEVTMPFS=y";
        message = "CONFIG_DEVTMPFS is not enabled!";
    ...
- In `/home/infinisil/src/nixpkgs/nixos/modules/system/boot/stage-1.nix':
    [
      {
        assertion = <function>;
        configLine = "CONFIG_TMPFS=y";
        message = "CONFIG_TMPFS is not yes!";
    ...
- In `/home/infinisil/src/nixpkgs/nixos/modules/system/boot/loader/systemd-boot/systemd-boot.nix'
- In `/home/infinisil/src/nixpkgs/nixos/modules/system/boot/kernel.nix':
    [
      {
        assertion = <function>;
        configLine = "CONFIG_MODULES=y";
        message = "CONFIG_MODULES is not yes!";
    ...
- In `/home/infinisil/src/nixpkgs/nixos/modules/services/hardware/udev.nix':
    [
      {
        assertion = <function>;
        configLine = "CONFIG_UNIX=y";
        message = "CONFIG_UNIX is not enabled!";
    ...
- In `/home/infinisil/src/nixpkgs/nixos/modules/programs/systemtap.nix'
- In `/home/infinisil/src/nixpkgs/nixos/modules/programs/criu.nix'
- In `/home/infinisil/src/nixpkgs/nixos/modules/hardware/video/amdgpu-pro.nix'
- In `/home/infinisil/src/nixpkgs/nixos/modules/config/zram.nix'
- In `/home/infinisil/src/nixpkgs/nixos/modules/config/swap.nix'

[infinisil]

This PR improves toPretty for it to be possible to render only the first couple lines, without evaluating the whole thing: #98761