lib/modules.nix: provide detailed error locations #65024

TomSmeets · 2019-07-18T12:48:19Z

If a user makes a mistake in his/her configuration.nix an error used to look like this:

error: The option value `services.xserver.enable' in `/etc/nixos/configuration.nix' is not of type `boolean'.
(use '--show-trace' to show detailed location information)

After this change it will look like this

error: The option value `services.xserver.enable' in `/etc/nixos/configuration.nix:17:7' is not of type `boolean'.
(use '--show-trace' to show detailed location information)

This change uses builtins.unsafeGetAttrPos to extract the exact location of nixos configuration options.
Most cases of <unknown-file> will also disappear.

Examples

tests/hello.nix:

{ config, lib, ... }: with lib; {
  options.hello = mkOption {
    type = types.str;
    default = "hi";
  };
}

tests/wrongType.nix:

{ ... }: {
  config.hello = 4;
}

tests/redefinition.nix:

{ config, lib, ... }: with lib; {
  options.hello = mkOption {
    type = types.bool;
    default = false;
  };
}

Failing option check

(evalModules {
  modules = [
    ./tests/hello.nix
    ./tests/wrongType.nix
  ];
}).config.hello

Before

error: The option value `hello' in `/home/tom/nixpkgs/tests/wrongType.nix' is not of type `string'.
(use '--show-trace' to show detailed location information)

After

error: The option value `hello' in `/home/tom/nixpkgs/tests/wrongType.nix:2:3' is not of type `string'.
(use '--show-trace' to show detailed location information)

Option redefinition

(evalModules {
  modules = [
    ./tests/hello.nix
    ./tests/redefinition.nix
  ];
}).config.hello

Before

error: The option `hello' in `/home/tom/nixpkgs/tests/hello.nix' is already declared in `/home/tom/nixpkgs/tests/redefinition.nix'.

After

error: The option `hello' in `/home/tom/nixpkgs/tests/hello.nix:2:3' is already declared in `/home/tom/nixpkgs/tests/redefinition.nix:2:3'.

Inside imports

(evalModules {
  modules = [
    ./tests/hello.nix
    (import ./tests/wrongType.nix)
  ];
}).config.hello

Before

error: The option value `hello' in `<unknown-file>' is not of type `string'.
(use '--show-trace' to show detailed location information)

After

error: The option value `hello' in `/home/tom/nixpkgs/tests/wrongType.nix:2:3' is not of type `string'.
(use '--show-trace' to show detailed location information)

Issues

`mkIf` and friends

The wrappers mkIf, mkOverride and mkOrder (also mkForce and mkDefault) will lose this information.
Here the unsafeGetAttrPos returns null. In this case it reverts back to the old behavior.
For some reason however, mkMerge appears to work work correctly.

tests/issue.nix:

{ lib, ...}: with lib; {
  config = mkIf true {
    hello = 4;
  };
}

This results in:

error: The option value `hello' in `/home/tom/nixpkgs/tests/issue.nix' is not of type `string'.
(use '--show-trace' to show detailed location information)

However if we use mkIf like this

{ lib, ...}: with lib; {
  config = {
    hello = mkIf true 4;
  };
}

It works fine:

error: The option value `hello' in `/home/tom/nixpkgs/tests/issue.nix:3:5' is not of type `string'.
(use '--show-trace' to show detailed location information)

