Skip to content

Introduce types.raw#160489

Merged
roberth merged 2 commits intoNixOS:masterfrom
infinisil:types.raw
Feb 23, 2022
Merged

Introduce types.raw#160489
roberth merged 2 commits intoNixOS:masterfrom
infinisil:types.raw

Conversation

@infinisil
Copy link
Member

Motivation for this change
  • Introduce types.raw as a type that doesn't do any nested processing for mkIf, mkForce, etc. on its values. This is useful when this processing would throw errors or where it would be too expensive (such as with package sets like nixpkgs).
  • Use types.raw for _module.args's type. This finally fixes _module.args.name gets merged on assignment #53458

This is split off from #132448. This is also motivated by wanting to deprecate types.attrs and types.unspecified, for which types.attrsOf types.raw and types.raw (or types.anything) is an adequate general replacement.

Things done
  • Wrote and ran tests
  • Wrote docs

@infinisil infinisil added the 6.topic: module system About "NixOS" module system internals label Feb 17, 2022
@infinisil infinisil requested a review from roberth February 17, 2022 17:19
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation labels Feb 17, 2022
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Feb 17, 2022
Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure.

Is this the only use case?

lib/modules.nix Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the right trade-off. It's already lazy and when you do access an attr value, like that of pkgs, you'll force it anyway. Checking its type attr before returning it is a cheap and one-time operation.

Copy link
Member

@roberth roberth Feb 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I don't think we should trade the ability to mkForce in _module.args for arguably no benefit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #160489 (comment), mkForce and co. still work on the spine

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must have read too much into "raw".
The tests demonstrate it clearly.

Perhaps the docs can clarify that it's not entirely raw?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the confusion comes from #132448 where types.raw originally worked like that, not doing any processing at all, but I changed that after discussing it with you :)

I think mentioning that in the docs is a good idea

Comment on lines 102 to 103

This comment was marked as outdated.

@infinisil
Copy link
Member Author

infinisil commented Feb 17, 2022

@roberth This type still works with mkForce, mkIf and co., just not deeply. E.g. this works and gives a result of 30:

let lib = import ./lib; in

lib.evalModules {
  modules = [
    {
      options.foo = lib.mkOption {
        type = lib.types.raw;
      };

      config.foo = lib.mkMerge [
        (lib.mkIf false 10)
        20
        (lib.mkIf true (lib.mkForce 30))
      ];
    }
  ];
}

types.anything is similar to types.raw in that it accepts any value, however types.anything recurses deeply into the structure to apply any mkForce and co. And that is what makes types.anything very unsuitable for below-described use cases.

The main use case for types.raw is values that aren't meant for the module system to be evaluated and merged, because they come from outside the module system. So e.g. things like package sets, which have their own merging strategies incompatible with the module system: overlays, overrides, extends, etc. The module system also shouldn't try to inspect these values for any mkForces and co, which would make it excessively strict in evaluation without any benefit, since you won't be using mkForce and co. for e.g. pkgs.hello

Other than _module.args I can also think of nixpkgs.pkgs and options like services.xserver.windowManager.xmonad.haskellPackages that could benefit from this.

We also need to deprecate types.unspecified due to its implicit and arbitrary merging behavior, and replace it with either types.anything or types.raw.

@roberth
Copy link
Member

roberth commented Feb 17, 2022

Is this the only use case?

This one seems valid.

#157056 (comment)

Comment on lines 69 to 73
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what's important is that

  • it doesn't merge or check
  • it does apply priorities
  • it should be used if and only if merging and checking are not desirable

"Especially useful" and "recommended" are a bit too encouraging for something that should be applied with attention.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, I updated the docs

Comment on lines +68 to +75
: A type which doesn't do any checking, merging or nested evaluation. It
accepts a single arbitrary value that is not recursed into, making it
useful for values coming from outside the module system, such as package
sets or arbitrary data. Options of this type are still evaluated according
to priorities and conditionals, so `mkForce`, `mkIf` and co. still work on
the option value itself, but not for any value nested within it. This type
should only be used when checking, merging and nested evaluation are not
desirable.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roberth Does this sound good?

@roberth roberth merged commit 6225804 into NixOS:master Feb 23, 2022
@infinisil infinisil deleted the types.raw branch February 23, 2022 17:39
@ncfavier
Copy link
Member

As expected, this broke my config because I was adding my own stuff to _module.args.utils. What was less expected is that instead of a module system error I got an infinite recursion (logs).

Probably just a "me" problem not worth looking into as I couldn't reproduce that error on a minimal config.

@roberth
Copy link
Member

roberth commented Feb 24, 2022

I'd like utils to be removed, as it is nonsensical.

@ncfavier
Copy link
Member

Yeah, I guess it's supposed to be a middle ground between lib and pkgs? Maybe the things it defines should go into config.lib instead?

@roberth
Copy link
Member

roberth commented Feb 24, 2022

As a general rule, utils is the name for the place people create to put the stuff they don't know where to put. config.lib is only slightly better.

The systemd logic can move into importable modules instead, after #156533

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-25/18003/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: module system About "NixOS" module system internals 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

_module.args.name gets merged on assignment

4 participants