Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introduce types.anything #97119

Merged
merged 5 commits into from Sep 21, 2020
Merged

Introduce types.anything #97119

merged 5 commits into from Sep 21, 2020

Conversation

infinisil
Copy link
Member

Motivation for this change

This introduces the new type types.anything, which implements a safe and reasonable merge strategy if you don't care about the type. Specifically:

  • It recursively merges attribute sets together
    • Except if it's string-coercible (aka, packages and co.), then only one definition is allowed
  • It doesn't concatenate list values together (since list concatenation doesn't always make sense)
  • If it's an atomic value (not a set or a list), all definitions must be equal
  • It applies mkIf's and co. for all values

This is in contrast with the often-used types.attrs and types.unspecified, which can be problematic because they don't adhere to above points. If both this PR and #97114 are merged, I can start deprecating those in favor of this one.

Note that the doc entry for this new type is intentionally kept small, because its behavior is very natural and doesn't need a special explanation.

Ping @rycee @roberth @Profpatsch

Things done
  • Wrote a doc entry for the new type
  • Wrote tests for the new type

@infinisil infinisil added the 6.topic: module system About NixOS module system internals label Sep 4, 2020
@edolstra
Copy link
Member

edolstra commented Sep 4, 2020

Hm, what's the use case for this? Presumably you always do care about the type of an option since you need to use it somewhere eventually.

@rycee
Copy link
Member

rycee commented Sep 4, 2020 •
edited

@edolstra I don't think the anything type is very useful by itself but I would happily use, e.g., attrsOf anything instead of attrs or attrsOf unspecified. That is, when I know the type is an attribute set but not the specific content. I've encountered this mainly when defining settings-like options that gets serialized to some hierarchical configuration format. For example the programs.taskwarrier.config or programs.waybar.settings.*.modules options in Home Manager.

@infinisil
Copy link
Member Author

Yeah exactly, the intention is for this type to replace types.attrs and types.unspecified, which are currently used when the type isn't known, but as mentioned they have some surprising behavior which you wouldn't expect.

Profpatsch
Profpatsch requested changes Sep 14, 2020
View reviewed changes
nixos/doc/manual/development/option-types.xml Outdated Show resolved Hide resolved
lib/types.nix Show resolved Hide resolved
@infinisil
nixos/doc: Soft deprecate types.attrs
b381016
@infinisil
lib/options: Fix mergeEqualOption for singular functions
6e7bc2c 
Previously it would error out for a single function definition
@infinisil
lib/types: Introduce types.anything
67551f4 
This new type has unsurprising merge behavior: Only attribute sets are
merged together (recursively), and only if they don't conflict.

This is in contrast to the existing types:
- types.attrs is problematic because later definitions completely
  override attributes of earlier definitions, and it doesn't support
  mkIf and co.
- types.unspecified is very similar to types.attrs, but it has smart
  merging behavior that often doesn't make sense, and it doesn't support
  all types
@infinisil
Copy link
Member Author

Changes since the last revision:

  • I moved types.attrs to the bottom of the type list and added a warning about its problems
  • Added an example for types.anything in the docs
  • Fixed mergeEqualOption for when a single function is passed. Now it doesn't error out anymore. Also added a test case to types.anything for this

Profpatsch
Profpatsch approved these changes Sep 18, 2020
View reviewed changes
Copy link
Member

@Profpatsch Profpatsch left a comment

Choose a reason for hiding this comment

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

LGTM

@roberth roberth merged commit f3893d8 into NixOS:master Sep 21, 2020
@roberth
Copy link
Member

roberth commented Sep 21, 2020

Thanks @infinisil !

@infinisil infinisil deleted the types.anything branch September 21, 2020 11:05
@arcnmx arcnmx mentioned this pull request Aug 27, 2021
Merged
7 tasks
This was referenced Dec 9, 2021
Merged
Closed
@infinisil infinisil mentioned this pull request Jan 5, 2022
Closed
13 tasks
@infinisil infinisil mentioned this pull request Mar 1, 2022
Merged
13 tasks
@ncfavier ncfavier mentioned this pull request Dec 29, 2022
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roberth roberth roberth approved these changes

@Profpatsch Profpatsch Profpatsch approved these changes

@edolstra edolstra Awaiting requested review from edolstra

@nbp nbp Awaiting requested review from nbp

Assignees
No one assigned
Labels
6.topic: module system About NixOS module system internals 6.topic: nixos 8.has: documentation 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@infinisil @edolstra @rycee @roberth @Profpatsch