This is an issue for the NixOs modules as many of them use config = mkIf enable {
An user's configuration.nix wont have this problem most of the time.

Performance

I did notice that builtins.unsafeGetAttrPos will be called for every configuration definition. This will have an impact on the evaluation performance.
The evaluation time went up by around 0.3 seconds after a very quick and dirty time nixos-rebuild build and time nix-instantiate '<nixpkgs/nixos> -A config.system.build.toplevel
I don't know if it is possible to just call builtins.unsafeGetAttrPos on the failing options. As I don't think that nix will nix hold on to this information.

Conclusion

I am myself not sure if the benefits outweigh the costs.
Hopefully someone will look at this and come up with a better solution.
Let me know what you think in the comments.

Related Issues

#15417

cc @edolstra and @nbp

If a user makes a mistake in his/her `configuration.nix` an error used to look like this: ``` error: The option value `services.xserver.enable' in `/etc/nixos/configuration.nix' is not of type `boolean'. (use '--show-trace' to show detailed location information) ``` After this change it will look like this ``` error: The option value `services.xserver.enable' in `/etc/nixos/configuration.nix:17:7' is not of type `boolean'. (use '--show-trace' to show detailed location information) ``` This change uses `builtins.unsafeGetAttrPos` to extract the exact location of nixos configuration options. Most cases of `<unknown-file>` will also disappear.

stale · 2020-09-29T05:02:41Z

Hello, I'm a bot and I thank you in the name of the community for your contributions.

Nixpkgs is a busy repository, and unfortunately sometimes PRs get left behind for too long. Nevertheless, we'd like to help committers reach the PRs that are still important. This PR 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.

If this is still important to you and you'd like to remove the stale label, we ask that you leave a comment. Your comment can be as simple as "still important to me". But there's a bit more you can do:

If you received an approval by an unprivileged maintainer and you are just waiting for a merge, you can @ mention someone with merge permissions and ask them to help. You might be able to find someone relevant by using Git blame on the relevant files, or via GitHub's web interface. You can see if someone's a member of the nixpkgs-committers team, by hovering with the mouse over their username on the web interface, or by searching them directly on the list.

If your PR wasn't reviewed at all, it might help to find someone who's perhaps a user of the package or module you are changing, or alternatively, ask once more for a review by the maintainer of the package/module this is about. If you don't know any, you can use Git blame on the relevant files, or GitHub's web interface to find someone who touched the relevant files in the past.

If your PR has had reviews and nevertheless got stale, make sure you've responded to all of the reviewer's requests / questions. Usually when PR authors show responsibility and dedication, reviewers (privileged or not) show dedication as well. If you've pushed a change, it's possible the reviewer wasn't notified about your push via email, so you can always officially request them for a review, or just @ mention them and say you've addressed their comments.

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

stale · 2021-06-05T00:56:06Z

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

What it does: line and column level *declaration* position information: $ nix repl . nix-repl> :p nixosConfigurations.micro.options.environment.systemPackages.declarationsWithLocations [ { column = 7; file = "/nix/store/24aj3k7fgqv3ly7qkbf98qvphasrw9nb-source/nixos/modules/config/system-path.nix"; line = 63; } ] Use cases: - ctags over NixOS options, as will be presented at NixCon 2023 ;) - improving the documentation pages to go to the exact line of the declarations. Related work: - NixOS#65024 This one does it for all *definitions* rather than declarations, and it was not followed through with due to performance worries. - NixOS#208173 The basis for this change. This change is just a rebase of that one. I split it out to add the capability before adding users of it, in order to simplify review. However, the ctags script in there is a sample user of this feature. Benchmarks: conducted by evaluating my own reasonably complex NixOS configuration with the command: `hyperfine -S none -w 1 -- "nix eval .#nixosConfigurations.snowflake.config.system.build.toplevel.outPath"` ``` Benchmark 1: nix eval .#nixosConfigurations.snowflake.config.system.build.toplevel.outPath Time (mean ± σ): 8.971 s ± 0.254 s [User: 5.872 s, System: 1.388 s] Range (min … max): 8.574 s … 9.327 s 10 runs Benchmark 1: nix eval .#nixosConfigurations.snowflake.config.system.build.toplevel.outPath Time (mean ± σ): 8.766 s ± 0.160 s [User: 5.873 s, System: 1.346 s] Range (min … max): 8.496 s … 9.033 s 10 runs ``` Summary of results: it seems to be in the noise, this does not cause any visible regression in times.

