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

Deprecate types.loaOf #63103

Merged
merged 3 commits into from Jan 6, 2020
Merged

Deprecate types.loaOf #63103

merged 3 commits into from Jan 6, 2020

Conversation

@rnhmjoj
Copy link
Contributor

@rnhmjoj rnhmjoj commented Jun 13, 2019

Motivation for this change

Fix issue #1800.

It turns out this is a lot of work: there are many instances of lists being used instead of sets.
The main culprits are users.users and environment.etc.
I think I have identified most of them but there are still some, moreover all this changes should be properly tested so I definitely need some help.

@rnhmjoj rnhmjoj requested review from edolstra and nbp as code owners Jun 13, 2019
@rnhmjoj rnhmjoj force-pushed the rnhmjoj:loaof branch from 5103153 to ff20e1a Jun 13, 2019
@Infinisil
Copy link
Member

@Infinisil Infinisil commented Jun 14, 2019

If we want to deprecate this, there needs to be some deprecation warning for users. We can't just flip the switch and possibly break every second person's config. A good way to do that is to change the loaOf type to issue a warning when the list case is used, something like

Warning: You're assigning the list [ { name = "foo"; bar = <CODE>; ... } ... ] to
<option name>, which won't be supported in the future, use an attribute set like
{ foo = { bar = <CODE>; ... }; ... } instead. See
https://github.com/NixOS/nixpkgs/pull/63103 for more info.
@rnhmjoj rnhmjoj force-pushed the rnhmjoj:loaof branch 3 times, most recently from 4c0cfbd to 2412d3b Jun 14, 2019
@rnhmjoj
Copy link
Contributor Author

@rnhmjoj rnhmjoj commented Jun 14, 2019

Ok, I removed the attempts to replace loaOf with attrsOf and added a deprecation warning.
I don't know how to do something like that though

