Skip to content

nixos/unbound: allow list of strings in top-level settings option type#121927

Merged
roberth merged 1 commit intoNixOS:masterfrom
rissson:nixos-unbound-fix-top-level-include
May 8, 2021
Merged

nixos/unbound: allow list of strings in top-level settings option type#121927
roberth merged 1 commit intoNixOS:masterfrom
rissson:nixos-unbound-fix-top-level-include

Conversation

@rissson
Copy link
Member

@rissson rissson commented May 6, 2021

Motivation for this change

Closes #121878

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels May 6, 2021
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels May 6, 2021
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
settingsType = oneOf [ string (attrsOf validSettingsTypes) ];
settingsType = oneOf [ str (attrsOf validSettingsTypes) ];

string is a deprecated type.

Copy link
Contributor

@rnhmjoj rnhmjoj May 7, 2021

Choose a reason for hiding this comment

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

The intermediate types could also use a human readable description to avoid gollum showing up in error messages:

error: A definition for option services.unbound.settings.include.[definition 1-entry 1] is not of type attribute set of signed integer or string or boolean or floating point number or list of signed integer or string or boolean or floating point numberss.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed the deprecated string type. Sorry about that.

Copy link
Member

Choose a reason for hiding this comment

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

Don't worry about it. We don't have a usable warning for it. #99132 broke other things.

Signed-off-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>
@rissson rissson force-pushed the nixos-unbound-fix-top-level-include branch from d66dc21 to 0340cd2 Compare May 8, 2021 17:55
@roberth roberth merged commit 4433ba9 into NixOS:master May 8, 2021
@rissson rissson deleted the nixos-unbound-fix-top-level-include branch May 8, 2021 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nixos/unbound: Impossible to specify multiple "include" directives.

3 participants