What it does: line and column level *declaration* position information: $ nix repl . nix-repl> :p nixosConfigurations.micro.options.environment.systemPackages.declarationPositions [ { column = 7; file = "/nix/store/24aj3k7fgqv3ly7qkbf98qvphasrw9nb-source/nixos/modules/config/system-path.nix"; line = 63; } ] Use cases: - ctags over NixOS options, as will be presented at NixCon 2023 ;) - improving the documentation pages to go to the exact line of the declarations. Related work: - NixOS#65024 This one does it for all *definitions* rather than declarations, and it was not followed through with due to performance worries. - NixOS#208173 The basis for this change. This change is just a rebase of that one. I split it out to add the capability before adding users of it, in order to simplify review. However, the ctags script in there is a sample user of this feature. Benchmarks: conducted by evaluating my own reasonably complex NixOS configuration with the command: `hyperfine -S none -w 1 -- "nix eval .#nixosConfigurations.snowflake.config.system.build.toplevel.outPath"` ``` Benchmark 1: nix eval .#nixosConfigurations.snowflake.config.system.build.toplevel.outPath Time (mean ± σ): 8.971 s ± 0.254 s [User: 5.872 s, System: 1.388 s] Range (min … max): 8.574 s … 9.327 s 10 runs Benchmark 1: nix eval .#nixosConfigurations.snowflake.config.system.build.toplevel.outPath Time (mean ± σ): 8.766 s ± 0.160 s [User: 5.873 s, System: 1.346 s] Range (min … max): 8.496 s … 9.033 s 10 runs ``` Summary of results: it seems to be in the noise, this does not cause any visible regression in times.

What it does: line and column level *declaration* position information: $ nix repl . nix-repl> :p nixosConfigurations.micro.options.environment.systemPackages.declarationPositions [ { column = 7; file = "/nix/store/24aj3k7fgqv3ly7qkbf98qvphasrw9nb-source/nixos/modules/config/system-path.nix"; line = 63; } ] Use cases: - ctags over NixOS options, as will be presented at NixCon 2023 ;) - improving the documentation pages to go to the exact line of the declarations. Related work: - NixOS/nixpkgs#65024 This one does it for all *definitions* rather than declarations, and it was not followed through with due to performance worries. - NixOS/nixpkgs#208173 The basis for this change. This change is just a rebase of that one. I split it out to add the capability before adding users of it, in order to simplify review. However, the ctags script in there is a sample user of this feature. Benchmarks: conducted by evaluating my own reasonably complex NixOS configuration with the command: `hyperfine -S none -w 1 -- "nix eval .#nixosConfigurations.snowflake.config.system.build.toplevel.outPath"` ``` Benchmark 1: nix eval .#nixosConfigurations.snowflake.config.system.build.toplevel.outPath Time (mean ± σ): 8.971 s ± 0.254 s [User: 5.872 s, System: 1.388 s] Range (min … max): 8.574 s … 9.327 s 10 runs Benchmark 1: nix eval .#nixosConfigurations.snowflake.config.system.build.toplevel.outPath Time (mean ± σ): 8.766 s ± 0.160 s [User: 5.873 s, System: 1.346 s] Range (min … max): 8.496 s … 9.033 s 10 runs ``` Summary of results: it seems to be in the noise, this does not cause any visible regression in times.

TomSmeets requested review from edolstra and nbp as code owners July 18, 2019 12:48

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 0 labels Jul 18, 2019

infinisil added the 6.topic: module system About NixOS module system internals label Apr 2, 2020

infinisil mentioned this pull request Sep 17, 2020

Improved module errors #98155

Merged

1 task

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 29, 2020

ryantm added the 2.status: merge conflict label Oct 3, 2020

stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 3, 2020

ryantm added 2.status: merge conflict and removed 2.status: merge conflict labels Oct 3, 2020

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 5, 2021

lf- mentioned this pull request Dec 29, 2022

nixos/docs: add a ctags script for nixos options #208173

Closed

13 tasks

lf- mentioned this pull request Aug 15, 2023

nixos/modules: Add declarationPositions #249243

Merged

12 tasks

wegank marked this pull request as draft March 20, 2024 15:12

stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024

wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib/modules.nix: provide detailed error locations #65024

lib/modules.nix: provide detailed error locations #65024

TomSmeets commented Jul 18, 2019

stale bot commented Sep 29, 2020

stale bot commented Jun 5, 2021

lib/modules.nix: provide detailed error locations #65024

Are you sure you want to change the base?

lib/modules.nix: provide detailed error locations #65024

Conversation

TomSmeets commented Jul 18, 2019

Examples

Failing option check

Option redefinition

Inside imports

Issues

mkIf and friends

Performance

Conclusion

Related Issues

stale bot commented Sep 29, 2020

stale bot commented Jun 5, 2021

`mkIf` and friends