@@ -318,7 +318,7 @@ rec {
def;
attrOnly = attrsOf elemType;
in mkOptionType rec {
name = "loaOf";
name = builtins.trace "types.loaOf is deprecated; use types.attrsOf instead" "loaOf";

This comment has been minimized.

@Infinisil

Infinisil Jun 14, 2019
Member

Yeah this isn't what we need. The warning needs to go in the merge function. There one has access to all definitions, which can then be filtered to lists only, and for each of them one can print a warning as I showed in my comment. would be even neater to include the file and line where the user needs to do this change.

I generally think our error messages suck too much. It's not too difficult to make them better, especially with all the info the module system gives access to.

This comment has been minimized.

@rnhmjoj

rnhmjoj Jun 14, 2019
Author Contributor

Ok, I'm working on it.

This comment has been minimized.

@rnhmjoj

rnhmjoj Jun 15, 2019
Author Contributor

I took me a while but I think this should do it:

trace: warning: In file <nixpkgs/nixos/modules/services/misc/nix-daemon.nix>
a list is being assigned to the option config.users.users.
This will soon be an error as type loaOf is deprecated.
See https://git.io/fj2zm for more information.
Do
  users.users =
    { nixbld1 = {...}; nixbld2 = {...}; nixbld3 = {...}; ...}
instead of
  users.users =
    [ { name = "nixbld1"; ...} { name = "nixbld2"; ...} { name = "nixbld3"; ...} ...]

This comment has been minimized.

@rnhmjoj

rnhmjoj Jun 15, 2019
Author Contributor

Uhm, no it doesn't work when the set is missing the name attribute.

This comment has been minimized.

@rnhmjoj

rnhmjoj Aug 28, 2019
Author Contributor

I mean, it does but it's not pretty.

@rycee
Copy link
Member

@rycee rycee commented Jun 14, 2019

This will break the programs.ssh.matchBlocks option in Home Manager. That option allows use of list when the order matters and an attribute set as a convenience for users whose setup does not require a certain order.

I believe I originally had it as a listOf but got complaints that it was cumbersome for simple setups.

I don't particularly mind switching back to listOf since I, in principle, agree about the rationale for deprecating loaOf but thought I'd make a note of this use-case.

@rnhmjoj rnhmjoj force-pushed the rnhmjoj:loaof branch from 2412d3b to e08e64f Jun 15, 2019
@rnhmjoj
Copy link
Contributor Author

@rnhmjoj rnhmjoj commented Jun 15, 2019

@rycee I don't know how home-manager works: are you somehow bound to nixos option types? Couldn't you simply define an equivalent loaOf for home-manager?

@rnhmjoj rnhmjoj force-pushed the rnhmjoj:loaof branch 2 times, most recently from ef6ce8b to fdab0a3 Jun 15, 2019
@rnhmjoj rnhmjoj force-pushed the rnhmjoj:loaof branch from fdab0a3 to 609a370 Jun 16, 2019
@rnhmjoj rnhmjoj force-pushed the rnhmjoj:loaof branch 3 times, most recently from 5d7cfe4 to 57440d5 Jun 17, 2019
@rnhmjoj rnhmjoj force-pushed the rnhmjoj:loaof branch from 57440d5 to f863cfc Jul 4, 2019
@rnhmjoj rnhmjoj force-pushed the rnhmjoj:loaof branch from f863cfc to b68425c Jul 26, 2019
@rnhmjoj rnhmjoj mentioned this pull request Jul 28, 2019
@rnhmjoj rnhmjoj mentioned this pull request Aug 26, 2019
0 of 10 tasks complete
Profpatsch added a commit to Profpatsch/nixpkgs that referenced this pull request Jan 21, 2020
The error message is already helpfully verbose, so there's little
reason to shorten the informational URL.
Jassob added a commit to Jassob/nixos that referenced this pull request Feb 5, 2020
NixOS/nixpkgs deprecated a primitive (loaOf, read more [here]), these
are the required changes.

[here]: NixOS/nixpkgs#63103
@mmilata mmilata mentioned this pull request Feb 10, 2020
0 of 10 tasks complete
ben0x539 added a commit to ben0x539/nixpkgs that referenced this pull request Feb 19, 2020
rnhmjoj added a commit that referenced this pull request Feb 19, 2020
silence warning from #63103 in encrypted-devices.nix
edwtjo added a commit that referenced this pull request Feb 19, 2020
As a comment to 1d61efb
Note that collect returns a list from a set
dguibert added a commit to dguibert/nixpkgs that referenced this pull request Mar 5, 2020
@dguibert dguibert mentioned this pull request Mar 5, 2020
2 of 10 tasks complete
dtzWill added a commit to dtzWill/nixpkgs that referenced this pull request Mar 6, 2020
related to NixOS#63103.

(cherry picked from commit bbc2cd8)
dmeijboom pushed a commit to dmeijboom/nixpkgs that referenced this pull request Mar 6, 2020
related to NixOS#63103.
dasj19 added a commit to dasj19/nixpkgs that referenced this pull request Mar 12, 2020
svanderburg added a commit that referenced this pull request Mar 14, 2020
related to #63103.

(cherry picked from commit bbc2cd8)
orivej added a commit to orivej/musnix that referenced this pull request Mar 25, 2020
The current usage triggers the warning from
NixOS/nixpkgs@0330989:

trace: warning: In file /home/uj/nix/musnix/modules/rtirq.nix
a list is being assigned to the option config.environment.etc.
This will soon be an error as type loaOf is deprecated.
See NixOS/nixpkgs#63103 for more information.
Do
  environment.etc =
    { rtirq.conf = {...}; }
instead of
  environment.etc =
    [ { target = "rtirq.conf"; ...} ]
orivej added a commit to orivej/musnix that referenced this pull request Mar 25, 2020
The current usage triggers the warning from
NixOS/nixpkgs@0330989:

    trace: warning: In file /home/uj/nix/musnix/modules/rtirq.nix
    a list is being assigned to the option config.environment.etc.
    This will soon be an error as type loaOf is deprecated.
    See NixOS/nixpkgs#63103 for more information.
    Do
      environment.etc =
        { rtirq.conf = {...}; }
    instead of
      environment.etc =
        [ { target = "rtirq.conf"; ...} ]
henrytill added a commit to musnix/musnix that referenced this pull request Mar 27, 2020
The current usage triggers the warning from
NixOS/nixpkgs@0330989:

    trace: warning: In file /home/uj/nix/musnix/modules/rtirq.nix
    a list is being assigned to the option config.environment.etc.
    This will soon be an error as type loaOf is deprecated.
    See NixOS/nixpkgs#63103 for more information.
    Do
      environment.etc =
        { rtirq.conf = {...}; }
    instead of
      environment.etc =
        [ { target = "rtirq.conf"; ...} ]
zetavg added a commit to zetavg/nixos-configs that referenced this pull request Apr 4, 2020
warning: In file [...]
a list is being assigned to the option config.users.users.
This will soon be an error as type loaOf is deprecated.
See NixOS/nixpkgs#63103 for more information.
yokodake added a commit to yokodake/dots that referenced this pull request Apr 10, 2020
trace: warning: In file hw-laptop.nix
a list is being assigned to the option config.boot.initrd.luks.devices.
This will soon be an error as type loaOf is deprecated.
See NixOS/nixpkgs#63103 for more information.
Do
  boot.initrd.luks.devices =
    { main = {...}; }
instead of
  boot.initrd.luks.devices =
    [ { name = "main"; ...} ]

trace: warning: The option `i18n.consoleKeyMap' defined in `configuration.nix' has been renamed to `console.keyMap'.

trace: warning: The following options are deprecated:
  - services.xserver.desktopManager.default
  - services.xserver.windowManager.default
Please use
  services.xserver.displayManager.defaultSession = "none+xmonad";
instead.
mmilata added a commit to mmilata/nixpkgs that referenced this pull request May 2, 2020
@aanderse aanderse mentioned this pull request May 5, 2020
1 of 10 tasks complete
@aanderse aanderse mentioned this pull request May 13, 2020
2 of 10 tasks complete
woffs added a commit to woffs/nixpkgs that referenced this pull request May 25, 2020
As a comment to 1d61efb
Note that collect returns a list from a set

(cherry picked from commit 9bab9e2)
rnhmjoj added a commit that referenced this pull request May 26, 2020
[20.03] nixos/i2pd: address #63103
worldofpeace added a commit that referenced this pull request Jun 18, 2020
…warning

[20.03] silence warning from #63103 in encrypted-devices.nix
github-actions bot added a commit to pbogdan/nix-hie that referenced this pull request Jun 19, 2020
## Motivation

Dependencies should be up to date.

## Changelog for stable:
Commits: [NixOS/nixpkgs@a84b797...2b417708](NixOS/nixpkgs@a84b797...2b41770)

* [`4a41762c`](NixOS/nixpkgs@4a41762) silence warning from NixOS/nixpkgs#63103 in encrypted-devices.nix
* [`fc9ca529`](NixOS/nixpkgs@fc9ca52) ssm-session-manager-plugin: init at 1.1.61.0
* [`88306909`](NixOS/nixpkgs@8830690) samba: 4.11.5 -> 4.11.9
* [`5cd1d383`](NixOS/nixpkgs@5cd1d38) firmwareLinuxNonfree: 2020-01-22 -> 2020-05-19
* [`cd6ea1df`](NixOS/nixpkgs@cd6ea1d) xpra: fix NixOS/nixpkgs#85694
* [`79ac3258`](NixOS/nixpkgs@79ac325) xpra: fix NixOS/nixpkgs#41106
* [`82ef74d0`](NixOS/nixpkgs@82ef74d) xpra: adjust patches
* [`90057c5b`](NixOS/nixpkgs@90057c5) jool: 4.0.5 -> 4.0.9
* [`392b8bc9`](NixOS/nixpkgs@392b8bc) nixos-artwork: add file path attributes
* [`20bfce50`](NixOS/nixpkgs@20bfce5) nixos/lightdm: change background type to path
* [`489ebe45`](NixOS/nixpkgs@489ebe4) nixos/gnome3: install nixos wallpapers
* [`b2bd9376`](NixOS/nixpkgs@b2bd937) nixos/pantheon: install nixos wallpaper
* [`4a644cc7`](NixOS/nixpkgs@4a644cc) sympa: 6.2.54 -> 6.2.56
* [`2678e4fc`](NixOS/nixpkgs@2678e4f) pantheon.appcenter: 3.2.4 -> 3.3.0
* [`9fdefdc9`](NixOS/nixpkgs@9fdefdc) pantheon.switchboard: 2.3.9 -> 2.4.0
* [`016f42b3`](NixOS/nixpkgs@016f42b) pantheon.elementary-icon-theme: 5.2.0 -> 5.3.0
* [`3b4dbf34`](NixOS/nixpkgs@3b4dbf3) pantheon.granite: 5.3.1 -> 5.4.0
* [`c064cd05`](NixOS/nixpkgs@c064cd0) pantheon.wingpanel-applications-menu: 2.6.0 -> 2.7.0
* [`c0c768b6`](NixOS/nixpkgs@c0c768b) pantheon.gala: 3.3.1 -> 3.3.2
* [`044c9a43`](NixOS/nixpkgs@044c9a4) pantheon.appcenter: 3.3.0 -> 3.4.0
* [`3c96a131`](NixOS/nixpkgs@3c96a13) pantheon.elementary-icon-theme: 5.3.0 -> 5.3.1
* [`adacfd02`](NixOS/nixpkgs@adacfd0) pantheon.elementary-calendar: 5.0.4 -> 5.0.5
* [`d71bdb2e`](NixOS/nixpkgs@d71bdb2) pantheon.elementary-files: 4.4.2 -> 4.4.3
* [`adca4a37`](NixOS/nixpkgs@adca4a3) pantheon.sideload: 1.1.0 -> 1.1.1
* [`85146f0f`](NixOS/nixpkgs@85146f0) pantheon.elementary-greeter: 5.0.3 -> 5.0.4
* [`8f24062f`](NixOS/nixpkgs@8f24062) pantheon.pantheon-agent-polkit: 1.0.1 -> 1.0.2
* [`0d8445d0`](NixOS/nixpkgs@0d8445d) pantheon.wingpanel-indicator-network: 2.2.3 -> 2.2.4
* [`4a25ae59`](NixOS/nixpkgs@4a25ae5) pantheon.wingpanel-applications-menu: 2.7.0 -> 2.7.1
* [`ae8c62cb`](NixOS/nixpkgs@ae8c62c) pantheon.elementary-shortcut-overlay: 1.1.1 -> 1.1.2
* [`ac21fd75`](NixOS/nixpkgs@ac21fd7) pantheon.elementary-onboarding: 1.2.0 -> 1.2.1
* [`df72656b`](NixOS/nixpkgs@df72656) pantheon.switchboard-plug-about: 2.6.2 -> 2.6.3
* [`77804794`](NixOS/nixpkgs@7780479) pantheon.switchboard-plug-bluetooth: 2.3.1 -> 2.3.2
* [`34ce3952`](NixOS/nixpkgs@34ce395) pantheon.switchboard-plug-datetime: 2.1.7 -> 2.1.9
* [`d3805ad5`](NixOS/nixpkgs@d3805ad) pantheon.switchboard-plug-display: 2.2.1 -> 2.2.2
* [`489245ed`](NixOS/nixpkgs@489245e) pantheon.switchboard-plug-mouse-touchpad: 2.4.1 -> 2.4.2
* [`68d7569d`](NixOS/nixpkgs@68d7569) pantheon.switchboard-plug-network: 2.3.0 -> 2.3.1
* [`e40d5b5f`](NixOS/nixpkgs@e40d5b5) pantheon.switchboard-plug-notifications: 2.1.6 -> 2.1.7
* [`d8cac264`](NixOS/nixpkgs@d8cac26) pantheon.switchboard-plug-power: 2.4.1 -> 2.4.2
* [`5421f401`](NixOS/nixpkgs@5421f40) pantheon.switchboard-plug-printers: 2.1.8 -> 2.1.9
* [`003af9ca`](NixOS/nixpkgs@003af9c) pantheon.switchboard-plug-security-privacy: 2.2.3 -> 2.2.4
* [`b3a58026`](NixOS/nixpkgs@b3a5802) pantheon.switchboard-plug-sound: 2.2.3 -> 2.2.4
* [`1763fe3f`](NixOS/nixpkgs@1763fe3) microcodeIntel: 20200508 → 20200520
* [`57f7f175`](NixOS/nixpkgs@57f7f17) microcodeIntel: 20200520 -> 20200609
* [`3f31c0d2`](NixOS/nixpkgs@3f31c0d) nixos/sympa: fix PATH_INFO splitting for sympa-6.2.56
* [`807d383c`](NixOS/nixpkgs@807d383) palemoon: 28.9.3 -> 28.10.0
* [`098fdc6b`](NixOS/nixpkgs@098fdc6) treewide: central.maven.org -> repo1.maven.org
* [`b0bfe9b5`](NixOS/nixpkgs@b0bfe9b) chromium: Build with VA-API but disable it by default
* [`0d8250ae`](NixOS/nixpkgs@0d8250a) chromium: 83.0.4103.97 -> 83.0.4103.106
* [`70ecf772`](NixOS/nixpkgs@70ecf77) graylog: 3.3.0 -> 3.3.1
* [`60da5edb`](NixOS/nixpkgs@60da5ed) graylogPlugins: Update plugins
* [`02b95cf5`](NixOS/nixpkgs@02b95cf) strace-graph: fix strace-graph shebang which points to perl
* [`dce9ef51`](NixOS/nixpkgs@dce9ef5) youtube-dl: 2020.06.06 -> 2020.06.16.1
* [`c27866c8`](NixOS/nixpkgs@c27866c) microcodeIntel: 20200609 -> 20200616
* [`724d123a`](NixOS/nixpkgs@724d123) riot-web: 1.6.4 -> 1.6.5
* [`8ab112eb`](NixOS/nixpkgs@8ab112e) riot-desktop: 1.6.4 -> 1.6.5
* [`b2a6d2fa`](NixOS/nixpkgs@b2a6d2f) linuxPackages.wireguard: 1.0.20200520 -> 1.0.20200611
* [`243f698a`](NixOS/nixpkgs@243f698) vagrant: 2.2.8 -> 2.2.9
* [`c5fd298d`](NixOS/nixpkgs@c5fd298) vlc: 3.0.8 -> 3.0.11 (security)
* [`bb4fd7eb`](NixOS/nixpkgs@bb4fd7e) haskellPackages.geojson: Unmark as broken
* [`583ccf8c`](NixOS/nixpkgs@583ccf8) Don't enable nix-bash-completions when using Nix 2.4
* [`80b50f32`](NixOS/nixpkgs@80b50f3) fwupd: add patch for CVE-2020-10759
* [`725c4fdb`](NixOS/nixpkgs@725c4fd) google-chrome: add coreutils to PATH
* [`9e379117`](NixOS/nixpkgs@9e37911) xdg_utils: xdg-open: add $out to PATH
* [`73d7516c`](NixOS/nixpkgs@73d7516) matrix-synapse: 1.14.0 -> 1.15.1
* [`f70a5992`](NixOS/nixpkgs@f70a599) nixos/gnome3: nixos-artwork -> pkgs.nixos-artwork
* [`87cde1cf`](NixOS/nixpkgs@87cde1c) pantheon.elementary-code: 3.4.0 -> 3.4.1
* [`30c703cb`](NixOS/nixpkgs@30c703c) nixos/sudo: default rule should be first
* [`788764b1`](NixOS/nixpkgs@788764b) transmission: patch CVE-2018-10756
* [`2b417708`](NixOS/nixpkgs@2b41770) Remove Google Talk Plugin
@rnhmjoj rnhmjoj mentioned this pull request Aug 22, 2020
0 of 10 tasks complete
@Infinisil Infinisil mentioned this pull request Sep 4, 2020
1 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.